From 96130f298bc51465bb7b471c70446f549a00aeeb Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Mon, 14 Jul 2025 14:47:04 -0500 Subject: [PATCH 1/4] update to handle new page arg order --- .../all-segment-events.test.ts | 2 +- .../sandbox-analytics-runtime.test.ts | 460 ++++++++++++++++++ .../src/core/processor/arg-resolvers.ts | 15 - .../src/core/processor/argument-types.ts | 55 +++ .../processor/sandbox-analytics-runtime.ts | 128 +++++ .../signals/src/core/processor/sandbox.ts | 125 +---- 6 files changed, 651 insertions(+), 134 deletions(-) create mode 100644 packages/signals/signals/src/core/processor/__tests__/sandbox-analytics-runtime.test.ts delete mode 100644 packages/signals/signals/src/core/processor/arg-resolvers.ts create mode 100644 packages/signals/signals/src/core/processor/argument-types.ts create mode 100644 packages/signals/signals/src/core/processor/sandbox-analytics-runtime.ts diff --git a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/all-segment-events.test.ts b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/all-segment-events.test.ts index 188cdaf28..e7b6a994d 100644 --- a/packages/signals/signals-integration-tests/src/tests/signals-vanilla/all-segment-events.test.ts +++ b/packages/signals/signals-integration-tests/src/tests/signals-vanilla/all-segment-events.test.ts @@ -43,7 +43,7 @@ test('Segment events', async ({ page }) => { analytics.group('foo', { hello: 'world' }) analytics.alias('john', 'johnsmith') analytics.track('a track call', {foo: 'bar'}) - analytics.page('Retail Page', 'Home', { url: 'http://my-home.com', title: 'Some Title' }); + analytics.page('Home', 'Retail Page', { url: 'http://my-home.com', title: 'Some Title' }); } }` diff --git a/packages/signals/signals/src/core/processor/__tests__/sandbox-analytics-runtime.test.ts b/packages/signals/signals/src/core/processor/__tests__/sandbox-analytics-runtime.test.ts new file mode 100644 index 000000000..ed7e1f61a --- /dev/null +++ b/packages/signals/signals/src/core/processor/__tests__/sandbox-analytics-runtime.test.ts @@ -0,0 +1,460 @@ +import { AnalyticsRuntime } from '../sandbox-analytics-runtime' + +describe('AnalyticsRuntime', () => { + let analyticsRuntime: AnalyticsRuntime + + beforeEach(() => { + analyticsRuntime = new AnalyticsRuntime() + // Spy on console.error to prevent test output pollution + jest.spyOn(console, 'error').mockImplementation(() => {}) + }) + + afterEach(() => { + jest.restoreAllMocks() + }) + + describe('initialization', () => { + it('should initialize with empty calls object', () => { + const calls = analyticsRuntime.getCalls() + expect(calls).toEqual({ + page: [], + identify: [], + track: [], + alias: [], + screen: [], + group: [], + reset: [], + }) + }) + }) + + describe('track method', () => { + it('should record track calls with all parameters', () => { + const name = 'Button Clicked' + const properties = { buttonId: 'cta-button', color: 'blue' } + const context = { ip: '127.0.0.1' } + + analyticsRuntime.track(name, properties, context) + + const calls = analyticsRuntime.getCalls() + expect(calls.track).toHaveLength(1) + expect(calls.track[0]).toEqual([ + name, + properties, + { context: { ...context, __eventOrigin: { type: 'Signal' } } }, + ]) + }) + + it('should record track calls with minimal parameters', () => { + analyticsRuntime.track('Event Name', undefined, undefined) + + const calls = analyticsRuntime.getCalls() + expect(calls.track).toHaveLength(1) + expect(calls.track[0]).toEqual(['Event Name', undefined, {}]) + }) + + it('should handle multiple track calls', () => { + analyticsRuntime.track('Event 1', { prop: 'value1' }, undefined) + analyticsRuntime.track('Event 2', { prop: 'value2' }, undefined) + + const calls = analyticsRuntime.getCalls() + expect(calls.track).toHaveLength(2) + expect(calls.track[0][0]).toBe('Event 1') + expect(calls.track[1][0]).toBe('Event 2') + }) + + it('should handle errors gracefully', () => { + const consoleSpy = jest.spyOn(console, 'error') + + // Mock createOptions to throw an error + const originalCreateOptions = (analyticsRuntime as any).createOptions + ;(analyticsRuntime as any).createOptions = jest.fn(() => { + throw new Error('Test error') + }) + + analyticsRuntime.track('Event Name', {}, {}) + + expect(consoleSpy).toHaveBeenCalledWith(expect.any(Error)) + + // Restore original method + ;(analyticsRuntime as any).createOptions = originalCreateOptions + }) + }) + + describe('identify method', () => { + it('should record identify calls with all parameters', () => { + const userId = 'user-123' + const traits = { email: 'user@example.com', name: 'John Doe' } + const context = { userAgent: 'Chrome' } + + analyticsRuntime.identify(userId, traits, context) + + const calls = analyticsRuntime.getCalls() + expect(calls.identify).toHaveLength(1) + expect(calls.identify[0]).toEqual([ + userId, + traits, + { context: { ...context, __eventOrigin: { type: 'Signal' } } }, + ]) + }) + + it('should record identify calls with undefined values', () => { + analyticsRuntime.identify(undefined, undefined, undefined) + + const calls = analyticsRuntime.getCalls() + expect(calls.identify).toHaveLength(1) + expect(calls.identify[0]).toEqual([undefined, undefined, {}]) + }) + + it('should handle errors gracefully', () => { + const consoleSpy = jest.spyOn(console, 'error') + + // Force an error by making calls.identify.push throw + const originalPush = analyticsRuntime.getCalls().identify.push + analyticsRuntime.getCalls().identify.push = jest.fn(() => { + throw new Error('Test error') + }) + + analyticsRuntime.identify('user-123', {}, {}) + + expect(consoleSpy).toHaveBeenCalledWith(expect.any(Error)) + + // Restore original method + analyticsRuntime.getCalls().identify.push = originalPush + }) + }) + + describe('alias method', () => { + it('should record alias calls with all parameters', () => { + const userId = 'new-user-id' + const previousId = 'anonymous-id' + const context = { source: 'mobile' } + + analyticsRuntime.alias(userId, previousId, context) + + const calls = analyticsRuntime.getCalls() + expect(calls.alias).toHaveLength(1) + expect(calls.alias[0]).toEqual([ + userId, + previousId, + { context: { ...context, __eventOrigin: { type: 'Signal' } } }, + ]) + }) + + it('should record alias calls with undefined previousId', () => { + analyticsRuntime.alias('new-user-id', undefined, undefined) + + const calls = analyticsRuntime.getCalls() + expect(calls.alias).toHaveLength(1) + expect(calls.alias[0]).toEqual(['new-user-id', undefined, {}]) + }) + + it('should handle errors gracefully', () => { + const consoleSpy = jest.spyOn(console, 'error') + + // Force an error + const originalPush = analyticsRuntime.getCalls().alias.push + analyticsRuntime.getCalls().alias.push = jest.fn(() => { + throw new Error('Test error') + }) + + analyticsRuntime.alias('user-id', 'prev-id', {}) + + expect(consoleSpy).toHaveBeenCalledWith(expect.any(Error)) + + // Restore original method + analyticsRuntime.getCalls().alias.push = originalPush + }) + }) + + describe('group method', () => { + it('should record group calls with all parameters', () => { + const groupId = 'group-123' + const traits = { name: 'Acme Inc', plan: 'enterprise' } + const context = { library: 'signals' } + + analyticsRuntime.group(groupId, traits, context) + + const calls = analyticsRuntime.getCalls() + expect(calls.group).toHaveLength(1) + expect(calls.group[0]).toEqual([ + groupId, + traits, + { context: { ...context, __eventOrigin: { type: 'Signal' } } }, + ]) + }) + + it('should record group calls with undefined values', () => { + analyticsRuntime.group(undefined, undefined, undefined) + + const calls = analyticsRuntime.getCalls() + expect(calls.group).toHaveLength(1) + expect(calls.group[0]).toEqual([undefined, undefined, {}]) + }) + + it('should handle errors gracefully', () => { + const consoleSpy = jest.spyOn(console, 'error') + + // Force an error + const originalPush = analyticsRuntime.getCalls().group.push + analyticsRuntime.getCalls().group.push = jest.fn(() => { + throw new Error('Test error') + }) + + analyticsRuntime.group('group-id', {}, {}) + + expect(consoleSpy).toHaveBeenCalledWith(expect.any(Error)) + + // Restore original method + analyticsRuntime.getCalls().group.push = originalPush + }) + }) + + describe('page method', () => { + it('should record page calls with all parameters', () => { + const name = 'Home' + const category = 'Website' + const properties = { url: '/home', title: 'Home Page' } + const context = { referrer: '/landing' } + + analyticsRuntime.page(name, category, properties, context) + + const calls = analyticsRuntime.getCalls() + expect(calls.page).toHaveLength(1) + expect(calls.page[0]).toEqual([ + category, + name, + properties, + { context: { ...context, __eventOrigin: { type: 'Signal' } } }, + ]) + }) + + it('should handle legacy behavior when name is undefined but category is provided', () => { + const category = 'Website' + const properties = { url: '/page' } + + analyticsRuntime.page(undefined, category, properties, undefined) + + const calls = analyticsRuntime.getCalls() + expect(calls.page).toHaveLength(1) + expect(calls.page[0]).toEqual([ + category, + '', // name defaults to empty string + properties, + {}, + ]) + }) + + it('should preserve name when both name and category are provided', () => { + analyticsRuntime.page('Page Name', 'Category', {}, undefined) + + const calls = analyticsRuntime.getCalls() + expect(calls.page[0][1]).toBe('Page Name') + }) + + it('should preserve undefined name when category is also undefined', () => { + analyticsRuntime.page(undefined, undefined, {}, undefined) + + const calls = analyticsRuntime.getCalls() + expect(calls.page[0][1]).toBeUndefined() + }) + + it('should handle errors gracefully', () => { + const consoleSpy = jest.spyOn(console, 'error') + + // Force an error + const originalPush = analyticsRuntime.getCalls().page.push + analyticsRuntime.getCalls().page.push = jest.fn(() => { + throw new Error('Test error') + }) + + analyticsRuntime.page('Page', 'Category', {}, {}) + + expect(consoleSpy).toHaveBeenCalledWith(expect.any(Error)) + + // Restore original method + analyticsRuntime.getCalls().page.push = originalPush + }) + }) + + describe('screen method', () => { + it('should record screen calls with all parameters', () => { + const name = 'Login Screen' + const category = 'Authentication' + const properties = { version: '2.0', experiment: 'new-ui' } + const context = { app: { version: '1.2.3' } } + + analyticsRuntime.screen(name, category, properties, context) + + const calls = analyticsRuntime.getCalls() + expect(calls.screen).toHaveLength(1) + expect(calls.screen[0]).toEqual([ + category, + name, + properties, + { context: { ...context, __eventOrigin: { type: 'Signal' } } }, + ]) + }) + + it('should handle legacy behavior when name is undefined but category is provided', () => { + const category = 'Mobile' + const properties = { screenId: 'main' } + + analyticsRuntime.screen(undefined, category, properties, undefined) + + const calls = analyticsRuntime.getCalls() + expect(calls.screen).toHaveLength(1) + expect(calls.screen[0]).toEqual([ + category, + '', // name defaults to empty string + properties, + {}, + ]) + }) + + it('should preserve name when both name and category are provided', () => { + analyticsRuntime.screen('Screen Name', 'Category', {}, undefined) + + const calls = analyticsRuntime.getCalls() + expect(calls.screen[0][1]).toBe('Screen Name') + }) + + it('should handle errors gracefully', () => { + const consoleSpy = jest.spyOn(console, 'error') + + // Force an error + const originalPush = analyticsRuntime.getCalls().screen.push + analyticsRuntime.getCalls().screen.push = jest.fn(() => { + throw new Error('Test error') + }) + + analyticsRuntime.screen('Screen', 'Category', {}, {}) + + expect(consoleSpy).toHaveBeenCalledWith(expect.any(Error)) + + // Restore original method + analyticsRuntime.getCalls().screen.push = originalPush + }) + }) + + describe('reset method', () => { + it('should record reset calls', () => { + analyticsRuntime.reset() + + const calls = analyticsRuntime.getCalls() + expect(calls.reset).toHaveLength(1) + expect(calls.reset[0]).toEqual([]) + }) + + it('should record multiple reset calls', () => { + analyticsRuntime.reset() + analyticsRuntime.reset() + + const calls = analyticsRuntime.getCalls() + expect(calls.reset).toHaveLength(2) + }) + }) + + describe('createOptions method', () => { + it('should return empty object when context is undefined', () => { + const result = (analyticsRuntime as any).createOptions(undefined) + expect(result).toEqual({}) + }) + + it('should return empty object when context is null', () => { + const result = (analyticsRuntime as any).createOptions(null) + expect(result).toEqual({}) + }) + + it('should add __eventOrigin to context', () => { + const context = { userId: '123', custom: 'value' } + const result = (analyticsRuntime as any).createOptions(context) + + expect(result).toEqual({ + context: { + userId: '123', + custom: 'value', + __eventOrigin: { type: 'Signal' }, + }, + }) + }) + + it('should preserve existing context properties', () => { + const context = { + ip: '127.0.0.1', + userAgent: 'Chrome', + nested: { prop: 'value' }, + } + const result = (analyticsRuntime as any).createOptions(context) + + expect(result.context.ip).toBe('127.0.0.1') + expect(result.context.userAgent).toBe('Chrome') + expect(result.context.nested).toEqual({ prop: 'value' }) + expect(result.context.__eventOrigin).toEqual({ type: 'Signal' }) + }) + + it('should not mutate the original context object', () => { + const originalContext = { userId: '123' } + const contextCopy = { ...originalContext } + + ;(analyticsRuntime as any).createOptions(originalContext) + + expect(originalContext).toEqual(contextCopy) + expect((originalContext as any).__eventOrigin).toBeUndefined() + }) + }) + + describe('method calls isolation', () => { + it('should maintain separate call arrays for each method', () => { + analyticsRuntime.track('event', {}, {}) + analyticsRuntime.identify('user', {}, {}) + analyticsRuntime.page('page', 'category', {}, {}) + analyticsRuntime.screen('screen', 'category', {}, {}) + analyticsRuntime.group('group', {}, {}) + analyticsRuntime.alias('new', 'old', {}) + analyticsRuntime.reset() + + const calls = analyticsRuntime.getCalls() + expect(calls.track).toHaveLength(1) + expect(calls.identify).toHaveLength(1) + expect(calls.page).toHaveLength(1) + expect(calls.screen).toHaveLength(1) + expect(calls.group).toHaveLength(1) + expect(calls.alias).toHaveLength(1) + expect(calls.reset).toHaveLength(1) + }) + + it('should return the same calls object reference', () => { + const calls1 = analyticsRuntime.getCalls() + const calls2 = analyticsRuntime.getCalls() + + expect(calls1).toBe(calls2) + }) + }) + + describe('bound methods behavior', () => { + it('should maintain context when methods are destructured', () => { + const { track, identify, page } = analyticsRuntime + + track('event', {}, {}) + identify('user', {}, {}) + page('page', 'category', {}, {}) + + const calls = analyticsRuntime.getCalls() + expect(calls.track).toHaveLength(1) + expect(calls.identify).toHaveLength(1) + expect(calls.page).toHaveLength(1) + }) + + it('should work when methods are called with different context', () => { + const track = analyticsRuntime.track + const otherObject = { track } + + otherObject.track('event', {}, {}) + + const calls = analyticsRuntime.getCalls() + expect(calls.track).toHaveLength(1) + expect(calls.track[0][0]).toBe('event') + }) + }) +}) diff --git a/packages/signals/signals/src/core/processor/arg-resolvers.ts b/packages/signals/signals/src/core/processor/arg-resolvers.ts deleted file mode 100644 index 74abdb901..000000000 --- a/packages/signals/signals/src/core/processor/arg-resolvers.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { - resolveAliasArguments, - resolveArguments, - resolvePageArguments, - resolveUserArguments, -} from '@segment/analytics-next' - -export const resolvers = { - resolveAliasArguments, - resolveArguments, - resolvePageArguments, - resolveUserArguments: resolveUserArguments({ - id: () => undefined, - }), -} diff --git a/packages/signals/signals/src/core/processor/argument-types.ts b/packages/signals/signals/src/core/processor/argument-types.ts new file mode 100644 index 000000000..f54bba8a0 --- /dev/null +++ b/packages/signals/signals/src/core/processor/argument-types.ts @@ -0,0 +1,55 @@ +/** + * analytics.track("name", { prop1: "value" }, {fooCtx: '123'}); + */ +export type TrackArgs = [ + name: string, + properties: Record | undefined, + context: Record | undefined +] + +/** + * analytics.identify("userId", { trait1: "value" }, {fooCtx: '123'}); + */ +export type IdentifyArgs = [ + userId: string | undefined, + traits: Record | undefined, + context: Record | undefined +] + +/** + * analytics.page("name", "category", { prop1: "value" }, {fooCtx: '123'}) + */ +export type PageArgs = [ + name: string | undefined, + category: string | undefined, + properties: Record | undefined, + context: Record | undefined +] + +/** + * analytics.screen("name", "category", { prop1: "value" }, {fooCtx: '123'}); + */ +export type ScreenArgs = [ + name: string | undefined, + category: string | undefined, + properties: Record | undefined, + context: Record | undefined +] + +/** + * analytics.group("groupId", { trait1: "value" }, {fooCtx: '123'}); + */ +export type GroupArgs = [ + groupId: string | undefined, + traits: Record | undefined, + context: Record | undefined +] + +/** + * analytics.alias("userId", "previousId", {fooCtx: '123'}); + */ +export type AliasArgs = [ + userId: string, + previousId: string | undefined, // from + context: Record | undefined +] diff --git a/packages/signals/signals/src/core/processor/sandbox-analytics-runtime.ts b/packages/signals/signals/src/core/processor/sandbox-analytics-runtime.ts new file mode 100644 index 000000000..c82ad3c90 --- /dev/null +++ b/packages/signals/signals/src/core/processor/sandbox-analytics-runtime.ts @@ -0,0 +1,128 @@ +import { AnalyticsRuntimePublicApi } from '../../types' +import { + TrackArgs, + IdentifyArgs, + AliasArgs, + GroupArgs, + PageArgs, + ScreenArgs, +} from './argument-types' + +export type MethodName = + | 'page' + | 'identify' + | 'track' + | 'alias' + | 'screen' + | 'group' +/** + * Buffer of any analytics calls made during the processing of a signal + */ +export type AnalyticsMethodCalls = Record & { + reset: unknown[] +} +/** + * Proxy around the analytics client + */ +export class AnalyticsRuntime implements AnalyticsRuntimePublicApi { + private calls: AnalyticsMethodCalls = { + page: [], + identify: [], + track: [], + alias: [], + screen: [], + group: [], + reset: [], + } + + getCalls(): AnalyticsMethodCalls { + return this.calls + } + + /** + * Stamp the context with the event origin to prevent infinite signal-event loops. + */ + private createOptions(context?: Record): Record { + if (!context) { + return {} + } + return { + context: { ...context, __eventOrigin: { type: 'Signal' } }, + } + } + + // these methods need to be bound to the instance, rather than the prototype, in order to serialize correctly in the sandbox. + track = (...args: TrackArgs) => { + const [name, properties, context] = args + try { + this.calls.track.push([name, properties, this.createOptions(context)]) + } catch (err) { + // wrapping all methods in a try/catch because throwing an error won't cause the error to surface inside of workerboxjs + console.error(err) + } + } + + identify = (...args: IdentifyArgs) => { + try { + // @ts-ignore + const [id, traits, context] = args + this.calls.identify.push([id, traits, this.createOptions(context)]) + } catch (err) { + console.error(err) + } + } + + alias = (...args: AliasArgs) => { + try { + const [userId, previousId, context] = args + this.calls.alias.push([userId, previousId, this.createOptions(context)]) + } catch (err) { + console.error(err) + } + } + group = (...args: GroupArgs) => { + try { + // @ts-ignore + const [id, traits, context] = args + this.calls.group.push([id, traits, this.createOptions(context)]) + } catch (err) { + console.error(err) + } + } + + page = (...args: PageArgs) => { + try { + const [name, category, props, context] = args + // If name is not provided, but category is, we default to an empty string + // This is a legacy behavior from the argument resolver + const nameStr = !name && category ? '' : name + this.calls.page.push([ + category, + nameStr, + props, + this.createOptions(context), + ]) + } catch (err) { + console.error(err) + } + } + + screen = (...args: ScreenArgs) => { + try { + const [name, category, props, context] = args + const nameStr = !name && category ? '' : name + this.calls.screen.push([ + category, + nameStr, + props, + this.createOptions(context), + ]) + } catch (err) { + console.error(err) + } + } + + reset = () => { + this.calls.reset.push([]) + } +} diff --git a/packages/signals/signals/src/core/processor/sandbox.ts b/packages/signals/signals/src/core/processor/sandbox.ts index 957790cbf..30c21b1b7 100644 --- a/packages/signals/signals/src/core/processor/sandbox.ts +++ b/packages/signals/signals/src/core/processor/sandbox.ts @@ -1,129 +1,18 @@ import { logger } from '../../lib/logger' import { createWorkerBox, WorkerBoxAPI } from '../../lib/workerbox' -import { resolvers } from './arg-resolvers' -import { AnalyticsRuntimePublicApi, ProcessSignal } from '../../types' +import { ProcessSignal } from '../../types' import { replaceBaseUrl } from '../../lib/replace-base-url' import { Signal, WebSignalsRuntime } from '@segment/analytics-signals-runtime' import { getRuntimeCode } from '@segment/analytics-signals-runtime' import { polyfills } from './polyfills' import { loadScript } from '../../lib/load-script' +import { + AnalyticsMethodCalls, + AnalyticsRuntime, + MethodName, +} from './sandbox-analytics-runtime' -export type MethodName = - | 'page' - | 'identify' - | 'track' - | 'alias' - | 'screen' - | 'group' - -/** - * Buffer of any analytics calls made during the processing of a signal - */ -export type AnalyticsMethodCalls = Record & { - reset: unknown[] -} - -/** - * Proxy around the analytics client - */ -class AnalyticsRuntime implements AnalyticsRuntimePublicApi { - private calls: AnalyticsMethodCalls = { - page: [], - identify: [], - track: [], - alias: [], - screen: [], - group: [], - reset: [], - } - - getCalls(): AnalyticsMethodCalls { - return this.calls - } - - /** - * Stamp the context with the event origin to prevent infinite signal-event loops. - */ - private stamp(options: Record): Record { - if (!options) { - options = {} - } - options.context = { ...options.context, __eventOrigin: { type: 'Signal' } } - return options - } - - // these methods need to be bound to the instance, rather than the prototype, in order to serialize correctly in the sandbox. - track = (...args: any[]) => { - try { - // @ts-ignore - const [eventName, props, options, cb] = resolvers.resolveArguments( - // @ts-ignore - ...args - ) - this.calls.track.push([eventName, props, this.stamp(options), cb]) - } catch (err) { - // wrapping all methods in a try/catch because throwing an error won't cause the error to surface inside of workerboxjs - console.error(err) - } - } - - identify = (...args: any[]) => { - try { - // @ts-ignore - const [id, traits, options, cb] = resolvers.resolveUserArguments(...args) - this.calls.identify.push([id, traits, this.stamp(options), cb]) - } catch (err) { - console.error(err) - } - } - - alias = (...args: any[]) => { - try { - const [userId, previousId, options, cb] = resolvers.resolveAliasArguments( - // @ts-ignore - ...args - ) - this.calls.alias.push([userId, previousId, this.stamp(options), cb]) - } catch (err) { - console.error(err) - } - } - group = (...args: any[]) => { - try { - // @ts-ignore - const [id, traits, options, cb] = resolvers.resolveUserArguments(...args) - this.calls.group.push([id, traits, this.stamp(options), cb]) - } catch (err) { - console.error(err) - } - } - - page = (...args: any[]) => { - try { - const [category, name, props, options, cb] = - resolvers.resolvePageArguments(...args) - this.stamp(options) - this.calls.page.push([category, name, props, this.stamp(options), cb]) - } catch (err) { - console.error(err) - } - } - - screen = (...args: any[]) => { - try { - const [category, name, props, options, cb] = - resolvers.resolvePageArguments(...args) - this.stamp(options) - this.calls.screen.push([category, name, props, this.stamp(options), cb]) - } catch (err) { - console.error(err) - } - } - - reset = () => { - this.calls.reset.push([]) - } -} +export type { AnalyticsMethodCalls, MethodName } interface CodeSandbox { run: (fn: string, scope: Record) => Promise From 153d8d6946519d8f3613928610e5669b080f84da Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Mon, 14 Jul 2025 15:06:25 -0500 Subject: [PATCH 2/4] add origin --- .../sandbox-analytics-runtime.test.ts | 115 ++++++------------ .../processor/sandbox-analytics-runtime.ts | 3 - 2 files changed, 40 insertions(+), 78 deletions(-) diff --git a/packages/signals/signals/src/core/processor/__tests__/sandbox-analytics-runtime.test.ts b/packages/signals/signals/src/core/processor/__tests__/sandbox-analytics-runtime.test.ts index ed7e1f61a..3d731313c 100644 --- a/packages/signals/signals/src/core/processor/__tests__/sandbox-analytics-runtime.test.ts +++ b/packages/signals/signals/src/core/processor/__tests__/sandbox-analytics-runtime.test.ts @@ -50,7 +50,11 @@ describe('AnalyticsRuntime', () => { const calls = analyticsRuntime.getCalls() expect(calls.track).toHaveLength(1) - expect(calls.track[0]).toEqual(['Event Name', undefined, {}]) + expect(calls.track[0]).toEqual([ + 'Event Name', + undefined, + { context: { __eventOrigin: { type: 'Signal' } } }, + ]) }) it('should handle multiple track calls', () => { @@ -62,23 +66,6 @@ describe('AnalyticsRuntime', () => { expect(calls.track[0][0]).toBe('Event 1') expect(calls.track[1][0]).toBe('Event 2') }) - - it('should handle errors gracefully', () => { - const consoleSpy = jest.spyOn(console, 'error') - - // Mock createOptions to throw an error - const originalCreateOptions = (analyticsRuntime as any).createOptions - ;(analyticsRuntime as any).createOptions = jest.fn(() => { - throw new Error('Test error') - }) - - analyticsRuntime.track('Event Name', {}, {}) - - expect(consoleSpy).toHaveBeenCalledWith(expect.any(Error)) - - // Restore original method - ;(analyticsRuntime as any).createOptions = originalCreateOptions - }) }) describe('identify method', () => { @@ -103,7 +90,11 @@ describe('AnalyticsRuntime', () => { const calls = analyticsRuntime.getCalls() expect(calls.identify).toHaveLength(1) - expect(calls.identify[0]).toEqual([undefined, undefined, {}]) + expect(calls.identify[0]).toEqual([ + undefined, + undefined, + { context: { __eventOrigin: { type: 'Signal' } } }, + ]) }) it('should handle errors gracefully', () => { @@ -146,7 +137,11 @@ describe('AnalyticsRuntime', () => { const calls = analyticsRuntime.getCalls() expect(calls.alias).toHaveLength(1) - expect(calls.alias[0]).toEqual(['new-user-id', undefined, {}]) + expect(calls.alias[0]).toEqual([ + 'new-user-id', + undefined, + { context: { __eventOrigin: { type: 'Signal' } } }, + ]) }) it('should handle errors gracefully', () => { @@ -189,7 +184,11 @@ describe('AnalyticsRuntime', () => { const calls = analyticsRuntime.getCalls() expect(calls.group).toHaveLength(1) - expect(calls.group[0]).toEqual([undefined, undefined, {}]) + expect(calls.group[0]).toEqual([ + undefined, + undefined, + { context: { __eventOrigin: { type: 'Signal' } } }, + ]) }) it('should handle errors gracefully', () => { @@ -241,7 +240,7 @@ describe('AnalyticsRuntime', () => { category, '', // name defaults to empty string properties, - {}, + { context: { __eventOrigin: { type: 'Signal' } } }, ]) }) @@ -249,14 +248,24 @@ describe('AnalyticsRuntime', () => { analyticsRuntime.page('Page Name', 'Category', {}, undefined) const calls = analyticsRuntime.getCalls() - expect(calls.page[0][1]).toBe('Page Name') + expect(calls.page[0]).toEqual([ + 'Category', + 'Page Name', + {}, + { context: { __eventOrigin: { type: 'Signal' } } }, + ]) }) it('should preserve undefined name when category is also undefined', () => { analyticsRuntime.page(undefined, undefined, {}, undefined) const calls = analyticsRuntime.getCalls() - expect(calls.page[0][1]).toBeUndefined() + expect(calls.page[0]).toEqual([ + undefined, + undefined, + {}, + { context: { __eventOrigin: { type: 'Signal' } } }, + ]) }) it('should handle errors gracefully', () => { @@ -308,7 +317,7 @@ describe('AnalyticsRuntime', () => { category, '', // name defaults to empty string properties, - {}, + { context: { __eventOrigin: { type: 'Signal' } } }, ]) }) @@ -316,7 +325,12 @@ describe('AnalyticsRuntime', () => { analyticsRuntime.screen('Screen Name', 'Category', {}, undefined) const calls = analyticsRuntime.getCalls() - expect(calls.screen[0][1]).toBe('Screen Name') + expect(calls.screen[0]).toEqual([ + 'Category', + 'Screen Name', + {}, + { context: { __eventOrigin: { type: 'Signal' } } }, + ]) }) it('should handle errors gracefully', () => { @@ -355,55 +369,6 @@ describe('AnalyticsRuntime', () => { }) }) - describe('createOptions method', () => { - it('should return empty object when context is undefined', () => { - const result = (analyticsRuntime as any).createOptions(undefined) - expect(result).toEqual({}) - }) - - it('should return empty object when context is null', () => { - const result = (analyticsRuntime as any).createOptions(null) - expect(result).toEqual({}) - }) - - it('should add __eventOrigin to context', () => { - const context = { userId: '123', custom: 'value' } - const result = (analyticsRuntime as any).createOptions(context) - - expect(result).toEqual({ - context: { - userId: '123', - custom: 'value', - __eventOrigin: { type: 'Signal' }, - }, - }) - }) - - it('should preserve existing context properties', () => { - const context = { - ip: '127.0.0.1', - userAgent: 'Chrome', - nested: { prop: 'value' }, - } - const result = (analyticsRuntime as any).createOptions(context) - - expect(result.context.ip).toBe('127.0.0.1') - expect(result.context.userAgent).toBe('Chrome') - expect(result.context.nested).toEqual({ prop: 'value' }) - expect(result.context.__eventOrigin).toEqual({ type: 'Signal' }) - }) - - it('should not mutate the original context object', () => { - const originalContext = { userId: '123' } - const contextCopy = { ...originalContext } - - ;(analyticsRuntime as any).createOptions(originalContext) - - expect(originalContext).toEqual(contextCopy) - expect((originalContext as any).__eventOrigin).toBeUndefined() - }) - }) - describe('method calls isolation', () => { it('should maintain separate call arrays for each method', () => { analyticsRuntime.track('event', {}, {}) diff --git a/packages/signals/signals/src/core/processor/sandbox-analytics-runtime.ts b/packages/signals/signals/src/core/processor/sandbox-analytics-runtime.ts index c82ad3c90..950e45063 100644 --- a/packages/signals/signals/src/core/processor/sandbox-analytics-runtime.ts +++ b/packages/signals/signals/src/core/processor/sandbox-analytics-runtime.ts @@ -43,9 +43,6 @@ export class AnalyticsRuntime implements AnalyticsRuntimePublicApi { * Stamp the context with the event origin to prevent infinite signal-event loops. */ private createOptions(context?: Record): Record { - if (!context) { - return {} - } return { context: { ...context, __eventOrigin: { type: 'Signal' } }, } From 36fcabd478372a802c04371be34d89042e342e83 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Mon, 14 Jul 2025 15:22:52 -0500 Subject: [PATCH 3/4] update analytics.js --- packages/signals/signals/src/core/processor/argument-types.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/signals/signals/src/core/processor/argument-types.ts b/packages/signals/signals/src/core/processor/argument-types.ts index f54bba8a0..73c3688a2 100644 --- a/packages/signals/signals/src/core/processor/argument-types.ts +++ b/packages/signals/signals/src/core/processor/argument-types.ts @@ -1,3 +1,5 @@ +// https://github.com/segment-integrations/signals-rules-engine/blob/main/packages/rules-engine/src/types/arguments-types.ts + /** * analytics.track("name", { prop1: "value" }, {fooCtx: '123'}); */ From 31854f7522bbdabee2fcc8f85c9329152f078641 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Mon, 14 Jul 2025 15:30:15 -0500 Subject: [PATCH 4/4] add more tests --- .../__tests__/index.test.ts | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) 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 55582da61..f41426f36 100644 --- a/packages/browser/src/core/arguments-resolver/__tests__/index.test.ts +++ b/packages/browser/src/core/arguments-resolver/__tests__/index.test.ts @@ -235,6 +235,30 @@ describe(resolvePageArguments, () => { expect(cb).toEqual(undefined) }) + it('should accept ("name")', () => { + const [category, name, properties, options, cb] = + resolvePageArguments('name') + + expect(name).toEqual('name') + expect(category).toEqual(null) + expect(properties).toEqual({}) + expect(options).toEqual({}) + expect(cb).toEqual(undefined) + }) + + it('should accept ("", "name")', () => { + const [category, name, properties, options, cb] = resolvePageArguments( + '', + 'name' + ) + + expect(name).toEqual('name') + expect(category).toEqual('') + 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(