Skip to content

Commit

Permalink
Make page context resilient to quick navigation changes (#852)
Browse files Browse the repository at this point in the history
  • Loading branch information
silesky authored Oct 5, 2023
1 parent 4ca56fb commit 897f4cc
Show file tree
Hide file tree
Showing 40 changed files with 1,947 additions and 1,106 deletions.
5 changes: 5 additions & 0 deletions .changeset/dirty-snakes-sip.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@segment/analytics-core': patch
---

Tighten isPlainObject type guard
6 changes: 6 additions & 0 deletions .changeset/smooth-crabs-pull.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@segment/analytics-next': minor
---

- Capture page context information faster, so context.campaign and context.page are more resilient to quick navigation changes.
- Parse UTM params into context.campaign if users pass an object to a page call.
233 changes: 233 additions & 0 deletions examples/standalone-playground/pages/index-buffered-page-ctx.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,233 @@
<html>

<head>
<style>
body {
font-family: monospace;
}

#event {
margin: 2em 0;
min-height: 200px;
min-width: 700px;
}
</style>

<form method="get">
<input type="text" name="writeKey" placeholder="Writekey" />
<button>Load</button>
</form>

<button id="reset">Rerun Test (Reset Params + Reload)</button>

<script>
const { searchParams } = new URL(document.location);
const writeKey = searchParams.get("writeKey");
document.querySelector("input").value = writeKey;


// add random QPs for testing bug
const addRandomQp = () => {
const url = new URL(window.location.href);
const id = Math.round(Math.random() * 10000);
url.searchParams.set("id", id);
url.searchParams.set("utm_source", `source-${id}`);
url.searchParams.set("utm_campaign", `campaign-${id}`);
window.history.replaceState(null, "", url);
}

// reset added QP
document.getElementById('reset').addEventListener('click', () => {
let url = location.origin + location.pathname
const params = new URLSearchParams(location.search);
const wk = params.get('writeKey')
if (wk) {
url += `?writeKey=${wk}`
}
window.history.replaceState(null, "", url);
window.location.reload()
})


document.addEventListener('DOMContentLoaded', () => addRandomQp())



if (writeKey) {
console.profile('snippet')
console.time('snippet')
!(function () {

var analytics = (window.analytics = window.analytics || [])
if (!analytics.initialize)
if (analytics.invoked)
window.console &&
console.error &&
console.error('Segment snippet included twice.')
else {

analytics.invoked = !0
analytics.methods = [
'screen',
'register',
'deregister',
'trackSubmit',
'trackClick',
'trackLink',
'trackForm',
'pageview',
'identify',
'reset',
'group',
'track',
'ready',
'alias',
'debug',
'page',
'once',
'off',
'on',
'addSourceMiddleware',
'addIntegrationMiddleware',
'setAnonymousId',
'addDestinationMiddleware',
]

analytics.factory = function (e) {
// add this function - using single key names to save bytes
function p() {
var c = document.querySelector("link[rel='canonical']");
return {
__t: 'bpc', // discriminant key for extra uniqueness guarantee
c: c && c.getAttribute('href'),
p: location.pathname,
u: location.href,
s: location.search,
t: document.title,
r: document.referrer,
}
}
return function () {
var t = Array.prototype.slice.call(arguments)
// add buffered page context to these calls
if (['track', 'screen', 'alias', 'group', 'page', 'identify'].indexOf(e) > -1) t.push(p());
t.unshift(e)
analytics.push(t)
return analytics
}
}
for (var e = 0; e < analytics.methods.length; e++) {
var key = analytics.methods[e]
analytics[key] = analytics.factory(key)
}
analytics.load = function (key, e) {
var t = document.createElement('script')
t.type = 'text/javascript'
t.async = !0
t.src = '/node_modules/@segment/analytics-next/dist/umd/standalone.js'
var n = document.getElementsByTagName('script')[0]
n.parentNode.insertBefore(t, n)
analytics._loadOptions = e
}
analytics.SNIPPET_VERSION = '4.13.1'
analytics._writeKey = writeKey
analytics.load()
analytics.page()
}
})()
}
</script>
</head>

