Fix query string api not respecting source middleware (#1207)

segmentio · Jan 7, 2025 · 32582be · 32582be
1 parent 9883151
commit 32582be
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 40 deletions.
diff --git a/.changeset/few-starfishes-bake.md 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
@@ -1,19 +1,30 @@
 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<typeof parseFetchCall>[] = []
+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
 
 describe('queryString', () => {
   let jsd: JSDOM
 
   beforeEach(async () => {
-    jest.restoreAllMocks()
-    jest.resetAllMocks()
-
     const html = `
     <!DOCTYPE html>
       <head>
@@ -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
@@ -208,14 +208,29 @@ function flushPreBuffer(
  */
 async function flushFinalBuffer(
   analytics: Analytics,
+  queryString: string,
   buffer: PreInitMethodCallBuffer
 ): Promise<void> {
-  // 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<void> => {
+  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
@@ -44,7 +44,7 @@
     },
     "lint": {
       "dependsOn": ["build"],
-      "inputs": ["**/tsconfig*.json", "**/*.ts", "**/*.tsx", "**/*.js"],
+      "inputs": ["**/tsconfig*.json", "**/*.ts", "**/*.tsx"],
       "outputs": []
     },
     "test:cloudflare-workers": {