Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make page context resilient to quick navigation changes #852

Merged
merged 50 commits into from
Oct 5, 2023
Merged
Show file tree
Hide file tree
Changes from 44 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
58ef434
UTM parameters
silesky Sep 26, 2023
7c54e06
wip
silesky Sep 26, 2023
3d6cc64
wip
silesky Sep 26, 2023
e1a6cd0
wip
silesky Sep 26, 2023
f24219d
wip
silesky Sep 26, 2023
b68f94e
wip
silesky Sep 26, 2023
e69a168
move tests
silesky Sep 26, 2023
64f6cb8
wip
silesky Sep 26, 2023
5dc37be
wip
silesky Sep 26, 2023
821f94d
Update smooth-crabs-pull.md
silesky Sep 26, 2023
f8bab65
wip
silesky Sep 26, 2023
0b2e40c
fix accidental metadata
silesky Sep 26, 2023
569ef0a
wip
silesky Sep 26, 2023
96aca3e
wip
silesky Sep 26, 2023
f938dfc
wip
silesky Sep 26, 2023
07ca499
wip
silesky Sep 26, 2023
906b672
wip
silesky Sep 26, 2023
c3b82e5
wip
silesky Sep 26, 2023
8073c3c
wip
silesky Sep 26, 2023
77f4cec
wip
silesky Sep 26, 2023
2660709
wip
silesky Sep 27, 2023
ccb1183
wip
silesky Sep 27, 2023
b81a603
wip
silesky Sep 27, 2023
6107fbc
wip
silesky Sep 27, 2023
f2aa03d
wip
silesky Sep 27, 2023
e6ddf62
wip
silesky Sep 27, 2023
11d9f04
wip
silesky Sep 27, 2023
fcdfcca
wip
silesky Sep 27, 2023
8193b1f
wip
silesky Sep 27, 2023
6148282
export getDefaultBufferedPageContext
silesky Sep 27, 2023
24b5b92
wip
silesky Sep 27, 2023
265f780
wip
silesky Sep 27, 2023
36097ab
wip
silesky Sep 27, 2023
d521087
wip
silesky Sep 27, 2023
57ebbe0
wip
silesky Sep 27, 2023
bb1e51a
add example test
silesky Sep 28, 2023
03eccf1
Respond to feedback
silesky Oct 2, 2023
3d528f6
address feedback
silesky Oct 3, 2023
d34bf46
wip
silesky Oct 3, 2023
bf29ad4
wip
silesky Oct 3, 2023
ce52a39
wip
silesky Oct 3, 2023
b28dd48
wip
silesky Oct 3, 2023
7c8b20c
update objToQueryString to handle arrays
silesky Oct 4, 2023
e8cd08c
wip
silesky Oct 4, 2023
a86f8e1
improve tests
silesky Oct 5, 2023
0383f3b
wip
silesky Oct 5, 2023
b759087
wip
silesky Oct 5, 2023
7ebc578
wip
silesky Oct 5, 2023
e685e5c
wip
silesky Oct 5, 2023
d3da1c8
wip
silesky Oct 5, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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