<body>
<p>
This is for testing that the page context is buffered when users make quick navigation changes.
When this page first loads, it should immediately add some query parameters (similuating a quick navigation change).
The page call should still have the correct campaign and page parameters.
</p>
<form>
<textarea name="event" id="event">
{
"name": "hi",
"properties": { },
"traits": { },
"options": { }
}
</textarea>
<div>
<button id="track">Track</button>
<button id="identify">Identify</button>
</div>
</form>

<pre id="ready-logs"></pre>
<pre id="logs"></pre>

<script type="text/javascript">
if (window.analytics) {
// misc
analytics.on('page', (...args) => console.log('analytics.on("page")', ...args))

window.analytics.ready(function onReady() {
console.profileEnd('snippet')
console.timeEnd('snippet')
document.querySelector('#ready-logs').textContent = 'ready!'
})

document.querySelector('#track').addEventListener('click', function (e) {
e.preventDefault()
var contents = document.querySelector('#event').value
var evt = JSON.parse(contents)
console.profile('track')
console.time('track')
var promise = window.analytics.track(
evt.name || '',
evt.properties || {},
evt.options || {}
)

promise &&
promise.then &&
promise.then(function (ctx) {
console.timeEnd('track')
console.profileEnd('track')
ctx.flush()
document.querySelector('#logs').textContent = JSON.stringify(
ctx.event,
null,
' '
)
})
})

document
.querySelector('#identify')
.addEventListener('click', function (e) {
e.preventDefault()
var contents = document.querySelector('#event').value
var evt = JSON.parse(contents)
console.time('identify')
var promise = window.analytics.identify(
evt.name || '',
evt.properties || {},
evt.options || {}
)

promise &&
promise.then &&
promise.then(function (ctx) {
console.timeEnd('identify')
ctx.flush()
document.querySelector('#logs').textContent = JSON.stringify(
ctx.event,
null,
' '
)
})
})
}
</script>

</body>

