fix(forwarders): reset configuredForwarders on each init to prevent kit accumulation - #1332
Draft
alexs-mparticle wants to merge 6 commits into
Draft
fix(forwarders): reset configuredForwarders on each init to prevent kit accumulation#1332alexs-mparticle wants to merge 6 commits into
alexs-mparticle wants to merge 6 commits into
Conversation
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.
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.
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.
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.
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.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
_Store.configuredForwardersto[]at the start ofprocessForwardersso that eachmParticle.init()call starts with a clean forwarder list.mParticle.init()on every navigation accumulate duplicate kit instances — after N inits,activeForwarderscontains N Rokt kit instances and the same event fires N times.Root cause
configuredForwardersis initialised once to[]in theStoreconstructor and never reset.processForwarders→configureUIEnabledKitpushes a new kit instance on every call, but the previous instances are never evicted.initForwardersthen promotes all of them toactiveForwarders.Why the reset belongs here
processForwardersis the only function that writes toconfiguredForwarders. Resetting at the top makes the function idempotent and safe to call multiple times — any future call site gets correct behaviour automatically without needing to remember to reset first.Logging added (remove before merge)
Verbose logging shows
cleared N existing forwarder(s) before re-initand the final configured count on every init, making it easy to verify in local testing.Test plan
npm test)cleared 1 existing forwarder(s)and produce exactly 1 kit event, not 2Notes
Draft — pending local verification on SPA partner site. Companion to #1331.