fix(apv): prevent duplicate page views on SPA re-init (stacked wrapper + init guard) - #1331
fix(apv): prevent duplicate page views on SPA re-init (stacked wrapper + init guard)#1331alexs-mparticle wants to merge 8 commits into
Conversation
PR SummaryMedium Risk Overview
SDK init creates the tracker only when missing, gates the first page view behind the init flag, still calls New unit and integration tests cover SPA re-init, Next.js-style module re-evaluation, stale tracker teardown, and pending deferred page views. Reviewed by Cursor Bugbot for commit 1d6bdc4. Bugbot is set up for automated code reviews on this repo. Configure here. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Two root causes fixed: 1. mParticle.init() re-called by SPA framework (e.g. on each route change): logPageView() fired on every completeSDKInitialization even when the PageViewTracker was already active. Fixed by guarding behind a window.__mpApvInitPVLogged__ flag (survives module re-evaluation) so the initial page view fires only once per hard page load. 2. Next.js re-evaluates the SDK module on each SPA navigation, resetting all module-level state. Each fresh module created a new PageViewTracker and patched window.history.pushState on top of the previous wrapper, stacking N wrappers after N navigations and firing N page views per click. Fixed by storing the active tracker on window.__mpApvTracker__ and tearing down any stale tracker from a previous module load before installing the new one. _resetForTests now directly deletes both APV window flags so stale state cannot leak across test cases regardless of instance assignment. Adds a regression test for the re-init duplicate case.
40352d1 to
7257d7e
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7257d7e. Configure here.
rmi22186
left a comment
rmi22186
left a comment
There was a problem hiding this comment.
Re-reviewed with multiple-instance support explicitly treated as out of scope for this Rokt/legacy APV feature. I found two lifecycle edge cases and a test-coverage gap. One additional question: development already marks wrappers with __mpApvWrapped__ and skips re-wrapping, so it would be useful to confirm whether the observed production duplication is from accumulated listeners/stale ownership rather than stacked pushState wrappers.
| const prev = win[WIN_TRACKER_KEY]; | ||
| if (prev && prev !== this) { | ||
| this.mpInstance.Logger.verbose( | ||
| 'mParticle APV: [init] found stale tracker from previous module load — tearing down to prevent stacked wrappers' |
There was a problem hiding this comment.
Could this handoff preserve or flush a page view already queued by prev? A route change queues its event with setTimeout; if module re-evaluation reaches this synchronous init before that timer runs, teardown() marks the previous tracker inactive and its callback aborts. The replacement tracker seeds the destination as lastPath, so that navigation is never logged. A regression test where the old tracker detects a route, the replacement initializes, and then timers flush would cover the target scenario.
There was a problem hiding this comment.
| } | ||
| const win = window as WindowWithApvFlags; | ||
| delete win[WIN_INIT_PV_KEY]; | ||
| delete win[WIN_TRACKER_KEY]; |
There was a problem hiding this comment.
This can delete the only reference to an active stale tracker without tearing it down. After module re-evaluation, instance._PageViewTracker may be absent or differ from win[WIN_TRACKER_KEY]; in that case its history wrapper/listener remains installed but becomes undiscoverable. Please teardown the tracker referenced by the window registry before clearing it (while avoiding a second teardown when both references are the same).
| ); | ||
| }); | ||
|
|
||
| it('should not log a duplicate page view when init() is called again (SPA re-init)', async () => { |
There was a problem hiding this comment.
This verifies repeated init() on the same SDK instance, where _PageViewTracker already exists. It does not exercise the new window-level handoff or WIN_INIT_PV_KEY path used when a fresh module creates a fresh tracker. Could we add a focused module-replacement test with an old tracker stored on window, including a pending route event and cleanup of stale global state?
There was a problem hiding this comment.
…ForTests, document edge cases
…iewTracker module
…cker static methods
…thApvFlags directly
… and WIN_INIT_PV_KEY path
|
2 participants
Summary
window.__mpApvTracker__singleton so that when Next.js re-evaluates the SDK module on each SPA navigation, the new module's tracker tears down the previous module's tracker before patchingpushState. This eliminates stacked wrappers (N page views per navigation).logPageView()incompleteSDKInitializationbehindwindow.__mpApvInitPVLogged__so the initial page view fires only once per hard page load, not on everymParticle.init()call._resetForTeststo tear down any activePageViewTrackerand clear both APV window flags between tests, preventing stale state from leaking across test cases.Test plan
AutoLogPageView: TruemParticle.init()again on re-navigation does not fire a duplicate initial page viewnpm testpasses (2176 tests, 0 failures)Notes
The companion PR for kit double-registration via
configuredForwardersaccumulation is being reviewed separately as a draft.