wip

wip

wip

.changeset/smooth-crabs-pull.md

@@ -0,0 +1,5 @@
+---
+'@segment/analytics-next': patch
+---
+
+Refactor page enrichment and remove as plugin, add page context to buffered events.

packages/browser/package.json

@@ -30,7 +30,7 @@
     "eslint": "yarn run -T eslint",
     "tsc": "yarn run -T tsc",
     "jest": "yarn run -T jest",
-    "concurrently": "yarn run -T concurrently",
+    "concurrently": "yarn run -T concurrently --raw",
     "watch": "yarn concurrently 'NODE_ENV=production WATCH=true yarn umd --watch' 'yarn pkg --watch'",
     "build": "yarn clean && yarn build-prep && yarn concurrently 'NODE_ENV=production yarn umd' 'yarn pkg' 'yarn cjs'",
     "release:cdn": "yarn run -T browser+deps build && NODE_ENV=production bash scripts/release.sh && NODE_ENV=stage bash scripts/release.sh",

packages/browser/src/browser/__tests__/analytics-lazy-init.integration.test.ts

@@ -3,6 +3,7 @@ import unfetch from 'unfetch'
 import { AnalyticsBrowser } from '..'
 import { Analytics } from '../../core/analytics'
 import { createSuccess } from '../../test-helpers/factories'
+import { createPageCtx } from '../../test-helpers/fixtures'
 
 jest.mock('unfetch')
 
@@ -12,6 +13,7 @@ const mockFetchSettingsSuccessResponse = () => {
     .mockImplementation(() => createSuccess({ integrations: {} }))
 }
 
