Skip to content

Commit 0a88243

Browse files
absorbbabsorbb
committed
fix(jitsu-js): fix regression introduced by #1135. Canonical URLs once again not used for page events properties
1 parent 14561f7 commit 0a88243

File tree

3 files changed

+197
-1
lines changed

3 files changed

+197
-1
lines changed

‎libs/jitsu-js/__tests__/playwright/cases/spa-navigation.html‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
<!DOCTYPE html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="utf-8" />
5+
<meta name="viewport" content="width=device-width, initial-scale=1" />
6+
<link rel="canonical" href="<%=trackingBase%>" />
7+
<title>SPA Navigation Test</title>
8+
<script>
9+
window.jitsuConfig = {
10+
debug: true
11+
};
12+
13+
window.testOnload = async j => {
14+
window.jitsu = j;
15+
16+
j.page();
17+
j.track("test-loaded");
18+
19+
function simulateNavigation(path, query, hash) {
20+
const url = new URL(window.location.href);
21+
url.pathname = path;
22+
url.search = query;
23+
url.hash = hash;
24+
25+
window.history.pushState({}, '', url.toString());
26+
j.page();
27+
j.track("navigation");
28+
}
29+
30+
setTimeout(() => {
31+
simulateNavigation('/page1', '', '');
32+
}, 100);
33+
34+
setTimeout(() => {
35+
simulateNavigation('/page2', '?param1=value1', '');
36+
}, 200);
37+
38+
setTimeout(() => {
39+
simulateNavigation('/page3', '?param1=value1&param2=value2', '#section1');
40+
}, 300);
41+
42+
setTimeout(() => {
43+
simulateNavigation('/page4/subpage', '', '#section2');
44+
}, 400);
45+
46+
setTimeout(() => {
47+
simulateNavigation('/final', '?utm_source=test&utm_medium=spa', '#top');
48+
}, 500);
49+
50+
setTimeout(() => {
51+
window.__spaTestComplete = true;
52+
}, 600);
53+
};
54+
</script>
55+
<script
56+
type="text/javascript"
57+
src="<%=trackingBase%>/p.js"
58+
data-onload="testOnload"
59+
data-init-only="true"
60+
data-debug="true"
61+
defer
62+
></script>
63+
</head>
64+
<body>
65+
<h1>SPA Navigation Test</h1>
66+
<div id="content">
67+
<p>This page simulates a Single Page Application navigation</p>
68+
</div>
69+
</body>
70+
</html>

‎libs/jitsu-js/__tests__/playwright/integration.test.ts‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,104 @@ test("cookie-names", async ({ browser }) => {
542542
expect(requestLog[1].body.anonymousId).toEqual(anonymousId);
543543
});
544544

