From ea8681624d741cd9707c291b6fd5bbcdbe3a9ff6 Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Wed, 5 Aug 2026 17:35:51 -0400 Subject: [PATCH 1/5] feat: detect SPA page changes for AutoLogPageView (POC) Add PageViewTracker to auto-log a page view on client-side (SPA) navigations when the AutoLogPageView feature flag is enabled. The tracker monkey-patches history.pushState/replaceState and listens for popstate/hashchange, deduping by pathname and firing the deferred _Events.logPageView() so the document title has settled. Wired into completeSDKInitialization: constructs and inits the tracker when the flag is on, and tears it down if the flag is off on re-init. This is a validation POC: each detection stage emits a console.warn('Rokt APV:', ...) so the logic can be verified locally via browser overrides. The debug logging is not intended to ship. --- src/mp-instance.ts | 15 +++ src/pageViewTracker.ts | 228 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 243 insertions(+) create mode 100644 src/pageViewTracker.ts diff --git a/src/mp-instance.ts b/src/mp-instance.ts index 91409ec8e..a94cd8491 100644 --- a/src/mp-instance.ts +++ b/src/mp-instance.ts @@ -57,6 +57,7 @@ import { LoggingDispatcher } from './reporting/loggingDispatcher'; import { IErrorReportingService, ILoggingService } from './reporting/types'; import { logDeprecatedMethodUsage } from './reporting/deprecatedMethodLogger'; import { normalizeRoktLauncherOptions } from './roktLauncherOptions'; +import { PageViewTracker } from './pageViewTracker'; export interface IErrorLogMessage { message?: string; @@ -87,6 +88,7 @@ export interface IMParticleWebSDKInstance extends MParticleWebSDK { _IdentityAPIClient: typeof IdentityAPIClient; _IntegrationCapture: IntegrationCapture; _NativeSdkHelpers: INativeSdkHelpers; + _PageViewTracker?: PageViewTracker; _Persistence: IPersistence; _CookieConsentManager: ICookieConsentManager; _ErrorReportingDispatcher: ErrorReportingDispatcher; @@ -1584,7 +1586,20 @@ function completeSDKInitialization(apiKey, config, mpInstance) { mpInstance._Events.logAST(); if (getFeatureFlag(AutoLogPageView)) { + // Fire the initial (landing) page view immediately, mirroring the + // MPA behavior. mpInstance._Events.logPageView(); + + // Start the SPA page-change tracker so subsequent client-side + // navigations (pushState/replaceState/popstate/hashchange) also + // auto-log a page view. Idempotent: tears down internally first. + if (!mpInstance._PageViewTracker) { + mpInstance._PageViewTracker = new PageViewTracker(mpInstance); + } + mpInstance._PageViewTracker.init(); + } else if (mpInstance._PageViewTracker) { + // Flag flipped off on a re-init: stop tracking and clean up. + mpInstance._PageViewTracker.teardown(); } processIdentityCallback( diff --git a/src/pageViewTracker.ts b/src/pageViewTracker.ts new file mode 100644 index 000000000..9e8f4b077 --- /dev/null +++ b/src/pageViewTracker.ts @@ -0,0 +1,228 @@ +import { IMParticleWebSDKInstance } from './mp-instance'; + +/** + * PageViewTracker detects client-side (SPA) navigations and fires an + * auto-logged page view for each one, when the AutoLogPageView feature flag + * is enabled. + * + * No framework detection is needed: every client-side router ultimately + * drives navigation through the same browser primitives — + * - history.pushState() (forward navigation) + * - history.replaceState() (redirects / canonicalization) + * - popstate (back / forward buttons) + * - hashchange (hash routers) + * + * pushState/replaceState fire no native event, so we monkey-patch them + * (the standard technique used by GA4, Segment, Amplitude, Datadog RUM) and + * listen for popstate/hashchange. Navigations are deduped by pathname. MPAs + * never trigger these, so with the flag off the tracker is never constructed + * and the listeners stay inert. + * + * Modeled on BatchUploader: the constructor is side-effect-free; init() does + * the patching/listening and tears down internally first for idempotency. + * + * NOTE (POC): every detection stage emits console.warn('Rokt APV:', ...) so we + * can validate the detection logic locally. These logs are for the prototype + * only and would be removed before shipping. + */ + +type HistoryStateMethod = History['pushState']; + +export class PageViewTracker { + mpInstance: IMParticleWebSDKInstance; + + // The path we last fired for; used to dedupe repeated navigations to the + // same pathname. Seeded in init() with the landing page. + private lastPath: string | null = null; + + private isActive = false; + + // Original references so we can restore on teardown (good-neighbor patch). + private originalPushState: HistoryStateMethod | null = null; + private originalReplaceState: HistoryStateMethod | null = null; + + // Named listener refs so teardown removes exactly what init added. + private popStateListener: (() => void) | null = null; + private hashChangeListener: (() => void) | null = null; + + constructor(mpInstance: IMParticleWebSDKInstance) { + this.mpInstance = mpInstance; + } + + /** + * Guards against non-browser / webview contexts where the History API is + * unavailable. + */ + private isSupportedEnvironment(): boolean { + return ( + typeof window !== 'undefined' && + typeof window.history !== 'undefined' && + typeof window.history.pushState === 'function' && + typeof window.addEventListener === 'function' + ); + } + + public init(): void { + if (!this.isSupportedEnvironment()) { + // eslint-disable-next-line no-console + console.warn( + 'Rokt APV: [init] unsupported environment (no History API), not starting' + ); + return; + } + + // Idempotent: tear down any prior patch/listeners first so repeated + // init() calls (e.g. re-init) don't stack wrappers or listeners. + // eslint-disable-next-line no-console + console.warn('Rokt APV: [init] starting (teardown-first for idempotency)'); + this.teardown(); + + this.isActive = true; + + // Seed lastPath with the current landing page so the first real + // navigation registers as a change rather than a spurious fire. + this.lastPath = window.location.pathname; + // eslint-disable-next-line no-console + console.warn('Rokt APV: [init] seeded lastPath', { + lastPath: this.lastPath, + }); + + this.patchHistoryMethods(); + this.addNavigationListeners(); + + // eslint-disable-next-line no-console + console.warn( + 'Rokt APV: [init] patched pushState/replaceState + listening for popstate/hashchange' + ); + } + + private patchHistoryMethods(): void { + this.originalPushState = window.history.pushState; + this.originalReplaceState = window.history.replaceState; + + const self = this; + + // Wrap with original.apply(this, args) so the router's own behavior is + // untouched, then handle the navigation. Patch both identically — + // path-changing replaceState (redirects) should also log a view; + // under-counting is worse than a rare extra view. + window.history.pushState = function( + this: History, + ...args: Parameters + ): void { + const result = self.originalPushState!.apply(this, args); + self.handleNavigation('pushState'); + return result; + }; + + window.history.replaceState = function( + this: History, + ...args: Parameters + ): void { + const result = self.originalReplaceState!.apply(this, args); + self.handleNavigation('replaceState'); + return result; + }; + } + + private addNavigationListeners(): void { + this.popStateListener = () => this.handleNavigation('popstate'); + this.hashChangeListener = () => this.handleNavigation('hashchange'); + + window.addEventListener('popstate', this.popStateListener); + window.addEventListener('hashchange', this.hashChangeListener); + } + + /** + * Called by every navigation primitive. Captures the candidate path + * synchronously, dedupes by pathname, and defers the fire to the next + * macrotask so the SPA's document.title has settled. + */ + private handleNavigation(source: string): void { + const candidatePath = window.location.pathname; + + // eslint-disable-next-line no-console + console.warn('Rokt APV: [detect] navigation signal', { + source, + candidatePath, + lastPath: this.lastPath, + }); + + if (candidatePath === this.lastPath) { + // eslint-disable-next-line no-console + console.warn('Rokt APV: [dedupe] pathname unchanged, skipping', { + source, + path: candidatePath, + }); + return; + } + + // eslint-disable-next-line no-console + console.warn('Rokt APV: [accept] pathname changed, scheduling fire', { + source, + from: this.lastPath, + to: candidatePath, + }); + this.lastPath = candidatePath; + + // Defer via setTimeout(fn, 0) — chosen over requestAnimationFrame + // (throttled in background tabs) and queueMicrotask (may run before the + // render commit, yielding a stale title). + setTimeout(() => { + if (!this.isActive) { + // eslint-disable-next-line no-console + console.warn( + 'Rokt APV: [defer] fire aborted, tracker inactive (torn down before flush)' + ); + return; + } + // eslint-disable-next-line no-console + console.warn('Rokt APV: [fire] deferred flush -> _Events.logPageView()', { + path: candidatePath, + title: window.document.title, + }); + this.mpInstance._Events.logPageView(); + }, 0); + } + + /** + * Removes listeners unconditionally and restores the original history + * methods only if the wrapper is still ours (someone may have patched on + * top of us). Otherwise we simply mark inactive so the deferred callback + * no-ops. + */ + public teardown(): void { + if (this.popStateListener) { + window.removeEventListener('popstate', this.popStateListener); + this.popStateListener = null; + } + if (this.hashChangeListener) { + window.removeEventListener('hashchange', this.hashChangeListener); + this.hashChangeListener = null; + } + + const ourWrapperStillInstalled = + this.originalPushState !== null && + window.history.pushState !== this.originalPushState; + + if (this.originalPushState && this.originalReplaceState) { + if (ourWrapperStillInstalled) { + window.history.pushState = this.originalPushState; + window.history.replaceState = this.originalReplaceState; + // eslint-disable-next-line no-console + console.warn( + 'Rokt APV: [teardown] restored original pushState/replaceState' + ); + } else { + // eslint-disable-next-line no-console + console.warn( + 'Rokt APV: [teardown] wrapper no longer ours; leaving history methods, gating callback to no-op' + ); + } + this.originalPushState = null; + this.originalReplaceState = null; + } + + this.isActive = false; + } +} From 63f784eae191ad2c9d7525c34c913a82584cdf09 Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Thu, 6 Aug 2026 15:36:57 -0400 Subject: [PATCH 2/5] refactor: use mParticle verbose logging in PageViewTracker Replace console.warn debug calls in PageViewTracker with mpInstance.Logger.verbose so SPA page-view tracking respects the SDK log level. Adds jest coverage and .nvmrc. --- .nvmrc | 1 + src/mp-instance.ts | 6 - src/pageViewTracker.ts | 239 ++++++++-------- test/jest/pageViewTracker.spec.ts | 451 ++++++++++++++++++++++++++++++ 4 files changed, 575 insertions(+), 122 deletions(-) create mode 100644 .nvmrc create mode 100644 test/jest/pageViewTracker.spec.ts diff --git a/.nvmrc b/.nvmrc new file mode 100644 index 000000000..eb6ead3cf --- /dev/null +++ b/.nvmrc @@ -0,0 +1 @@ +v24.16.0 diff --git a/src/mp-instance.ts b/src/mp-instance.ts index a94cd8491..0e45a74c9 100644 --- a/src/mp-instance.ts +++ b/src/mp-instance.ts @@ -1586,19 +1586,13 @@ function completeSDKInitialization(apiKey, config, mpInstance) { mpInstance._Events.logAST(); if (getFeatureFlag(AutoLogPageView)) { - // Fire the initial (landing) page view immediately, mirroring the - // MPA behavior. mpInstance._Events.logPageView(); - // Start the SPA page-change tracker so subsequent client-side - // navigations (pushState/replaceState/popstate/hashchange) also - // auto-log a page view. Idempotent: tears down internally first. if (!mpInstance._PageViewTracker) { mpInstance._PageViewTracker = new PageViewTracker(mpInstance); } mpInstance._PageViewTracker.init(); } else if (mpInstance._PageViewTracker) { - // Flag flipped off on a re-init: stop tracking and clean up. mpInstance._PageViewTracker.teardown(); } diff --git a/src/pageViewTracker.ts b/src/pageViewTracker.ts index 9e8f4b077..2671188a9 100644 --- a/src/pageViewTracker.ts +++ b/src/pageViewTracker.ts @@ -1,47 +1,25 @@ import { IMParticleWebSDKInstance } from './mp-instance'; -/** - * PageViewTracker detects client-side (SPA) navigations and fires an - * auto-logged page view for each one, when the AutoLogPageView feature flag - * is enabled. - * - * No framework detection is needed: every client-side router ultimately - * drives navigation through the same browser primitives — - * - history.pushState() (forward navigation) - * - history.replaceState() (redirects / canonicalization) - * - popstate (back / forward buttons) - * - hashchange (hash routers) - * - * pushState/replaceState fire no native event, so we monkey-patch them - * (the standard technique used by GA4, Segment, Amplitude, Datadog RUM) and - * listen for popstate/hashchange. Navigations are deduped by pathname. MPAs - * never trigger these, so with the flag off the tracker is never constructed - * and the listeners stay inert. - * - * Modeled on BatchUploader: the constructor is side-effect-free; init() does - * the patching/listening and tears down internally first for idempotency. - * - * NOTE (POC): every detection stage emits console.warn('Rokt APV:', ...) so we - * can validate the detection logic locally. These logs are for the prototype - * only and would be removed before shipping. - */ - type HistoryStateMethod = History['pushState']; +const WRAPPED_MARKER = '__mpApvWrapped__'; + +type MarkedHistoryMethod = HistoryStateMethod & { + [WRAPPED_MARKER]?: boolean; +}; + export class PageViewTracker { mpInstance: IMParticleWebSDKInstance; - // The path we last fired for; used to dedupe repeated navigations to the - // same pathname. Seeded in init() with the landing page. private lastPath: string | null = null; - private isActive = false; - // Original references so we can restore on teardown (good-neighbor patch). private originalPushState: HistoryStateMethod | null = null; private originalReplaceState: HistoryStateMethod | null = null; - // Named listener refs so teardown removes exactly what init added. + private pushStateWrapper: HistoryStateMethod | null = null; + private replaceStateWrapper: HistoryStateMethod | null = null; + private popStateListener: (() => void) | null = null; private hashChangeListener: (() => void) | null = null; @@ -49,10 +27,6 @@ export class PageViewTracker { this.mpInstance = mpInstance; } - /** - * Guards against non-browser / webview contexts where the History API is - * unavailable. - */ private isSupportedEnvironment(): boolean { return ( typeof window !== 'undefined' && @@ -63,134 +37,167 @@ export class PageViewTracker { } public init(): void { + this.mpInstance.Logger.verbose( + 'mParticle APV: [init] PageViewTracker Init' + ); if (!this.isSupportedEnvironment()) { - // eslint-disable-next-line no-console - console.warn( - 'Rokt APV: [init] unsupported environment (no History API), not starting' + this.mpInstance.Logger.verbose( + 'mParticle APV: [init] unsupported environment (no History API), not starting' ); return; } - // Idempotent: tear down any prior patch/listeners first so repeated - // init() calls (e.g. re-init) don't stack wrappers or listeners. - // eslint-disable-next-line no-console - console.warn('Rokt APV: [init] starting (teardown-first for idempotency)'); - this.teardown(); + if (this.isActive) { + this.mpInstance.Logger.verbose( + 'mParticle APV: [init] starting (teardown-first for idempotency)' + ); + this.teardown(); + } this.isActive = true; - // Seed lastPath with the current landing page so the first real - // navigation registers as a change rather than a spurious fire. - this.lastPath = window.location.pathname; - // eslint-disable-next-line no-console - console.warn('Rokt APV: [init] seeded lastPath', { - lastPath: this.lastPath, - }); + const { pathname, search, hash } = window.location; + this.lastPath = pathname + search + hash; + + this.mpInstance.Logger.verbose( + `mParticle APV: [init] seeded lastPath: ${this.lastPath}` + ); this.patchHistoryMethods(); this.addNavigationListeners(); - // eslint-disable-next-line no-console - console.warn( - 'Rokt APV: [init] patched pushState/replaceState + listening for popstate/hashchange' + this.mpInstance.Logger.verbose( + 'mParticle APV: [init] patched pushState/replaceState + listening for popstate/hashchange' ); } private patchHistoryMethods(): void { - this.originalPushState = window.history.pushState; - this.originalReplaceState = window.history.replaceState; - const self = this; - // Wrap with original.apply(this, args) so the router's own behavior is - // untouched, then handle the navigation. Patch both identically — - // path-changing replaceState (redirects) should also log a view; - // under-counting is worse than a rare extra view. - window.history.pushState = function( + const installed = window.history.pushState as MarkedHistoryMethod; + if (installed[WRAPPED_MARKER]) { + this.mpInstance.Logger.verbose( + 'mParticle APV: [patch] history already wrapped, skipping to avoid double-wrap' + ); + return; + } + + const originalPushState = window.history.pushState; + const originalReplaceState = window.history.replaceState; + this.originalPushState = originalPushState; + this.originalReplaceState = originalReplaceState; + + const pushStateWrapper = function( this: History, ...args: Parameters ): void { - const result = self.originalPushState!.apply(this, args); - self.handleNavigation('pushState'); + const result = originalPushState.apply(this, args); + self.safeHandleNavigation('pushState'); return result; }; - window.history.replaceState = function( + const replaceStateWrapper = function( this: History, ...args: Parameters ): void { - const result = self.originalReplaceState!.apply(this, args); - self.handleNavigation('replaceState'); + const result = originalReplaceState.apply(this, args); + self.safeHandleNavigation('replaceState'); return result; }; + + Object.defineProperty(pushStateWrapper, WRAPPED_MARKER, { + value: true, + enumerable: false, + }); + Object.defineProperty(replaceStateWrapper, WRAPPED_MARKER, { + value: true, + enumerable: false, + }); + + this.pushStateWrapper = pushStateWrapper; + this.replaceStateWrapper = replaceStateWrapper; + + try { + window.history.pushState = pushStateWrapper; + window.history.replaceState = replaceStateWrapper; + } catch (e) { + this.mpInstance.Logger.verbose( + `mParticle APV: [error] failed to patch history methods (frozen/sealed), rolling back: ${e}` + ); + try { + if (window.history.pushState === pushStateWrapper) { + window.history.pushState = originalPushState; + } + if (window.history.replaceState === replaceStateWrapper) { + window.history.replaceState = originalReplaceState; + } + } catch (restoreError) { + this.mpInstance.Logger.verbose( + `mParticle APV: [error] failed to restore history methods after patch failure: ${restoreError}` + ); + } + this.pushStateWrapper = null; + this.replaceStateWrapper = null; + this.originalPushState = null; + this.originalReplaceState = null; + } } private addNavigationListeners(): void { - this.popStateListener = () => this.handleNavigation('popstate'); - this.hashChangeListener = () => this.handleNavigation('hashchange'); + this.popStateListener = () => this.safeHandleNavigation('popstate'); + this.hashChangeListener = () => this.safeHandleNavigation('hashchange'); window.addEventListener('popstate', this.popStateListener); window.addEventListener('hashchange', this.hashChangeListener); } - /** - * Called by every navigation primitive. Captures the candidate path - * synchronously, dedupes by pathname, and defers the fire to the next - * macrotask so the SPA's document.title has settled. - */ + private safeHandleNavigation(source: string): void { + try { + this.handleNavigation(source); + } catch (e) { + this.mpInstance.Logger.verbose( + `mParticle APV: [error] navigation handler threw (${source}), page view skipped: ${e}` + ); + } + } + private handleNavigation(source: string): void { - const candidatePath = window.location.pathname; + const { pathname, search, hash } = window.location; + const candidatePath = pathname + search + hash; - // eslint-disable-next-line no-console - console.warn('Rokt APV: [detect] navigation signal', { - source, - candidatePath, - lastPath: this.lastPath, - }); + this.mpInstance.Logger.verbose( + `mParticle APV: [detect] navigation signal (source: ${source}, candidatePath: ${candidatePath}, lastPath: ${this.lastPath})` + ); if (candidatePath === this.lastPath) { - // eslint-disable-next-line no-console - console.warn('Rokt APV: [dedupe] pathname unchanged, skipping', { - source, - path: candidatePath, - }); + this.mpInstance.Logger.verbose( + `mParticle APV: [dedupe] pathname unchanged, skipping (source: ${source}, path: ${candidatePath})` + ); return; } - // eslint-disable-next-line no-console - console.warn('Rokt APV: [accept] pathname changed, scheduling fire', { - source, - from: this.lastPath, - to: candidatePath, - }); + this.mpInstance.Logger.verbose( + `mParticle APV: [accept] pathname changed, scheduling fire (source: ${source}, from: ${this.lastPath}, to: ${candidatePath})` + ); this.lastPath = candidatePath; - // Defer via setTimeout(fn, 0) — chosen over requestAnimationFrame - // (throttled in background tabs) and queueMicrotask (may run before the - // render commit, yielding a stale title). setTimeout(() => { if (!this.isActive) { - // eslint-disable-next-line no-console - console.warn( - 'Rokt APV: [defer] fire aborted, tracker inactive (torn down before flush)' + this.mpInstance.Logger.verbose( + 'mParticle APV: [defer] fire aborted, tracker inactive (torn down before flush)' ); return; } - // eslint-disable-next-line no-console - console.warn('Rokt APV: [fire] deferred flush -> _Events.logPageView()', { - path: candidatePath, - title: window.document.title, - }); + + this.mpInstance._SessionManager.resetSessionTimer(); + + this.mpInstance.Logger.verbose( + `mParticle APV: [fire] deferred flush -> _Events.logPageView() (path: ${candidatePath}, title: ${window.document.title})` + ); this.mpInstance._Events.logPageView(); }, 0); } - /** - * Removes listeners unconditionally and restores the original history - * methods only if the wrapper is still ours (someone may have patched on - * top of us). Otherwise we simply mark inactive so the deferred callback - * no-ops. - */ public teardown(): void { if (this.popStateListener) { window.removeEventListener('popstate', this.popStateListener); @@ -202,25 +209,25 @@ export class PageViewTracker { } const ourWrapperStillInstalled = - this.originalPushState !== null && - window.history.pushState !== this.originalPushState; + this.pushStateWrapper !== null && + window.history.pushState === this.pushStateWrapper; if (this.originalPushState && this.originalReplaceState) { if (ourWrapperStillInstalled) { window.history.pushState = this.originalPushState; window.history.replaceState = this.originalReplaceState; - // eslint-disable-next-line no-console - console.warn( - 'Rokt APV: [teardown] restored original pushState/replaceState' + this.mpInstance.Logger.verbose( + 'mParticle APV: [teardown] restored original pushState/replaceState' ); } else { - // eslint-disable-next-line no-console - console.warn( - 'Rokt APV: [teardown] wrapper no longer ours; leaving history methods, gating callback to no-op' + this.mpInstance.Logger.verbose( + 'mParticle APV: [teardown] wrapper no longer ours; leaving history methods, gating callback to no-op' ); } this.originalPushState = null; this.originalReplaceState = null; + this.pushStateWrapper = null; + this.replaceStateWrapper = null; } this.isActive = false; diff --git a/test/jest/pageViewTracker.spec.ts b/test/jest/pageViewTracker.spec.ts new file mode 100644 index 000000000..9a77f91fe --- /dev/null +++ b/test/jest/pageViewTracker.spec.ts @@ -0,0 +1,451 @@ +import { PageViewTracker } from '../../src/pageViewTracker'; +import { IMParticleWebSDKInstance } from '../../src/mp-instance'; + +// Capture the genuinely-native history methods at import time, before any +// tracker has a chance to monkey-patch them. Used to reset state between tests +// and to drive navigation without going through the tracker's wrapper. +const NATIVE_PUSH_STATE = window.history.pushState; +const NATIVE_REPLACE_STATE = window.history.replaceState; + +const WRAPPED_MARKER = '__mpApvWrapped__'; + +describe('PageViewTracker', () => { + let mpInstance: IMParticleWebSDKInstance; + let logPageView: jest.Mock; + let resetSessionTimer: jest.Mock; + let verbose: jest.Mock; + + // Every tracker built during a test is registered here so afterEach can + // tear it down. Trackers add global window listeners in init(); without + // teardown those listeners leak across tests and fire on later navigations. + let trackers: PageViewTracker[]; + + const createTracker = (): PageViewTracker => { + const tracker = new PageViewTracker(mpInstance); + trackers.push(tracker); + return tracker; + }; + + // Change the URL without triggering the tracker's patched pushState. + const navigateNatively = (path: string): void => { + NATIVE_PUSH_STATE.call(window.history, {}, '', path); + }; + + beforeEach(() => { + jest.useFakeTimers(); + trackers = []; + + logPageView = jest.fn(); + resetSessionTimer = jest.fn(); + verbose = jest.fn(); + + mpInstance = ({ + Logger: { verbose }, + _SessionManager: { resetSessionTimer }, + _Events: { logPageView }, + } as unknown) as IMParticleWebSDKInstance; + }); + + afterEach(() => { + // Tear down every tracker so its window listeners don't leak into the + // next test. Swallow errors from trackers whose mocks were rigged to + // throw. + trackers.forEach(tracker => { + try { + tracker.teardown(); + } catch (e) { + /* ignore */ + } + }); + + // Restore the native history methods and reset the URL so each test + // starts from a clean `http://localhost/` (jsdom's default origin). + window.history.pushState = NATIVE_PUSH_STATE; + window.history.replaceState = NATIVE_REPLACE_STATE; + NATIVE_REPLACE_STATE.call(window.history, {}, '', '/'); + + jest.clearAllTimers(); + jest.useRealTimers(); + jest.restoreAllMocks(); + }); + + describe('#constructor', () => { + it('should be side-effect free and store the mpInstance', () => { + const addEventListenerSpy = jest.spyOn(window, 'addEventListener'); + + const tracker = createTracker(); + + expect(tracker.mpInstance).toBe(mpInstance); + expect(tracker['isActive']).toBe(false); + expect(tracker['lastPath']).toBeNull(); + expect(addEventListenerSpy).not.toHaveBeenCalled(); + expect(window.history.pushState).toBe(NATIVE_PUSH_STATE); + }); + }); + + describe('#init - environment support', () => { + it('should not start when the History API is unavailable', () => { + // Simulate an unsupported environment by removing pushState. + Object.defineProperty(window.history, 'pushState', { + value: undefined, + configurable: true, + writable: true, + }); + + const tracker = createTracker(); + const addEventListenerSpy = jest.spyOn(window, 'addEventListener'); + + expect(() => tracker.init()).not.toThrow(); + expect(tracker['isActive']).toBe(false); + expect(addEventListenerSpy).not.toHaveBeenCalled(); + + // Restore for afterEach cleanup. + Object.defineProperty(window.history, 'pushState', { + value: NATIVE_PUSH_STATE, + configurable: true, + writable: true, + }); + }); + }); + + describe('#init', () => { + it('should seed lastPath with the full relative URL at init time', () => { + navigateNatively('/dashboard?tab=1#section'); + + const tracker = createTracker(); + tracker.init(); + + expect(tracker['lastPath']).toBe('/dashboard?tab=1#section'); + expect(tracker['isActive']).toBe(true); + }); + + it('should patch pushState/replaceState and register listeners', () => { + const addEventListenerSpy = jest.spyOn(window, 'addEventListener'); + + const tracker = createTracker(); + tracker.init(); + + expect(window.history.pushState).not.toBe(NATIVE_PUSH_STATE); + expect(window.history.replaceState).not.toBe(NATIVE_REPLACE_STATE); + expect(addEventListenerSpy).toHaveBeenCalledWith( + 'popstate', + expect.any(Function) + ); + expect(addEventListenerSpy).toHaveBeenCalledWith( + 'hashchange', + expect.any(Function) + ); + }); + + // Q9.4 - Double init() patches only once without stacking (idempotency). + it('should tear down first on a second init(), not stacking wrappers', () => { + const tracker = createTracker(); + tracker.init(); + const firstWrapper = window.history.pushState; + + const teardownSpy = jest.spyOn(tracker, 'teardown'); + tracker.init(); + + expect(teardownSpy).toHaveBeenCalled(); + // After teardown-first + re-patch, the installed wrapper is a fresh + // one wrapping the native method, never a wrapper wrapping a wrapper. + expect(window.history.pushState).not.toBe(firstWrapper); + expect(tracker['originalPushState']).toBe(NATIVE_PUSH_STATE); + }); + }); + + describe('#patchHistoryMethods', () => { + // Q9.5 - A second wrapper/instance already present skips patching. + it('should skip patching when history is already wrapped (marker guard)', () => { + const foreignWrapper = function() { + /* someone else's wrapper */ + } as History['pushState']; + Object.defineProperty(foreignWrapper, WRAPPED_MARKER, { + value: true, + enumerable: false, + }); + window.history.pushState = foreignWrapper; + + const tracker = createTracker(); + tracker.init(); + + expect(window.history.pushState).toBe(foreignWrapper); + expect(tracker['originalPushState']).toBeNull(); + expect(tracker['pushStateWrapper']).toBeNull(); + expect(verbose).toHaveBeenCalledWith( + expect.stringContaining('already wrapped') + ); + }); + + it('should mark its wrapper as non-enumerable', () => { + const tracker = createTracker(); + tracker.init(); + + const wrapper = window.history.pushState as History['pushState'] & + Record; + expect(wrapper[WRAPPED_MARKER]).toBe(true); + expect(Object.keys(wrapper)).not.toContain(WRAPPED_MARKER); + }); + + // Q9.8 - Frozen/sealed history throws on assignment, rolls back, and + // init() doesn't escape while listeners still get added. + it('should roll back and not throw when history assignment fails', () => { + // Make pushState non-writable so assignment throws in strict mode. + Object.defineProperty(window.history, 'pushState', { + value: NATIVE_PUSH_STATE, + writable: false, + configurable: true, + }); + const addEventListenerSpy = jest.spyOn(window, 'addEventListener'); + + const tracker = createTracker(); + + try { + expect(() => tracker.init()).not.toThrow(); + + // Assignment failed, so the native method is left in place... + expect(window.history.pushState).toBe(NATIVE_PUSH_STATE); + expect(tracker['pushStateWrapper']).toBeNull(); + expect(tracker['originalPushState']).toBeNull(); + + // ...but navigation listeners are still registered. + expect(addEventListenerSpy).toHaveBeenCalledWith( + 'popstate', + expect.any(Function) + ); + expect(addEventListenerSpy).toHaveBeenCalledWith( + 'hashchange', + expect.any(Function) + ); + } finally { + // Restore writability so afterEach can reset. + Object.defineProperty(window.history, 'pushState', { + value: NATIVE_PUSH_STATE, + writable: true, + configurable: true, + }); + } + }); + }); + + describe('navigation detection', () => { + let tracker: PageViewTracker; + + beforeEach(() => { + navigateNatively('/'); + tracker = createTracker(); + tracker.init(); + }); + + // Q9.1 - Same-path replaceState should not fire a view (dedup). + // Q9.9 - Landing page isn't double-logged after the init-time view. + it('should not fire a page view when the path is unchanged', () => { + window.history.replaceState({}, '', '/'); + jest.runAllTimers(); + + expect(logPageView).not.toHaveBeenCalled(); + }); + + // Q9.2 - Query-only change fires exactly one view via the full-URL key. + it('should fire one view on a query-string-only change', () => { + window.history.pushState({}, '', '/?tab=settings'); + jest.runAllTimers(); + + expect(logPageView).toHaveBeenCalledTimes(1); + }); + + // Q9.2 - Hash-only change fires exactly one view via the full-URL key. + it('should fire one view on a hash-only change', () => { + window.history.pushState({}, '', '/#/details'); + jest.runAllTimers(); + + expect(logPageView).toHaveBeenCalledTimes(1); + }); + + // Q9.3 - A real path change produces one deferred view after flush. + it('should defer the page view until the timer flushes', () => { + window.history.pushState({}, '', '/products'); + + // Deferred: nothing fires synchronously. + expect(logPageView).not.toHaveBeenCalled(); + + jest.runAllTimers(); + expect(logPageView).toHaveBeenCalledTimes(1); + }); + + it('should update lastPath synchronously when a change is accepted', () => { + window.history.pushState({}, '', '/products'); + expect(tracker['lastPath']).toBe('/products'); + }); + + it('should fire a view on popstate navigation', () => { + navigateNatively('/back-target'); + window.dispatchEvent(new PopStateEvent('popstate')); + jest.runAllTimers(); + + expect(logPageView).toHaveBeenCalledTimes(1); + }); + + it('should fire a view on hashchange navigation', () => { + navigateNatively('/#/new-hash'); + window.dispatchEvent(new HashChangeEvent('hashchange')); + jest.runAllTimers(); + + expect(logPageView).toHaveBeenCalledTimes(1); + }); + + it('should fire a view via replaceState when the path changes', () => { + window.history.replaceState({}, '', '/replaced'); + jest.runAllTimers(); + + expect(logPageView).toHaveBeenCalledTimes(1); + }); + }); + + // Q9.10 - Rapid same-tick /a -> /b -> /c fires two events. + describe('rapid same-tick navigation', () => { + it('should fire one view per accepted change', () => { + navigateNatively('/a'); + const tracker = createTracker(); + tracker.init(); // seeds lastPath = '/a' + + window.history.pushState({}, '', '/b'); + window.history.pushState({}, '', '/c'); + + jest.runAllTimers(); + + // /a is the seed; /b and /c are the two accepted changes. + expect(logPageView).toHaveBeenCalledTimes(2); + }); + }); + + describe('deferred fire behavior', () => { + let tracker: PageViewTracker; + + beforeEach(() => { + navigateNatively('/'); + tracker = createTracker(); + tracker.init(); + }); + + // Q9.11 - Navigation triggers resetSessionTimer() to renew the session. + it('should call resetSessionTimer before logging the page view', () => { + const callOrder: string[] = []; + resetSessionTimer.mockImplementation(() => + callOrder.push('resetSessionTimer') + ); + logPageView.mockImplementation(() => callOrder.push('logPageView')); + + window.history.pushState({}, '', '/next'); + jest.runAllTimers(); + + expect(callOrder).toEqual(['resetSessionTimer', 'logPageView']); + }); + + it('should abort a queued fire if torn down before the timer flushes', () => { + window.history.pushState({}, '', '/pending'); + + // Tear down before the deferred callback runs. + tracker.teardown(); + jest.runAllTimers(); + + expect(logPageView).not.toHaveBeenCalled(); + expect(resetSessionTimer).not.toHaveBeenCalled(); + }); + + // safeHandleNavigation isolates errors from the *synchronous* handler + // so a throwing handler never breaks the app's own pushState call. + it('should isolate synchronous errors from the navigation handler', () => { + jest.spyOn( + tracker as PageViewTracker & { + handleNavigation: () => void; + }, + 'handleNavigation' + ).mockImplementation(() => { + throw new Error('boom'); + }); + + expect(() => + window.history.pushState({}, '', '/explode') + ).not.toThrow(); + expect(verbose).toHaveBeenCalledWith( + expect.stringContaining('navigation handler threw') + ); + }); + }); + + describe('#teardown', () => { + // Q9.6 - After init() then teardown, the original is restored when + // still our wrapper. + it('should restore native history methods and remove listeners', () => { + const removeEventListenerSpy = jest.spyOn( + window, + 'removeEventListener' + ); + + const tracker = createTracker(); + tracker.init(); + tracker.teardown(); + + expect(window.history.pushState).toBe(NATIVE_PUSH_STATE); + expect(window.history.replaceState).toBe(NATIVE_REPLACE_STATE); + expect(removeEventListenerSpy).toHaveBeenCalledWith( + 'popstate', + expect.any(Function) + ); + expect(removeEventListenerSpy).toHaveBeenCalledWith( + 'hashchange', + expect.any(Function) + ); + expect(tracker['isActive']).toBe(false); + }); + + // Q9.7 - Teardown with another wrapper on top leaves the original in + // place but still removes listeners. + it('should leave a foreign wrapper in place but still clean up', () => { + const removeEventListenerSpy = jest.spyOn( + window, + 'removeEventListener' + ); + + const tracker = createTracker(); + tracker.init(); + + // A third party patches pushState on top of ours after init. + const foreignWrapper = function() { + /* someone else's wrapper */ + } as History['pushState']; + window.history.pushState = foreignWrapper; + + tracker.teardown(); + + // Our wrapper is no longer installed, so we must not clobber the + // foreign one by restoring the native method. + expect(window.history.pushState).toBe(foreignWrapper); + expect(removeEventListenerSpy).toHaveBeenCalledWith( + 'popstate', + expect.any(Function) + ); + expect(tracker['isActive']).toBe(false); + }); + + it('should be safe to call before init()', () => { + const tracker = createTracker(); + expect(() => tracker.teardown()).not.toThrow(); + expect(tracker['isActive']).toBe(false); + }); + + it('should stop firing page views after teardown', () => { + const tracker = createTracker(); + tracker.init(); + tracker.teardown(); + + // Navigate via the (now restored) native method + event. + navigateNatively('/after-teardown'); + window.dispatchEvent(new PopStateEvent('popstate')); + jest.runAllTimers(); + + expect(logPageView).not.toHaveBeenCalled(); + }); + }); +}); From 39a5594c1b76aabc45b8920c8714d914333d4e5a Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Thu, 6 Aug 2026 23:24:54 -0400 Subject: [PATCH 3/5] fix: capture per-navigation path and restore replaceState independently Bug 1 (teardown asymmetry): evaluate pushState and replaceState restoration independently by identity, so a still-ours replaceState wrapper is restored even when a third party has replaced only pushState. Prevents a leaked wrapper from being double-wrapped on re-init. Bug 2 (final-URL capture): snapshot the settled path at accept time and pass it through to the deferred flush, so each fire reports its own navigation rather than reading the live location (which a same-tick navigation would overwrite). Extract the fire logic into a testable firePageView(path) that emits via _Events.logEvent(PageView) carrying path, title, and hostname. --- src/pageViewTracker.ts | 65 ++++++++++++++++----- test/jest/pageViewTracker.spec.ts | 94 +++++++++++++++++++++++++------ 2 files changed, 130 insertions(+), 29 deletions(-) diff --git a/src/pageViewTracker.ts b/src/pageViewTracker.ts index 2671188a9..8ac57ac4e 100644 --- a/src/pageViewTracker.ts +++ b/src/pageViewTracker.ts @@ -1,4 +1,5 @@ import { IMParticleWebSDKInstance } from './mp-instance'; +import { EventType, MessageType } from './types'; type HistoryStateMethod = History['pushState']; @@ -181,6 +182,13 @@ export class PageViewTracker { ); this.lastPath = candidatePath; + // Snapshot the path now: it is already settled at this point, and a + // same-tick navigation would otherwise overwrite window.location + // before the deferred flush reads it. The title is intentionally read + // later (in the deferred flush) so the router's post-navigation render + // commit has a chance to update document.title first. + const capturedPath = candidatePath; + setTimeout(() => { if (!this.isActive) { this.mpInstance.Logger.verbose( @@ -190,14 +198,31 @@ export class PageViewTracker { } this.mpInstance._SessionManager.resetSessionTimer(); - - this.mpInstance.Logger.verbose( - `mParticle APV: [fire] deferred flush -> _Events.logPageView() (path: ${candidatePath}, title: ${window.document.title})` - ); - this.mpInstance._Events.logPageView(); + this.firePageView(capturedPath); }, 0); } + // Mirrors the event shape of the public mParticle.logPageView(), but carries + // the captured SPA path rather than reading the live location. + private firePageView(path: string): void { + const title = window.document.title; + + this.mpInstance.Logger.verbose( + `mParticle APV: [fire] deferred flush -> _Events.logEvent(PageView) (path: ${path}, title: ${title})` + ); + + this.mpInstance._Events.logEvent({ + messageType: MessageType.PageView, + name: 'PageView', + data: { + hostname: window.location.hostname, + title, + path, + }, + eventType: EventType.Unknown, + }); + } + public teardown(): void { if (this.popStateListener) { window.removeEventListener('popstate', this.popStateListener); @@ -208,25 +233,39 @@ export class PageViewTracker { this.hashChangeListener = null; } - const ourWrapperStillInstalled = + const pushStateStillOurs = this.pushStateWrapper !== null && window.history.pushState === this.pushStateWrapper; - - if (this.originalPushState && this.originalReplaceState) { - if (ourWrapperStillInstalled) { + if (this.originalPushState) { + if (pushStateStillOurs) { window.history.pushState = this.originalPushState; - window.history.replaceState = this.originalReplaceState; this.mpInstance.Logger.verbose( - 'mParticle APV: [teardown] restored original pushState/replaceState' + 'mParticle APV: [teardown] restored original pushState' ); } else { this.mpInstance.Logger.verbose( - 'mParticle APV: [teardown] wrapper no longer ours; leaving history methods, gating callback to no-op' + 'mParticle APV: [teardown] pushState no longer ours; leaving in place, gating callback to no-op' ); } this.originalPushState = null; - this.originalReplaceState = null; this.pushStateWrapper = null; + } + + const replaceStateStillOurs = + this.replaceStateWrapper !== null && + window.history.replaceState === this.replaceStateWrapper; + if (this.originalReplaceState) { + if (replaceStateStillOurs) { + window.history.replaceState = this.originalReplaceState; + this.mpInstance.Logger.verbose( + 'mParticle APV: [teardown] restored original replaceState' + ); + } else { + this.mpInstance.Logger.verbose( + 'mParticle APV: [teardown] replaceState no longer ours; leaving in place, gating callback to no-op' + ); + } + this.originalReplaceState = null; this.replaceStateWrapper = null; } diff --git a/test/jest/pageViewTracker.spec.ts b/test/jest/pageViewTracker.spec.ts index 9a77f91fe..3b2958770 100644 --- a/test/jest/pageViewTracker.spec.ts +++ b/test/jest/pageViewTracker.spec.ts @@ -1,5 +1,6 @@ import { PageViewTracker } from '../../src/pageViewTracker'; import { IMParticleWebSDKInstance } from '../../src/mp-instance'; +import { MessageType } from '../../src/types'; // Capture the genuinely-native history methods at import time, before any // tracker has a chance to monkey-patch them. Used to reset state between tests @@ -11,7 +12,7 @@ const WRAPPED_MARKER = '__mpApvWrapped__'; describe('PageViewTracker', () => { let mpInstance: IMParticleWebSDKInstance; - let logPageView: jest.Mock; + let logEvent: jest.Mock; let resetSessionTimer: jest.Mock; let verbose: jest.Mock; @@ -35,14 +36,14 @@ describe('PageViewTracker', () => { jest.useFakeTimers(); trackers = []; - logPageView = jest.fn(); + logEvent = jest.fn(); resetSessionTimer = jest.fn(); verbose = jest.fn(); mpInstance = ({ Logger: { verbose }, _SessionManager: { resetSessionTimer }, - _Events: { logPageView }, + _Events: { logEvent }, } as unknown) as IMParticleWebSDKInstance; }); @@ -243,7 +244,7 @@ describe('PageViewTracker', () => { window.history.replaceState({}, '', '/'); jest.runAllTimers(); - expect(logPageView).not.toHaveBeenCalled(); + expect(logEvent).not.toHaveBeenCalled(); }); // Q9.2 - Query-only change fires exactly one view via the full-URL key. @@ -251,7 +252,7 @@ describe('PageViewTracker', () => { window.history.pushState({}, '', '/?tab=settings'); jest.runAllTimers(); - expect(logPageView).toHaveBeenCalledTimes(1); + expect(logEvent).toHaveBeenCalledTimes(1); }); // Q9.2 - Hash-only change fires exactly one view via the full-URL key. @@ -259,7 +260,7 @@ describe('PageViewTracker', () => { window.history.pushState({}, '', '/#/details'); jest.runAllTimers(); - expect(logPageView).toHaveBeenCalledTimes(1); + expect(logEvent).toHaveBeenCalledTimes(1); }); // Q9.3 - A real path change produces one deferred view after flush. @@ -267,10 +268,10 @@ describe('PageViewTracker', () => { window.history.pushState({}, '', '/products'); // Deferred: nothing fires synchronously. - expect(logPageView).not.toHaveBeenCalled(); + expect(logEvent).not.toHaveBeenCalled(); jest.runAllTimers(); - expect(logPageView).toHaveBeenCalledTimes(1); + expect(logEvent).toHaveBeenCalledTimes(1); }); it('should update lastPath synchronously when a change is accepted', () => { @@ -283,7 +284,7 @@ describe('PageViewTracker', () => { window.dispatchEvent(new PopStateEvent('popstate')); jest.runAllTimers(); - expect(logPageView).toHaveBeenCalledTimes(1); + expect(logEvent).toHaveBeenCalledTimes(1); }); it('should fire a view on hashchange navigation', () => { @@ -291,14 +292,14 @@ describe('PageViewTracker', () => { window.dispatchEvent(new HashChangeEvent('hashchange')); jest.runAllTimers(); - expect(logPageView).toHaveBeenCalledTimes(1); + expect(logEvent).toHaveBeenCalledTimes(1); }); it('should fire a view via replaceState when the path changes', () => { window.history.replaceState({}, '', '/replaced'); jest.runAllTimers(); - expect(logPageView).toHaveBeenCalledTimes(1); + expect(logEvent).toHaveBeenCalledTimes(1); }); }); @@ -315,7 +316,48 @@ describe('PageViewTracker', () => { jest.runAllTimers(); // /a is the seed; /b and /c are the two accepted changes. - expect(logPageView).toHaveBeenCalledTimes(2); + expect(logEvent).toHaveBeenCalledTimes(2); + }); + + // Each deferred fire must carry the path captured when its navigation + // was accepted, not the final live location. Before the fix both fires + // read window.location at flush time and both reported '/c'. + it('should record each intermediate path, not the final URL', () => { + navigateNatively('/a'); + const tracker = createTracker(); + tracker.init(); + + window.history.pushState({}, '', '/b'); + window.history.pushState({}, '', '/c'); + + jest.runAllTimers(); + + const paths = logEvent.mock.calls.map(([event]) => event.data.path); + expect(paths).toEqual(['/b', '/c']); + }); + }); + + describe('page view payload', () => { + it('should fire a PageView event carrying path, title, and hostname', () => { + navigateNatively('/'); + const tracker = createTracker(); + tracker.init(); + + window.document.title = 'Next Page'; + window.history.pushState({}, '', '/next?q=1#top'); + jest.runAllTimers(); + + expect(logEvent).toHaveBeenCalledTimes(1); + const [event] = logEvent.mock.calls[0]; + expect(event).toMatchObject({ + messageType: MessageType.PageView, + name: 'PageView', + data: { + hostname: 'localhost', + title: 'Next Page', + path: '/next?q=1#top', + }, + }); }); }); @@ -334,12 +376,12 @@ describe('PageViewTracker', () => { resetSessionTimer.mockImplementation(() => callOrder.push('resetSessionTimer') ); - logPageView.mockImplementation(() => callOrder.push('logPageView')); + logEvent.mockImplementation(() => callOrder.push('logEvent')); window.history.pushState({}, '', '/next'); jest.runAllTimers(); - expect(callOrder).toEqual(['resetSessionTimer', 'logPageView']); + expect(callOrder).toEqual(['resetSessionTimer', 'logEvent']); }); it('should abort a queued fire if torn down before the timer flushes', () => { @@ -349,7 +391,7 @@ describe('PageViewTracker', () => { tracker.teardown(); jest.runAllTimers(); - expect(logPageView).not.toHaveBeenCalled(); + expect(logEvent).not.toHaveBeenCalled(); expect(resetSessionTimer).not.toHaveBeenCalled(); }); @@ -429,6 +471,26 @@ describe('PageViewTracker', () => { expect(tracker['isActive']).toBe(false); }); + // A third party may patch only one of the two methods. Teardown must + // decide per-method: leave the foreign pushState in place while still + // restoring our untouched replaceState (otherwise it leaks, and a + // later re-init stacks a second wrapper on top of it). + it('should restore replaceState even when pushState is foreign', () => { + const tracker = createTracker(); + tracker.init(); + + const foreignWrapper = function() { + /* someone else's wrapper */ + } as History['pushState']; + window.history.pushState = foreignWrapper; + + tracker.teardown(); + + expect(window.history.pushState).toBe(foreignWrapper); + expect(window.history.replaceState).toBe(NATIVE_REPLACE_STATE); + expect(tracker['isActive']).toBe(false); + }); + it('should be safe to call before init()', () => { const tracker = createTracker(); expect(() => tracker.teardown()).not.toThrow(); @@ -445,7 +507,7 @@ describe('PageViewTracker', () => { window.dispatchEvent(new PopStateEvent('popstate')); jest.runAllTimers(); - expect(logPageView).not.toHaveBeenCalled(); + expect(logEvent).not.toHaveBeenCalled(); }); }); }); From b7445f05c421ceadb205f1302e87b74c204b5d02 Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Thu, 6 Aug 2026 23:27:16 -0400 Subject: [PATCH 4/5] refactor: extract getCurrentKey() to keep seed and compare in sync The init() seed and handleNavigation() comparison derived the dedup key by hand in two places; a future edit to one could silently desync the other, mis-firing or suppressing the first navigation after init. Extract a single private getCurrentKey() used by both. --- src/pageViewTracker.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/pageViewTracker.ts b/src/pageViewTracker.ts index 8ac57ac4e..c1144b37e 100644 --- a/src/pageViewTracker.ts +++ b/src/pageViewTracker.ts @@ -57,8 +57,7 @@ export class PageViewTracker { this.isActive = true; - const { pathname, search, hash } = window.location; - this.lastPath = pathname + search + hash; + this.lastPath = this.getCurrentKey(); this.mpInstance.Logger.verbose( `mParticle APV: [init] seeded lastPath: ${this.lastPath}` @@ -162,9 +161,13 @@ export class PageViewTracker { } } - private handleNavigation(source: string): void { + private getCurrentKey(): string { const { pathname, search, hash } = window.location; - const candidatePath = pathname + search + hash; + return pathname + search + hash; + } + + private handleNavigation(source: string): void { + const candidatePath = this.getCurrentKey(); this.mpInstance.Logger.verbose( `mParticle APV: [detect] navigation signal (source: ${source}, candidatePath: ${candidatePath}, lastPath: ${this.lastPath})` From 540838c8f79a65b5dc110584b09d3837701c0266 Mon Sep 17 00:00:00 2001 From: Alexander Sapountzis Date: Thu, 6 Aug 2026 23:54:32 -0400 Subject: [PATCH 5/5] fix: dedup page views on pathname only, drop search and hash Including search in the dedup key over-counted page views (transient UI state written to the query string via replaceState produced a distinct key per change) and, combined with the per-fire resetSessionTimer() call, could keep a session alive indefinitely from background query-string churn with no user present. Key and report pathname only. Hash-router support is deferred to a follow-up along with the anchor-link-vs-route-hash question; the hashchange listener is removed until then. --- src/pageViewTracker.ts | 14 ++---------- test/jest/pageViewTracker.spec.ts | 38 ++++++++++++++----------------- 2 files changed, 19 insertions(+), 33 deletions(-) diff --git a/src/pageViewTracker.ts b/src/pageViewTracker.ts index c1144b37e..c8ce49ed3 100644 --- a/src/pageViewTracker.ts +++ b/src/pageViewTracker.ts @@ -22,7 +22,6 @@ export class PageViewTracker { private replaceStateWrapper: HistoryStateMethod | null = null; private popStateListener: (() => void) | null = null; - private hashChangeListener: (() => void) | null = null; constructor(mpInstance: IMParticleWebSDKInstance) { this.mpInstance = mpInstance; @@ -67,7 +66,7 @@ export class PageViewTracker { this.addNavigationListeners(); this.mpInstance.Logger.verbose( - 'mParticle APV: [init] patched pushState/replaceState + listening for popstate/hashchange' + 'mParticle APV: [init] patched pushState/replaceState + listening for popstate' ); } @@ -145,10 +144,7 @@ export class PageViewTracker { private addNavigationListeners(): void { this.popStateListener = () => this.safeHandleNavigation('popstate'); - this.hashChangeListener = () => this.safeHandleNavigation('hashchange'); - window.addEventListener('popstate', this.popStateListener); - window.addEventListener('hashchange', this.hashChangeListener); } private safeHandleNavigation(source: string): void { @@ -162,8 +158,7 @@ export class PageViewTracker { } private getCurrentKey(): string { - const { pathname, search, hash } = window.location; - return pathname + search + hash; + return window.location.pathname; } private handleNavigation(source: string): void { @@ -231,11 +226,6 @@ export class PageViewTracker { window.removeEventListener('popstate', this.popStateListener); this.popStateListener = null; } - if (this.hashChangeListener) { - window.removeEventListener('hashchange', this.hashChangeListener); - this.hashChangeListener = null; - } - const pushStateStillOurs = this.pushStateWrapper !== null && window.history.pushState === this.pushStateWrapper; diff --git a/test/jest/pageViewTracker.spec.ts b/test/jest/pageViewTracker.spec.ts index 3b2958770..2f6d06364 100644 --- a/test/jest/pageViewTracker.spec.ts +++ b/test/jest/pageViewTracker.spec.ts @@ -110,13 +110,13 @@ describe('PageViewTracker', () => { }); describe('#init', () => { - it('should seed lastPath with the full relative URL at init time', () => { + it('should seed lastPath with the pathname only at init time', () => { navigateNatively('/dashboard?tab=1#section'); const tracker = createTracker(); tracker.init(); - expect(tracker['lastPath']).toBe('/dashboard?tab=1#section'); + expect(tracker['lastPath']).toBe('/dashboard'); expect(tracker['isActive']).toBe(true); }); @@ -132,7 +132,7 @@ describe('PageViewTracker', () => { 'popstate', expect.any(Function) ); - expect(addEventListenerSpy).toHaveBeenCalledWith( + expect(addEventListenerSpy).not.toHaveBeenCalledWith( 'hashchange', expect.any(Function) ); @@ -209,15 +209,11 @@ describe('PageViewTracker', () => { expect(tracker['pushStateWrapper']).toBeNull(); expect(tracker['originalPushState']).toBeNull(); - // ...but navigation listeners are still registered. + // ...but the navigation listener is still registered. expect(addEventListenerSpy).toHaveBeenCalledWith( 'popstate', expect.any(Function) ); - expect(addEventListenerSpy).toHaveBeenCalledWith( - 'hashchange', - expect.any(Function) - ); } finally { // Restore writability so afterEach can reset. Object.defineProperty(window.history, 'pushState', { @@ -247,20 +243,22 @@ describe('PageViewTracker', () => { expect(logEvent).not.toHaveBeenCalled(); }); - // Q9.2 - Query-only change fires exactly one view via the full-URL key. - it('should fire one view on a query-string-only change', () => { + // Dedup keys on pathname only, so a query-string-only change is treated + // as the same page and does not fire. + it('should not fire a view on a query-string-only change', () => { window.history.pushState({}, '', '/?tab=settings'); jest.runAllTimers(); - expect(logEvent).toHaveBeenCalledTimes(1); + expect(logEvent).not.toHaveBeenCalled(); }); - // Q9.2 - Hash-only change fires exactly one view via the full-URL key. - it('should fire one view on a hash-only change', () => { + // Hash support is deferred to a follow-up; a hash-only change leaves the + // pathname unchanged and does not fire. + it('should not fire a view on a hash-only change', () => { window.history.pushState({}, '', '/#/details'); jest.runAllTimers(); - expect(logEvent).toHaveBeenCalledTimes(1); + expect(logEvent).not.toHaveBeenCalled(); }); // Q9.3 - A real path change produces one deferred view after flush. @@ -287,12 +285,14 @@ describe('PageViewTracker', () => { expect(logEvent).toHaveBeenCalledTimes(1); }); - it('should fire a view on hashchange navigation', () => { + // Hash support is deferred to a follow-up: the tracker no longer listens + // for hashchange, so a hash-route navigation does not fire. + it('should not fire a view on hashchange navigation', () => { navigateNatively('/#/new-hash'); window.dispatchEvent(new HashChangeEvent('hashchange')); jest.runAllTimers(); - expect(logEvent).toHaveBeenCalledTimes(1); + expect(logEvent).not.toHaveBeenCalled(); }); it('should fire a view via replaceState when the path changes', () => { @@ -355,7 +355,7 @@ describe('PageViewTracker', () => { data: { hostname: 'localhost', title: 'Next Page', - path: '/next?q=1#top', + path: '/next', }, }); }); @@ -435,10 +435,6 @@ describe('PageViewTracker', () => { 'popstate', expect.any(Function) ); - expect(removeEventListenerSpy).toHaveBeenCalledWith( - 'hashchange', - expect.any(Function) - ); expect(tracker['isActive']).toBe(false); });