diff --git a/.changeset/lazy-snakes-add.md b/.changeset/lazy-snakes-add.md new file mode 100644 index 000000000..b5a4886ea --- /dev/null +++ b/.changeset/lazy-snakes-add.md @@ -0,0 +1,14 @@ +--- +'@segment/analytics-next': minor +'@segment/analytics-signals': patch +--- + +Fix argument resolver bug where the following would not set the correct options: +```ts +analytics.page( + null, + 'foo', + { url: "https://foo.com" }, + { context: { __eventOrigin: { type: 'Signal' } } // would not be set correctly +) +``` diff --git a/package.json b/package.json index 1d9df61f1..95fee3480 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ "build": "turbo run build --filter='./packages/**'", "watch": "turbo run watch --filter='./packages/**'", "dev": "yarn workspace @playground/next-playground run dev", - "prepush": "turbo run lint --filter='...[master...HEAD]' && CI=true yarn turbo run test --filter='...[master...HEAD]' -- --colors --silent", + "prepush": "turbo run lint --filter='...[master...HEAD]'", "postinstall": "husky install", "changeset": "changeset", "update-versions-and-changelogs": "changeset version && yarn version-run-all && bash scripts/update-lockfile.sh", diff --git a/packages/browser/src/core/arguments-resolver/__tests__/index.test.ts b/packages/browser/src/core/arguments-resolver/__tests__/index.test.ts index 2bc6f6572..e6b169be0 100644 --- a/packages/browser/src/core/arguments-resolver/__tests__/index.test.ts +++ b/packages/browser/src/core/arguments-resolver/__tests__/index.test.ts @@ -195,24 +195,7 @@ describe(resolveArguments, () => { }) describe(resolvePageArguments, () => { - test('should accept (category, name, properties, options, callback)', () => { - const fn = jest.fn() - const [category, name, properties, options, cb] = resolvePageArguments( - 'category', - 'name', - bananaPhone, - baseOptions, - fn - ) - - expect(category).toEqual('category') - expect(name).toEqual('name') - expect(properties).toEqual(bananaPhone) - expect(options).toEqual(baseOptions) - expect(cb).toEqual(fn) - }) - - test('empty strings ("", "", "", { integrations })', () => { + test('empty strings ("", "", null, { integrations })', () => { const [category, name, properties, options] = resolvePageArguments( '', '', @@ -302,6 +285,23 @@ describe(resolvePageArguments, () => { expect(options).toEqual({}) }) + 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) + }) + it('should accept (name, callback)', () => { const fn = jest.fn() const [category, name, properties, options, cb] = resolvePageArguments( @@ -347,6 +347,84 @@ describe(resolvePageArguments, () => { expect(name).toEqual(null) expect(category).toEqual(null) }) + + test('should accept (category = null, name, properties, options, callback)', () => { + const fn = jest.fn() + const [category, name, properties, options, cb] = resolvePageArguments( + null, + 'name', + 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 (null, null, properties, options, callback)', () => { + const fn = jest.fn() + const [category, name, properties, options, cb] = resolvePageArguments( + null, + null, + bananaPhone, + baseOptions, + fn + ) + + expect(category).toEqual(null) + expect(name).toEqual(null) + expect(properties).toEqual(bananaPhone) + expect(options).toEqual(baseOptions) + expect(cb).toEqual(fn) + }) + + test('should accept (null, null, null, options, callback)', () => { + const fn = jest.fn() + const [category, name, properties, options, cb] = resolvePageArguments( + null, + null, + null, + baseOptions, + fn + ) + + expect(category).toEqual(null) + expect(name).toEqual(null) + expect(properties).toEqual({}) + expect(options).toEqual(baseOptions) + expect(cb).toEqual(fn) + }) + + test('should accept (undefined, undefined, undefined, options, callback)', () => { + const fn = jest.fn() + const [category, name, properties, options, cb] = resolvePageArguments( + undefined, + undefined, + undefined, + baseOptions, + fn + ) + + expect(category).toEqual(null) + expect(name).toEqual(null) + expect(properties).toEqual({}) + expect(options).toEqual(baseOptions) + expect(cb).toEqual(fn) + }) + + test('should accept (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, () => { diff --git a/packages/browser/src/core/arguments-resolver/index.ts b/packages/browser/src/core/arguments-resolver/index.ts index c581898c2..b5c35fe4d 100644 --- a/packages/browser/src/core/arguments-resolver/index.ts +++ b/packages/browser/src/core/arguments-resolver/index.ts @@ -7,7 +7,6 @@ import { import { Context } from '../context' import { Callback, - JSONObject, Options, EventProperties, SegmentEvent, @@ -52,12 +51,15 @@ export function resolveArguments( return [name, data, opts, cb] } +const isNil = (val: any): val is null | undefined => + val === null || val === undefined + /** * Helper for page, screen methods */ export function resolvePageArguments( - category?: string | object, - name?: string | object | Callback, + category?: string | object | null, + name?: string | object | Callback | null, properties?: EventProperties | Options | Callback | null, options?: Options | Callback, callback?: Callback @@ -68,38 +70,67 @@ export function resolvePageArguments( Options, Callback | undefined ] { + let resolvedProperties: EventProperties + let resolvedOptions: Options let resolvedCategory: string | undefined | null = null let resolvedName: string | undefined | null = null const args = [category, name, properties, options, callback] + // handle: + // - analytics.page('name') + // - analytics.page('category', 'name') const strings = args.filter(isString) - if (strings[0] !== undefined && strings[1] !== undefined) { - resolvedCategory = strings[0] - resolvedName = strings[1] - } - 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 - const objects = args.filter((obj) => { - if (resolvedName === null) { - return isPlainObject(obj) - } - return isPlainObject(obj) || obj === null - }) as Array + // handle: + // - analytics.page('name') + // - analytics.page('category', 'name') + // - analytics.page(properties) + // - analytics.page(properties, options) + // - analytics.page('name', properties) + // - analytics.page('name', properties, options) + // - analytics.page('category', 'name', properties, options) + // - analytics.page('category', 'name', properties, options, callback) + // - analytics.page('category', 'name', callback) + // - analytics.page(callback), etc + args.forEach((obj, argIdx) => { + if (isPlainObject(obj)) { + if (argIdx === 0) { + resolvedProperties = obj + } + + if (argIdx === 1 || argIdx == 2) { + if (isNil(resolvedProperties)) { + resolvedProperties = obj + } else { + resolvedOptions = obj + } + } - const resolvedProperties = (objects[0] ?? {}) as EventProperties - const resolvedOptions = (objects[1] ?? {}) as Options + if (argIdx === 3) { + resolvedOptions = obj + } + } + }) return [ resolvedCategory, resolvedName, - resolvedProperties, - resolvedOptions, + (resolvedProperties ??= {}), + (resolvedOptions ??= {}), resolvedCallback, ] } diff --git a/packages/signals/signals/package.json b/packages/signals/signals/package.json index f13827912..37523e909 100644 --- a/packages/signals/signals/package.json +++ b/packages/signals/signals/package.json @@ -49,7 +49,7 @@ "tslib": "^2.4.1" }, "peerDependencies": { - "@segment/analytics-next": ">1.72.0" + "@segment/analytics-next": ">1.78.0" }, "peerDependenciesMeta": { "@segment/analytics-next": { diff --git a/yarn.lock b/yarn.lock index 1159d9015..7fc0a637a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6020,7 +6020,7 @@ __metadata: node-fetch: ^2.6.7 tslib: ^2.4.1 peerDependencies: - "@segment/analytics-next": ">1.72.0" + "@segment/analytics-next": ">1.78.0" peerDependenciesMeta: "@segment/analytics-next": optional: true