refactor [NT-3339]: Refactor auto page emitter architecture#303
Merged
Lotfi Anwar L Arif (Lotfi-Arif) merged 5 commits intoJun 3, 2026
Merged
Conversation
…d add buildAutoPagePayload helper
Shrink useAutoPageEmitter to a router-agnostic emitter that owns dedup
and emission only. The hook now accepts { enabled, routeKey, buildPayload }
and no longer knows about routers, route contexts, payload merging, or the
consumer's pagePayload/getPagePayload props.
Move payload composition into a new buildAutoPagePayload helper that merges
three explicit layers (router-derived, consumer static, consumer dynamic)
and forwards the emission context to getPagePayload. composePagePayload is
extended to accept any number of layers so the helper can compose them
cleanly; the previous two-arg call sites continue to work.
These changes prepare the four router adapters to build their own finished
page payloads. The hook becomes trivial to test without a router; the merge
precedence becomes one explicit data flow inside the helper rather than
inline logic in the hook.
All four router adapters (TanStack, React Router, Next.js App Router,
Next.js Pages Router) now follow the same five-step structure:
1. Read router state via the router's React API.
2. Build a router-derived payload (path, url, search, hash, query) from
authoritative router state, not a window.location read at send time.
3. Build the emission context for the consumer's getPagePayload callback.
4. Compose router payload + consumer static + consumer dynamic via the
shared buildAutoPagePayload helper, inside a buildPayload callback.
5. Hand routeKey + buildPayload to useAutoPageEmitter.
The router-derived payload becomes the bottom layer of the merge stack.
Consumer-supplied pagePayload and getPagePayload still take precedence on
key conflicts, so existing consumer code works unchanged.
This restructure also fixes a class of stale-URL bugs across all four
adapters (originally reported for TanStack Router): page event payloads
now reflect the route React just rendered, not whatever window.location
shows at emission time. The Next.js App Router adapter intentionally
omits hash, since Next does not expose it; getPageProperties() supplies
it from window.location.hash, which is not subject to the same staleness
as pathname/search.
- Add pagePayload.test.ts: standalone tests for composePagePayload's
variadic merge semantics and buildAutoPagePayload's three-layer
precedence + context forwarding. Pure helper, no React or routers.
- Rewrite useAutoPageEmitter.test.tsx for the slim hook surface
({ enabled, routeKey, buildPayload }). Adds tests that
buildPayload only runs when an emission would actually happen
(i.e. not on dedup short-circuit) and that isInitialEmission is
signalled correctly across emissions.
- Update each router adapter test (tanstack-router, react-router,
next-app, next-pages) to assert the new payload shape that includes
the router-derived path / url / search / hash / query keys, alongside
whatever static or dynamic consumer overrides each test supplies.
- Update OptimizationProvider.onStatesReady.test.tsx for the new hook
call shape.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
CodeQL flagged remote property injection in the four router adapters
where user-controlled query keys were assigned to plain {} objects via
`obj[key] = value`. A malicious URL containing keys like __proto__ or
constructor could mutate the prototype chain, affecting downstream code
that reads from these query dictionaries.
Replace the imperative for-loop assignments with Object.fromEntries,
which uses [[DefineOwnProperty]] semantics and is not subject to the
same prototype-setter trap. This satisfies CodeQL's remote-property-
injection sink check without an unsafe type assertion.
Lotfi Anwar L Arif (Lotfi-Arif)
requested a review
from Charles Hudson (phobetron)
as a code owner
Charles Hudson (phobetron)
approved these changes
Jun 3, 2026
Lotfi Anwar L Arif (Lotfi-Arif)
deleted the
refactor-auto-page-emitter-architecture
branch
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
3 participants
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
Restructures the auto-page tracker stack so each router adapter (TanStack Router, React Router, Next.js App Router, Next.js Pages Router) owns its own page event payload, while
useAutoPageEmitterbecomes a generic emitter with a single responsibility: dedup + emit.Each of the four router adapters now follows the same five-step structure:
What this fixes
The TanStack stale-URL bug from NT-3339 — page events reflecting the previous route after navigation — is fixed as a side effect of step 2. The same fix is applied uniformly across all four adapters; none of them now rely on window.location for path/url/search data. The Next.js App Router adapter intentionally omits hash (Next does not expose it), so the SDK still fills it in from window.location.hash, which is not subject to the same staleness.