</html>
4 changes: 2 additions & 2 deletions packages/browser/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
],
"sideEffects": false,
"scripts": {
".": "yarn run -T turbo run --filter=@segment/analytics-next",
".": "yarn run -T turbo run --filter=@segment/analytics-next...",
"build-prep": "sh scripts/build-prep.sh",
"version": "yarn run build-prep && git add src/generated/version.ts",
"umd": "webpack",
Expand All @@ -44,7 +44,7 @@
"size-limit": [
{
"path": "dist/umd/index.js",
"limit": "28.5 KB"
"limit": "28.8 KB"
}
],
"dependencies": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { sleep } from '@segment/analytics-core'
import { getBufferedPageCtxFixture } from '../../test-helpers/fixtures'
import unfetch from 'unfetch'
import { AnalyticsBrowser } from '..'
import { Analytics } from '../../core/analytics'
Expand Down Expand Up @@ -27,7 +28,7 @@ describe('Lazy initialization', () => {
expect(trackSpy).not.toBeCalled()
analytics.load({ writeKey: 'abc' })
await track
expect(trackSpy).toBeCalledWith('foo')
expect(trackSpy).toBeCalledWith('foo', getBufferedPageCtxFixture())
})

it('.load method return an analytics instance', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import * as Factory from '../../test-helpers/factories'
import { sleep } from '../../lib/sleep'
import { setGlobalCDNUrl } from '../../lib/parse-cdn'
import { User } from '../../core/user'
import { getBufferedPageCtxFixture } from '../../test-helpers/fixtures'

jest.mock('unfetch')

Expand Down Expand Up @@ -61,7 +62,11 @@ describe('Pre-initialization', () => {
const trackCtxPromise = ajsBrowser.track('foo', { name: 'john' })
const result = await trackCtxPromise
expect(result).toBeInstanceOf(Context)
expect(trackSpy).toBeCalledWith('foo', { name: 'john' })
expect(trackSpy).toBeCalledWith(
'foo',
{ name: 'john' },
getBufferedPageCtxFixture()
)
expect(trackSpy).toBeCalledTimes(1)
})

Expand Down Expand Up @@ -107,11 +112,19 @@ describe('Pre-initialization', () => {

await Promise.all([trackCtxPromise, trackCtxPromise2, identifyCtxPromise])

expect(trackSpy).toBeCalledWith('foo', { name: 'john' })
expect(trackSpy).toBeCalledWith('bar', { age: 123 })
expect(trackSpy).toBeCalledWith(
'foo',
{ name: 'john' },
getBufferedPageCtxFixture()
)
expect(trackSpy).toBeCalledWith(
'bar',
{ age: 123 },
getBufferedPageCtxFixture()
)
expect(trackSpy).toBeCalledTimes(2)

expect(identifySpy).toBeCalledWith('hello')
expect(identifySpy).toBeCalledWith('hello', getBufferedPageCtxFixture())
expect(identifySpy).toBeCalledTimes(1)
})

Expand Down Expand Up @@ -234,8 +247,8 @@ describe('Pre-initialization', () => {
await AnalyticsBrowser.standalone(writeKey)

await sleep(100) // the snippet does not return a promise (pre-initialization) ... it sometimes has a callback as the third argument.
expect(trackSpy).toBeCalledWith('foo')
expect(trackSpy).toBeCalledWith('bar')
expect(trackSpy).toBeCalledWith('foo', getBufferedPageCtxFixture())
expect(trackSpy).toBeCalledWith('bar', getBufferedPageCtxFixture())
expect(trackSpy).toBeCalledTimes(2)

expect(identifySpy).toBeCalledTimes(1)
Expand All @@ -262,11 +275,11 @@ describe('Pre-initialization', () => {
await AnalyticsBrowser.standalone(writeKey)

await sleep(100) // the snippet does not return a promise (pre-initialization) ... it sometimes has a callback as the third argument.
expect(trackSpy).toBeCalledWith('foo')
expect(trackSpy).toBeCalledWith('bar')
expect(trackSpy).toBeCalledWith('foo', getBufferedPageCtxFixture())
expect(trackSpy).toBeCalledWith('bar', getBufferedPageCtxFixture())
expect(trackSpy).toBeCalledTimes(2)

expect(identifySpy).toBeCalledWith()
expect(identifySpy).toBeCalledWith(getBufferedPageCtxFixture())
expect(identifySpy).toBeCalledTimes(1)
expect(consoleErrorSpy).toBeCalledTimes(1)

Expand All @@ -292,8 +305,8 @@ describe('Pre-initialization', () => {
})

await sleep(100) // the snippet does not return a promise (pre-initialization) ... it sometimes has a callback as the third argument.
expect(trackSpy).toBeCalledWith('foo')
expect(trackSpy).toBeCalledWith('bar')
expect(trackSpy).toBeCalledWith('foo', getBufferedPageCtxFixture())
expect(trackSpy).toBeCalledWith('bar', getBufferedPageCtxFixture())
expect(trackSpy).toBeCalledTimes(2)

expect(identifySpy).toBeCalledTimes(1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ describe('Inspector', () => {

await deliveryPromise

expect(enrichedFn).toHaveBeenCalledTimes(2)
expect(enrichedFn).toHaveBeenCalledTimes(2) // will be called once for every before or enrichment plugin.
expect(deliveredFn).toHaveBeenCalledTimes(1)
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,17 @@ const mockFetchCdnSettings = (cdnSettings: any = {}) => {
.mockImplementation(createMockFetchImplementation(cdnSettings))
}

jest.spyOn(console, 'warn').mockImplementation((...errMsgs) => {
if (errMsgs[0].includes('deprecate')) {
// get rid of deprecation wawrning spam
return
}
console.warn(
'Unexpected console.warn spam in your jest test - please stub out. ' +
JSON.stringify(errMsgs)
)
})

describe('Integrations', () => {
beforeEach(async () => {
mockFetchCdnSettings()
Expand Down
Loading

0 comments on commit 897f4cc

Please sign in to comment.