+const pageCtxFixture = createPageCtx()
 describe('Lazy initialization', () => {
   let trackSpy: jest.SpiedFunction<Analytics['track']>
   let fetched: jest.MockedFn<typeof unfetch>
@@ -27,7 +29,7 @@ describe('Lazy initialization', () => {
     expect(trackSpy).not.toBeCalled()
     analytics.load({ writeKey: 'abc' })
     await track
-    expect(trackSpy).toBeCalledWith('foo')
+    expect(trackSpy).toBeCalledWith('foo', pageCtxFixture)
   })
 
   it('.load method return an analytics instance', async () => {

packages/browser/src/browser/__tests__/analytics-pre-init.integration.test.ts

@@ -3,10 +3,13 @@ import unfetch from 'unfetch'
 import { Analytics } from '../../core/analytics'
 import { Context } from '../../core/context'
 import * as Factory from '../../test-helpers/factories'
+import { createPageCtx } from '../../test-helpers/fixtures'
 import { sleep } from '../../lib/sleep'
 import { setGlobalCDNUrl } from '../../lib/parse-cdn'
 import { User } from '../../core/user'
 
+const pageCtxFixture = createPageCtx()
+
 jest.mock('unfetch')
 
 const mockFetchSettingsSuccessResponse = () => {
@@ -61,7 +64,7 @@ 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' }, pageCtxFixture)
       expect(trackSpy).toBeCalledTimes(1)
     })
 
@@ -107,11 +110,11 @@ 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' }, pageCtxFixture)
+      expect(trackSpy).toBeCalledWith('bar', { age: 123 }, pageCtxFixture)
       expect(trackSpy).toBeCalledTimes(2)
 
-      expect(identifySpy).toBeCalledWith('hello')
+      expect(identifySpy).toBeCalledWith('hello', pageCtxFixture)
       expect(identifySpy).toBeCalledTimes(1)
     })
 
@@ -237,8 +240,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', pageCtxFixture)
+      expect(trackSpy).toBeCalledWith('bar', pageCtxFixture)
       expect(trackSpy).toBeCalledTimes(2)
 
       expect(identifySpy).toBeCalledTimes(1)
@@ -265,11 +268,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', pageCtxFixture)
+      expect(trackSpy).toBeCalledWith('bar', pageCtxFixture)
       expect(trackSpy).toBeCalledTimes(2)
 
-      expect(identifySpy).toBeCalledWith()
+      expect(identifySpy).toBeCalledWith(pageCtxFixture)
       expect(identifySpy).toBeCalledTimes(1)
       expect(consoleErrorSpy).toBeCalledTimes(1)
 
@@ -320,7 +323,7 @@ describe('Pre-initialization', () => {
       await onReadyPromise
       expect(readySpy).toHaveBeenCalledTimes(1)
       expect(onReadyCb).toHaveBeenCalledTimes(1)
-      expect(readySpy).toHaveBeenCalledWith(expect.any(Function))
+      expect(readySpy).toHaveBeenCalledWith(onReadyCb)
     })
 
     test('Should work with "on" events if a track event is called after load is complete', async () => {

packages/browser/src/browser/__tests__/inspector.integration.test.ts

@@ -55,7 +55,7 @@ describe('Inspector', () => {
 
     await deliveryPromise
 
-    expect(enrichedFn).toHaveBeenCalledTimes(2)
+    expect(enrichedFn).toHaveBeenCalledTimes(1)
     expect(deliveredFn).toHaveBeenCalledTimes(1)
   })
 

packages/browser/src/browser/__tests__/integration.test.ts

@@ -521,7 +521,6 @@ describe('Dispatch', () => {
         "message_dispatched",
         "plugin_time",
         "plugin_time",
-        "plugin_time",
         "message_delivered",
         "plugin_time",
         "delivered",

packages/browser/src/plugins/page-enrichment/__tests__/index.test.ts → packages/browser/src/browser/__tests__/page-enrichment.integration.test.ts

@@ -1,12 +1,13 @@
-import { Analytics } from '../../../core/analytics'
-import { pageEnrichment, pageDefaults } from '..'
-import { pick } from '../../../lib/pick'
+import { Analytics } from '../../core/analytics'
+import { pick } from '../../lib/pick'
+import { PageContext, PAGE_CTX_DISCRIMINANT } from '../../core/page'
 
 let ajs: Analytics
 
 const helpers = {
-  get pageProps() {
+  get pageProps(): PageContext {
     return {
+      __type: PAGE_CTX_DISCRIMINANT,
       url: 'http://foo.com/bar?foo=hello_world',
       path: '/bar',
       search: '?foo=hello_world',
@@ -21,15 +22,14 @@ describe('Page Enrichment', () => {
     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 {
+        "__type": "page_ctx",
         "name": "Checkout",
         "path": "/",
         "referrer": "",
@@ -47,6 +47,7 @@ describe('Page Enrichment', () => {
 
     expect(ctx.event.context?.page).toMatchInlineSnapshot(`
       Object {
+        "__type": "page_ctx",
         "path": "/",
         "referrer": "",
         "search": "",
@@ -107,27 +108,29 @@ describe('Page Enrichment', () => {
     )
 
     expect(ctx.event.context?.page).toMatchInlineSnapshot(`
-          Object {
-            "path": "/",
-            "referrer": "",
-            "search": "",
-            "title": "",
-            "url": "not-localhost",
-          }
-      `)
+      Object {
+        "__type": "page_ctx",
+        "path": "/",
+        "referrer": "",
+        "search": "",
+        "title": "",
+        "url": "not-localhost",
+      }
+    `)
   })
-  test('enriches page events using properties', async () => {
+  test.only('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/",
-          }
-      `)
+      Object {
+        "__type": "page_ctx",
+        "path": "/",
+        "referrer": "foo",
+        "search": "",
+        "title": "",
+        "url": "http://localhost/",
+      }
+    `)
   })
 
   test('in page events, event.name overrides event.properties.name', async () => {
@@ -151,6 +154,7 @@ describe('Page Enrichment', () => {
 
     expect(ctx.event.context?.page).toMatchInlineSnapshot(`
       Object {
+        "__type": "page_ctx",
         "path": "/",
         "referrer": "",
         "search": "",
@@ -176,50 +180,3 @@ describe('Page Enrichment', () => {
     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)
-  })
-})

packages/browser/src/browser/__tests__/standalone-analytics.test.ts

@@ -8,6 +8,7 @@ import { PersistedPriorityQueue } from '../../lib/priority-queue/persisted'
 import { sleep } from '../../lib/sleep'
 import * as Factory from '../../test-helpers/factories'
 import { EventQueue } from '../../core/queue/event-queue'
+import { createPageCtx } from '../../test-helpers/fixtures'
 
 const track = jest.fn()
 const identify = jest.fn()
@@ -38,6 +39,8 @@ jest.mock('unfetch', () => {
   return jest.fn()
 })
 
+const pageCtxFixture = createPageCtx({ url: 'https://segment.com/' })
+
 describe('standalone bundle', () => {
   const segmentDotCom = `foo`
 
@@ -65,6 +68,7 @@ describe('standalone bundle', () => {
     `.trim()
 
     const virtualConsole = new jsdom.VirtualConsole()
+
     const jsd = new JSDOM(html, {
       runScripts: 'dangerously',
       resources: 'usable',
@@ -156,12 +160,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: ['🍌', '🍇'],
+      },
+      pageCtxFixture
+    )
+    expect(identify).toHaveBeenCalledWith(
+      'netto',
+      {
+        employer: 'segment',
+      },
+      pageCtxFixture
+    )
 
     expect(page).toHaveBeenCalled()
   })
@@ -267,13 +279,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: ['🍌', '🍇'],
+      },
+      pageCtxFixture
+    )
+    expect(track).toHaveBeenCalledWith(
+      'race conditions',
+      { foo: 'bar' },
+      pageCtxFixture
+    )
+    expect(identify).toHaveBeenCalledWith(
+      'netto',
+      {
+        employer: 'segment',
+      },
+      pageCtxFixture
+    )
 
     expect(page).toHaveBeenCalled()
   })