545+
test("spa-navigation", async ({ browser }) => {
546+
clearRequestLog();
547+
const browserContext = await browser.newContext();
548+
const { page, uncaughtErrors } = await createLoggingPage(browserContext);
549+
550+
const initialUrl = `${server.baseUrl}/spa-navigation.html`;
551+
const pageResult = await page.goto(initialUrl);
552+
553+
await page.waitForFunction(() => window["jitsu"] !== undefined, undefined, {
554+
timeout: 1000,
555+
polling: 100,
556+
});
557+
558+
expect(pageResult.status()).toBe(200);
559+
560+
await page.waitForFunction(() => window["__spaTestComplete"] === true, undefined, {
561+
timeout: 2000,
562+
polling: 100,
563+
});
564+
565+
await new Promise(resolve => setTimeout(resolve, 500));
566+
567+
expect(uncaughtErrors.length).toEqual(0);
568+
569+
console.log(
570+
`📝 Request log size of ${requestLog.length}`,
571+
requestLog.map(x => describeEvent(x.type, x.body))
572+
);
573+
574+
const pageEvents = requestLog.filter(x => x.type === "page");
575+
const trackEvents = requestLog.filter(x => x.type === "track");
576+
577+
expect(pageEvents.length).toBe(6);
578+
expect(trackEvents.length).toBe(6);
579+
580+
//expect(pageEvents[0].body.name).toBe("initial-page");
581+
expect(pageEvents[0].body.properties.path).toBe("/spa-navigation.html");
582+
expect(pageEvents[0].body.properties.url).toContain("/spa-navigation.html");
583+
expect(pageEvents[0].body.context.page.path).toBe("/spa-navigation.html");
584+
expect(pageEvents[0].body.context.page.url).toContain("/spa-navigation.html");
585+
586+
expect(pageEvents[1].body.properties.path).toBe("/page1");
587+
expect(pageEvents[1].body.properties.url).toContain("/page1");
588+
expect(pageEvents[1].body.properties.search).toBe("");
589+
expect(pageEvents[1].body.properties.hash).toBe("");
590+
expect(pageEvents[1].body.context.page.path).toBe("/page1");
591+
expect(pageEvents[1].body.context.page.url).toContain("/page1");
592+
expect(pageEvents[1].body.context.page.search).toBe("");
593+
594+
expect(pageEvents[2].body.properties.path).toBe("/page2");
595+
expect(pageEvents[2].body.properties.url).toContain("/page2?param1=value1");
596+
expect(pageEvents[2].body.properties.search).toBe("?param1=value1");
597+
expect(pageEvents[2].body.properties.hash).toBe("");
598+
expect(pageEvents[2].body.context.page.path).toBe("/page2");
599+
expect(pageEvents[2].body.context.page.url).toContain("/page2?param1=value1");
600+
expect(pageEvents[2].body.context.page.search).toBe("?param1=value1");
601+
602+
expect(pageEvents[3].body.properties.path).toBe("/page3");
603+
expect(pageEvents[3].body.properties.url).toContain("/page3?param1=value1&param2=value2");
604+
expect(pageEvents[3].body.properties.search).toBe("?param1=value1&param2=value2");
605+
expect(pageEvents[3].body.properties.hash).toBe("#section1");
606+
expect(pageEvents[3].body.context.page.path).toBe("/page3");
607+
expect(pageEvents[3].body.context.page.url).toContain("/page3?param1=value1&param2=value2");
608+
expect(pageEvents[3].body.context.page.search).toBe("?param1=value1&param2=value2");
609+
610+
expect(pageEvents[4].body.properties.path).toBe("/page4/subpage");
611+
expect(pageEvents[4].body.properties.url).toContain("/page4/subpage");
612+
expect(pageEvents[4].body.properties.search).toBe("");
613+
expect(pageEvents[4].body.properties.hash).toBe("#section2");
614+
expect(pageEvents[4].body.context.page.path).toBe("/page4/subpage");
615+
expect(pageEvents[4].body.context.page.url).toContain("/page4/subpage");
616+
expect(pageEvents[4].body.context.page.search).toBe("");
617+
618+
expect(pageEvents[5].body.properties.path).toBe("/final");
619+
expect(pageEvents[5].body.properties.url).toContain("/final?utm_source=test&utm_medium=spa");
620+
expect(pageEvents[5].body.properties.search).toBe("?utm_source=test&utm_medium=spa");
621+
expect(pageEvents[5].body.properties.hash).toBe("#top");
622+
expect(pageEvents[5].body.context.page.path).toBe("/final");
623+
expect(pageEvents[5].body.context.page.url).toContain("/final?utm_source=test&utm_medium=spa");
624+
expect(pageEvents[5].body.context.page.search).toBe("?utm_source=test&utm_medium=spa");
625+
626+
const navTrackEvents = trackEvents.filter(x => x.body.event === "navigation");
627+
expect(navTrackEvents.length).toBe(5);
628+
629+
expect(navTrackEvents[0].body.context.page.path).toBe("/page1");
630+
expect(navTrackEvents[1].body.context.page.url).toContain("/page2?param1=value1");
631+
expect(navTrackEvents[1].body.context.page.path).toBe("/page2");
632+
expect(navTrackEvents[1].body.context.page.search).toBe("?param1=value1");
633+
expect(navTrackEvents[2].body.context.page.url).toContain("/page3?param1=value1&param2=value2#section1");
634+
expect(navTrackEvents[2].body.context.page.path).toBe("/page3");
635+
expect(navTrackEvents[2].body.context.page.search).toBe("?param1=value1&param2=value2");
636+
expect(navTrackEvents[3].body.context.page.url).toContain("/page4/subpage#section2");
637+
expect(navTrackEvents[3].body.context.page.path).toBe("/page4/subpage");
638+
expect(navTrackEvents[4].body.context.page.url).toContain("/final?utm_source=test&utm_medium=spa#top");
639+
expect(navTrackEvents[4].body.context.page.path).toBe("/final");
640+
expect(navTrackEvents[4].body.context.page.search).toBe("?utm_source=test&utm_medium=spa");
641+
});
642+
545643
test("basic", async ({ browser }) => {
546644
clearRequestLog();
547645
const browserContext = await browser.newContext();

‎libs/jitsu-js/src/analytics-plugin.ts‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,27 @@ function urlPath(url) {
466466
return "/" + pathMatch;
467467
}
468468

469+
function canonicalUrl() {
470+
if (!isInBrowser()) return;
471+
const tags = document.getElementsByTagName("link");
472+
for (var i = 0, tag; (tag = tags[i]); i++) {
473+
if (tag.getAttribute("rel") === "canonical") {
474+
return tag.getAttribute("href");
475+
}
476+
}
477+
}
478+
479+
/**
480+
* bugged analytics.js logic that produces 'url' parameter by concating canonical URL with current search part
481+
* I produces broken results in some cases like SPA where path is changed but canonical URL is not updated
482+
*/
483+
function analyticsJsUrl() {
484+
if (!isInBrowser()) return;
485+
const canonical = canonicalUrl();
486+
if (!canonical) return window.location.href.replace(hashRegex, "");
487+
return canonical.match(/\?/) ? canonical : canonical + window.location.search;
488+
}
489+
469490
function adjustPayload(
470491
payload: any,
471492
config: JitsuOptions,
@@ -479,9 +500,16 @@ function adjustPayload(
479500
const properties = payload.properties || {};
480501

481502
if (payload.type === "page" && (properties.url || url)) {
482-
const targetUrl = properties.url || url;
503+
// we don't trust analytics.js URL logic since it's sticks with canonical URL on SPA pages
504+
let targetUrl = url || properties.url;
505+
if (properties.url && properties.url !== analyticsJsUrl()) {
506+
// properties.url is not the same as provided by analytics.js
507+
// it means that it was not overridden by user and we should use it
508+
targetUrl = properties.url;
509+
}
483510
properties.url = targetUrl.replace(hashRegex, "");
484511
properties.path = fixPath(urlPath(targetUrl));
512+
// other properties are correctly based on window.location in analytics.js
485513
}
486514

487515
const customContext = deepMerge(

0 commit comments

Comments
 (0)