refactor arg resolver on page to address edge cases

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/arguments-resolver/__tests__/index.test.ts

@@ -194,7 +194,7 @@ describe(resolveArguments, () => {
   })
 })
 
-describe(resolvePageArguments, () => {
+describe('resolvePageArguments - analytics.page([category], [name], [properties], [options], [callback])', () => {
   test('empty strings ("", "", null, { integrations })', () => {
     const [category, name, properties, options] = resolvePageArguments(
       '',
@@ -221,6 +221,36 @@ describe(resolvePageArguments, () => {
     })
   })
 
+  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)', () => {
     const fn = jest.fn()
     const [category, name, properties, options, cb] = resolvePageArguments(
@@ -254,52 +284,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 +374,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,7 +391,7 @@ describe(resolvePageArguments, () => {
     expect(cb).toEqual(fn)
   })
 
-  test('should accept (category, name = null, properties, options, callback)', () => {
+  test('should accept (category, null, properties, options, callback)', () => {
     const fn = jest.fn()
     const [category, name, properties, options, cb] = resolvePageArguments(
       'category',
@@ -376,12 +402,43 @@ describe(resolvePageArguments, () => {
     )
 
     expect(category).toEqual('category')
-    expect(name).toEqual('name')
+    expect(name).toEqual(null)
     expect(properties).toEqual(bananaPhone)
     expect(options).toEqual(baseOptions)
     expect(cb).toEqual(fn)
   })
 
+  test('should accept (category, undefined, properties, options, callback)', () => {
+    const fn = jest.fn()
+    const [category, name, properties, options, cb] = resolvePageArguments(
+      'category',
+      undefined,
+      bananaPhone,
+      baseOptions,
+      fn
+    )
+
+    expect(category).toEqual('category')
+    expect(name).toEqual(undefined)
+    expect(properties).toEqual(bananaPhone)
+    expect(options).toEqual(baseOptions)
+    expect(cb).toEqual(fn)
+  })
+
+  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(
@@ -442,6 +499,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, () => {

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

@@ -70,29 +70,12 @@ export function resolvePageArguments(
   Options,
   Callback | undefined
 ] {
-  let resolvedProperties: EventProperties
-  let resolvedOptions: Options
   let resolvedCategory: string | undefined | null = null
   let resolvedName: string | undefined | null = null
+  let resolvedProperties: EventProperties
+  let resolvedOptions: Options
   const args = [category, name, properties, options, callback]
 
-  // handle:
-  // - analytics.page('name')
-  // - analytics.page('category', 'name')
-  const strings = args.filter(isString)
-  if (strings.length === 1) {
-    resolvedCategory = null
-    resolvedName = strings[0]
-  } else if (strings.length === 2) {
-    if (typeof args[0] === 'string') {
-      resolvedCategory = args[0]
-    }
-    if (typeof args[1] === 'string') {
-      resolvedName = args[1]
-    }
-  }
-
-  // handle: analytics.page('category', 'name', properties, options, callback)
   const resolvedCallback = args.find(isFunction) as Callback | undefined
 
   // handle:
@@ -106,22 +89,43 @@ export function resolvePageArguments(
   // - analytics.page('category', 'name', properties, options, callback)
   // - analytics.page('category', 'name', callback)
   // - analytics.page(callback), etc
-  args.forEach((obj, argIdx) => {
-    if (isPlainObject(obj)) {
+  args.forEach((arg, argIdx) => {
+    if (isString(arg)) {
       if (argIdx === 0) {
-        resolvedProperties = obj
+        if (isString(arg)) {
+          resolvedName = arg
+        }
+      }
+      if (argIdx === 1) {
+        resolvedCategory = args[0] as string | undefined | null
+        resolvedName = arg
+      }
+    }
+
+    if (isPlainObject(arg)) {
+      if (argIdx === 0) {
+        resolvedProperties = arg
       }
 
       if (argIdx === 1 || argIdx == 2) {
         if (isNil(resolvedProperties)) {
-          resolvedProperties = obj
+          resolvedProperties = arg
         } else {
-          resolvedOptions = obj
+          resolvedOptions = arg
         }
       }
 
       if (argIdx === 3) {
-        resolvedOptions = obj
+        resolvedOptions = arg
+      }
+    }
+
+    if (isNil(arg)) {
+      if (argIdx === 1) {
+        if (isString(args[0])) {
+          resolvedName = arg
+          resolvedCategory = args[0]
+        }
       }
     }
   })