From 897f4cc69de4cdd38efd0cd70567bfed0c454fec Mon Sep 17 00:00:00 2001 From: Seth Silesky <5115498+silesky@users.noreply.github.com> Date: Thu, 5 Oct 2023 06:16:26 -0500 Subject: [PATCH] Make page context resilient to quick navigation changes (#852) --- .changeset/dirty-snakes-sip.md | 5 + .changeset/smooth-crabs-pull.md | 6 + .../pages/index-buffered-page-ctx.html | 233 ++++++++++ packages/browser/package.json | 4 +- .../analytics-lazy-init.integration.test.ts | 3 +- .../analytics-pre-init.integration.test.ts | 35 +- .../__tests__/inspector.integration.test.ts | 2 +- .../integrations.integration.test.ts | 11 + .../page-enrichment.integration.test.ts | 216 +++++++++ .../__tests__/standalone-analytics.test.ts | 52 ++- packages/browser/src/browser/index.ts | 8 +- .../src/core/__tests__/auto-track.test.ts | 440 ------------------ .../src/core/__tests__/track-form.test.ts | 193 ++++++++ .../src/core/__tests__/track-link.test.ts | 252 ++++++++++ packages/browser/src/core/analytics/index.ts | 26 +- .../src/core/buffer/__tests__/index.test.ts | 258 ++++++---- packages/browser/src/core/buffer/index.ts | 142 ++++-- packages/browser/src/core/buffer/snippet.ts | 46 -- .../src/core/events/__tests__/index.test.ts | 63 ++- packages/browser/src/core/events/index.ts | 131 +++--- .../src/core/page/__tests__/index.test.ts | 130 ++++++ .../browser/src/core/page/add-page-context.ts | 33 ++ .../browser/src/core/page/get-page-context.ts | 140 ++++++ packages/browser/src/core/page/index.ts | 2 + .../core/storage/__tests__/test-helpers.ts | 23 +- .../__tests__/universalStorage.test.ts | 29 +- .../src/core/storage/universalStorage.ts | 26 +- .../browser/src/lib/__tests__/pick.test.ts | 2 +- .../browser/src/lib/__tests__/pick.typedef.ts | 39 ++ packages/browser/src/lib/pick.ts | 19 +- .../__tests__/index.test.ts | 308 +++--------- .../index.ts | 128 ++--- .../plugins/segmentio/__tests__/index.test.ts | 6 +- .../segmentio/__tests__/retries.test.ts | 4 +- .../src/test-helpers/fixtures/cdn-settings.ts | 8 +- .../fixtures/create-fetch-method.ts | 2 +- .../src/test-helpers/fixtures/index.ts | 4 + .../src/test-helpers/fixtures/page-context.ts | 11 + packages/core/src/validation/helpers.ts | 2 +- packages/node/README.md | 11 +- 40 files changed, 1947 insertions(+), 1106 deletions(-) create mode 100644 .changeset/dirty-snakes-sip.md create mode 100644 .changeset/smooth-crabs-pull.md create mode 100644 examples/standalone-playground/pages/index-buffered-page-ctx.html create mode 100644 packages/browser/src/browser/__tests__/page-enrichment.integration.test.ts delete mode 100644 packages/browser/src/core/__tests__/auto-track.test.ts create mode 100644 packages/browser/src/core/__tests__/track-form.test.ts create mode 100644 packages/browser/src/core/__tests__/track-link.test.ts delete mode 100644 packages/browser/src/core/buffer/snippet.ts create mode 100644 packages/browser/src/core/page/__tests__/index.test.ts create mode 100644 packages/browser/src/core/page/add-page-context.ts create mode 100644 packages/browser/src/core/page/get-page-context.ts create mode 100644 packages/browser/src/core/page/index.ts create mode 100644 packages/browser/src/lib/__tests__/pick.typedef.ts rename packages/browser/src/plugins/{page-enrichment => env-enrichment}/__tests__/index.test.ts (59%) rename packages/browser/src/plugins/{page-enrichment => env-enrichment}/index.ts (64%) create mode 100644 packages/browser/src/test-helpers/fixtures/index.ts create mode 100644 packages/browser/src/test-helpers/fixtures/page-context.ts diff --git a/.changeset/dirty-snakes-sip.md b/.changeset/dirty-snakes-sip.md new file mode 100644 index 000000000..a99dcf054 --- /dev/null +++ b/.changeset/dirty-snakes-sip.md @@ -0,0 +1,5 @@ +--- +'@segment/analytics-core': patch +--- + +Tighten isPlainObject type guard diff --git a/.changeset/smooth-crabs-pull.md b/.changeset/smooth-crabs-pull.md new file mode 100644 index 000000000..b369014d4 --- /dev/null +++ b/.changeset/smooth-crabs-pull.md @@ -0,0 +1,6 @@ +--- +'@segment/analytics-next': minor +--- + +- Capture page context information faster, so context.campaign and context.page are more resilient to quick navigation changes. +- Parse UTM params into context.campaign if users pass an object to a page call. diff --git a/examples/standalone-playground/pages/index-buffered-page-ctx.html b/examples/standalone-playground/pages/index-buffered-page-ctx.html new file mode 100644 index 000000000..f973c900d --- /dev/null +++ b/examples/standalone-playground/pages/index-buffered-page-ctx.html @@ -0,0 +1,233 @@ + + + + + +
+ + +
+ + + + + + + +

+ This is for testing that the page context is buffered when users make quick navigation changes. + When this page first loads, it should immediately add some query parameters (similuating a quick navigation change). + The page call should still have the correct campaign and page parameters. +

+
+ +
+ + +
+
+ +

+  

+
+  
+
+
+
+
diff --git a/packages/browser/package.json b/packages/browser/package.json
index 2ae317f89..89e9ef76b 100644
--- a/packages/browser/package.json
+++ b/packages/browser/package.json
@@ -24,7 +24,7 @@
   ],
   "sideEffects": false,
   "scripts": {
-    ".": "yarn run -T turbo run --filter=@segment/analytics-next",
+    ".": "yarn run -T turbo run --filter=@segment/analytics-next...",
     "build-prep": "sh scripts/build-prep.sh",
     "version": "yarn run build-prep && git add src/generated/version.ts",
     "umd": "webpack",
@@ -44,7 +44,7 @@
   "size-limit": [
     {
       "path": "dist/umd/index.js",
-      "limit": "28.5 KB"
+      "limit": "28.8 KB"
     }
   ],
   "dependencies": {
diff --git a/packages/browser/src/browser/__tests__/analytics-lazy-init.integration.test.ts b/packages/browser/src/browser/__tests__/analytics-lazy-init.integration.test.ts
index 526a7a972..00247bc42 100644
--- a/packages/browser/src/browser/__tests__/analytics-lazy-init.integration.test.ts
+++ b/packages/browser/src/browser/__tests__/analytics-lazy-init.integration.test.ts
@@ -1,4 +1,5 @@
 import { sleep } from '@segment/analytics-core'
+import { getBufferedPageCtxFixture } from '../../test-helpers/fixtures'
 import unfetch from 'unfetch'
 import { AnalyticsBrowser } from '..'
 import { Analytics } from '../../core/analytics'
@@ -27,7 +28,7 @@ describe('Lazy initialization', () => {
     expect(trackSpy).not.toBeCalled()
     analytics.load({ writeKey: 'abc' })
     await track
-    expect(trackSpy).toBeCalledWith('foo')
+    expect(trackSpy).toBeCalledWith('foo', getBufferedPageCtxFixture())
   })
 
   it('.load method return an analytics instance', async () => {
diff --git a/packages/browser/src/browser/__tests__/analytics-pre-init.integration.test.ts b/packages/browser/src/browser/__tests__/analytics-pre-init.integration.test.ts
index 10384fca6..74952b455 100644
--- a/packages/browser/src/browser/__tests__/analytics-pre-init.integration.test.ts
+++ b/packages/browser/src/browser/__tests__/analytics-pre-init.integration.test.ts
@@ -6,6 +6,7 @@ import * as Factory from '../../test-helpers/factories'
 import { sleep } from '../../lib/sleep'
 import { setGlobalCDNUrl } from '../../lib/parse-cdn'
 import { User } from '../../core/user'
+import { getBufferedPageCtxFixture } from '../../test-helpers/fixtures'
 
 jest.mock('unfetch')
 
@@ -61,7 +62,11 @@ describe('Pre-initialization', () => {
       const trackCtxPromise = ajsBrowser.track('foo', { name: 'john' })
       const result = await trackCtxPromise
       expect(result).toBeInstanceOf(Context)
-      expect(trackSpy).toBeCalledWith('foo', { name: 'john' })
+      expect(trackSpy).toBeCalledWith(
+        'foo',
+        { name: 'john' },
+        getBufferedPageCtxFixture()
+      )
       expect(trackSpy).toBeCalledTimes(1)
     })
 
@@ -107,11 +112,19 @@ describe('Pre-initialization', () => {
 
       await Promise.all([trackCtxPromise, trackCtxPromise2, identifyCtxPromise])
 
-      expect(trackSpy).toBeCalledWith('foo', { name: 'john' })
-      expect(trackSpy).toBeCalledWith('bar', { age: 123 })
+      expect(trackSpy).toBeCalledWith(
+        'foo',
+        { name: 'john' },
+        getBufferedPageCtxFixture()
+      )
+      expect(trackSpy).toBeCalledWith(
+        'bar',
+        { age: 123 },
+        getBufferedPageCtxFixture()
+      )
       expect(trackSpy).toBeCalledTimes(2)
 
-      expect(identifySpy).toBeCalledWith('hello')
+      expect(identifySpy).toBeCalledWith('hello', getBufferedPageCtxFixture())
       expect(identifySpy).toBeCalledTimes(1)
     })
 
@@ -234,8 +247,8 @@ describe('Pre-initialization', () => {
       await AnalyticsBrowser.standalone(writeKey)
 
       await sleep(100) // the snippet does not return a promise (pre-initialization) ... it sometimes has a callback as the third argument.
-      expect(trackSpy).toBeCalledWith('foo')
-      expect(trackSpy).toBeCalledWith('bar')
+      expect(trackSpy).toBeCalledWith('foo', getBufferedPageCtxFixture())
+      expect(trackSpy).toBeCalledWith('bar', getBufferedPageCtxFixture())
       expect(trackSpy).toBeCalledTimes(2)
 
       expect(identifySpy).toBeCalledTimes(1)
@@ -262,11 +275,11 @@ describe('Pre-initialization', () => {
       await AnalyticsBrowser.standalone(writeKey)
 
       await sleep(100) // the snippet does not return a promise (pre-initialization) ... it sometimes has a callback as the third argument.
-      expect(trackSpy).toBeCalledWith('foo')
-      expect(trackSpy).toBeCalledWith('bar')
+      expect(trackSpy).toBeCalledWith('foo', getBufferedPageCtxFixture())
+      expect(trackSpy).toBeCalledWith('bar', getBufferedPageCtxFixture())
       expect(trackSpy).toBeCalledTimes(2)
 
-      expect(identifySpy).toBeCalledWith()
+      expect(identifySpy).toBeCalledWith(getBufferedPageCtxFixture())
       expect(identifySpy).toBeCalledTimes(1)
       expect(consoleErrorSpy).toBeCalledTimes(1)
 
@@ -292,8 +305,8 @@ describe('Pre-initialization', () => {
       })
 
       await sleep(100) // the snippet does not return a promise (pre-initialization) ... it sometimes has a callback as the third argument.
-      expect(trackSpy).toBeCalledWith('foo')
-      expect(trackSpy).toBeCalledWith('bar')
+      expect(trackSpy).toBeCalledWith('foo', getBufferedPageCtxFixture())
+      expect(trackSpy).toBeCalledWith('bar', getBufferedPageCtxFixture())
       expect(trackSpy).toBeCalledTimes(2)
 
       expect(identifySpy).toBeCalledTimes(1)
diff --git a/packages/browser/src/browser/__tests__/inspector.integration.test.ts b/packages/browser/src/browser/__tests__/inspector.integration.test.ts
index c56e6d986..a5b8df713 100644
--- a/packages/browser/src/browser/__tests__/inspector.integration.test.ts
+++ b/packages/browser/src/browser/__tests__/inspector.integration.test.ts
@@ -55,7 +55,7 @@ describe('Inspector', () => {
 
     await deliveryPromise
 
-    expect(enrichedFn).toHaveBeenCalledTimes(2)
+    expect(enrichedFn).toHaveBeenCalledTimes(2) // will be called once for every before or enrichment plugin.
     expect(deliveredFn).toHaveBeenCalledTimes(1)
   })
 
diff --git a/packages/browser/src/browser/__tests__/integrations.integration.test.ts b/packages/browser/src/browser/__tests__/integrations.integration.test.ts
index a70f12095..107e8905b 100644
--- a/packages/browser/src/browser/__tests__/integrations.integration.test.ts
+++ b/packages/browser/src/browser/__tests__/integrations.integration.test.ts
@@ -19,6 +19,17 @@ const mockFetchCdnSettings = (cdnSettings: any = {}) => {
     .mockImplementation(createMockFetchImplementation(cdnSettings))
 }
 
+jest.spyOn(console, 'warn').mockImplementation((...errMsgs) => {
+  if (errMsgs[0].includes('deprecate')) {
+    // get rid of deprecation wawrning spam
+    return
+  }
+  console.warn(
+    'Unexpected console.warn spam in your jest test - please stub out. ' +
+      JSON.stringify(errMsgs)
+  )
+})
+
 describe('Integrations', () => {
   beforeEach(async () => {
     mockFetchCdnSettings()
diff --git a/packages/browser/src/browser/__tests__/page-enrichment.integration.test.ts b/packages/browser/src/browser/__tests__/page-enrichment.integration.test.ts
new file mode 100644
index 000000000..3b72278c9
--- /dev/null
+++ b/packages/browser/src/browser/__tests__/page-enrichment.integration.test.ts
@@ -0,0 +1,216 @@
+import unfetch from 'unfetch'
+import { Analytics, AnalyticsBrowser } from '../..'
+import { PageContext } from '../../core/page'
+import {
+  cdnSettingsMinimal,
+  createMockFetchImplementation,
+} from '../../test-helpers/fixtures'
+
+jest.mock('unfetch')
+jest.mocked(unfetch).mockImplementation(createMockFetchImplementation())
+
+let ajs: Analytics
+
+beforeEach(async () => {
+  const [analytics] = await AnalyticsBrowser.load({
+    writeKey: 'abc_123',
+    cdnSettings: { ...cdnSettingsMinimal },
+  })
+  ajs = analytics
+})
+describe('Page Enrichment', () => {
+  it('enriches page calls', async () => {
+    const ctx = await ajs.page('Checkout', {})
+
+    expect(ctx.event.properties).toMatchInlineSnapshot(`
+      Object {
+        "name": "Checkout",
+        "path": "/",
+        "referrer": "",
+        "search": "",
+        "title": "",
+        "url": "http://localhost/",
+      }
+    `)
+  })
+
+  it('enriches track events with the page context', async () => {
+    const ctx = await ajs.track('My event', {
+      banana: 'phone',
+    })
+    expect(ctx.event.context?.page).toMatchInlineSnapshot(`
+      Object {
+        "path": "/",
+        "referrer": "",
+        "search": "",
+        "title": "",
+        "url": "http://localhost/",
+      }
+    `)
+  })
+
+  describe('event.properties override behavior', () => {
+    it('special page properties in event.properties (url, referrer, etc) are copied to context.page', async () => {
+      const pageProps: PageContext & { [key: string]: unknown } = Object.freeze(
+        {
+          path: 'foo',
+          referrer: 'bar',
+          search: 'baz',
+          title: 'qux',
+          url: 'http://fake.com',
+          should_not_show_up: 'hello',
+        }
+      )
+      const ctx = await ajs.page('My Event', pageProps)
+      const page = ctx.event.context!.page
+      expect(page).toMatchInlineSnapshot(`
+        Object {
+          "path": "foo",
+          "referrer": "bar",
+          "search": "baz",
+          "title": "qux",
+          "url": "http://fake.com",
+        }
+      `)
+    })
+
+    it('special page properties in event.properties (url, referrer, etc) are not copied to context.page in non-page calls', async () => {
+      const eventProps = Object.freeze({
+        path: 'foo',
+        referrer: 'bar',
+        search: 'baz',
+        url: 'http://fake.com',
+        foo: 'hello',
+      })
+      const ctx = await ajs.track('My Event', eventProps)
+      expect(ctx.event.properties).toMatchInlineSnapshot(`
+        Object {
+          "foo": "hello",
+          "path": "foo",
+          "referrer": "bar",
+          "search": "baz",
+          "url": "http://fake.com",
+        }
+      `)
+      expect(ctx.event.context!.page).toMatchInlineSnapshot(`
+        Object {
+          "path": "/",
+          "referrer": "",
+          "search": "",
+          "title": "",
+          "url": "http://localhost/",
+        }
+      `)
+    })
+
+    it('page properties should override defaults in page calls', async () => {
+      const pageProps = Object.freeze({ path: 'override' })
+      const ctx = await ajs.page('My Event', pageProps)
+      const page = ctx.event.context!.page
+      expect(page).toMatchInlineSnapshot(`
+        Object {
+          "path": "override",
+          "referrer": "",
+          "search": "",
+          "title": "",
+          "url": "http://localhost/",
+        }
+      `)
+    })
+
+    it('undefined / null / empty string properties on event get overridden as usual', async () => {
+      const eventProps = Object.freeze({
+        referrer: '',
+        path: undefined,
+        title: null,
+      })
+
+      const ctx = await ajs.page('My Event', eventProps)
+      const page = ctx.event.context!.page
+      expect(page).toEqual(
+        expect.objectContaining({ referrer: '', path: undefined, title: null })
+      )
+    })
+  })
+
+  it('enriches page events with the page context', async () => {
+    const ctx = await ajs.page(
+      'My event',
+      { banana: 'phone' },
+      { page: { url: 'not-localhost' } }
+    )
+
+    expect(ctx.event.context?.page).toMatchInlineSnapshot(`
+          Object {
+            "path": "/",
+            "referrer": "",
+            "search": "",
+            "title": "",
+            "url": "not-localhost",
+          }
+      `)
+  })
+  it('enriches page events using properties', async () => {
+    const ctx = await ajs.page('My event', { banana: 'phone', referrer: 'foo' })
+
+    expect(ctx.event.context?.page).toMatchInlineSnapshot(`
+          Object {
+            "path": "/",
+            "referrer": "foo",
+            "search": "",
+            "title": "",
+            "url": "http://localhost/",
+          }
+      `)
+  })
+
+  it('in page events, event.name overrides event.properties.name', async () => {
+    const ctx = await ajs.page('My Event', undefined, undefined, {
+      name: 'some propery name',
+    })
+    expect(ctx.event.properties!.name).toBe('My Event')
+  })
+
+  it('in non-page events, event.name does not override event.properties.name', async () => {
+    const ctx = await ajs.track('My Event', {
+      name: 'some propery name',
+    })
+    expect(ctx.event.properties!.name).toBe('some propery name')
+  })
+
+  it('enriches identify events with the page context', async () => {
+    const ctx = await ajs.identify('Netto', {
+      banana: 'phone',
+    })
+
+    expect(ctx.event.context?.page).toMatchInlineSnapshot(`
+      Object {
+        "path": "/",
+        "referrer": "",
+        "search": "",
+        "title": "",
+        "url": "http://localhost/",
+      }
+    `)
+  })
+
+  it('page object is accessible in all plugins', async () => {
+    await ajs.addSourceMiddleware(({ payload, next }) => {
+      expect(payload.obj?.context?.page).toMatchInlineSnapshot(`
+        Object {
+          "path": "/",
+          "referrer": "",
+          "search": "",
+          "title": "",
+          "url": "http://localhost/",
+        }
+      `)
+      next(payload)
+    })
+
+    await ajs.track('My event', {
+      banana: 'phone',
+    })
+    expect.assertions(1)
+  })
+})
diff --git a/packages/browser/src/browser/__tests__/standalone-analytics.test.ts b/packages/browser/src/browser/__tests__/standalone-analytics.test.ts
index c13c5f426..330360d3f 100644
--- a/packages/browser/src/browser/__tests__/standalone-analytics.test.ts
+++ b/packages/browser/src/browser/__tests__/standalone-analytics.test.ts
@@ -9,6 +9,7 @@ import { sleep } from '../../lib/sleep'
 import * as Factory from '../../test-helpers/factories'
 import { EventQueue } from '../../core/queue/event-queue'
 import { AnalyticsStandalone } from '../standalone-interface'
+import { getBufferedPageCtxFixture } from '../../test-helpers/fixtures'
 
 const track = jest.fn()
 const identify = jest.fn()
@@ -28,9 +29,7 @@ jest.mock('../../core/analytics', () => ({
     register,
     emit: jest.fn(),
     on,
-    queue: new EventQueue(
-      new PersistedPriorityQueue(1, 'foo:event-queue') as any
-    ),
+    queue: new EventQueue(new PersistedPriorityQueue(1, 'event-queue') as any),
     options,
   }),
 }))
@@ -68,6 +67,7 @@ describe('standalone bundle', () => {
     `.trim()
 
     const virtualConsole = new jsdom.VirtualConsole()
+
     const jsd = new JSDOM(html, {
       runScripts: 'dangerously',
       resources: 'usable',
@@ -159,12 +159,20 @@ describe('standalone bundle', () => {
 
     await sleep(0)
 
-    expect(track).toHaveBeenCalledWith('fruit basket', {
-      fruits: ['🍌', '🍇'],
-    })
-    expect(identify).toHaveBeenCalledWith('netto', {
-      employer: 'segment',
-    })
+    expect(track).toHaveBeenCalledWith(
+      'fruit basket',
+      {
+        fruits: ['🍌', '🍇'],
+      },
+      getBufferedPageCtxFixture()
+    )
+    expect(identify).toHaveBeenCalledWith(
+      'netto',
+      {
+        employer: 'segment',
+      },
+      getBufferedPageCtxFixture()
+    )
 
     expect(page).toHaveBeenCalled()
   })
@@ -270,13 +278,25 @@ describe('standalone bundle', () => {
 
     await sleep(0)
 
-    expect(track).toHaveBeenCalledWith('fruit basket', {
-      fruits: ['🍌', '🍇'],
-    })
-    expect(track).toHaveBeenCalledWith('race conditions', { foo: 'bar' })
-    expect(identify).toHaveBeenCalledWith('netto', {
-      employer: 'segment',
-    })
+    expect(track).toHaveBeenCalledWith(
+      'fruit basket',
+      {
+        fruits: ['🍌', '🍇'],
+      },
+      getBufferedPageCtxFixture()
+    )
+    expect(track).toHaveBeenCalledWith(
+      'race conditions',
+      { foo: 'bar' },
+      getBufferedPageCtxFixture()
+    )
+    expect(identify).toHaveBeenCalledWith(
+      'netto',
+      {
+        employer: 'segment',
+      },
+      getBufferedPageCtxFixture()
+    )
 
     expect(page).toHaveBeenCalled()
   })
diff --git a/packages/browser/src/browser/index.ts b/packages/browser/src/browser/index.ts
index 1c59af549..39ac2ee83 100644
--- a/packages/browser/src/browser/index.ts
+++ b/packages/browser/src/browser/index.ts
@@ -9,7 +9,7 @@ import { Plugin } from '../core/plugin'
 import { MetricsOptions } from '../core/stats/remote-metrics'
 import { mergedOptions } from '../lib/merged-options'
 import { createDeferred } from '../lib/create-deferred'
-import { pageEnrichment } from '../plugins/page-enrichment'
+import { envEnrichment } from '../plugins/env-enrichment'
 import {
   PluginFactory,
   remoteLoader,
@@ -26,7 +26,6 @@ import {
   flushSetAnonymousID,
   flushOn,
 } from '../core/buffer'
-import { popSnippetWindowBuffer } from '../core/buffer/snippet'
 import { ClassicIntegrationSource } from '../plugins/ajs-destination/types'
 import { attachInspector } from '../core/inspector'
 import { Stats } from '../core/stats'
@@ -155,7 +154,6 @@ function flushPreBuffer(
   analytics: Analytics,
   buffer: PreInitMethodCallBuffer
 ): void {
-  buffer.push(...popSnippetWindowBuffer())
   flushSetAnonymousID(analytics, buffer)
   flushOn(analytics, buffer)
 }
@@ -169,9 +167,7 @@ async function flushFinalBuffer(
 ): Promise {
   // Call popSnippetWindowBuffer before each flush task since there may be
   // analytics calls during async function calls.
-  buffer.push(...popSnippetWindowBuffer())
   await flushAddSourceMiddleware(analytics, buffer)
-  buffer.push(...popSnippetWindowBuffer())
   flushAnalyticsCallsInNewTask(analytics, buffer)
   // Clear buffer, just in case analytics is loaded twice; we don't want to fire events off again.
   buffer.clear()
@@ -250,7 +246,7 @@ async function registerPlugins(
 
   const toRegister = [
     validation,
-    pageEnrichment,
+    envEnrichment,
     ...plugins,
     ...legacyDestinations,
     ...remotePlugins,
diff --git a/packages/browser/src/core/__tests__/auto-track.test.ts b/packages/browser/src/core/__tests__/auto-track.test.ts
deleted file mode 100644
index 1c9d27ebd..000000000
--- a/packages/browser/src/core/__tests__/auto-track.test.ts
+++ /dev/null
@@ -1,440 +0,0 @@
-import { JSDOM } from 'jsdom'
-import { Analytics } from '../analytics'
-
-const sleep = (time: number): Promise =>
-  new Promise((resolve) => {
-    setTimeout(resolve, time)
-  })
-
-async function resolveWhen(
-  condition: () => boolean,
-  timeout?: number
-): Promise {
-  return new Promise((resolve, _reject) => {
-    if (condition()) {
-      resolve()
-      return
-    }
-
-    const check = () =>
-      setTimeout(() => {
-        if (condition()) {
-          resolve()
-        } else {
-          check()
-        }
-      }, timeout)
-
-    check()
-  })
-}
-
-describe('track helpers', () => {
-  describe('trackLink', () => {
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    let link: any
-    let wrap: SVGSVGElement
-    let svg: SVGAElement
-
-    let analytics = new Analytics({ writeKey: 'foo' })
-    let mockTrack = jest.spyOn(analytics, 'track')
-
-    beforeEach(() => {
-      analytics = new Analytics({ writeKey: 'foo' })
-
-      // @ts-ignore
-      global.jQuery = require('jquery')
-
-      const jsd = new JSDOM('', {
-        runScripts: 'dangerously',
-        resources: 'usable',
-      })
-      // eslint-disable-next-line no-global-assign
-      document = jsd.window.document
-
-      jest.spyOn(console, 'error').mockImplementationOnce(() => {})
-
-      document.querySelector('html')!.innerHTML = `
-      
-        
-          
-          
-
- -
-
- - ` - - link = document.getElementById('foo') - wrap = document.createElementNS('http://www.w3.org/2000/svg', 'svg') - svg = document.createElementNS('http://www.w3.org/2000/svg', 'a') - wrap.appendChild(svg) - document.body.appendChild(wrap) - - jest.spyOn(window, 'location', 'get').mockReturnValue({ - ...window.location, - }) - - mockTrack = jest.spyOn(analytics, 'track') - - // We need to mock the track function for the .catch() call not to break when testing - // eslint-disable-next-line @typescript-eslint/unbound-method - mockTrack.mockImplementation(Analytics.prototype.track) - }) - - it('should respect options object', async () => { - await analytics.trackLink( - link!, - 'foo', - {}, - { context: { ip: '0.0.0.0' } } - ) - link.click() - - expect(mockTrack).toHaveBeenCalledWith( - 'foo', - {}, - { context: { ip: '0.0.0.0' } } - ) - }) - - it('should stay on same page with blank href', async () => { - link.href = '' - await analytics.trackLink(link!, 'foo') - link.click() - - expect(mockTrack).toHaveBeenCalled() - expect(window.location.href).toBe('http://localhost/') - }) - - it('should work with nested link', async () => { - const nested = document.getElementById('bar') - await analytics.trackLink(nested, 'foo') - nested!.click() - - expect(mockTrack).toHaveBeenCalled() - - await resolveWhen(() => window.location.href === 'bar.com') - expect(window.location.href).toBe('bar.com') - }) - - it('should make a track call', async () => { - await analytics.trackLink(link!, 'foo') - link.click() - - expect(mockTrack).toHaveBeenCalled() - }) - - it('should still navigate even if the track call fails', async () => { - mockTrack.mockClear() - - let rejected = false - mockTrack.mockImplementationOnce(() => { - rejected = true - return Promise.reject(new Error('boo!')) - }) - - const nested = document.getElementById('bar') - await analytics.trackLink(nested, 'foo') - nested!.click() - - await resolveWhen(() => rejected) - await resolveWhen(() => window.location.href === 'bar.com') - expect(window.location.href).toBe('bar.com') - }) - - it('should still navigate even if the track call times out', async () => { - mockTrack.mockClear() - - let timedOut = false - mockTrack.mockImplementationOnce(async () => { - await sleep(600) - timedOut = true - return Promise.resolve() as any - }) - - const nested = document.getElementById('bar') - await analytics.trackLink(nested, 'foo') - nested!.click() - - await resolveWhen(() => window.location.href === 'bar.com') - expect(window.location.href).toBe('bar.com') - expect(timedOut).toBe(false) - - await resolveWhen(() => timedOut) - }) - - it('should accept a jquery object for an element', async () => { - const $link = jQuery(link) - await analytics.trackLink($link, 'foo') - link.click() - expect(mockTrack).toBeCalled() - }) - - it('accepts array of elements', async () => { - const links = [link, link] - await analytics.trackLink(links, 'foo') - link.click() - - expect(mockTrack).toHaveBeenCalled() - }) - - it('should send an event and properties', async () => { - await analytics.trackLink(link, 'event', { property: true }) - link.click() - - expect(mockTrack).toBeCalledWith('event', { property: true }, {}) - }) - - it('should accept an event function', async () => { - function event(el: Element): string { - return el.nodeName - } - await analytics.trackLink(link, event, { foo: 'bar' }) - link.click() - - expect(mockTrack).toBeCalledWith('A', { foo: 'bar' }, {}) - }) - - it('should accept a properties function', async () => { - function properties(el: Record): Record { - return { type: el.nodeName } - } - await analytics.trackLink(link, 'event', properties) - link.click() - - expect(mockTrack).toBeCalledWith('event', { type: 'A' }, {}) - }) - - it('should load an href on click', async () => { - link.href = '#test' - await analytics.trackLink(link, 'foo') - link.click() - - await resolveWhen(() => window.location.href === '#test') - expect(global.window.location.href).toBe('#test') - }) - - it('should only navigate after the track call has been completed', async () => { - link.href = '#test' - await analytics.trackLink(link, 'foo') - link.click() - - await resolveWhen(() => mockTrack.mock.calls.length === 1) - await resolveWhen(() => window.location.href === '#test') - - expect(global.window.location.href).toBe('#test') - }) - - it('should support svg .href attribute', async () => { - svg.setAttribute('href', '#svg') - await analytics.trackLink(svg, 'foo') - const clickEvent = new Event('click') - svg.dispatchEvent(clickEvent) - - await resolveWhen(() => window.location.href === '#svg') - expect(global.window.location.href).toBe('#svg') - }) - - it('should fallback to getAttributeNS', async () => { - svg.setAttributeNS('http://www.w3.org/1999/xlink', 'href', '#svg') - await analytics.trackLink(svg, 'foo') - const clickEvent = new Event('click') - svg.dispatchEvent(clickEvent) - - await resolveWhen(() => window.location.href === '#svg') - expect(global.window.location.href).toBe('#svg') - }) - - it('should support xlink:href', async () => { - svg.setAttribute('xlink:href', '#svg') - await analytics.trackLink(svg, 'foo') - const clickEvent = new Event('click') - svg.dispatchEvent(clickEvent) - - await resolveWhen(() => window.location.href === '#svg') - expect(global.window.location.href).toBe('#svg') - }) - - it('should not load an href for a link with a blank target', async () => { - link.href = '/base/test/support/mock.html' - link.target = '_blank' - await analytics.trackLink(link, 'foo') - link.click() - - await sleep(300) - expect(global.window.location.href).not.toBe('#test') - }) - }) - - describe('trackForm', () => { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - let form: any - // eslint-disable-next-line @typescript-eslint/no-explicit-any - let submit: any - - const analytics = new Analytics({ writeKey: 'foo' }) - let mockTrack = jest.spyOn(analytics, 'track') - - beforeEach(() => { - document.querySelector('html')!.innerHTML = ` - - -
- -
- - ` - form = document.getElementById('form') - submit = document.getElementById('submit') - - // @ts-ignore - global.jQuery = require('jquery') - - mockTrack = jest.spyOn(analytics, 'track') - // eslint-disable-next-line @typescript-eslint/unbound-method - mockTrack.mockImplementation(Analytics.prototype.track) - }) - - afterEach(() => { - window.location.hash = '' - document.body.removeChild(form) - }) - - it('should not error or send track event on null form', async () => { - const form = document.getElementById('fake-form') as HTMLFormElement - - await analytics.trackForm(form, 'Signed Up', { - plan: 'Premium', - revenue: 99.0, - }) - submit.click() - expect(mockTrack).not.toBeCalled() - }) - - it('should respect options object', async () => { - await analytics.trackForm(form, 'foo', {}, { context: { ip: '0.0.0.0' } }) - submit.click() - - expect(mockTrack).toHaveBeenCalledWith( - 'foo', - {}, - { context: { ip: '0.0.0.0' } } - ) - }) - - it('should trigger a track on a form submit', async () => { - await analytics.trackForm(form, 'foo') - submit.click() - expect(mockTrack).toBeCalled() - }) - - it('should accept a jquery object for an element', async () => { - await analytics.trackForm(form, 'foo') - submit.click() - expect(mockTrack).toBeCalled() - }) - - it('should not accept a string for an element', async () => { - try { - // @ts-expect-error - await analytics.trackForm('foo') - submit.click() - } catch (e) { - expect(e instanceof TypeError).toBe(true) - } - expect(mockTrack).not.toBeCalled() - }) - - it('should send an event and properties', async () => { - await analytics.trackForm(form, 'event', { property: true }) - submit.click() - expect(mockTrack).toBeCalledWith('event', { property: true }, {}) - }) - - it('should accept an event function', async () => { - function event(): string { - return 'event' - } - await analytics.trackForm(form, event, { foo: 'bar' }) - submit.click() - expect(mockTrack).toBeCalledWith('event', { foo: 'bar' }, {}) - }) - - it('should accept a properties function', async () => { - function properties(): Record { - return { property: true } - } - await analytics.trackForm(form, 'event', properties) - submit.click() - expect(mockTrack).toBeCalledWith('event', { property: true }, {}) - }) - - it('should call submit after a timeout', async () => { - const submitSpy = jest.spyOn(form, 'submit') - const mockedTrack = jest.fn() - - // eslint-disable-next-line @typescript-eslint/unbound-method - mockedTrack.mockImplementation(Analytics.prototype.track) - - analytics.track = mockedTrack - await analytics.trackForm(form, 'foo') - - submit.click() - - await sleep(500) - - expect(submitSpy).toHaveBeenCalled() - }) - - it('should trigger an existing submit handler', async () => { - const submitPromise = new Promise((resolve) => { - form.addEventListener('submit', () => { - resolve() - }) - }) - - await analytics.trackForm(form, 'foo') - submit.click() - await submitPromise - }) - - it('should trigger an existing jquery submit handler', async () => { - const $form = jQuery(form) - - const submitPromise = new Promise((resolve) => { - $form.submit(function () { - resolve() - }) - }) - - await analytics.trackForm(form, 'foo') - submit.click() - await submitPromise - }) - - it('should track on a form submitted via jquery', async () => { - const $form = jQuery(form) - - await analytics.trackForm(form, 'foo') - $form.submit() - - expect(mockTrack).toBeCalled() - }) - - it('should trigger an existing jquery submit handler on a form submitted via jquery', async () => { - const $form = jQuery(form) - - const submitPromise = new Promise((resolve) => { - $form.submit(function () { - resolve() - }) - }) - - await analytics.trackForm(form, 'foo') - $form.submit() - await submitPromise - }) - }) -}) diff --git a/packages/browser/src/core/__tests__/track-form.test.ts b/packages/browser/src/core/__tests__/track-form.test.ts new file mode 100644 index 000000000..92fff0d58 --- /dev/null +++ b/packages/browser/src/core/__tests__/track-form.test.ts @@ -0,0 +1,193 @@ +import { Analytics } from '../analytics' +import { sleep } from '../../lib/sleep' +import { getDefaultPageContext } from '../page' + +let analytics: Analytics +let mockTrack: jest.SpiedFunction +const ogLocation = { + ...global.window.location, +} + +beforeEach(() => { + // @ts-ignore + global.jQuery = require('jquery') + + jest.spyOn(console, 'error').mockImplementationOnce(() => {}) + Object.defineProperty(window, 'location', { + value: ogLocation, + writable: true, + }) + mockTrack = jest.spyOn(Analytics.prototype, 'track') + analytics = new Analytics({ writeKey: 'foo' }) +}) + +describe('trackForm', () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let form: any + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let submit: any + + beforeEach(() => { + document.querySelector('html')!.innerHTML = ` + + +
+ +
+ + ` + form = document.getElementById('form') + submit = document.getElementById('submit') + + // @ts-ignore + global.jQuery = require('jquery') + }) + + afterEach(() => { + document.querySelector('html')!.innerHTML = '' + }) + + it('should have the correct page context', async () => { + window.location.href = 'http://bar.com?foo=123' + window.location.search = '?foo=123' + await analytics.trackForm(form, 'foo', {}, { context: { ip: '0.0.0.0' } }) + submit.click() + const [, , properties] = mockTrack.mock.lastCall as any[] + + expect((properties.context as any).page).toEqual({ + ...getDefaultPageContext(), + url: 'http://bar.com?foo=123', + search: '?foo=123', + }) + }) + + it('should not error or send track event on null form', async () => { + const form = document.getElementById('fake-form') as HTMLFormElement + + await analytics.trackForm(form, 'Signed Up', { + plan: 'Premium', + revenue: 99.0, + }) + submit.click() + expect(mockTrack).not.toBeCalled() + }) + + it('should respect options object', async () => { + await analytics.trackForm(form, 'foo', {}, { context: { ip: '0.0.0.0' } }) + submit.click() + + expect(mockTrack).toHaveBeenCalledWith( + 'foo', + {}, + { context: expect.objectContaining({ ip: '0.0.0.0' }) } + ) + }) + + it('should trigger a track on a form submit', async () => { + await analytics.trackForm(form, 'foo') + submit.click() + expect(mockTrack).toBeCalled() + }) + + it('should accept a jquery object for an element', async () => { + await analytics.trackForm(form, 'foo') + submit.click() + expect(mockTrack).toBeCalled() + }) + + it('should not accept a string for an element', async () => { + try { + // @ts-expect-error + await analytics.trackForm('foo') + submit.click() + } catch (e) { + expect(e instanceof TypeError).toBe(true) + } + expect(mockTrack).not.toBeCalled() + }) + + it('should send an event and properties', async () => { + await analytics.trackForm(form, 'event', { property: true }) + submit.click() + expect(mockTrack).toBeCalledWith('event', { property: true }, {}) + }) + + it('should accept an event function', async () => { + function event(): string { + return 'event' + } + await analytics.trackForm(form, event, { foo: 'bar' }) + submit.click() + expect(mockTrack).toBeCalledWith('event', { foo: 'bar' }, {}) + }) + + it('should accept a properties function', async () => { + function properties(): Record { + return { property: true } + } + await analytics.trackForm(form, 'event', properties) + submit.click() + expect(mockTrack).toBeCalledWith('event', { property: true }, {}) + }) + + it('should call submit after a timeout', async () => { + const submitSpy = jest.spyOn(form, 'submit') + + await analytics.trackForm(form, 'foo') + + submit.click() + + await sleep(300) + + expect(submitSpy).toHaveBeenCalled() + }) + + it('should trigger an existing submit handler', async () => { + const submitPromise = new Promise((resolve) => { + form.addEventListener('submit', () => { + resolve() + }) + }) + + await analytics.trackForm(form, 'foo') + submit.click() + await submitPromise + }) + + it('should trigger an existing jquery submit handler', async () => { + const $form = jQuery(form) + + const submitPromise = new Promise((resolve) => { + $form.submit(function () { + resolve() + }) + }) + + await analytics.trackForm(form, 'foo') + submit.click() + await submitPromise + }) + + it('should track on a form submitted via jquery', async () => { + const $form = jQuery(form) + + await analytics.trackForm(form, 'foo') + $form.submit() + + expect(mockTrack).toBeCalled() + }) + + it('should trigger an existing jquery submit handler on a form submitted via jquery', async () => { + const $form = jQuery(form) + + const submitPromise = new Promise((resolve) => { + $form.submit(function () { + resolve() + }) + }) + + await analytics.trackForm(form, 'foo') + $form.submit() + await submitPromise + }) +}) diff --git a/packages/browser/src/core/__tests__/track-link.test.ts b/packages/browser/src/core/__tests__/track-link.test.ts new file mode 100644 index 000000000..363de0cfd --- /dev/null +++ b/packages/browser/src/core/__tests__/track-link.test.ts @@ -0,0 +1,252 @@ +import { Analytics } from '../analytics' +import { sleep } from '../../lib/sleep' + +async function resolveWhen( + condition: () => boolean, + timeout?: number +): Promise { + return new Promise((resolve, _reject) => { + if (condition()) { + resolve() + return + } + + const check = () => + setTimeout(() => { + if (condition()) { + resolve() + } else { + check() + } + }, timeout) + + check() + }) +} + +const ogLocation = { + ...global.window.location, +} + +let analytics: Analytics +let mockTrack: jest.SpiedFunction +beforeEach(() => { + Object.defineProperty(window, 'location', { + value: ogLocation, + writable: true, + }) + mockTrack = jest.spyOn(Analytics.prototype, 'track') + analytics = new Analytics({ writeKey: 'foo' }) +}) +describe('trackLink', () => { + let link: any + let wrap: SVGSVGElement + let svg: SVGAElement + + beforeEach(() => { + // @ts-ignore + global.jQuery = require('jquery') + + jest.spyOn(console, 'error').mockImplementationOnce(() => {}) + + document.querySelector('html')!.innerHTML = ` + + + +
+
+ +
+
+ + ` + + link = document.getElementById('foo') + wrap = document.createElementNS('http://www.w3.org/2000/svg', 'svg') + svg = document.createElementNS('http://www.w3.org/2000/svg', 'a') + wrap.appendChild(svg) + document.body.appendChild(wrap) + }) + + afterEach(() => { + document.querySelector('html')!.innerHTML = '' + }) + it('should respect options object', async () => { + await analytics.trackLink(link!, 'foo', {}, { context: { ip: '0.0.0.0' } }) + link.click() + + expect(mockTrack).toHaveBeenCalledWith( + 'foo', + {}, + { context: expect.objectContaining({ ip: '0.0.0.0' }) } + ) + }) + + it('should stay on same page with blank href', async () => { + link.href = '' + await analytics.trackLink(link!, 'foo') + link.click() + + expect(mockTrack).toHaveBeenCalled() + expect(window.location.href).toBe('http://localhost/') + }) + + it('should work with nested link', async () => { + const nested = document.getElementById('bar') + await analytics.trackLink(nested, 'foo') + nested!.click() + + expect(mockTrack).toHaveBeenCalled() + + await resolveWhen(() => window.location.href === 'bar.com') + expect(window.location.href).toBe('bar.com') + }) + + it('should make a track call', async () => { + await analytics.trackLink(link!, 'foo') + link.click() + + expect(mockTrack).toHaveBeenCalled() + }) + + it('should still navigate even if the track call fails', async () => { + mockTrack.mockClear() + + let rejected = false + mockTrack.mockImplementationOnce(() => { + rejected = true + return Promise.reject(new Error('boo!')) + }) + + const nested = document.getElementById('bar') + await analytics.trackLink(nested, 'foo') + nested!.click() + + await resolveWhen(() => rejected) + await resolveWhen(() => window.location.href === 'bar.com') + expect(window.location.href).toBe('bar.com') + }) + + it('should still navigate even if the track call times out', async () => { + mockTrack.mockClear() + + let timedOut = false + mockTrack.mockImplementationOnce(async () => { + await sleep(600) + timedOut = true + return Promise.resolve() as any + }) + + const nested = document.getElementById('bar') + await analytics.trackLink(nested, 'foo') + nested!.click() + + await resolveWhen(() => window.location.href === 'bar.com') + expect(window.location.href).toBe('bar.com') + expect(timedOut).toBe(false) + + await resolveWhen(() => timedOut) + }) + + it('should accept a jquery object for an element', async () => { + const $link = jQuery(link) + await analytics.trackLink($link, 'foo') + link.click() + expect(mockTrack).toBeCalled() + }) + + it('accepts array of elements', async () => { + const links = [link, link] + await analytics.trackLink(links, 'foo') + link.click() + + expect(mockTrack).toHaveBeenCalled() + }) + + it('should send an event and properties', async () => { + await analytics.trackLink(link, 'event', { property: true }) + link.click() + + expect(mockTrack).toBeCalledWith('event', { property: true }, {}) + }) + + it('should accept an event function', async () => { + function event(el: Element): string { + return el.nodeName + } + await analytics.trackLink(link, event, { foo: 'bar' }) + link.click() + + expect(mockTrack).toBeCalledWith('A', { foo: 'bar' }, {}) + }) + + it('should accept a properties function', async () => { + function properties(el: Record): Record { + return { type: el.nodeName } + } + await analytics.trackLink(link, 'event', properties) + link.click() + + expect(mockTrack).toBeCalledWith('event', { type: 'A' }, {}) + }) + + it('should load an href on click', async () => { + link.href = '#test' + await analytics.trackLink(link, 'foo') + link.click() + + await resolveWhen(() => window.location.href === '#test') + expect(global.window.location.href).toBe('#test') + }) + + it('should only navigate after the track call has been completed', async () => { + link.href = '#test' + await analytics.trackLink(link, 'foo') + link.click() + + await resolveWhen(() => mockTrack.mock.calls.length === 1) + await resolveWhen(() => window.location.href === '#test') + + expect(global.window.location.href).toBe('#test') + }) + + it('should support svg .href attribute', async () => { + svg.setAttribute('href', '#svg') + await analytics.trackLink(svg, 'foo') + const clickEvent = new Event('click') + svg.dispatchEvent(clickEvent) + + await resolveWhen(() => window.location.href === '#svg') + expect(global.window.location.href).toBe('#svg') + }) + + it('should fallback to getAttributeNS', async () => { + svg.setAttributeNS('http://www.w3.org/1999/xlink', 'href', '#svg') + await analytics.trackLink(svg, 'foo') + const clickEvent = new Event('click') + svg.dispatchEvent(clickEvent) + + await resolveWhen(() => window.location.href === '#svg') + expect(global.window.location.href).toBe('#svg') + }) + + it('should support xlink:href', async () => { + svg.setAttribute('xlink:href', '#svg') + await analytics.trackLink(svg, 'foo') + const clickEvent = new Event('click') + svg.dispatchEvent(clickEvent) + + await resolveWhen(() => window.location.href === '#svg') + expect(global.window.location.href).toBe('#svg') + }) + + it('should not load an href for a link with a blank target', async () => { + link.href = '/base/test/support/mock.html' + link.target = '_blank' + await analytics.trackLink(link, 'foo') + link.click() + + await sleep(300) + expect(global.window.location.href).not.toBe('#test') + }) +}) diff --git a/packages/browser/src/core/analytics/index.ts b/packages/browser/src/core/analytics/index.ts index 8b88630c8..ca22c5924 100644 --- a/packages/browser/src/core/analytics/index.ts +++ b/packages/browser/src/core/analytics/index.ts @@ -54,6 +54,7 @@ import { } from '../storage' import { PluginFactory } from '../../plugins/remote-loader' import { setGlobalAnalytics } from '../../lib/global-analytics-helper' +import { popPageContext } from '../buffer' const deprecationWarning = 'This is being deprecated and will be not be available in future releases of Analytics JS' @@ -202,7 +203,6 @@ export class Analytics this.eventFactory = new EventFactory(this._user) this.integrations = options?.integrations ?? {} this.options = options ?? {} - autoBind(this) } @@ -252,13 +252,15 @@ export class Analytics } async track(...args: EventParams): Promise { + const pageCtx = popPageContext(args) const [name, data, opts, cb] = resolveArguments(...args) const segmentEvent = this.eventFactory.track( name, data as EventProperties, opts, - this.integrations + this.integrations, + pageCtx ) return this._dispatch(segmentEvent, cb).then((ctx) => { @@ -268,6 +270,7 @@ export class Analytics } async page(...args: PageParams): Promise { + const pageCtx = popPageContext(args) const [category, page, properties, options, callback] = resolvePageArguments(...args) @@ -276,7 +279,8 @@ export class Analytics page, properties, options, - this.integrations + this.integrations, + pageCtx ) return this._dispatch(segmentEvent, callback).then((ctx) => { @@ -286,6 +290,7 @@ export class Analytics } async identify(...args: IdentifyParams): Promise { + const pageCtx = popPageContext(args) const [id, _traits, options, callback] = resolveUserArguments(this._user)( ...args ) @@ -295,7 +300,8 @@ export class Analytics this._user.id(), this._user.traits(), options, - this.integrations + this.integrations, + pageCtx ) return this._dispatch(segmentEvent, callback).then((ctx) => { @@ -312,6 +318,7 @@ export class Analytics group(): Group group(...args: GroupParams): Promise group(...args: GroupParams): Promise | Group { + const pageCtx = popPageContext(args) if (args.length === 0) { return this._group } @@ -328,7 +335,8 @@ export class Analytics groupId, groupTraits, options, - this.integrations + this.integrations, + pageCtx ) return this._dispatch(segmentEvent, callback).then((ctx) => { @@ -338,12 +346,14 @@ export class Analytics } async alias(...args: AliasParams): Promise { + const pageCtx = popPageContext(args) const [to, from, options, callback] = resolveAliasArguments(...args) const segmentEvent = this.eventFactory.alias( to, from, options, - this.integrations + this.integrations, + pageCtx ) return this._dispatch(segmentEvent, callback).then((ctx) => { this.emit('alias', to, from, ctx.event.options) @@ -352,6 +362,7 @@ export class Analytics } async screen(...args: PageParams): Promise { + const pageCtx = popPageContext(args) const [category, page, properties, options, callback] = resolvePageArguments(...args) @@ -360,7 +371,8 @@ export class Analytics page, properties, options, - this.integrations + this.integrations, + pageCtx ) return this._dispatch(segmentEvent, callback).then((ctx) => { this.emit( diff --git a/packages/browser/src/core/buffer/__tests__/index.test.ts b/packages/browser/src/core/buffer/__tests__/index.test.ts index 071a4b6c1..5ff1c0dbc 100644 --- a/packages/browser/src/core/buffer/__tests__/index.test.ts +++ b/packages/browser/src/core/buffer/__tests__/index.test.ts @@ -4,65 +4,146 @@ import { PreInitMethodCall, flushAnalyticsCallsInNewTask, PreInitMethodCallBuffer, + PreInitMethodName, } from '..' import { Analytics } from '../../analytics' import { Context } from '../../context' import { sleep } from '../../../lib/sleep' import { User } from '../../user' +import { getBufferedPageCtxFixture } from '../../../test-helpers/fixtures' +import * as GlobalAnalytics from '../../../lib/global-analytics-helper' -describe('PreInitMethodCallBuffer', () => { - describe('push', () => { - it('should return this', async () => { - const buffer = new PreInitMethodCallBuffer() - const result = buffer.push({} as any) - expect(result).toBeInstanceOf(PreInitMethodCallBuffer) +describe(PreInitMethodCallBuffer, () => { + beforeEach(() => { + GlobalAnalytics.setGlobalAnalytics(undefined as any) + }) + + describe('toArray()', () => { + it('should convert the map back to an array', () => { + const call1 = new PreInitMethodCall('identify', [], jest.fn()) + const call2 = new PreInitMethodCall('identify', [], jest.fn()) + const call3 = new PreInitMethodCall('group', [], jest.fn()) + const buffer = new PreInitMethodCallBuffer(call1, call2, call3) + expect(buffer.toArray()).toEqual([call1, call2, call3]) + }) + + it('should also read from global analytics buffer', () => { + const call1 = new PreInitMethodCall('identify', ['foo'], jest.fn()) + ;(window as any).analytics = [['track', 'snippet']] + + const buffer = new PreInitMethodCallBuffer(call1) + const calls = buffer.toArray() + expect(calls.length).toBe(2) + expect(calls[0]).toEqual( + expect.objectContaining>({ + method: 'track', + args: ['snippet', getBufferedPageCtxFixture()], + }) + ) + expect(calls[1]).toEqual(call1) }) }) - describe('toArray should return an array', () => { - it('toArray() should convert the map back to an array', async () => { + + describe('push()', () => { + it('should add method calls', () => { + const call1 = new PreInitMethodCall('identify', [], jest.fn()) const buffer = new PreInitMethodCallBuffer() - const method1 = { method: 'foo' } as any - const method2 = { method: 'foo', args: [1] } as any - const method3 = { method: 'bar' } as any - buffer.push(method1, method2, method3) - expect(buffer.toArray()).toEqual([method1, method2, method3]) + buffer.push(call1) + expect(buffer.toArray()).toEqual([call1]) + }) + + it('should work if the calls were added at different times or in different ways', () => { + const call1 = new PreInitMethodCall('identify', [], jest.fn()) + const call2 = new PreInitMethodCall('identify', [], jest.fn()) + const call3 = new PreInitMethodCall('group', [], jest.fn()) + const call4 = new PreInitMethodCall('group', [], jest.fn()) + const buffer = new PreInitMethodCallBuffer(call1) + buffer.push(call2, call3) + buffer.push(call4) + expect(buffer.toArray()).toEqual([call1, call2, call3, call4]) }) }) - describe('clear', () => { - it('should return this', async () => { + describe('getCalls()', () => { + it('should fetch calls by name', async () => { const buffer = new PreInitMethodCallBuffer() - const result = buffer.push({} as any) - expect(result).toBeInstanceOf(PreInitMethodCallBuffer) + const call1 = new PreInitMethodCall('identify', [], jest.fn()) + const call2 = new PreInitMethodCall('identify', [], jest.fn()) + const call3 = new PreInitMethodCall('group', [], jest.fn()) + buffer.push(call1, call2, call3) + expect(buffer.getCalls('identify')).toEqual([call1, call2]) + expect(buffer.getCalls('group')).toEqual([call3]) + }) + it('should read from Snippet Buffer', () => { + const call1 = new PreInitMethodCall('identify', ['foo'], jest.fn()) + GlobalAnalytics.setGlobalAnalytics([['identify', 'snippet']] as any) + + const buffer = new PreInitMethodCallBuffer(call1) + const calls = buffer.getCalls('identify') + expect(calls.length).toBe(2) + expect(calls[0]).toEqual( + expect.objectContaining>({ + method: 'identify', + args: ['snippet', getBufferedPageCtxFixture()], + }) + ) + expect(calls[1]).toEqual(call1) + }) + }) + describe('clear()', () => { + it('should clear calls', () => { + const call1 = new PreInitMethodCall('identify', [], jest.fn()) + const call2 = new PreInitMethodCall('identify', [], jest.fn()) + const call3 = new PreInitMethodCall('group', [], jest.fn()) + GlobalAnalytics.setGlobalAnalytics([['track', 'bar']] as any) + const buffer = new PreInitMethodCallBuffer(call1, call2, call3) + buffer.clear() + expect(buffer.toArray()).toEqual([]) + expect(GlobalAnalytics.getGlobalAnalytics()).toEqual([]) }) }) - describe('getCalls', () => { - it('should return calls', async () => { - const buffer = new PreInitMethodCallBuffer() - - const fooCall1 = { - method: 'foo', - args: ['bar'], - } as any - - const barCall = { - method: 'bar', - args: ['foobar'], - } as any - const fooCall2 = { - method: 'foo', - args: ['baz'], - } as any + describe('Snippet buffer (method calls)', () => { + it('should be read from the global analytics instance', () => { + const getGlobalAnalyticsSpy = jest.spyOn( + GlobalAnalytics, + 'getGlobalAnalytics' + ) - const calls: PreInitMethodCall[] = [fooCall1, fooCall2, barCall] - const result = buffer.push(...calls) - expect(result.getCalls('foo' as any)).toEqual([fooCall1, fooCall2]) + const buffer = new PreInitMethodCallBuffer() + expect(getGlobalAnalyticsSpy).not.toBeCalled() + buffer.toArray() + expect(getGlobalAnalyticsSpy).toBeCalled() + }) + }) + describe('BufferedPageContext', () => { + test.each([ + 'track', + 'screen', + 'alias', + 'group', + 'page', + 'identify', + ] as PreInitMethodName[])('should be appended to %p calls.', (method) => { + const buffer = new PreInitMethodCallBuffer( + new PreInitMethodCall(method, ['foo'], jest.fn()) + ) + expect(buffer.getCalls(method)[0].args).toEqual([ + 'foo', + getBufferedPageCtxFixture(), + ]) + }) + it('should not be appended for other method calls', () => { + const fn = jest.fn() + const onCall = new PreInitMethodCall('on', ['foo', fn]) + expect(onCall.args).toEqual(['foo', fn]) + const setAnonIdCall = new PreInitMethodCall('setAnonymousId', []) + expect(setAnonIdCall.args).toEqual([]) }) }) }) -describe('AnalyticsBuffered', () => { +describe(AnalyticsBuffered, () => { describe('Happy path', () => { it('should return a promise-like object', async () => { const ajs = new Analytics({ writeKey: 'foo' }) @@ -220,7 +301,7 @@ describe('AnalyticsBuffered', () => { }) }) -describe('callAnalyticsMethod', () => { +describe(callAnalyticsMethod, () => { let ajs!: Analytics let resolveSpy!: jest.Mock let rejectSpy!: jest.Mock @@ -228,14 +309,12 @@ describe('callAnalyticsMethod', () => { beforeEach(() => { resolveSpy = jest.fn().mockImplementation((el) => `resolved: ${el}`) rejectSpy = jest.fn().mockImplementation((el) => `rejected: ${el}`) - methodCall = { - args: ['foo', {}], - called: false, - method: 'track', - resolve: resolveSpy, - reject: rejectSpy, - } as PreInitMethodCall - + methodCall = new PreInitMethodCall( + 'track', + ['foo', {}], + resolveSpy, + rejectSpy + ) ajs = new Analytics({ writeKey: 'abc', }) @@ -297,7 +376,7 @@ describe('callAnalyticsMethod', () => { }) }) -describe('flushAnalyticsCallsInNewTask', () => { +describe(flushAnalyticsCallsInNewTask, () => { test('should defer buffered method calls, regardless of whether or not they are async', async () => { // @ts-ignore Analytics.prototype['synchronousMethod'] = () => 123 @@ -305,27 +384,21 @@ describe('flushAnalyticsCallsInNewTask', () => { // @ts-ignore Analytics.prototype['asyncMethod'] = () => Promise.resolve(123) - const synchronousMethod = { - method: 'synchronousMethod' as any, - args: ['foo'], - called: false, - resolve: jest.fn(), - reject: jest.fn(), - } as PreInitMethodCall - - const asyncMethod = { - method: 'asyncMethod' as any, - args: ['foo'], - called: false, - resolve: jest.fn(), - reject: jest.fn(), - } as PreInitMethodCall - - const buffer = new PreInitMethodCallBuffer().push( - synchronousMethod, - asyncMethod + const synchronousMethod = new PreInitMethodCall( + 'synchronousMethod' as any, + ['foo'], + jest.fn(), + jest.fn() + ) + + const asyncMethod = new PreInitMethodCall( + 'asyncMethod' as any, + ['foo'], + jest.fn(), + jest.fn() ) + const buffer = new PreInitMethodCallBuffer(synchronousMethod, asyncMethod) flushAnalyticsCallsInNewTask(new Analytics({ writeKey: 'abc' }), buffer) expect(synchronousMethod.resolve).not.toBeCalled() expect(asyncMethod.resolve).not.toBeCalled() @@ -338,15 +411,14 @@ describe('flushAnalyticsCallsInNewTask', () => { // @ts-ignore Analytics.prototype['asyncMethod'] = () => Promise.reject('oops!') - const asyncMethod = { - method: 'asyncMethod' as any, - args: ['foo'], - called: false, - resolve: jest.fn(), - reject: jest.fn(), - } as PreInitMethodCall + const asyncMethod = new PreInitMethodCall( + 'asyncMethod' as any, + ['foo'], + jest.fn(), + jest.fn() + ) - const buffer = new PreInitMethodCallBuffer().push(asyncMethod) + const buffer = new PreInitMethodCallBuffer(asyncMethod) flushAnalyticsCallsInNewTask(new Analytics({ writeKey: 'abc' }), buffer) await sleep(0) expect(asyncMethod.reject).toBeCalledWith('oops!') @@ -361,29 +433,23 @@ describe('flushAnalyticsCallsInNewTask', () => { throw new Error('Ooops!') } - const synchronousMethod = { - method: 'synchronousMethod' as any, - args: ['foo'], - called: false, - resolve: jest.fn(), - reject: jest.fn(), - } as PreInitMethodCall - - const asyncMethod = { - method: 'asyncMethod' as any, - args: ['foo'], - called: false, - resolve: jest.fn(), - reject: jest.fn(), - } as PreInitMethodCall - - const buffer = new PreInitMethodCallBuffer().push( - synchronousMethod, - asyncMethod + const synchronousMethod = new PreInitMethodCall( + 'synchronousMethod' as any, + ['foo'], + jest.fn(), + jest.fn() + ) + const asyncMethod = new PreInitMethodCall( + 'asyncMethod' as any, + ['foo'], + jest.fn(), + jest.fn() ) + + const buffer = new PreInitMethodCallBuffer(synchronousMethod, asyncMethod) flushAnalyticsCallsInNewTask(new Analytics({ writeKey: 'abc' }), buffer) await sleep(0) - expect(synchronousMethod.reject).toBeCalled() - expect(asyncMethod.resolve).toBeCalled() + expect(synchronousMethod.reject).toBeCalledTimes(1) + expect(asyncMethod.resolve).toBeCalledTimes(1) }) }) diff --git a/packages/browser/src/core/buffer/index.ts b/packages/browser/src/core/buffer/index.ts index d7b42edb0..e5604cad1 100644 --- a/packages/browser/src/core/buffer/index.ts +++ b/packages/browser/src/core/buffer/index.ts @@ -3,6 +3,14 @@ import { Context } from '../context' import { isThenable } from '../../lib/is-thenable' import { AnalyticsBrowserCore } from '../analytics/interfaces' import { version } from '../../generated/version' +import { getGlobalAnalytics } from '../../lib/global-analytics-helper' +import { + isBufferedPageContext, + BufferedPageContext, + getDefaultBufferedPageContext, + createPageContext, + PageContext, +} from '../page' /** * The names of any AnalyticsBrowser methods that also exist on Analytics @@ -80,10 +88,24 @@ export const flushAnalyticsCallsInNewTask = ( }) } +export const popPageContext = (args: unknown[]): PageContext | undefined => { + if (hasBufferedPageContextAsLastArg(args)) { + const ctx = args.pop() as BufferedPageContext + return createPageContext(ctx) + } +} + +export const hasBufferedPageContextAsLastArg = ( + args: unknown[] +): args is [...unknown[], BufferedPageContext] | [BufferedPageContext] => { + const lastArg = args[args.length - 1] + return isBufferedPageContext(lastArg) +} + /** * Represents a buffered method call that occurred before initialization. */ -export interface PreInitMethodCall< +export class PreInitMethodCall< MethodName extends PreInitMethodName = PreInitMethodName > { method: MethodName @@ -91,10 +113,23 @@ export interface PreInitMethodCall< called: boolean resolve: (v: ReturnType) => void reject: (reason: any) => void + constructor( + method: PreInitMethodCall['method'], + args: PreInitMethodParams, + resolve: PreInitMethodCall['resolve'] = () => {}, + reject: PreInitMethodCall['reject'] = console.error + ) { + this.method = method + this.resolve = resolve + this.reject = reject + this.called = false + this.args = args + } } export type PreInitMethodParams = - Parameters + | [...Parameters, BufferedPageContext] + | Parameters /** * Infer return type; if return type is promise, unwrap it. @@ -107,34 +142,91 @@ type ReturnTypeUnwrap = Fn extends (...args: any[]) => infer ReturnT type MethodCallMap = Partial> +type SnippetWindowBufferedMethodCall< + MethodName extends PreInitMethodName = PreInitMethodName +> = [MethodName, ...PreInitMethodParams] + +/** + * A list of the method calls before initialization for snippet users + * For example, [["track", "foo", {bar: 123}], ["page"], ["on", "ready", function(){..}] + */ +type SnippetBuffer = SnippetWindowBufferedMethodCall[] + /** * Represents any and all the buffered method calls that occurred before initialization. */ export class PreInitMethodCallBuffer { - private _value = {} as MethodCallMap + private _callMap: MethodCallMap = {} - toArray(): PreInitMethodCall[] { - return ([] as PreInitMethodCall[]).concat(...Object.values(this._value)) + constructor(...calls: PreInitMethodCall[]) { + this.push(...calls) + } + + /** + * Pull any buffered method calls from the window object, and use them to hydrate the instance buffer. + */ + private get calls() { + this._pushSnippetWindowBuffer() + return this._callMap + } + + private set calls(calls: MethodCallMap) { + this._callMap = calls } getCalls(methodName: T): PreInitMethodCall[] { - return (this._value[methodName] ?? []) as PreInitMethodCall[] + return (this.calls[methodName] ?? []) as PreInitMethodCall[] } - push(...calls: PreInitMethodCall[]): PreInitMethodCallBuffer { + push(...calls: PreInitMethodCall[]): void { calls.forEach((call) => { - if (this._value[call.method]) { - this._value[call.method]!.push(call) + const eventsExpectingPageContext: PreInitMethodName[] = [ + 'track', + 'screen', + 'alias', + 'group', + 'page', + 'identify', + ] + if ( + eventsExpectingPageContext.includes(call.method) && + !hasBufferedPageContextAsLastArg(call.args) + ) { + call.args = [...call.args, getDefaultBufferedPageContext()] + } + + if (this.calls[call.method]) { + this.calls[call.method]!.push(call) } else { - this._value[call.method] = [call] + this.calls[call.method] = [call] } }) - return this } - clear(): PreInitMethodCallBuffer { - this._value = {} as MethodCallMap - return this + clear(): void { + // clear calls in the global snippet buffered array. + this._pushSnippetWindowBuffer() + // clear calls in this instance + this.calls = {} + } + + toArray(): PreInitMethodCall[] { + return ([] as PreInitMethodCall[]).concat(...Object.values(this.calls)) + } + + /** + * Fetch the buffered method calls from the window object, + * normalize them, and use them to hydrate the buffer. + * This removes existing buffered calls from the window object. + */ + private _pushSnippetWindowBuffer(): void { + const wa = getGlobalAnalytics() + if (!Array.isArray(wa)) return undefined + const buffered: SnippetBuffer = wa.splice(0, wa.length) + const calls = buffered.map( + ([methodName, ...args]) => new PreInitMethodCall(methodName, args) + ) + this.push(...calls) } } @@ -176,9 +268,10 @@ export class AnalyticsBuffered { instance?: Analytics ctx?: Context - private _preInitBuffer = new PreInitMethodCallBuffer() + private _preInitBuffer: PreInitMethodCallBuffer private _promise: Promise<[Analytics, Context]> constructor(loader: AnalyticsLoader) { + this._preInitBuffer = new PreInitMethodCallBuffer() this._promise = loader(this._preInitBuffer) this._promise .then(([ajs, ctx]) => { @@ -251,15 +344,10 @@ export class AnalyticsBuffered const result = (this.instance[methodName] as Function)(...args) return Promise.resolve(result) } - return new Promise((resolve, reject) => { - this._preInitBuffer.push({ - method: methodName, - args, - resolve: resolve, - reject: reject, - called: false, - } as PreInitMethodCall) + this._preInitBuffer.push( + new PreInitMethodCall(methodName, args, resolve as any, reject) + ) }) } } @@ -274,13 +362,7 @@ export class AnalyticsBuffered void (this.instance[methodName] as Function)(...args) return this } else { - this._preInitBuffer.push({ - method: methodName, - args, - resolve: () => {}, - reject: console.error, - called: false, - } as PreInitMethodCall) + this._preInitBuffer.push(new PreInitMethodCall(methodName, args)) } return this diff --git a/packages/browser/src/core/buffer/snippet.ts b/packages/browser/src/core/buffer/snippet.ts deleted file mode 100644 index 73612b3f6..000000000 --- a/packages/browser/src/core/buffer/snippet.ts +++ /dev/null @@ -1,46 +0,0 @@ -import type { - PreInitMethodCall, - PreInitMethodName, - PreInitMethodParams, -} from '.' -import { getGlobalAnalytics } from '../../lib/global-analytics-helper' - -export function transformSnippetCall([ - methodName, - ...args -]: SnippetWindowBufferedMethodCall): PreInitMethodCall { - return { - method: methodName, - resolve: () => {}, - reject: console.error, - args, - called: false, - } -} - -const normalizeSnippetBuffer = (buffer: SnippetBuffer): PreInitMethodCall[] => { - return buffer.map(transformSnippetCall) -} - -type SnippetWindowBufferedMethodCall< - MethodName extends PreInitMethodName = PreInitMethodName -> = [MethodName, ...PreInitMethodParams] - -/** - * A list of the method calls before initialization for snippet users - * For example, [["track", "foo", {bar: 123}], ["page"], ["on", "ready", function(){..}] - */ -export type SnippetBuffer = SnippetWindowBufferedMethodCall[] - -/** - * Fetch the buffered method calls from the window object and normalize them. - * This removes existing buffered calls from the window object. - */ -export const popSnippetWindowBuffer = ( - buffer: unknown = getGlobalAnalytics() -): PreInitMethodCall[] => { - const wa = buffer - if (!Array.isArray(wa)) return [] - const buffered = wa.splice(0, wa.length) - return normalizeSnippetBuffer(buffered) -} diff --git a/packages/browser/src/core/events/__tests__/index.test.ts b/packages/browser/src/core/events/__tests__/index.test.ts index 1b939011f..d9b53250a 100644 --- a/packages/browser/src/core/events/__tests__/index.test.ts +++ b/packages/browser/src/core/events/__tests__/index.test.ts @@ -1,6 +1,7 @@ import uuid from '@lukeed/uuid' import { range, uniq } from 'lodash' import { EventFactory } from '..' +import { getDefaultPageContext } from '../../page' import { User } from '../../user' import { SegmentEvent, Options } from '../interfaces' @@ -11,10 +12,15 @@ describe('Event Factory', () => { const shoes = { product: 'shoes', total: '$35', category: 'category' } const shopper = { totalSpent: 100 } + const defaultContext = { + page: getDefaultPageContext(), + } + beforeEach(() => { user = new User() user.reset() factory = new EventFactory(user) + defaultContext.page = getDefaultPageContext() }) describe('alias', () => { @@ -67,7 +73,7 @@ describe('Event Factory', () => { it('accepts properties', () => { const page = factory.page('category', 'name', shoes) - expect(page.properties).toEqual(shoes) + expect(page.properties).toEqual(expect.objectContaining(shoes)) }) it('ignores category and page if not passed in', () => { @@ -153,7 +159,7 @@ describe('Event Factory', () => { const track = factory.track('Order Completed', shoes, { opt1: true, }) - expect(track.context).toEqual({ opt1: true }) + expect(track.context).toEqual({ ...defaultContext, opt1: true }) }) test('sets context correctly if property arg is undefined', () => { @@ -161,7 +167,10 @@ describe('Event Factory', () => { context: { page: { path: '/custom' } }, }) - expect(track.context).toEqual({ page: { path: '/custom' } }) + expect(track.context?.page).toEqual({ + ...defaultContext.page, + path: '/custom', + }) }) test('sets integrations', () => { @@ -243,7 +252,11 @@ describe('Event Factory', () => { }, }) - expect(track.context).toEqual({ opt1: true, opt2: '🥝' }) + expect(track.context).toEqual({ + ...defaultContext, + opt1: true, + opt2: '🥝', + }) }) test('should not move known options into `context`', () => { @@ -257,7 +270,11 @@ describe('Event Factory', () => { timestamp: new Date(), }) - expect(track.context).toEqual({ opt1: true, opt2: '🥝' }) + expect(track.context).toEqual({ + ...defaultContext, + opt1: true, + opt2: '🥝', + }) }) test('accepts an anonymous id', () => { @@ -265,7 +282,7 @@ describe('Event Factory', () => { anonymousId: 'anon-1', }) - expect(track.context).toEqual({}) + expect(track.context).toEqual(defaultContext) expect(track.anonymousId).toEqual('anon-1') }) @@ -275,7 +292,7 @@ describe('Event Factory', () => { timestamp, }) - expect(track.context).toEqual({}) + expect(track.context).toEqual(defaultContext) expect(track.timestamp).toEqual(timestamp) }) @@ -302,6 +319,7 @@ describe('Event Factory', () => { }) expect(track.context).toEqual({ + ...defaultContext, library: { name: 'ajs-next', version: '0.1.0', @@ -322,6 +340,7 @@ describe('Event Factory', () => { }) expect(track.context).toEqual({ + ...defaultContext, library: { name: 'ajs-next', version: '0.1.0', @@ -395,9 +414,37 @@ describe('Event Factory', () => { integrations: { Segment: true }, type: 'track', userId: 'user-id', - context: {}, + context: defaultContext, }) }) }) }) + + describe('Page context augmentation', () => { + // minimal tests -- more tests should be specifically around addPageContext + factory = new EventFactory(new User()) + it('adds a default pageContext if pageContext is not defined', () => { + const event = factory.identify('foo') + expect(event.context?.page).toEqual(defaultContext.page) + }) + + const pageCtx = { ...defaultContext.page, title: 'foo test' } + test.each([ + factory.identify('foo', undefined, undefined, undefined, { + ...pageCtx, + }), + factory.track('foo', undefined, undefined, undefined, { + ...pageCtx, + }), + factory.group('foo', undefined, undefined, undefined, { + ...pageCtx, + }), + factory.page('foo', 'bar', undefined, undefined, undefined, { + ...pageCtx, + }), + factory.alias('foo', 'bar', undefined, undefined, { ...pageCtx }), + ])(`$type: Event has the expected page properties`, async (event) => { + expect(event.context?.page).toEqual(pageCtx) + }) + }) }) diff --git a/packages/browser/src/core/events/index.ts b/packages/browser/src/core/events/index.ts index 8af4820f9..b213cc55b 100644 --- a/packages/browser/src/core/events/index.ts +++ b/packages/browser/src/core/events/index.ts @@ -9,30 +9,31 @@ import { SegmentEvent, } from './interfaces' import md5 from 'spark-md5' +import { addPageContext, PageContext } from '../page' export * from './interfaces' export class EventFactory { - user: User - - constructor(user: User) { - this.user = user - } + constructor(public user: User) {} track( event: string, properties?: EventProperties, options?: Options, - globalIntegrations?: Integrations + globalIntegrations?: Integrations, + pageCtx?: PageContext ): SegmentEvent { - return this.normalize({ - ...this.baseEvent(), - event, - type: 'track' as const, - properties, - options: { ...options }, - integrations: { ...globalIntegrations }, - }) + return this.normalize( + { + ...this.baseEvent(), + event, + type: 'track' as const, + properties, + options: { ...options }, + integrations: { ...globalIntegrations }, + }, + pageCtx + ) } page( @@ -40,7 +41,8 @@ export class EventFactory { page: string | null, properties?: EventProperties, options?: Options, - globalIntegrations?: Integrations + globalIntegrations?: Integrations, + pageCtx?: PageContext ): SegmentEvent { const event: Partial = { type: 'page' as const, @@ -59,10 +61,13 @@ export class EventFactory { event.name = page } - return this.normalize({ - ...this.baseEvent(), - ...event, - } as SegmentEvent) + return this.normalize( + { + ...this.baseEvent(), + ...event, + } as SegmentEvent, + pageCtx + ) } screen( @@ -70,7 +75,8 @@ export class EventFactory { screen: string | null, properties?: EventProperties, options?: Options, - globalIntegrations?: Integrations + globalIntegrations?: Integrations, + pageCtx?: PageContext ): SegmentEvent { const event: Partial = { type: 'screen' as const, @@ -86,50 +92,61 @@ export class EventFactory { if (screen !== null) { event.name = screen } - - return this.normalize({ - ...this.baseEvent(), - ...event, - } as SegmentEvent) + return this.normalize( + { + ...this.baseEvent(), + ...event, + } as SegmentEvent, + pageCtx + ) } identify( userId: ID, traits?: Traits, options?: Options, - globalIntegrations?: Integrations + globalIntegrations?: Integrations, + pageCtx?: PageContext ): SegmentEvent { - return this.normalize({ - ...this.baseEvent(), - type: 'identify' as const, - userId, - traits, - options: { ...options }, - integrations: { ...globalIntegrations }, - }) + return this.normalize( + { + ...this.baseEvent(), + type: 'identify' as const, + userId, + traits, + options: { ...options }, + integrations: { ...globalIntegrations }, + }, + pageCtx + ) } group( groupId: ID, traits?: Traits, options?: Options, - globalIntegrations?: Integrations + globalIntegrations?: Integrations, + pageCtx?: PageContext ): SegmentEvent { - return this.normalize({ - ...this.baseEvent(), - type: 'group' as const, - traits, - options: { ...options }, - integrations: { ...globalIntegrations }, - groupId, - }) + return this.normalize( + { + ...this.baseEvent(), + type: 'group' as const, + traits, + options: { ...options }, + integrations: { ...globalIntegrations }, + groupId, + }, + pageCtx + ) } alias( to: string, from: string | null, options?: Options, - globalIntegrations?: Integrations + globalIntegrations?: Integrations, + pageCtx?: PageContext ): SegmentEvent { const base: Partial = { userId: to, @@ -149,10 +166,13 @@ export class EventFactory { } as SegmentEvent) } - return this.normalize({ - ...this.baseEvent(), - ...base, - } as SegmentEvent) + return this.normalize( + { + ...this.baseEvent(), + ...base, + } as SegmentEvent, + pageCtx + ) } private baseEvent(): Partial { @@ -204,7 +224,7 @@ export class EventFactory { return [context, overrides] } - public normalize(event: SegmentEvent): SegmentEvent { + public normalize(event: SegmentEvent, pageCtx?: PageContext): SegmentEvent { // set anonymousId globally if we encounter an override //segment.com/docs/connections/sources/catalog/libraries/website/javascript/identity/#override-the-anonymous-id-using-the-options-object event.options?.anonymousId && @@ -235,21 +255,16 @@ export class EventFactory { const [context, overrides] = this.context(event) const { options, ...rest } = event - const body = { + const newEvent: SegmentEvent = { timestamp: new Date(), ...rest, context, integrations: allIntegrations, ...overrides, + messageId: 'ajs-next-' + md5.hash(JSON.stringify(event) + uuid()), } + addPageContext(newEvent, pageCtx) - const messageId = 'ajs-next-' + md5.hash(JSON.stringify(body) + uuid()) - - const evt: SegmentEvent = { - ...body, - messageId, - } - - return evt + return newEvent } } diff --git a/packages/browser/src/core/page/__tests__/index.test.ts b/packages/browser/src/core/page/__tests__/index.test.ts new file mode 100644 index 000000000..b1f166cff --- /dev/null +++ b/packages/browser/src/core/page/__tests__/index.test.ts @@ -0,0 +1,130 @@ +import { + BufferedPageContextDiscriminant, + getDefaultBufferedPageContext, + getDefaultPageContext, + isBufferedPageContext, +} from '../' +import { pickBy } from 'lodash' + +const originalLocation = window.location +beforeEach(() => { + Object.defineProperty(window, 'location', { + value: { + ...originalLocation, + }, + writable: true, + }) +}) + +describe(isBufferedPageContext, () => { + it('should return true if object is page context', () => { + expect(isBufferedPageContext(getDefaultBufferedPageContext())).toBe(true) + }) + it('should return false if object is not page context', () => { + expect(isBufferedPageContext(undefined)).toBe(false) + expect(isBufferedPageContext({})).toBe(false) + expect(isBufferedPageContext('')).toBe(false) + expect(isBufferedPageContext({ foo: false })).toBe(false) + expect(isBufferedPageContext({ u: 'hello' })).toBe(false) + expect(isBufferedPageContext(null)).toBe(false) + + expect( + isBufferedPageContext({ + ...getDefaultBufferedPageContext(), + some_unknown_key: 123, + }) + ).toBe(false) + + const missingDiscriminant = pickBy( + getDefaultBufferedPageContext(), + (v) => v !== BufferedPageContextDiscriminant + ) + // should not be missing the dscriminant + expect(isBufferedPageContext(missingDiscriminant)).toBe(false) + }) +}) + +describe(getDefaultPageContext, () => { + describe('hash', () => { + it('strips the hash from the URL', () => { + window.location.href = 'http://www.segment.local#test' + const defs = getDefaultPageContext() + expect(defs.url).toBe('http://www.segment.local') + + window.location.href = 'http://www.segment.local/#test' + const defs2 = getDefaultPageContext() + expect(defs2.url).toBe('http://www.segment.local/') + }) + }) + + describe('canonical URL', () => { + const el = document.createElement('link') + beforeEach(() => { + el.setAttribute('rel', 'canonical') + el.setAttribute('href', '') + document.clear() + }) + + it('returns location.href if canonical URL does not exist', () => { + el.setAttribute('rel', 'nope') + document.body.appendChild(el) + const defs = getDefaultPageContext() + expect(defs.url).toEqual(window.location.href) + }) + + it('does not throw an error if canonical URL is not a valid URL', () => { + el.setAttribute('href', 'foo.com/bar') + document.body.appendChild(el) + const defs = getDefaultPageContext() + expect(defs.url).toEqual('foo.com/bar') // this is analytics.js classic behavior + expect(defs.path).toEqual('/foo.com/bar') // this is analytics.js classic behavior + }) + + it('handles a leading slash', () => { + el.setAttribute('href', 'foo') + document.body.appendChild(el) + const defs = getDefaultPageContext() + expect(defs.url).toEqual('foo') + expect(defs.path).toEqual('/foo') // this is analytics.js classic behavior + }) + + it('handles canonical links', () => { + el.setAttribute('href', 'http://www.segment.local') + document.body.appendChild(el) + const defs = getDefaultPageContext() + expect(defs.url).toEqual('http://www.segment.local') + }) + + it('favors canonical path over location.pathname', () => { + window.location.pathname = '/nope' + el.setAttribute('href', 'http://www.segment.local/test') + document.body.appendChild(el) + const defs = getDefaultPageContext() + expect(defs.path).toEqual('/test') + }) + + it('handles canonical links with a path', () => { + el.setAttribute('href', 'http://www.segment.local/test') + document.body.appendChild(el) + const defs = getDefaultPageContext() + expect(defs.url).toEqual('http://www.segment.local/test') + expect(defs.path).toEqual('/test') + }) + + it('handles canonical links with search params in the url', () => { + el.setAttribute('href', 'http://www.segment.local?test=true') + document.body.appendChild(el) + const defs = getDefaultPageContext() + expect(defs.url).toEqual('http://www.segment.local?test=true') + }) + + it('will add search params from the document to the canonical path if it does not have search params', () => { + // This seems like weird behavior to me, but I found it in the codebase so adding a test for it. + window.location.search = '?foo=123' + el.setAttribute('href', 'http://www.segment.local') + document.body.appendChild(el) + const defs = getDefaultPageContext() + expect(defs.url).toEqual('http://www.segment.local?foo=123') + }) + }) +}) diff --git a/packages/browser/src/core/page/add-page-context.ts b/packages/browser/src/core/page/add-page-context.ts new file mode 100644 index 000000000..9d2c61148 --- /dev/null +++ b/packages/browser/src/core/page/add-page-context.ts @@ -0,0 +1,33 @@ +import { pick } from '../../lib/pick' +import { EventProperties, SegmentEvent } from '../events' +import { getDefaultPageContext } from './get-page-context' + +/** + * Augments a segment event with information about the current page. + * Page information like URL changes frequently, so this is meant to be captured as close to the event call as possible. + * Things like `userAgent` do not change, so they can be added later in the flow. + * We prefer not to add this information to this function, as it increases the main bundle size. + */ +export const addPageContext = ( + event: SegmentEvent, + pageCtx = getDefaultPageContext() +): void => { + const evtCtx = event.context! // Context should be set earlier in the flow + let pageContextFromEventProps: Pick | undefined + if (event.type === 'page') { + pageContextFromEventProps = + event.properties && pick(event.properties, Object.keys(pageCtx)) + + event.properties = { + ...pageCtx, + ...event.properties, + ...(event.name ? { name: event.name } : {}), + } + } + + evtCtx.page = { + ...pageCtx, + ...pageContextFromEventProps, + ...evtCtx.page, + } +} diff --git a/packages/browser/src/core/page/get-page-context.ts b/packages/browser/src/core/page/get-page-context.ts new file mode 100644 index 000000000..3a56bd783 --- /dev/null +++ b/packages/browser/src/core/page/get-page-context.ts @@ -0,0 +1,140 @@ +import { isPlainObject } from '@segment/analytics-core' + +/** + * Final Page Context object expected in the Segment Event context + */ +export interface PageContext { + path: string + referrer: string + search: string + title: string + url: string +} + +type CanonicalUrl = string | undefined + +export const BufferedPageContextDiscriminant = 'bpc' as const +/** + * Page Context expected to be built by the snippet. + * Note: The key names are super short because we want to keep the strings in the html snippet short to save bytes. + */ +export interface BufferedPageContext { + __t: typeof BufferedPageContextDiscriminant // for extra uniqeness + c: CanonicalUrl + p: PageContext['path'] + u: PageContext['url'] + s: PageContext['search'] + t: PageContext['title'] + r: PageContext['referrer'] +} + +/** + * `BufferedPageContext` object builder + */ +export const createBufferedPageContext = ( + url: string, + canonicalUrl: CanonicalUrl, + search: string, + path: string, + title: string, + referrer: string +): BufferedPageContext => ({ + __t: BufferedPageContextDiscriminant, + c: canonicalUrl, + p: path, + u: url, + s: search, + t: title, + r: referrer, +}) + +// my clever/dubious way of making sure this type guard does not get out sync with the type definition +const BUFFERED_PAGE_CONTEXT_KEYS = Object.keys( + createBufferedPageContext('', '', '', '', '', '') +) as (keyof BufferedPageContext)[] + +export function isBufferedPageContext( + bufferedPageCtx: unknown +): bufferedPageCtx is BufferedPageContext { + if (!isPlainObject(bufferedPageCtx)) return false + if (bufferedPageCtx.__t !== BufferedPageContextDiscriminant) return false + + // ensure obj has all the keys we expect, and none we don't. + for (const k in bufferedPageCtx) { + if (!BUFFERED_PAGE_CONTEXT_KEYS.includes(k as keyof BufferedPageContext)) { + return false + } + } + return true +} + +// Legacy logic: we are we appending search parameters to the canonical URL -- I guess the canonical URL is "not canonical enough" (lol) +const createCanonicalURL = (canonicalUrl: string, searchParams: string) => { + return canonicalUrl.indexOf('?') > -1 + ? canonicalUrl + : canonicalUrl + searchParams +} + +/** + * Strips hash from URL. + * http://www.segment.local#test -> http://www.segment.local + */ +const removeHash = (href: string) => { + const hashIdx = href.indexOf('#') + return hashIdx === -1 ? href : href.slice(0, hashIdx) +} + +const parseCanonicalPath = (canonicalUrl: string): string => { + try { + return new URL(canonicalUrl).pathname + } catch (_e) { + // this is classic behavior -- we assume that if the canonical URL is invalid, it's a raw path. + return canonicalUrl[0] === '/' ? canonicalUrl : '/' + canonicalUrl + } +} + +/** + * Create a `PageContext` from a `BufferedPageContext`. + * `BufferedPageContext` keys are minified to save bytes in the snippet. + */ +export const createPageContext = ({ + c: canonicalUrl, + p: pathname, + s: search, + u: url, + r: referrer, + t: title, +}: BufferedPageContext): PageContext => { + const newPath = canonicalUrl ? parseCanonicalPath(canonicalUrl) : pathname + const newUrl = canonicalUrl + ? createCanonicalURL(canonicalUrl, search) + : removeHash(url) + return { + path: newPath, + referrer, + search, + title, + url: newUrl, + } +} + +/** + * Get page properties from the browser window/document. + */ +export const getDefaultBufferedPageContext = (): BufferedPageContext => { + const c = document.querySelector("link[rel='canonical']") + return createBufferedPageContext( + location.href, + (c && c.getAttribute('href')) || undefined, + location.search, + location.pathname, + document.title, + document.referrer + ) +} + +/** + * Get page properties from the browser window/document. + */ +export const getDefaultPageContext = (): PageContext => + createPageContext(getDefaultBufferedPageContext()) diff --git a/packages/browser/src/core/page/index.ts b/packages/browser/src/core/page/index.ts new file mode 100644 index 000000000..c6ff6b3cf --- /dev/null +++ b/packages/browser/src/core/page/index.ts @@ -0,0 +1,2 @@ +export * from './get-page-context' +export * from './add-page-context' diff --git a/packages/browser/src/core/storage/__tests__/test-helpers.ts b/packages/browser/src/core/storage/__tests__/test-helpers.ts index 20faf069a..0971bdc57 100644 --- a/packages/browser/src/core/storage/__tests__/test-helpers.ts +++ b/packages/browser/src/core/storage/__tests__/test-helpers.ts @@ -1,16 +1,15 @@ import jar from 'js-cookie' +const throwDisabledError = () => { + throw new Error('__sorry brah, this storage is disabled__') +} /** * Disables Cookies * @returns jest spy */ export function disableCookies(): void { jest.spyOn(window.navigator, 'cookieEnabled', 'get').mockReturnValue(false) - jest.spyOn(jar, 'set').mockImplementation(() => { - throw new Error() - }) - jest.spyOn(jar, 'get').mockImplementation(() => { - throw new Error() - }) + jest.spyOn(jar, 'set').mockImplementation(throwDisabledError) + jest.spyOn(jar, 'get').mockImplementation(throwDisabledError) } /** @@ -18,10 +17,10 @@ export function disableCookies(): void { * @returns jest spy */ export function disableLocalStorage(): void { - jest.spyOn(Storage.prototype, 'setItem').mockImplementation(() => { - throw new Error() - }) - jest.spyOn(Storage.prototype, 'getItem').mockImplementation(() => { - throw new Error() - }) + jest + .spyOn(Storage.prototype, 'setItem') + .mockImplementation(throwDisabledError) + jest + .spyOn(Storage.prototype, 'getItem') + .mockImplementation(throwDisabledError) } diff --git a/packages/browser/src/core/storage/__tests__/universalStorage.test.ts b/packages/browser/src/core/storage/__tests__/universalStorage.test.ts index 3c813ebb9..5da7429e5 100644 --- a/packages/browser/src/core/storage/__tests__/universalStorage.test.ts +++ b/packages/browser/src/core/storage/__tests__/universalStorage.test.ts @@ -12,12 +12,14 @@ describe('UniversalStorage', function () { new MemoryStorage(), ] const getFromLS = (key: string) => JSON.parse(localStorage.getItem(key) ?? '') - beforeEach(function () { - clear() + jest.spyOn(console, 'warn').mockImplementation(() => { + // avoid accidental noise in console + throw new Error('console.warn should be mocked!') }) - afterEach(() => { + beforeEach(function () { jest.restoreAllMocks() + clear() }) function clear(): void { @@ -104,6 +106,10 @@ describe('UniversalStorage', function () { }) it('handles cookie errors gracefully', function () { + const consoleWarnSpy = jest + .spyOn(console, 'warn') + .mockImplementation(() => {}) + disableCookies() // Cookies is going to throw exceptions now const us = new UniversalStorage([ new LocalStorage(), @@ -113,9 +119,17 @@ describe('UniversalStorage', function () { us.set('ajs_test_key', '💰') expect(getFromLS('ajs_test_key')).toEqual('💰') expect(us.get('ajs_test_key')).toEqual('💰') + expect(consoleWarnSpy.mock.calls.length).toEqual(1) + expect(consoleWarnSpy.mock.lastCall[0]).toContain( + "CookieStorage: Can't set key" + ) }) it('does not write to LS when LS is not available', function () { + const consoleWarnSpy = jest + .spyOn(console, 'warn') + .mockImplementation(() => {}) + disableLocalStorage() // Localstorage will throw exceptions const us = new UniversalStorage([ new LocalStorage(), @@ -125,9 +139,14 @@ describe('UniversalStorage', function () { us.set('ajs_test_key', '💰') expect(jar.get('ajs_test_key')).toEqual('💰') expect(us.get('ajs_test_key')).toEqual('💰') + expect(consoleWarnSpy.mock.lastCall[0]).toContain('localStorage') }) it('handles cookie getter overrides gracefully', function () { + const consoleWarnSpy = jest + .spyOn(console, 'warn') + .mockImplementation(() => {}) + ;(document as any).__defineGetter__('cookie', function () { return '' }) @@ -139,6 +158,10 @@ describe('UniversalStorage', function () { us.set('ajs_test_key', '💰') expect(getFromLS('ajs_test_key')).toEqual('💰') expect(us.get('ajs_test_key')).toEqual('💰') + expect(consoleWarnSpy.mock.lastCall[0]).toContain( + "CookieStorage: Can't set key" + ) + expect(consoleWarnSpy.mock.lastCall[0]).toContain('TypeError') }) }) }) diff --git a/packages/browser/src/core/storage/universalStorage.ts b/packages/browser/src/core/storage/universalStorage.ts index 41cebf926..23dfefa3c 100644 --- a/packages/browser/src/core/storage/universalStorage.ts +++ b/packages/browser/src/core/storage/universalStorage.ts @@ -1,5 +1,17 @@ import { Store, StorageObject } from './types' +// not adding to private method because those method names do not get minified atm, and does not use 'this' +const _logStoreKeyError = ( + store: Store, + action: 'set' | 'get' | 'remove', + key: string, + err: unknown +) => { + console.warn( + `${store.constructor.name}: Can't ${action} key "${key}" | Err: ${err}` + ) +} + /** * Uses multiple storages in a priority list to get/set values in the order they are specified. */ @@ -20,28 +32,28 @@ export class UniversalStorage { return val } } catch (e) { - console.warn(`Can't access ${key}: ${e}`) + _logStoreKeyError(store, 'get', key, e) } } return null } set(key: K, value: Data[K] | null): void { - this.stores.forEach((s) => { + this.stores.forEach((store) => { try { - s.set(key, value) + store.set(key, value) } catch (e) { - console.warn(`Can't set ${key}: ${e}`) + _logStoreKeyError(store, 'set', key, e) } }) } clear(key: K): void { - this.stores.forEach((s) => { + this.stores.forEach((store) => { try { - s.remove(key) + store.remove(key) } catch (e) { - console.warn(`Can't remove ${key}: ${e}`) + _logStoreKeyError(store, 'remove', key, e) } }) } diff --git a/packages/browser/src/lib/__tests__/pick.test.ts b/packages/browser/src/lib/__tests__/pick.test.ts index 103dd5c27..2729ab492 100644 --- a/packages/browser/src/lib/__tests__/pick.test.ts +++ b/packages/browser/src/lib/__tests__/pick.test.ts @@ -24,7 +24,7 @@ describe(pick, () => { it('does not mutate object reference', () => { const e = { obj: { a: 1, '0': false, c: 3 }, - keys: ['a', '0'] as const, + keys: ['a', '0'], expected: { a: 1, '0': false }, } const ogObj = { ...e.obj } diff --git a/packages/browser/src/lib/__tests__/pick.typedef.ts b/packages/browser/src/lib/__tests__/pick.typedef.ts new file mode 100644 index 000000000..82182aab0 --- /dev/null +++ b/packages/browser/src/lib/__tests__/pick.typedef.ts @@ -0,0 +1,39 @@ +import { assertNotAny, assertIs } from '../../test-helpers/type-assertions' +import { pick } from '../pick' + +{ + // should work with literals + const res = pick({ id: 123 }, ['id']) + + assertIs<{ id: number }>(res) + assertNotAny(res) +} +{ + // should work if only keys are read-only + const obj: { id?: number } = {} + const res = pick(obj, ['id'] as const) + assertNotAny(res) + assertIs<{ id?: number }>(res) + + // @ts-expect-error + assertIs<{ id: number }>(res) +} + +{ + // should work with keys as string + const res = pick({ id: 123 }, [] as string[]) + assertNotAny(res) + + assertIs>(res) + // @ts-expect-error - should be partial + assertIs<{ id: number }>(res) +} + +{ + // should work with object type + const res = pick({} as object, ['id']) + assertNotAny(res) + assertIs(res) + // @ts-expect-error + assertIs<{ id: any }>(res) +} diff --git a/packages/browser/src/lib/pick.ts b/packages/browser/src/lib/pick.ts index e2c327dba..a71d5a18c 100644 --- a/packages/browser/src/lib/pick.ts +++ b/packages/browser/src/lib/pick.ts @@ -1,13 +1,23 @@ +export function pick, K extends keyof T>( + object: T, + keys: readonly K[] +): Pick + +export function pick>( + object: T, + keys: string[] +): Partial + /** * @example * pick({ 'a': 1, 'b': '2', 'c': 3 }, ['a', 'c']) * => { 'a': 1, 'c': 3 } */ -export const pick = ( +export function pick, K extends keyof T>( object: T, - keys: readonly K[] -): Pick => - Object.assign( + keys: string[] | K[] | readonly K[] +) { + return Object.assign( {}, ...keys.map((key) => { if (object && Object.prototype.hasOwnProperty.call(object, key)) { @@ -15,3 +25,4 @@ export const pick = ( } }) ) +} diff --git a/packages/browser/src/plugins/page-enrichment/__tests__/index.test.ts b/packages/browser/src/plugins/env-enrichment/__tests__/index.test.ts similarity index 59% rename from packages/browser/src/plugins/page-enrichment/__tests__/index.test.ts rename to packages/browser/src/plugins/env-enrichment/__tests__/index.test.ts index e0bee7514..a2d114de7 100644 --- a/packages/browser/src/plugins/page-enrichment/__tests__/index.test.ts +++ b/packages/browser/src/plugins/env-enrichment/__tests__/index.test.ts @@ -1,8 +1,7 @@ import cookie from 'js-cookie' import assert from 'assert' import { Analytics } from '../../../core/analytics' -import { pageEnrichment, pageDefaults } from '..' -import { pick } from '../../../lib/pick' +import { envEnrichment } from '..' import { SegmentioSettings } from '../../segmentio' import { version } from '../../../generated/version' import { CoreExtraContext } from '@segment/analytics-core' @@ -12,20 +11,6 @@ import { lowEntropyTestData, } from '../../../test-helpers/fixtures/client-hints' -let ajs: Analytics - -const helpers = { - get pageProps() { - return { - url: 'http://foo.com/bar?foo=hello_world', - path: '/bar', - search: '?foo=hello_world', - referrer: 'http://google.com', - title: 'Hello World', - } - }, -} - /** * Filters out the calls made for probing cookie availability */ @@ -40,230 +25,6 @@ const ignoreProbeCookieWrites = ( > ) => fn.mock.calls.filter((c) => c[0] !== 'ajs_cookies_check') -describe('Page Enrichment', () => { - beforeEach(async () => { - ajs = new Analytics({ - writeKey: 'abc_123', - }) - - await ajs.register(pageEnrichment) - }) - - test('enriches page calls', async () => { - const ctx = await ajs.page('Checkout', {}) - - expect(ctx.event.properties).toMatchInlineSnapshot(` - Object { - "name": "Checkout", - "path": "/", - "referrer": "", - "search": "", - "title": "", - "url": "http://localhost/", - } - `) - }) - - test('enriches track events with the page context', async () => { - const ctx = await ajs.track('My event', { - banana: 'phone', - }) - - expect(ctx.event.context?.page).toMatchInlineSnapshot(` - Object { - "path": "/", - "referrer": "", - "search": "", - "title": "", - "url": "http://localhost/", - } - `) - }) - - describe('event.properties override behavior', () => { - test('special page properties in event.properties (url, referrer, etc) are copied to context.page', async () => { - const eventProps = { ...helpers.pageProps } - ;(eventProps as any)['should_not_show_up'] = 'hello' - const ctx = await ajs.page('My Event', eventProps) - const page = ctx.event.context!.page - expect(page).toEqual( - pick(eventProps, ['url', 'path', 'referrer', 'search', 'title']) - ) - }) - - test('special page properties in event.properties (url, referrer, etc) are not copied to context.page in non-page calls', async () => { - const eventProps = { ...helpers.pageProps } - ;(eventProps as any)['should_not_show_up'] = 'hello' - const ctx = await ajs.track('My Event', eventProps) - const page = ctx.event.context!.page - expect(page).toMatchInlineSnapshot(` - Object { - "path": "/", - "referrer": "", - "search": "", - "title": "", - "url": "http://localhost/", - } - `) - }) - - test('event page properties should not be mutated', async () => { - const eventProps = { ...helpers.pageProps } - const ctx = await ajs.page('My Event', eventProps) - const page = ctx.event.context!.page - expect(page).toEqual(eventProps) - }) - - test('page properties should have defaults', async () => { - const eventProps = pick(helpers.pageProps, ['path', 'referrer']) - const ctx = await ajs.page('My Event', eventProps) - const page = ctx.event.context!.page - expect(page).toEqual({ - ...eventProps, - url: 'http://localhost/', - search: '', - title: '', - }) - }) - - test('undefined / null / empty string properties on event get overridden as usual', async () => { - const eventProps = { ...helpers.pageProps } - eventProps.referrer = '' - eventProps.path = undefined as any - eventProps.title = null as any - const ctx = await ajs.page('My Event', eventProps) - const page = ctx.event.context!.page - expect(page).toEqual( - expect.objectContaining({ referrer: '', path: undefined, title: null }) - ) - }) - }) - - test('enriches page events with the page context', async () => { - const ctx = await ajs.page( - 'My event', - { banana: 'phone' }, - { page: { url: 'not-localhost' } } - ) - - expect(ctx.event.context?.page).toMatchInlineSnapshot(` - Object { - "path": "/", - "referrer": "", - "search": "", - "title": "", - "url": "not-localhost", - } - `) - }) - test('enriches page events using properties', async () => { - const ctx = await ajs.page('My event', { banana: 'phone', referrer: 'foo' }) - - expect(ctx.event.context?.page).toMatchInlineSnapshot(` - Object { - "path": "/", - "referrer": "foo", - "search": "", - "title": "", - "url": "http://localhost/", - } - `) - }) - - test('in page events, event.name overrides event.properties.name', async () => { - const ctx = await ajs.page('My Event', undefined, undefined, { - name: 'some propery name', - }) - expect(ctx.event.properties!.name).toBe('My Event') - }) - - test('in non-page events, event.name does not override event.properties.name', async () => { - const ctx = await ajs.track('My Event', { - name: 'some propery name', - }) - expect(ctx.event.properties!.name).toBe('some propery name') - }) - - test('enriches identify events with the page context', async () => { - const ctx = await ajs.identify('Netto', { - banana: 'phone', - }) - - expect(ctx.event.context?.page).toMatchInlineSnapshot(` - Object { - "path": "/", - "referrer": "", - "search": "", - "title": "", - "url": "http://localhost/", - } - `) - }) - - test('runs before any other plugin', async () => { - let called = false - - await ajs.addSourceMiddleware(({ payload, next }) => { - called = true - expect(payload.obj?.context?.page).not.toBeFalsy() - next(payload) - }) - - await ajs.track('My event', { - banana: 'phone', - }) - - expect(called).toBe(true) - }) -}) - -describe('pageDefaults', () => { - const el = document.createElement('link') - el.setAttribute('rel', 'canonical') - - beforeEach(() => { - el.setAttribute('href', '') - document.clear() - }) - - afterEach(() => { - jest.restoreAllMocks() - }) - - it('handles no canonical links', () => { - const defs = pageDefaults() - expect(defs.url).not.toBeNull() - }) - - it('handles canonical links', () => { - el.setAttribute('href', 'http://www.segment.local') - document.body.appendChild(el) - const defs = pageDefaults() - expect(defs.url).toEqual('http://www.segment.local') - }) - - it('handles canonical links with a path', () => { - el.setAttribute('href', 'http://www.segment.local/test') - document.body.appendChild(el) - const defs = pageDefaults() - expect(defs.url).toEqual('http://www.segment.local/test') - expect(defs.path).toEqual('/test') - }) - - it('handles canonical links with search params in the url', () => { - el.setAttribute('href', 'http://www.segment.local?test=true') - document.body.appendChild(el) - const defs = pageDefaults() - expect(defs.url).toEqual('http://www.segment.local?test=true') - }) - - it('if canonical does not exist, returns fallback', () => { - document.body.appendChild(el) - const defs = pageDefaults() - expect(defs.url).toEqual(window.location.href) - }) -}) - describe('Other visitor metadata', () => { let options: SegmentioSettings let analytics: Analytics @@ -299,7 +60,7 @@ describe('Other visitor metadata', () => { options = { apiKey: 'foo' } analytics = new Analytics({ writeKey: options.apiKey }) - await analytics.register(pageEnrichment) + await analytics.register(envEnrichment) }) afterEach(() => { @@ -490,21 +251,76 @@ describe('Other visitor metadata', () => { }) it('should allow override of .search with object', async () => { + const searchParams = { + something_else: 'bar', + utm_custom: 'foo', + utm_campaign: 'hello', + } const ctx = await analytics.track( 'test', {}, { - context: amendSearchParams({ - someObject: 'foo', - }), + context: amendSearchParams(searchParams), } ) assert(ctx.event) assert(ctx.event.context) - assert(ctx.event.context.campaign === undefined) assert(ctx.event.context.referrer === undefined) + assert(ctx.event.context.campaign) + assert(ctx.event.context.page?.search) + expect(ctx.event.context.page.search).toEqual(searchParams) + expect(ctx.event.context.campaign).toEqual({ name: 'hello', custom: 'foo' }) }) + it('should not throw an error if the object is invalid', async () => { + const searchParams = { + invalidNested: { + foo: { + bar: null, + }, + }, + } + const ctx = await analytics.track( + 'test', + {}, + { + context: amendSearchParams(searchParams), + } + ) + assert(ctx.event) + assert(ctx.event.context) + assert(ctx.event.context.referrer === undefined) + expect(ctx.event.context.page?.search).toEqual(searchParams) + }) + + test.each([ + { + bar: ['123', '456'], + utm_campaign: 'hello', + utm_custom: ['foo', 'bar'], + }, + '?bar=123&bar=456&utm_campaign=hello&utm_custom=foo&utm_custom=bar', + ])( + 'should work as expected if there are multiple values for the same param (%p)', + async (params) => { + const ctx = await analytics.track( + 'test', + {}, + { + context: amendSearchParams(params), + } + ) + assert(ctx.event) + assert(ctx.event.context) + assert(ctx.event.context.referrer === undefined) + expect(ctx.event.context.page?.search).toEqual(params) + expect(ctx.event.context.campaign).toEqual({ + name: 'hello', + custom: 'bar', + }) + } + ) + it('should add .referrer.id and .referrer.type (cookies)', async () => { const ctx = await analytics.track( 'test', @@ -534,7 +350,7 @@ describe('Other visitor metadata', () => { { disableClientPersistence: true } ) - await analytics.register(pageEnrichment) + await analytics.register(envEnrichment) const ctx = await analytics.track( 'test', diff --git a/packages/browser/src/plugins/page-enrichment/index.ts b/packages/browser/src/plugins/env-enrichment/index.ts similarity index 64% rename from packages/browser/src/plugins/page-enrichment/index.ts rename to packages/browser/src/plugins/env-enrichment/index.ts index b113dc337..f1a981dc5 100644 --- a/packages/browser/src/plugins/page-enrichment/index.ts +++ b/packages/browser/src/plugins/env-enrichment/index.ts @@ -1,10 +1,9 @@ import jar from 'js-cookie' -import { pick } from '../../lib/pick' import type { Context } from '../../core/context' import type { Plugin } from '../../core/plugin' import { version } from '../../generated/version' import { SegmentEvent } from '../../core/events' -import { Campaign, EventProperties, PluginType } from '@segment/analytics-core' +import { Campaign, PluginType } from '@segment/analytics-core' import { getVersionType } from '../../lib/version-type' import { tld } from '../../core/user/tld' import { gracefulDecodeURIComponent } from '../../core/query-string/gracefulDecodeURIComponent' @@ -13,15 +12,6 @@ import { Analytics } from '../../core/analytics' import { clientHints } from '../../lib/client-hints' import { UADataValues } from '../../lib/client-hints/interfaces' -interface PageDefault { - [key: string]: unknown - path: string - referrer: string - search: string - title: string - url: string -} - let cookieOptions: jar.CookieAttributes | undefined function getCookieOptions(): jar.CookieAttributes { if (cookieOptions) { @@ -66,64 +56,6 @@ function ads(query: string): Ad | undefined { } } -/** - * Get the current page's canonical URL. - */ -function canonical(): string | undefined { - const canon = document.querySelector("link[rel='canonical']") - if (canon) { - return canon.getAttribute('href') || undefined - } -} - -/** - * Return the canonical path for the page. - */ - -function canonicalPath(): string { - const canon = canonical() - if (!canon) { - return window.location.pathname - } - - const a = document.createElement('a') - a.href = canon - const pathname = !a.pathname.startsWith('/') ? '/' + a.pathname : a.pathname - - return pathname -} - -/** - * Return the canonical URL for the page concat the given `search` - * and strip the hash. - */ - -export function canonicalUrl(search = ''): string { - const canon = canonical() - if (canon) { - return canon.includes('?') ? canon : `${canon}${search}` - } - const url = window.location.href - const i = url.indexOf('#') - return i === -1 ? url : url.slice(0, i) -} - -/** - * Return a default `options.context.page` object. - * - * https://segment.com/docs/spec/page/#properties - */ - -export function pageDefaults(): PageDefault { - return { - path: canonicalPath(), - referrer: document.referrer, - search: location.search, - title: document.title, - url: canonicalUrl(location.search), - } -} - export function utm(query: string): Campaign { if (query.startsWith('?')) { query = query.substring(1) @@ -133,7 +65,7 @@ export function utm(query: string): Campaign { return query.split('&').reduce((acc, str) => { const [k, v = ''] = str.split('=') if (k.includes('utm_') && k.length > 4) { - let utmParam = k.substr(4) as keyof Campaign + let utmParam = k.slice(4) as keyof Campaign if (utmParam === 'campaign') { utmParam = 'name' } @@ -174,7 +106,30 @@ function referrerId( storage.set('s:context.referrer', ad) } -class PageEnrichmentPlugin implements Plugin { +/** + * + * @param obj e.g. { foo: 'b', bar: 'd', baz: ['123', '456']} + * @returns e.g. 'foo=b&bar=d&baz=123&baz=456' + */ +const objectToQueryString = ( + obj: Record +): string => { + try { + const searchParams = new URLSearchParams() + Object.entries(obj).forEach(([k, v]) => { + if (Array.isArray(v)) { + v.forEach((value) => searchParams.append(k, value)) + } else { + searchParams.append(k, v) + } + }) + return searchParams.toString() + } catch { + return '' + } +} + +class EnvironmentEnrichmentPlugin implements Plugin { private instance!: Analytics private userAgentData: UADataValues | undefined @@ -195,32 +150,13 @@ class PageEnrichmentPlugin implements Plugin { } private enrich = (ctx: Context): Context => { - const event = ctx.event - const evtCtx = (event.context ??= {}) - - const defaultPageContext = pageDefaults() - - let pageContextFromEventProps: Pick | undefined + // Note: Types are off - context should never be undefined here, since it is set as part of event creation. + const evtCtx = ctx.event.context! - if (event.type === 'page') { - pageContextFromEventProps = - event.properties && - pick(event.properties, Object.keys(defaultPageContext)) - - event.properties = { - ...defaultPageContext, - ...event.properties, - ...(event.name ? { name: event.name } : {}), - } - } - - evtCtx.page = { - ...defaultPageContext, - ...pageContextFromEventProps, - ...evtCtx.page, - } + const search = evtCtx.page!.search || '' - const query: string = evtCtx.page.search || '' + const query = + typeof search === 'object' ? objectToQueryString(search) : search evtCtx.userAgent = navigator.userAgent evtCtx.userAgentData = this.userAgentData @@ -269,4 +205,4 @@ class PageEnrichmentPlugin implements Plugin { screen = this.enrich } -export const pageEnrichment = new PageEnrichmentPlugin() +export const envEnrichment = new EnvironmentEnrichmentPlugin() diff --git a/packages/browser/src/plugins/segmentio/__tests__/index.test.ts b/packages/browser/src/plugins/segmentio/__tests__/index.test.ts index fdb9be1d8..ec60ad350 100644 --- a/packages/browser/src/plugins/segmentio/__tests__/index.test.ts +++ b/packages/browser/src/plugins/segmentio/__tests__/index.test.ts @@ -3,7 +3,7 @@ import unfetch from 'unfetch' import { segmentio, SegmentioSettings } from '..' import { Analytics } from '../../../core/analytics' import { Plugin } from '../../../core/plugin' -import { pageEnrichment } from '../../page-enrichment' +import { envEnrichment } from '../../env-enrichment' import cookie from 'js-cookie' jest.mock('unfetch', () => { @@ -24,7 +24,7 @@ describe('Segment.io', () => { analytics = new Analytics({ writeKey: options.apiKey }) segment = await segmentio(analytics, options, {}) - await analytics.register(segment, pageEnrichment) + await analytics.register(segment, envEnrichment) window.localStorage.clear() @@ -55,7 +55,7 @@ describe('Segment.io', () => { } const analytics = new Analytics({ writeKey: options.apiKey }) const segment = await segmentio(analytics, options, {}) - await analytics.register(segment, pageEnrichment) + await analytics.register(segment, envEnrichment) // @ts-ignore test a valid ajsc page call await analytics.page(null, { foo: 'bar' }) diff --git a/packages/browser/src/plugins/segmentio/__tests__/retries.test.ts b/packages/browser/src/plugins/segmentio/__tests__/retries.test.ts index e73ce2ec9..0360b8aa1 100644 --- a/packages/browser/src/plugins/segmentio/__tests__/retries.test.ts +++ b/packages/browser/src/plugins/segmentio/__tests__/retries.test.ts @@ -3,7 +3,7 @@ import { Analytics } from '../../../core/analytics' // @ts-ignore isOffline mocked dependency is accused as unused import { isOffline } from '../../../core/connection' import { Plugin } from '../../../core/plugin' -import { pageEnrichment } from '../../page-enrichment' +import { envEnrichment } from '../../env-enrichment' import { scheduleFlush } from '../schedule-flush' import * as PPQ from '../../../lib/priority-queue/persisted' import * as PQ from '../../../lib/priority-queue' @@ -59,7 +59,7 @@ describe('Segment.io retries', () => { segment = await segmentio(analytics, options, {}) - await analytics.register(segment, pageEnrichment) + await analytics.register(segment, envEnrichment) }) test(`add events to the queue`, async () => { diff --git a/packages/browser/src/test-helpers/fixtures/cdn-settings.ts b/packages/browser/src/test-helpers/fixtures/cdn-settings.ts index fa22413b7..58a684208 100644 --- a/packages/browser/src/test-helpers/fixtures/cdn-settings.ts +++ b/packages/browser/src/test-helpers/fixtures/cdn-settings.ts @@ -1,7 +1,9 @@ -import { LegacySettings } from '../..' +import { AnalyticsBrowserSettings } from '../..' import { mockIntegrationName } from './classic-destination' -export const cdnSettingsKitchenSink: LegacySettings = { +type CDNSettings = NonNullable + +export const cdnSettingsKitchenSink: CDNSettings = { integrations: { [mockIntegrationName]: {}, 'Customer.io': { @@ -292,7 +294,7 @@ export const cdnSettingsKitchenSink: LegacySettings = { remotePlugins: [], } -export const cdnSettingsMinimal: LegacySettings = { +export const cdnSettingsMinimal: CDNSettings = { integrations: { [mockIntegrationName]: {}, }, diff --git a/packages/browser/src/test-helpers/fixtures/create-fetch-method.ts b/packages/browser/src/test-helpers/fixtures/create-fetch-method.ts index 3ba5b64c2..85cd7e41e 100644 --- a/packages/browser/src/test-helpers/fixtures/create-fetch-method.ts +++ b/packages/browser/src/test-helpers/fixtures/create-fetch-method.ts @@ -3,7 +3,7 @@ import { createSuccess } from '../factories' import { cdnSettingsMinimal } from './cdn-settings' export const createMockFetchImplementation = ( - cdnSettings: Partial = {} + cdnSettings: Partial = cdnSettingsMinimal ) => { return (...[url, req]: Parameters) => { const reqUrl = url.toString() diff --git a/packages/browser/src/test-helpers/fixtures/index.ts b/packages/browser/src/test-helpers/fixtures/index.ts new file mode 100644 index 000000000..9379c2698 --- /dev/null +++ b/packages/browser/src/test-helpers/fixtures/index.ts @@ -0,0 +1,4 @@ +export * from './page-context' +export * from './create-fetch-method' +export * from './classic-destination' +export * from './cdn-settings' diff --git a/packages/browser/src/test-helpers/fixtures/page-context.ts b/packages/browser/src/test-helpers/fixtures/page-context.ts new file mode 100644 index 000000000..c8949d4c5 --- /dev/null +++ b/packages/browser/src/test-helpers/fixtures/page-context.ts @@ -0,0 +1,11 @@ +import { + BufferedPageContext, + getDefaultBufferedPageContext, + getDefaultPageContext, + PageContext, +} from '../../core/page' + +export const getPageCtxFixture = (): PageContext => getDefaultPageContext() + +export const getBufferedPageCtxFixture = (): BufferedPageContext => + getDefaultBufferedPageContext() diff --git a/packages/core/src/validation/helpers.ts b/packages/core/src/validation/helpers.ts index b1d6de641..511b03a64 100644 --- a/packages/core/src/validation/helpers.ts +++ b/packages/core/src/validation/helpers.ts @@ -16,7 +16,7 @@ export function exists(val: unknown): val is NonNullable { export function isPlainObject( obj: unknown -): obj is Record { +): obj is Record { return ( Object.prototype.toString.call(obj).slice(8, -1).toLowerCase() === 'object' ) diff --git a/packages/node/README.md b/packages/node/README.md index f24687291..2d0c4bf0c 100644 --- a/packages/node/README.md +++ b/packages/node/README.md @@ -56,7 +56,7 @@ app.post('/cart', (req, res) => { ``` -## Settings & Configuration +## Settings & Configuration See the documentation: https://segment.com/docs/connections/sources/catalog/libraries/server/node/#configuration You can also see the complete list of settings in the [AnalyticsSettings interface](src/app/settings.ts). @@ -67,7 +67,8 @@ You can also see the complete list of settings in the [AnalyticsSettings interfa -## Usage in AWS Lambda +## Usage in non-node runtimes +### Usage in AWS Lambda - [AWS lambda execution environment](https://docs.aws.amazon.com/lambda/latest/dg/lambda-runtime-environment.html) is challenging for typically non-response-blocking async activites like tracking or logging, since the runtime terminates / freezes after a response is emitted. Here is an example of using analytics.js within a handler: @@ -97,7 +98,7 @@ module.exports.handler = async (event) => { }; ``` -## Usage in Vercel Edge Functions +### Usage in Vercel Edge Functions ```ts import { Analytics } from '@segment/analytics-node'; import { NextRequest, NextResponse } from 'next/server'; @@ -120,7 +121,7 @@ export default async (req: NextRequest) => { }; ``` -## Usage in Cloudflare Workers +### Usage in Cloudflare Workers ```ts import { Analytics, Context } from '@segment/analytics-node'; @@ -138,7 +139,7 @@ export default { await new Promise((resolve, reject) => analytics.track({ ... }, resolve) ); - + ... return new Response(...) },