From 32582be828d112a31a92b16e80b15727c3a36613 Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Tue, 7 Jan 2025 14:55:17 -0600 Subject: [PATCH] Fix query string api not respecting source middleware (#1207) --- .changeset/few-starfishes-bake.md | 5 ++ .../query-string.integration.test.ts | 69 ++++++++++++------- packages/browser/src/browser/index.ts | 39 +++++++---- turbo.json | 2 +- 4 files changed, 75 insertions(+), 40 deletions(-) create mode 100644 .changeset/few-starfishes-bake.md diff --git a/.changeset/few-starfishes-bake.md b/.changeset/few-starfishes-bake.md new file mode 100644 index 000000000..a22ba6362 --- /dev/null +++ b/.changeset/few-starfishes-bake.md @@ -0,0 +1,5 @@ +--- +'@segment/analytics-next': patch +--- + +Fix query string API not respecting source middleware diff --git a/packages/browser/src/browser/__tests__/query-string.integration.test.ts b/packages/browser/src/browser/__tests__/query-string.integration.test.ts index 0007d4cc5..176b69b9e 100644 --- a/packages/browser/src/browser/__tests__/query-string.integration.test.ts +++ b/packages/browser/src/browser/__tests__/query-string.integration.test.ts @@ -1,9 +1,23 @@ import { JSDOM } from 'jsdom' import { Analytics } from '../../core/analytics' -// @ts-ignore loadCDNSettings mocked dependency is accused as unused import { AnalyticsBrowser } from '..' import { setGlobalCDNUrl } from '../../lib/parse-cdn' import { TEST_WRITEKEY } from '../../test-helpers/test-writekeys' +import { createMockFetchImplementation } from '../../test-helpers/fixtures/create-fetch-method' +import { parseFetchCall } from '../../test-helpers/fetch-parse' +import { cdnSettingsKitchenSink } from '../../test-helpers/fixtures/cdn-settings' + +const fetchCalls: ReturnType[] = [] +jest.mock('unfetch', () => { + return { + __esModule: true, + default: (url: RequestInfo, body?: RequestInit) => { + const call = parseFetchCall([url, body]) + fetchCalls.push(call) + return createMockFetchImplementation(cdnSettingsKitchenSink)(url, body) + }, + } +}) const writeKey = TEST_WRITEKEY @@ -11,9 +25,6 @@ describe('queryString', () => { let jsd: JSDOM beforeEach(async () => { - jest.restoreAllMocks() - jest.resetAllMocks() - const html = ` @@ -37,33 +48,41 @@ describe('queryString', () => { setGlobalCDNUrl(undefined as any) }) - it('applies query string logic before analytics is finished initializing', async () => { - let analyticsInitializedBeforeQs: boolean | undefined - const originalQueryString = Analytics.prototype.queryString - const mockQueryString = jest - .fn() - .mockImplementation(async function (this: Analytics, ...args) { - // simulate network latency when retrieving the bundle - await new Promise((r) => setTimeout(r, 500)) - return originalQueryString.apply(this, args).then((result) => { - // ensure analytics has not finished initializing before querystring completes - analyticsInitializedBeforeQs = this.initialized - return result - }) - }) - Analytics.prototype.queryString = mockQueryString + it('querystring events that update anonymousId have priority over other buffered events', async () => { + const queryStringSpy = jest.spyOn(Analytics.prototype, 'queryString') jsd.reconfigure({ url: 'https://localhost/?ajs_aid=123', }) - const [analytics] = await AnalyticsBrowser.load({ writeKey }) - expect(mockQueryString).toHaveBeenCalledWith('?ajs_aid=123') - expect(analyticsInitializedBeforeQs).toBe(false) - // check that calls made immediately after analytics is loaded use correct anonymousId - const pageContext = await analytics.page() + const analytics = new AnalyticsBrowser() + const pagePromise = analytics.page() + await analytics.load({ writeKey }) + expect(queryStringSpy).toHaveBeenCalledWith('?ajs_aid=123') + const pageContext = await pagePromise expect(pageContext.event.anonymousId).toBe('123') - expect(analytics.user().anonymousId()).toBe('123') + const user = await analytics.user() + expect(user.anonymousId()).toBe('123') + }) + + it('querystring events have middleware applied like any other event', async () => { + jsd.reconfigure({ + url: 'https://localhost/?ajs_event=Clicked', + }) + + const analytics = new AnalyticsBrowser() + void analytics.addSourceMiddleware(({ next, payload }) => { + payload.obj.event = payload.obj.event + ' Middleware Applied' + return next(payload) + }) + await analytics.load({ writeKey }) + const trackCalls = fetchCalls.filter( + (call) => call.url === 'https://api.segment.io/v1/t' + ) + expect(trackCalls.length).toBe(1) + expect(trackCalls[0].body.event).toMatchInlineSnapshot( + `"Clicked Middleware Applied"` + ) }) it('applies query string logic if window.location.search is present', async () => { diff --git a/packages/browser/src/browser/index.ts b/packages/browser/src/browser/index.ts index 097175416..abc92716f 100644 --- a/packages/browser/src/browser/index.ts +++ b/packages/browser/src/browser/index.ts @@ -208,14 +208,29 @@ function flushPreBuffer( */ async function flushFinalBuffer( analytics: Analytics, + queryString: string, buffer: PreInitMethodCallBuffer ): Promise { - // Call popSnippetWindowBuffer before each flush task since there may be - // analytics calls during async function calls. - await flushAddSourceMiddleware(analytics, buffer) + await flushQueryString(analytics, queryString) flushAnalyticsCallsInNewTask(analytics, buffer) } +const getQueryString = (): string => { + const hash = window.location.hash ?? '' + const search = window.location.search ?? '' + const term = search.length ? search : hash.replace(/(?=#).*(?=\?)/, '') + return term +} + +const flushQueryString = async ( + analytics: Analytics, + queryString: string +): Promise => { + if (queryString.includes('ajs_')) { + await analytics.queryString(queryString).catch(console.error) + } +} + async function registerPlugins( writeKey: string, cdnSettings: CDNSettings, @@ -337,6 +352,9 @@ async function registerPlugins( }) } + // register any user-defined plugins added via analytics.addSourceMiddleware() + await flushAddSourceMiddleware(analytics, preInitBuffer) + return ctx } @@ -360,6 +378,9 @@ async function loadAnalytics( preInitBuffer.add(new PreInitMethodCall('page', [])) } + // reading the query string as early as possible in case the URL changes + const queryString = getQueryString() + const cdnURL = settings.cdnURL ?? getCDN() let cdnSettings = settings.cdnSettings ?? (await loadCDNSettings(settings.writeKey, cdnURL)) @@ -412,19 +433,9 @@ async function loadAnalytics( preInitBuffer ) - const search = window.location.search ?? '' - const hash = window.location.hash ?? '' - - const term = search.length ? search : hash.replace(/(?=#).*(?=\?)/, '') - - if (term.includes('ajs_')) { - await analytics.queryString(term).catch(console.error) - } - analytics.initialized = true analytics.emit('initialize', settings, options) - - await flushFinalBuffer(analytics, preInitBuffer) + await flushFinalBuffer(analytics, queryString, preInitBuffer) return [analytics, ctx] } diff --git a/turbo.json b/turbo.json index a4101e6e3..435551882 100644 --- a/turbo.json +++ b/turbo.json @@ -44,7 +44,7 @@ }, "lint": { "dependsOn": ["build"], - "inputs": ["**/tsconfig*.json", "**/*.ts", "**/*.tsx", "**/*.js"], + "inputs": ["**/tsconfig*.json", "**/*.ts", "**/*.tsx"], "outputs": [] }, "test:cloudflare-workers": {