Refactor argument resolver to address edge cases (#1259)

Author: silesky

.changeset/fast-lions-kick.md

@@ -0,0 +1,5 @@
+---
+'@segment/analytics-next': patch
+---
+
+Add support for more argument resolver edge cases

packages/browser/src/core/analytics/interfaces.ts

@@ -77,24 +77,169 @@ export interface AnalyticsClassic extends AnalyticsClassicStubs {
  * Interface implemented by concrete Analytics class (commonly accessible if you use "await" on AnalyticsBrowser.load())
  */
 export interface AnalyticsCore extends CoreAnalytics {
+  /**
+   * Tracks an event.
+   * @param args - Event parameters.
+   * @example
+   * ```ts
+   *  analytics.track('Event Name', {
+   *    property1: 'value1',
+   *    property2: 'value2'
+   *  });
+   * ```
+   * @link https://segment.com/docs/connections/sources/catalog/libraries/website/javascript/#track
+   * @returns A promise that resolves to a dispatched event.
+   */
   track(...args: EventParams): Promise<DispatchedEvent>
+
+  /**
+   * Tracks a page view.
+   * @param args - `[category], [name], [properties], [options], [callback]`
+   * @example
+   * ```ts
+   *  analytics.page('My Category', 'Pricing', {
+   *    title: 'My Overridden Title',
+   *    myExtraProp: 'foo'
+   *  })
+   *
+   *  analytics.page('Pricing', {
+   *    title: 'My Overridden Title',
+   *    myExtraProp: 'foo'
+   *  });
+   * ```
+   * @link https://segment.com/docs/connections/sources/catalog/libraries/website/javascript/#page
+   * @returns A promise that resolves to a dispatched event.
+   */
   page(...args: PageParams): Promise<DispatchedEvent>
+
+  /**
+   * Identifies a user.
+   * @param args - Identify parameters.
+   * @example
+   * ```ts
+   *  analytics.identify('userId123', {
+   *    email: 'user@example.com',
+   *    name: 'John Doe'
+   *  });
+   * ```
+   * @link https://segment.com/docs/connections/sources/catalog/libraries/website/javascript/#identify
+   * @returns A promise that resolves to a dispatched event.
+   */
   identify(...args: IdentifyParams): Promise<DispatchedEvent>
+
+  /**
+   * Gets the group.
+   * @returns The group.
+   */
   group(): Group
+
+  /**
+   * Sets the group.
+   * @param args - Group parameters.
+   * @example
+   * ```ts
+   *  analytics.group('groupId123', {
+   *    name: 'Company Inc.',
+   *    industry: 'Software'
+   *  });
+   * ```
+   * @link https://segment.com/docs/connections/sources/catalog/libraries/website/javascript/#group
+   * @returns A promise that resolves to a dispatched event.
+   */
   group(...args: GroupParams): Promise<DispatchedEvent>
+
+  /**
+   * Creates an alias for a user.
+   * @param args - Alias parameters.
+   * @example
+   * ```ts
+   *  analytics.alias('newUserId123', 'oldUserId456');
+   * ```
+   * @link https://segment.com/docs/connections/sources/catalog/libraries/website/javascript/#alias
+   * @returns A promise that resolves to a dispatched event.
+   */
   alias(...args: AliasParams): Promise<DispatchedEvent>
+
+  /**
+   * Tracks a screen view.
+   * @param args - Page parameters.
+   * @example
+   * ```ts
+   *  analytics.screen('Home Screen', {
+   *    title: 'Home',
+   *    section: 'Main'
+   *  });
+   * ```
+   * @link https://segment.com/docs/connections/sources/catalog/libraries/website/javascript/#screen
+   * @returns A promise that resolves to a dispatched event.
+   */
   screen(...args: PageParams): Promise<DispatchedEvent>
+
+  /**
+   * Registers plugins.
+   * @param plugins - Plugins to register.
+   * @example
+   * ```ts
+   *  analytics.register(plugin1, plugin2);
+   * ```
+   * @link https://segment.com/docs/connections/sources/catalog/libraries/website/javascript/#register
+   * @returns A promise that resolves to the context.
+   */
   register(...plugins: Plugin[]): Promise<Context>
+
+  /**
+   * Deregisters plugins.
+   * @param plugins - Plugin names to deregister.
+   * @example
+   * ```ts
+   *  analytics.deregister('plugin1', 'plugin2');
+   * ```
+   * @link https://segment.com/docs/connections/sources/catalog/libraries/website/javascript/#deregister
+   * @returns A promise that resolves to the context.
+   */
   deregister(...plugins: string[]): Promise<Context>
+
+  /**
+   * Gets the user.
+   * @returns The user.
+   */
   user(): User
+
+  /**
+   * The version of the analytics library.
+   */
   readonly VERSION: string
 }
 
 /**
  * Interface implemented by AnalyticsBrowser (buffered version of analytics) (commonly accessible through AnalyticsBrowser.load())
  */
-export type AnalyticsBrowserCore = Omit<AnalyticsCore, 'group' | 'user'> & {
+export interface AnalyticsBrowserCore
+  extends Omit<AnalyticsCore, 'group' | 'user'> {
+  /**
+   * Gets the group.
+   * @returns A promise that resolves to the group.
+   */
   group(): Promise<Group>
+
+  /**
+   * Sets the group.
+   * @param args - Group parameters.
+   * @example
+   * ```ts
+   *  analytics.group('groupId123', {
+   *    name: 'Company Inc.',
+   *    industry: 'Software'
+   *  });
+   * ```
+   * @link https://segment.com/docs/connections/sources/catalog/libraries/website/javascript/#group
+   * @returns A promise that resolves to a dispatched event.
+   */
   group(...args: GroupParams): Promise<DispatchedEvent>
+
+  /**
+   * Gets the user.
+   * @returns A promise that resolves to the user.
+   */
   user(): Promise<User>
 }

packages/browser/src/core/arguments-resolver/__tests__/index.test.ts

@@ -3,7 +3,7 @@ import {
   resolvePageArguments,
   resolveUserArguments,
   resolveAliasArguments,
-} from '../'
+} from '../index'
 import { Callback } from '../../events'
 import { User } from '../../user'
 
@@ -219,6 +219,37 @@ describe(resolvePageArguments, () => {
         },
       },
     })
