diff --git a/.changeset/smooth-crabs-pull.md b/.changeset/smooth-crabs-pull.md new file mode 100644 index 000000000..e3dcd68f1 --- /dev/null +++ b/.changeset/smooth-crabs-pull.md @@ -0,0 +1,5 @@ +--- +'@segment/analytics-next': minor +--- + +Fix stale page context information for buffered events, so page data is resilient to quick navigation changes. Remove page enrichment plugin. Fix bug in track event where properties like `{ title: 'foo', url: 'bar' }` would override context.page.title and context.page.url. 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..fd33b93a3 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 { bufferedPageCtxFixture } 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', bufferedPageCtxFixture) }) 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..4c027df71 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 { bufferedPageCtxFixture } 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' }, + bufferedPageCtxFixture + ) 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' }, + bufferedPageCtxFixture + ) + expect(trackSpy).toBeCalledWith( + 'bar', + { age: 123 }, + bufferedPageCtxFixture + ) expect(trackSpy).toBeCalledTimes(2) - expect(identifySpy).toBeCalledWith('hello') + expect(identifySpy).toBeCalledWith('hello', bufferedPageCtxFixture) 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', bufferedPageCtxFixture) + expect(trackSpy).toBeCalledWith('bar', bufferedPageCtxFixture) 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', bufferedPageCtxFixture) + expect(trackSpy).toBeCalledWith('bar', bufferedPageCtxFixture) expect(trackSpy).toBeCalledTimes(2) - expect(identifySpy).toBeCalledWith() + expect(identifySpy).toBeCalledWith(bufferedPageCtxFixture) expect(identifySpy).toBeCalledTimes(1) expect(consoleErrorSpy).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..f8f7a1d3b 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(1) // will be called once for every before or enrichment plugin. expect(deliveredFn).toHaveBeenCalledTimes(1) }) diff --git a/packages/browser/src/browser/__tests__/integration.test.ts b/packages/browser/src/browser/__tests__/integration.test.ts index f66471157..968421095 100644 --- a/packages/browser/src/browser/__tests__/integration.test.ts +++ b/packages/browser/src/browser/__tests__/integration.test.ts @@ -614,7 +614,6 @@ describe('Dispatch', () => { "message_dispatched", "plugin_time", "plugin_time", - "plugin_time", "message_delivered", "plugin_time", "delivered", diff --git a/packages/browser/src/browser/__tests__/standalone-analytics.test.ts b/packages/browser/src/browser/__tests__/standalone-analytics.test.ts index c13c5f426..44e37f1e1 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 { bufferedPageCtxFixture } 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: ['🍌', '🍇'], + }, + bufferedPageCtxFixture + ) + expect(identify).toHaveBeenCalledWith( + 'netto', + { + employer: 'segment', + }, + bufferedPageCtxFixture + ) 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: ['🍌', '🍇'], + }, + { ...bufferedPageCtxFixture } + ) + expect(track).toHaveBeenCalledWith( + 'race conditions', + { foo: 'bar' }, + { ...bufferedPageCtxFixture } + ) + expect(identify).toHaveBeenCalledWith( + 'netto', + { + employer: 'segment', + }, + { ...bufferedPageCtxFixture } + ) expect(page).toHaveBeenCalled() }) diff --git a/packages/browser/src/browser/index.ts b/packages/browser/src/browser/index.ts index 1c59af549..af47eb4ce 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 { 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,6 @@ async function registerPlugins( const toRegister = [ validation, - pageEnrichment, ...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..4018cb42c --- /dev/null +++ b/packages/browser/src/core/__tests__/track-form.test.ts @@ -0,0 +1,194 @@ +import { Analytics } from '../analytics' +import { eventFactoryCtxFixture } from '../../test-helpers/fixtures' +import { sleep } from '../../lib/sleep' + +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() + console.log(window.location.href) + const [, , properties] = mockTrack.mock.lastCall as any[] + + expect((properties.context as any).page).toEqual({ + ...eventFactoryCtxFixture.page, + 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..26fff07ef --- /dev/null +++ b/packages/browser/src/core/__tests__/track-link.test.ts @@ -0,0 +1,253 @@ +import { Analytics } from '../analytics' +import { sleep } from '../../lib/sleep' +import { eventFactoryCtxFixture } from '../../test-helpers/fixtures' + +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: { ip: '0.0.0.0', ...eventFactoryCtxFixture } } + ) + }) + + 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..3f7904a1a 100644 --- a/packages/browser/src/core/analytics/index.ts +++ b/packages/browser/src/core/analytics/index.ts @@ -54,6 +54,8 @@ import { } from '../storage' import { PluginFactory } from '../../plugins/remote-loader' import { setGlobalAnalytics } from '../../lib/global-analytics-helper' +import { hasBufferedPageContextAsLastArg } from '../buffer' +import { BufferedPageContext, PageContext, createPageContext } from '../page' const deprecationWarning = 'This is being deprecated and will be not be available in future releases of Analytics JS' @@ -134,6 +136,13 @@ function _stub(this: never) { console.warn(deprecationWarning) } +const popPageContext = (args: any[]): PageContext | undefined => { + if (hasBufferedPageContextAsLastArg(args)) { + const ctx = args.pop() as BufferedPageContext + return createPageContext(ctx) + } +} + export class Analytics extends Emitter implements AnalyticsCore, AnalyticsClassic @@ -199,10 +208,9 @@ export class Analytics }, cookieOptions ).load() - this.eventFactory = new EventFactory(this._user) this.integrations = options?.integrations ?? {} this.options = options ?? {} - + this.eventFactory = new EventFactory(this._user, this.options) autoBind(this) } @@ -252,13 +260,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 +278,7 @@ export class Analytics } async page(...args: PageParams): Promise { + const pageCtx = popPageContext(args) const [category, page, properties, options, callback] = resolvePageArguments(...args) @@ -276,7 +287,8 @@ export class Analytics page, properties, options, - this.integrations + this.integrations, + pageCtx ) return this._dispatch(segmentEvent, callback).then((ctx) => { @@ -286,6 +298,7 @@ export class Analytics } async identify(...args: IdentifyParams): Promise { + const pageCtx = popPageContext(args) const [id, _traits, options, callback] = resolveUserArguments(this._user)( ...args ) @@ -295,7 +308,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 +326,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 +343,8 @@ export class Analytics groupId, groupTraits, options, - this.integrations + this.integrations, + pageCtx ) return this._dispatch(segmentEvent, callback).then((ctx) => { @@ -338,12 +354,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 +370,7 @@ export class Analytics } async screen(...args: PageParams): Promise { + const pageCtx = popPageContext(args) const [category, page, properties, options, callback] = resolvePageArguments(...args) @@ -360,7 +379,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..e3fccdad4 100644 --- a/packages/browser/src/core/buffer/__tests__/index.test.ts +++ b/packages/browser/src/core/buffer/__tests__/index.test.ts @@ -12,52 +12,60 @@ import { User } from '../../user' describe('PreInitMethodCallBuffer', () => { describe('push', () => { - it('should return this', async () => { + it('should add method calls', async () => { + const call1 = new PreInitMethodCall('identify', [], jest.fn()) const buffer = new PreInitMethodCallBuffer() - const result = buffer.push({} as any) - expect(result).toBeInstanceOf(PreInitMethodCallBuffer) + buffer.push(call1) + expect(buffer.getCalls(call1.method)).toEqual([call1]) }) - }) - describe('toArray should return an array', () => { - it('toArray() should convert the map back to an array', async () => { - 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]) + + it('should add method calls if method calls exist', async () => { + const call1 = new PreInitMethodCall('identify', [], jest.fn()) + const buffer = new PreInitMethodCallBuffer(call1) + const call2 = new PreInitMethodCall('identify', [], jest.fn()) + const call3 = new PreInitMethodCall('group', [], jest.fn()) + buffer.push(call2, call3) + const call4 = new PreInitMethodCall('group', [], jest.fn()) + buffer.push(call4) + expect(buffer.getCalls('identify')).toEqual([call1, call2]) + expect(buffer.getCalls('group')).toEqual([call3, call4]) }) }) - describe('clear', () => { - it('should return this', async () => { - const buffer = new PreInitMethodCallBuffer() - const result = buffer.push({} as any) - expect(result).toBeInstanceOf(PreInitMethodCallBuffer) + describe('toArray', () => { + it('should convert the map back to an array', async () => { + 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]) }) }) + describe('getCalls', () => { - it('should return calls', async () => { + it('should fetch calls by name', async () => { const buffer = new PreInitMethodCallBuffer() - const fooCall1 = { - method: 'foo', - args: ['bar'], - } as any - - const barCall = { - method: 'bar', - args: ['foobar'], - } as any + const call1 = new PreInitMethodCall('identify', [], jest.fn()) + const call2 = new PreInitMethodCall('identify', [], jest.fn()) + const call3 = new PreInitMethodCall('group', [], jest.fn()) - const fooCall2 = { - method: 'foo', - args: ['baz'], - } as any - - const calls: PreInitMethodCall[] = [fooCall1, fooCall2, barCall] - const result = buffer.push(...calls) - expect(result.getCalls('foo' as any)).toEqual([fooCall1, fooCall2]) + const calls: PreInitMethodCall[] = [call1, call2, call3] + buffer.push(...calls) + expect(buffer.getCalls('identify')).toEqual([call1, call2]) + expect(buffer.getCalls('group')).toEqual([call3]) + }) + }) + describe('clear', () => { + it('should clear calls', async () => { + 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) + buffer.clear() + expect(buffer.getCalls('identify')).toEqual([]) + expect(buffer.getCalls('group')).toEqual([]) }) }) }) @@ -228,14 +236,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', }) @@ -305,27 +311,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 +338,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,26 +360,21 @@ 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) + buffer.push(synchronousMethod, asyncMethod) flushAnalyticsCallsInNewTask(new Analytics({ writeKey: 'abc' }), buffer) await sleep(0) expect(synchronousMethod.reject).toBeCalled() diff --git a/packages/browser/src/core/buffer/index.ts b/packages/browser/src/core/buffer/index.ts index d7b42edb0..c4e6c757e 100644 --- a/packages/browser/src/core/buffer/index.ts +++ b/packages/browser/src/core/buffer/index.ts @@ -4,6 +4,12 @@ import { isThenable } from '../../lib/is-thenable' import { AnalyticsBrowserCore } from '../analytics/interfaces' import { version } from '../../generated/version' +import { + isBufferedPageContext, + BufferedPageContext, + getDefaultBufferedPageContext, +} from '../page' + /** * The names of any AnalyticsBrowser methods that also exist on Analytics */ @@ -80,10 +86,17 @@ export const flushAnalyticsCallsInNewTask = ( }) } +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 +104,33 @@ 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 + + /** + * For specific events, we want to add page context here + */ + const shouldAddPageContext = ( + ['track', 'screen', 'alias', 'group', 'page', 'identify'] as MethodName[] + ).includes(method) + this.args = + shouldAddPageContext && !hasBufferedPageContextAsLastArg(args) + ? [...args, getDefaultBufferedPageContext()] + : args + } } export type PreInitMethodParams = - Parameters + | [...Parameters, BufferedPageContext] + | Parameters /** * Infer return type; if return type is promise, unwrap it. @@ -107,34 +143,66 @@ 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 - - toArray(): PreInitMethodCall[] { - return ([] as PreInitMethodCall[]).concat(...Object.values(this._value)) + private _calls: MethodCallMap = {} + constructor(...calls: PreInitMethodCall[]) { + this.push(...calls) } getCalls(methodName: T): PreInitMethodCall[] { - return (this._value[methodName] ?? []) as PreInitMethodCall[] + this._pushSnippetWindowBuffer() + 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) + 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 { + 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 = window.analytics + if (!Array.isArray(wa)) return undefined + const buffered: SnippetBuffer = wa.splice(0, wa.length) + const calls = buffered.map((v) => this._transformSnippetCall(v)) + this.push(...calls) + } + + private _transformSnippetCall([ + methodName, + ...args + ]: SnippetWindowBufferedMethodCall): PreInitMethodCall { + return new PreInitMethodCall(methodName, args) } } @@ -157,7 +225,6 @@ export async function callAnalyticsMethod( )(...call.args) if (isThenable(result)) { - // do not defer for non-async methods await result } @@ -176,9 +243,13 @@ export class AnalyticsBuffered { instance?: Analytics ctx?: Context - private _preInitBuffer = new PreInitMethodCallBuffer() + /** + * We're going to assume that page URL + */ + 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 +322,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 +340,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..cbe3261cd 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, isBufferedPageContext } from '../../page' import { User } from '../../user' import { SegmentEvent, Options } from '../interfaces' @@ -11,10 +12,21 @@ describe('Event Factory', () => { const shoes = { product: 'shoes', total: '$35', category: 'category' } const shopper = { totalSpent: 100 } + const defaultContext = { + locale: 'en-US', + library: { + // already tested in page-enrichment + name: 'analytics.js', + version: expect.stringContaining('npm:next-'), + }, + userAgent: navigator.userAgent, + page: getDefaultPageContext(), + } + beforeEach(() => { user = new User() user.reset() - factory = new EventFactory(user) + factory = new EventFactory(user, {}) }) describe('alias', () => { @@ -67,7 +79,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 +165,7 @@ describe('Event Factory', () => { const track = factory.track('Order Completed', shoes, { opt1: true, }) - expect(track.context).toEqual({ opt1: true }) + expect(track.context).toEqual({ opt1: true, ...defaultContext }) }) test('sets context correctly if property arg is undefined', () => { @@ -161,7 +173,10 @@ describe('Event Factory', () => { context: { page: { path: '/custom' } }, }) - expect(track.context).toEqual({ page: { path: '/custom' } }) + expect(track.context?.page).toEqual({ + ...getDefaultPageContext(), + path: '/custom', + }) }) test('sets integrations', () => { @@ -243,7 +258,11 @@ describe('Event Factory', () => { }, }) - expect(track.context).toEqual({ opt1: true, opt2: '🥝' }) + expect(track.context).toEqual({ + opt1: true, + opt2: '🥝', + ...defaultContext, + }) }) test('should not move known options into `context`', () => { @@ -257,7 +276,11 @@ describe('Event Factory', () => { timestamp: new Date(), }) - expect(track.context).toEqual({ opt1: true, opt2: '🥝' }) + expect(track.context).toEqual({ + opt1: true, + opt2: '🥝', + ...defaultContext, + }) }) test('accepts an anonymous id', () => { @@ -265,7 +288,7 @@ describe('Event Factory', () => { anonymousId: 'anon-1', }) - expect(track.context).toEqual({}) + expect(track.context).toEqual(defaultContext) expect(track.anonymousId).toEqual('anon-1') }) @@ -275,7 +298,7 @@ describe('Event Factory', () => { timestamp, }) - expect(track.context).toEqual({}) + expect(track.context).toEqual(defaultContext) expect(track.timestamp).toEqual(timestamp) }) @@ -302,6 +325,7 @@ describe('Event Factory', () => { }) expect(track.context).toEqual({ + ...defaultContext, library: { name: 'ajs-next', version: '0.1.0', @@ -322,6 +346,7 @@ describe('Event Factory', () => { }) expect(track.context).toEqual({ + ...defaultContext, library: { name: 'ajs-next', version: '0.1.0', @@ -395,7 +420,68 @@ describe('Event Factory', () => { integrations: { Segment: true }, type: 'track', userId: 'user-id', - context: {}, + context: defaultContext, + }) + }) + }) + }) + + describe('pageContext', () => { + let events: [ + SegmentEvent['type'], + NonNullable['page'] + ][] + beforeAll(() => { + events = [ + factory.identify( + 'foo', + undefined, + undefined, + undefined, + getDefaultPageContext() + ), + factory.track( + 'foo', + undefined, + undefined, + undefined, + getDefaultPageContext() + ), + factory.group( + 'foo', + undefined, + undefined, + undefined, + getDefaultPageContext() + ), + factory.page( + 'foo', + 'bar', + undefined, + undefined, + undefined, + getDefaultPageContext() + ), + factory.alias( + 'foo', + 'bar', + undefined, + undefined, + getDefaultPageContext() + ), + ].map((el) => [el.type, el.context?.page]) + }) + + test(`context.page has the expected properties`, async () => { + events.forEach(([type, page]) => { + expect({ + type, + isBufferedPageContext: isBufferedPageContext(page), + page, + }).toEqual({ + type, + isBufferedPageContext: false, + page: getDefaultPageContext(), }) }) }) diff --git a/packages/browser/src/core/events/index.ts b/packages/browser/src/core/events/index.ts index 8af4820f9..04579585b 100644 --- a/packages/browser/src/core/events/index.ts +++ b/packages/browser/src/core/events/index.ts @@ -9,30 +9,32 @@ import { SegmentEvent, } from './interfaces' import md5 from 'spark-md5' +import { addPageContext, PageContext } from '../page' +import { InitOptions } from 'index' export * from './interfaces' export class EventFactory { - user: User - - constructor(user: User) { - this.user = user - } + constructor(public user: User, public options: InitOptions) {} 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 +42,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 +62,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 +76,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 +93,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 +167,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 +225,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 +256,17 @@ 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()), } - const messageId = 'ajs-next-' + md5.hash(JSON.stringify(body) + uuid()) - - const evt: SegmentEvent = { - ...body, - messageId, - } + addPageContext(newEvent, pageCtx, this.options.disableClientPersistence) - 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..4c9e4b07a --- /dev/null +++ b/packages/browser/src/core/page/__tests__/index.test.ts @@ -0,0 +1,99 @@ +import { + getDefaultBufferedPageContext, + getDefaultPageContext, + isBufferedPageContext, + BUFFERED_PAGE_CONTEXT_DISCRIMINANT_KEY, +} from '../' + +const ogLocation = window.location +beforeEach(() => { + Object.defineProperty(window, 'location', { + value: { + ...ogLocation, + }, + writable: true, + }) +}) + +describe(isBufferedPageContext, () => { + it('should return true if object is page context', () => { + expect(isBufferedPageContext({})).toBe(false) + expect(isBufferedPageContext('')).toBe(false) + expect(isBufferedPageContext({ foo: false })).toBe(false) + expect( + isBufferedPageContext({ + __type: BUFFERED_PAGE_CONTEXT_DISCRIMINANT_KEY, + foo: 123, + }) + ).toBe(false) + + expect(isBufferedPageContext(getDefaultBufferedPageContext())).toBe(true) + + expect(isBufferedPageContext(null)).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('handles no canonical links', () => { + const defs = getDefaultPageContext() + expect(defs.url).not.toBeNull() + }) + + 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('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') + }) + + it('returns fallback if canonical does not exist', () => { + document.body.appendChild(el) + const defs = getDefaultPageContext() + expect(defs.url).toEqual(window.location.href) + }) + }) +}) 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..362423ff1 --- /dev/null +++ b/packages/browser/src/core/page/add-page-context.ts @@ -0,0 +1,174 @@ +import { Campaign } from '@segment/analytics-core' +import { pick } from '../../lib/pick' +import { EventProperties, SegmentEvent } from '../events' +import { getDefaultPageContext, PageContext } from './get-page-context' +import { gracefulDecodeURIComponent } from '../query-string/gracefulDecodeURIComponent' +import { tld } from '../user/tld' +import { getVersionType } from '../../lib/version-type' +import { UniversalStorage, CookieStorage } from '../storage' +import jar from 'js-cookie' +import { version } from '../../generated/version' + +let cookieOptions: jar.CookieAttributes | undefined +function getCookieOptions(): jar.CookieAttributes { + if (cookieOptions) { + return cookieOptions + } + + const domain = tld(window.location.href) + cookieOptions = { + expires: 31536000000, // 1 year + secure: false, + path: '/', + } + if (domain) { + cookieOptions.domain = domain + } + + return cookieOptions +} + +type Ad = { id: string; type: string } + +/** + * + * @param obj e.g. { foo: 'b', bar: 'd'} + * @returns e.g. 'foo=b&bar=d' + */ +export const objectToQueryString = (obj: Record): string => { + try { + return new URLSearchParams(obj).toString() + } catch { + return '' + } +} + +function ads(query: string): Ad | undefined { + const queryIds: Record = { + btid: 'dataxu', + urid: 'millennial-media', + } + + if (query.startsWith('?')) { + query = query.substring(1) + } + query = query.replace(/\?/g, '&') + const parts = query.split('&') + + for (const part of parts) { + const [k, v] = part.split('=') + if (queryIds[k]) { + return { + id: v, + type: queryIds[k], + } + } + } +} + +export function utm(query: string): Campaign { + if (query.startsWith('?')) { + query = query.substring(1) + } + query = query.replace(/\?/g, '&') + + return query.split('&').reduce((acc, str) => { + const [k, v = ''] = str.split('=') + if (k.includes('utm_') && k.length > 4) { + let utmParam = k.slice(4) as keyof Campaign + if (utmParam === 'campaign') { + utmParam = 'name' + } + acc[utmParam] = gracefulDecodeURIComponent(v) + } + return acc + }, {} as Campaign) +} + +export function ampId(): string | undefined { + const ampId = jar.get('_ga') + if (ampId && ampId.startsWith('amp')) { + return ampId + } +} + +function referrerId( + query: string, + ctx: SegmentEvent['context'], + disablePersistance: boolean +): void { + const storage = new UniversalStorage<{ + 's:context.referrer': Ad + }>(disablePersistance ? [] : [new CookieStorage(getCookieOptions())]) + + const stored = storage.get('s:context.referrer') + + const ad = ads(query) ?? stored + + if (!ad) { + return + } + + if (ctx) { + ctx.referrer = { ...ctx.referrer, ...ad } + } + + storage.set('s:context.referrer', ad) +} + +export function addPageContext( + event: SegmentEvent, + pageCtx: PageContext | undefined, + disableClientPersistence: boolean | undefined +): void { + const evtCtx = (event.context ??= {}) + pageCtx ??= getDefaultPageContext() + + 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, + } + + evtCtx.userAgent = navigator.userAgent + + // @ts-ignore + const locale = navigator.userLanguage || navigator.language + + if (typeof evtCtx.locale === 'undefined' && typeof locale !== 'undefined') { + evtCtx.locale = locale + } + + const search = evtCtx.page.search || '' + const query = + typeof search === 'object' ? objectToQueryString(search) : search + + if (query && !evtCtx.campaign) { + evtCtx.campaign = utm(query) + } + + const amp = ampId() + if (amp) { + evtCtx.amp = { id: amp } + } + + referrerId(query, evtCtx, disableClientPersistence ?? false) + + // Library is not page information, but leaving it here for now. + evtCtx.library ??= { + name: 'analytics.js', + version: `${getVersionType() === 'web' ? 'next' : 'npm:next'}-${version}`, + } +} 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..fc1639fec --- /dev/null +++ b/packages/browser/src/core/page/get-page-context.ts @@ -0,0 +1,107 @@ +import { isPlainObject } from '@segment/analytics-core' +export interface PageContext { + path: string + referrer: string + search: string + title: string + url: string +} + +export const BUFFERED_PAGE_CONTEXT_DISCRIMINANT_KEY = 'buffered_page_ctx' +export interface BufferedPageContext { + __type: typeof BUFFERED_PAGE_CONTEXT_DISCRIMINANT_KEY + pathname: string + href: string + search: string + title: string + referrer: string + canonicalUrl: string | undefined +} + +export const createBufferedPageContext = ( + href: string, + canonicalUrl: string | undefined, + search: string, + pathname: string, + title: string, + referrer: string +): BufferedPageContext => { + return { + __type: BUFFERED_PAGE_CONTEXT_DISCRIMINANT_KEY, + search: search, + canonicalUrl, + href, + pathname, + title, + referrer: referrer, + } +} + +export function isBufferedPageContext(v: unknown): v is BufferedPageContext { + return ( + isPlainObject(v) && + '__type' in v && + v['__type'] === BUFFERED_PAGE_CONTEXT_DISCRIMINANT_KEY && + 'href' in v && + typeof v['href'] === 'string' + ) +} + +// 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 formatCanonicalPath = (canonicalUrl: string) => { + const a = document.createElement('a') + a.href = canonicalUrl + // remove leading slash from canonical URL path + return a.pathname[0] === '/' ? a.pathname : '/' + a.pathname +} + +export const createPageContext = (u: BufferedPageContext): PageContext => { + const path = u.canonicalUrl ? formatCanonicalPath(u.canonicalUrl) : u.pathname + const url = u.canonicalUrl + ? createCanonicalURL(u.canonicalUrl, u.search) + : removeHash(u.href) + + // Why are we removing the anchor here but not the canonical URL? Also, why don't we include hash or any anchor arguments. (???) + // There's no way for a customer to get access to hash arguments without overriding url =S + return { + path, + referrer: u.referrer, + search: u.search, + title: u.title, + url, + } +} + +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 function getDefaultPageContext(): PageContext { + return 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/page-enrichment/__tests__/index.test.ts index 90723cfcd..6f10ef8a3 100644 --- a/packages/browser/src/plugins/page-enrichment/__tests__/index.test.ts +++ b/packages/browser/src/plugins/page-enrichment/__tests__/index.test.ts @@ -1,7 +1,7 @@ import cookie from 'js-cookie' import assert from 'assert' import { Analytics } from '../../../core/analytics' -import { pageEnrichment, pageDefaults } from '..' +import { pageEnrichment } from '..' import { pick } from '../../../lib/pick' import { SegmentioSettings } from '../../segmentio' import { version } from '../../../generated/version' @@ -212,53 +212,6 @@ describe('Page Enrichment', () => { }) }) -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 @@ -452,19 +405,46 @@ 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(searchParams), + } + ) + assert(ctx.event) + assert(ctx.event.context) + 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({ - someObject: 'foo', - }), + context: amendSearchParams(searchParams), } ) assert(ctx.event) assert(ctx.event.context) - assert(ctx.event.context.campaign === undefined) assert(ctx.event.context.referrer === undefined) + expect(ctx.event.context.page?.search).toEqual(searchParams) }) it('should add .referrer.id and .referrer.type (cookies)', async () => { diff --git a/packages/browser/src/plugins/page-enrichment/index.ts b/packages/browser/src/plugins/page-enrichment/index.ts index 91cf1162f..da9d51b03 100644 --- a/packages/browser/src/plugins/page-enrichment/index.ts +++ b/packages/browser/src/plugins/page-enrichment/index.ts @@ -10,15 +10,7 @@ import { tld } from '../../core/user/tld' import { gracefulDecodeURIComponent } from '../../core/query-string/gracefulDecodeURIComponent' import { CookieStorage, UniversalStorage } from '../../core/storage' import { Analytics } from '../../core/analytics' - -interface PageDefault { - [key: string]: unknown - path: string - referrer: string - search: string - title: string - url: string -} +import { getDefaultPageContext } from '../../core/page' let cookieOptions: jar.CookieAttributes | undefined function getCookieOptions(): jar.CookieAttributes { @@ -64,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) @@ -188,7 +122,7 @@ class PageEnrichmentPlugin implements Plugin { const event = ctx.event const evtCtx = (event.context ??= {}) - const defaultPageContext = pageDefaults() + const defaultPageContext = getDefaultPageContext() let pageContextFromEventProps: Pick | undefined diff --git a/packages/browser/src/test-helpers/fixtures/event-factory-context.ts b/packages/browser/src/test-helpers/fixtures/event-factory-context.ts new file mode 100644 index 000000000..4358f5be9 --- /dev/null +++ b/packages/browser/src/test-helpers/fixtures/event-factory-context.ts @@ -0,0 +1,8 @@ +import { pageCtxFixture } from './page-context' + +export const eventFactoryCtxFixture = { + page: pageCtxFixture, + library: expect.any(Object), + userAgent: expect.any(String), + locale: expect.any(String), +} 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..3e874a1fb --- /dev/null +++ b/packages/browser/src/test-helpers/fixtures/index.ts @@ -0,0 +1,5 @@ +export * from './page-context' +export * from './event-factory-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..11bbf5686 --- /dev/null +++ b/packages/browser/src/test-helpers/fixtures/page-context.ts @@ -0,0 +1,16 @@ +import { + BufferedPageContext, + createBufferedPageContext, + PageContext, +} from '../../core/page' + +export const pageCtxFixture: PageContext = { + path: '/', + referrer: '', + search: '', + title: '', + url: 'http://localhost/', +} + +export const bufferedPageCtxFixture: BufferedPageContext = + createBufferedPageContext(pageCtxFixture.url, undefined, '', '/', '', '')