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.

Sign up for GitHub

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

Support loading analytics into a custom global variable when using snippet version 5.2.1 or later #1032

Merged
merged 4 commits into from
Feb 1, 2024
Merged

Support loading analytics into a custom global variable when using snippet version 5.2.1 or later #1032

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a1012fb
Fix globalAnalyticsKey not working
zikaari Jan 26, 2024
fc0be16
update test
zikaari Jan 29, 2024
631923d
Update changeset description
zikaari Jan 29, 2024
d6caafa
changelog update
zikaari Jan 31, 2024
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
6 changes: 6 additions & 0 deletions .changeset/kind-crabs-report.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@segment/analytics-next': patch
Copy link
Contributor

@silesky silesky Jan 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would change this to minor, as this is a new feature IMO (even though it does kind of have a fix element)

'@internal/browser-integration-tests': patch
Copy link
Contributor

@silesky silesky Jan 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not really be bumped. It doesn't matter, but we should leave this off the changelog.

---

Support loading analytics into a custom global variable when using snippet version 5.2.1 or later
54 changes: 54 additions & 0 deletions packages/browser-integration-tests/src/custom-global-key.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { test, expect } from '@playwright/test'
import { standaloneMock } from './helpers/standalone-mock'
import { extractWriteKeyFromUrl } from './helpers/extract-writekey'
import { CDNSettingsBuilder } from '@internal/test-helpers'

test.describe('Segment with custom global key', () => {
test.beforeEach(standaloneMock)
test.beforeEach(async ({ context }) => {
await context.route(
'https://cdn.segment.com/v1/projects/*/settings',
(route, request) => {
if (request.method().toLowerCase() !== 'get') {
return route.continue()
}

const writeKey = extractWriteKeyFromUrl(request.url()) || 'writeKey'
return route.fulfill({
status: 200,
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify(new CDNSettingsBuilder({ writeKey }).build()),
})
}
)
})

test('supports using a custom global key', async ({ page }) => {
// Load analytics.js
await page.goto('/standalone-custom-key.html')
await page.evaluate(() => {
;(window as any).segment_analytics.track('track before load')
;(window as any).segment_analytics.load('fake-key')
})

const req = await page.waitForRequest('https://api.segment.io/v1/t')

// confirm that any events triggered before load have been sent
expect(req.postDataJSON().event).toBe('track before load')

const contextObj = await page.evaluate(() =>
(window as any).segment_analytics.track('track after load')
)

// confirm that any events triggered after load return a regular context object
expect(contextObj).toMatchObject(
expect.objectContaining({
attempts: expect.anything(),
event: expect.objectContaining({ event: 'track after load' }),
stats: expect.objectContaining({ metrics: expect.anything() }),
})
)
})
})
94 changes: 94 additions & 0 deletions packages/browser-integration-tests/standalone-custom-key.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
<!DOCTYPE html>
Copy link
Contributor

@silesky silesky Jan 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following the pattern of the other integration tests -- should we move this file to standalone-playground?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is as per how @chrisradek advised me to setup an integration test

<html>
<head>
<script>
!(function () {
var i = 'segment_analytics',
analytics = (window[i] = window[i] || [])
if (!analytics.initialize)
if (analytics.invoked)
window.console &&
console.error &&
console.error('Segment snippet included twice.')
else {
analytics.invoked = !0
analytics.methods = [
'trackSubmit',
'trackClick',
'trackLink',
'trackForm',
'pageview',
'identify',
'reset',
'group',
'track',
'ready',
'alias',
'debug',
'page',
'screen',
'once',
'off',
'on',
'addSourceMiddleware',
'addIntegrationMiddleware',
'setAnonymousId',
'addDestinationMiddleware',
'register',
]
analytics.factory = function (e) {
return function () {
if (window[i].initialized)
return window[i][e].apply(window[i], arguments)
var n = Array.prototype.slice.call(arguments)
if (
[
'track',
'screen',
'alias',
'group',
'page',
'identify',
].indexOf(e) > -1
) {
var c = document.querySelector("link[rel='canonical']")
n.push({
__t: 'bpc',
c: (c && c.getAttribute('href')) || void 0,
p: location.pathname,
u: location.href,
s: location.search,
t: document.title,
r: document.referrer,
})
}
n.unshift(e)
analytics.push(n)
return analytics
}
}
for (var n = 0; n < analytics.methods.length; n++) {
var key = analytics.methods[n]
analytics[key] = analytics.factory(key)
}
analytics.load = function (key, n) {
var t = document.createElement('script')
t.type = 'text/javascript'
t.async = !0
t.setAttribute('data-global-segment-analytics-key', i)
t.src =
'https://cdn.segment.com/analytics.js/v1/' +
key +
'/analytics.min.js'
Copy link
Contributor

@silesky silesky Jan 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this testing against the bundle that lives in the monorepo? This is loading the cdn bundle, which doesn't have the updated code AFAIK. Kind of wondering how the tests are passing.

(It's be even cleaner IMO to use the segment snippet npm package with new option).

See the other browser integration test's html for example FYI of how they are symlinked to this bundle.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, after it's been built

Copy link
Contributor

@silesky silesky Jan 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zikaari I am still a bit confused -- to rephrase:

If this is loading analytics from the CDN, wouldn't this be currently testing against the version of analytics that doesn't have the changes reflected in this PR (i.e if (globalAnalyticsKey))?

What am I missing? I would expect that the link to look more like https://github.com/segmentio/analytics-next/blob/master/playgrounds/standalone-playground/pages/index-local.html#L79

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does the redirection -

analytics-next/packages/browser-integration-tests/src/helpers/standalone-mock.ts

Lines 18 to 33 in d4f6975

await context.route(
'https://cdn.segment.com/analytics.js/v1/*/analytics.min.js',
(route, request) => {
if (request.method().toLowerCase() !== 'get') {
return route.continue()
}
return route.fulfill({
path: joinPath(ajsBasePath, 'standalone.js'),
status: 200,
headers: {
'Content-Type': 'text/javascript',
},
})
}
)

Copy link
Contributor

@silesky silesky Jan 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohhhh... I missed that detail! That's excellent!

var r = document.getElementsByTagName('script')[0]
r.parentNode.insertBefore(t, r)
analytics._loadOptions = n
analytics._writeKey = key
}
analytics.SNIPPET_VERSION = '5.2.0'
}
})()
</script>
</head>
<body></body>
</html>
11 changes: 11 additions & 0 deletions packages/browser/src/browser/standalone.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
loadAjsClassicFallback,
isAnalyticsCSPError,
} from '../lib/csp-detection'
import { setGlobalAnalyticsKey } from '../lib/global-analytics-helper'

let ajsIdentifiedCSP = false

Expand Down Expand Up @@ -71,6 +72,16 @@ async function attempt<T>(promise: () => Promise<T>) {
}
}

const globalAnalyticsKey = (
document.querySelector(
'script[data-global-segment-analytics-key]'
) as HTMLScriptElement
)?.dataset.globalSegmentAnalyticsKey

if (globalAnalyticsKey) {
setGlobalAnalyticsKey(globalAnalyticsKey)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a good idea to live here, so it doesn't get included in the npm bundle.

}

if (shouldPolyfill()) {
// load polyfills in order to get AJS to work with old browsers
const script = document.createElement('script')
Expand Down
Loading