+    expect(properties).toEqual({})
+  })
+
+  it('should accept (category, name)', () => {
+    const [category, name, properties, options, cb] = resolvePageArguments(
+      'category',
+      'name'
+    )
+
+    expect(name).toEqual('name')
+    expect(category).toEqual('category')
+    expect(properties).toEqual({})
+    expect(options).toEqual({})
+    expect(cb).toEqual(undefined)
+  })
+
+  it('should accept (category, name, properties, options, cb)', () => {
+    const fn = jest.fn()
+    const [category, name, properties, options, cb] = resolvePageArguments(
+      'foo',
+      'name',
+      bananaPhone,
+      baseOptions,
+      fn
+    )
+
+    expect(category).toEqual('foo')
+    expect(name).toEqual('name')
+    expect(properties).toEqual(bananaPhone)
+    expect(options).toEqual(baseOptions)
+    expect(cb).toEqual(fn)
   })
 
   test('should accept (category, name, properties, callback)', () => {
@@ -254,52 +285,48 @@ describe(resolvePageArguments, () => {
     expect(options).toEqual({})
   })
 
-  it('should accept (name, properties, options, callback)', () => {
-    const fn = jest.fn()
+  it('should accept (name, properties)', () => {
     const [category, name, properties, options, cb] = resolvePageArguments(
       'name',
-      bananaPhone,
-      baseOptions,
-      fn
+      bananaPhone
     )
 
-    expect(category).toEqual(null)
     expect(name).toEqual('name')
+    expect(category).toEqual(null)
     expect(properties).toEqual(bananaPhone)
-    expect(options).toEqual(baseOptions)
-    expect(cb).toEqual(fn)
+    expect(options).toEqual({})
+    expect(cb).toEqual(undefined)
   })
 
-  it('should accept (name, properties, callback)', () => {
+  it('should accept (name, properties, options, callback)', () => {
     const fn = jest.fn()
     const [category, name, properties, options, cb] = resolvePageArguments(
       'name',
       bananaPhone,
+      baseOptions,
       fn
     )
 
     expect(category).toEqual(null)
     expect(name).toEqual('name')
     expect(properties).toEqual(bananaPhone)
+    expect(options).toEqual(baseOptions)
     expect(cb).toEqual(fn)
-    expect(options).toEqual({})
   })
 
-  it('should accept (category, name, properties, options, cb)', () => {
+  it('should accept (name, properties, callback)', () => {
     const fn = jest.fn()
     const [category, name, properties, options, cb] = resolvePageArguments(
-      'foo',
       'name',
       bananaPhone,
-      baseOptions,
       fn
     )
 
-    expect(category).toEqual('foo')
+    expect(category).toEqual(null)
     expect(name).toEqual('name')
     expect(properties).toEqual(bananaPhone)
-    expect(options).toEqual(baseOptions)
     expect(cb).toEqual(fn)
+    expect(options).toEqual({})
   })
 
   it('should accept (name, callback)', () => {
@@ -348,7 +375,7 @@ describe(resolvePageArguments, () => {
     expect(category).toEqual(null)
   })
 
-  test('should accept (category = null, name, properties, options, callback)', () => {
+  test('should accept (null, name, properties, options, callback)', () => {
     const fn = jest.fn()
     const [category, name, properties, options, cb] = resolvePageArguments(
       null,
@@ -365,6 +392,64 @@ describe(resolvePageArguments, () => {
     expect(cb).toEqual(fn)
   })
 
+  test('should accept (name, null, properties, options, callback)', () => {
+    const fn = jest.fn()
+    const [category, name, properties, options, cb] = resolvePageArguments(
+      'name',
+      null,
+      bananaPhone,
+      baseOptions,
+      fn
+    )
+
+    expect(category).toEqual(null)
+    expect(name).toEqual('name')
+    expect(properties).toEqual(bananaPhone)
+    expect(options).toEqual(baseOptions)
+    expect(cb).toEqual(fn)
+  })
+
+  test('should accept (name, null, properties)', () => {
+    const [category, name, properties, options] = resolvePageArguments(
+      'name',
+      null,
+      bananaPhone
+    )
+
+    expect(name).toEqual('name')
+    expect(category).toEqual(null)
+    expect(properties).toEqual(bananaPhone)
+    expect(options).toEqual({})
+  })
+
+  test('should accept (name, null, null, options)', () => {
+    const [category, name, properties, options] = resolvePageArguments(
+      'name',
+      null,
+      null,
+      baseOptions
+    )
+
+    expect(name).toEqual('name')
+    expect(category).toEqual(null)
+    expect(properties).toEqual({})
+    expect(options).toEqual(baseOptions)
+  })
+
+  test('should accept (null, null, properties)', () => {
+    const [category, name, properties, options, cb] = resolvePageArguments(
+      null,
+      null,
+      bananaPhone
+    )
+
+    expect(category).toEqual(null)
+    expect(name).toEqual(null)
+    expect(properties).toEqual(bananaPhone)
+    expect(options).toEqual({})
+    expect(cb).toEqual(undefined)
+  })
+
   test('should accept (null, null, properties, options, callback)', () => {
     const fn = jest.fn()
     const [category, name, properties, options, cb] = resolvePageArguments(
@@ -425,6 +510,16 @@ describe(resolvePageArguments, () => {
     expect(options).toEqual({})
     expect(cb).toEqual(fn)
   })
+
+  test('should accept (null, null, null, null, callback)', () => {
+    const fn = jest.fn()
+    const [category, name, properties, options, cb] = resolvePageArguments(fn)
+    expect(category).toEqual(null)
+    expect(name).toEqual(null)
+    expect(properties).toEqual({})
+    expect(options).toEqual({})
+    expect(cb).toEqual(fn)
+  })
 })
 
 describe(resolveUserArguments, () => {