Skip to content

Defer initial adInit until after React hydration on Next.js App Router - #945

Open
aram356 wants to merge 6 commits into
mainfrom
fix/react418-hydration-safety
Open

Defer initial adInit until after React hydration on Next.js App Router#945
aram356 wants to merge 6 commits into
mainfrom
fix/react418-hydration-safety

Conversation

@aram356

@aram356 aram356 commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

On a Next.js App Router publisher, adInit() defines GPT slots on the publisher's -container wrappers, mutating those ad-slot subtrees. The </body> bids bootstrap called it synchronously at parse time, landing that mutation inside React's hydration window — React threw #418 and re-rendered the affected subtrees (visible flashes/reflow).

A live A/B, toggling TS on the same page via the tester cookie, isolated the trigger:

Condition React #418
Pure publisher (TS inactive) 0
TS active 1–2

The count tracks whether adInit processes ad slots — not the injection position. adInit is now deferred to after hydration: gated on window load, then a double requestAnimationFrame. Run-once, no retry timer.

The deferral lifecycle lives in the GPT bundle module as tsjs.scheduleInitialAdInit (crates/trusted-server-js/lib/src/integrations/gpt/index.ts); the server-emitted </body> bids script assigns tsjs.bids and delegates to it. The GPT module ships in the synchronous head bundle, so it has always executed before the inline script runs.

Scope

Deliberately reduced to the single change with a measured effect on #418. Two other hydration-safety changes were considered and dropped from this PR, because neither moved the observed #418 count and review surfaced real defects in both:

  • Appending the tsjs head bundle instead of prepending — reordered origin blocking scripts ahead of TS's bootstraps, which breaks the GPT shim's first-party rewrite of dynamically loaded scripts and can install the disableInitialLoad detector too late.
  • Rewriting integration hosts in the RSC flight — as written it HTML-escaped the serialized payload (corrupting bytes and desyncing recomputed T-chunk lengths), was not safe across lol_html text-chunk or cross-script boundaries, and rewrote every matching URL in flight data rather than only script src values.

A correct flight rewrite needs the existing RSC stream state machine, raw (non-escaping) insertion, and src-targeted parsing — that belongs in its own PR.

The gate's timing (window load is dominated by page weight — ~52s on heavy publishers) is addressed in the agreed stacked follow-up 958-adinit-hydration-chunk-gate (spec: docs/superpowers/specs/2026-07-24-adinit-hydration-gate-design.md), which waits on the async /_next/static/chunks/ hydration scripts and keeps load as the can't-hang fallback. Per that spec's PR strategy, this PR lands the conservative gate; the timing change gets its own PR and its own #418 A/B.

Review findings addressed

  • Stale deferred callback vs. SPA navigation — the deferred callback now captures tsjs.navGeneration, a monotonic counter the SPA auction hook increments synchronously the moment it accepts a pathname navigation, and no-ops if it moved. Unlike the earlier URL-equality guard, this cannot diverge from the hook's pathname-only route identity: a query-only replaceState before load no longer cancels the initial adInit, and an /a → /b → /a round trip (URL compares equal again) now correctly stands the stale callback down.
  • Executable lifecycle coverage — the bootstrap moved out of the Rust format string into the bundle, so its lifecycle is executed (not string-matched) under Vitest: test/integrations/gpt/schedule_initial_ad_init.test.ts covers load + two-frame ordering, already-complete documents, exactly-once invocation, query-only history changes, /a → /b → /a cancellation, and a publisher-displayed slot receiving setTargeting before the refresh that delivers it. The Rust tests pin the inline script's delegation and XSS safety.
  • Browser globals shadowing — the scheduler window-qualifies requestAnimationFrame / addEventListener.
  • Possible loss of the first targeted impression — measured rather than assumed. Instrumenting real GPT ad requests, the first request (t≈3.7s) carries ts_initial=1, hb_pb, and hb_bidder, so server-side targeting is applied to the first impression on the tested publisher; the Investigate refresh too quickly after SSAT with client side auction (timing issue with GAM) #958 wire investigation confirmed the TS slot's first fetch is adindex 0 with ts_initial=1 and that publisher-owned slots are distinct. Residual risk remains in principle for a publisher whose GPT activation precedes window load; the follow-up chunk gate narrows that window to seconds.

Verification

  • Executable Vitest lifecycle suite schedule_initial_ad_init.test.ts (6 tests) plus a navGeneration accounting test in spa_hook.test.ts; full JS suite 432 passed, 0 failed, format clean, bundles build.
  • Rust: cargo test-fastly 1734 passed, 0 failed (+ 21 adapter tests), cargo test-axum green, parity suite green, cargo fmt clean, clippy clean on trusted-server-core.
  • End to end through the dev proxy against a live App Router publisher: JS bundle hash recomputed on every HTML response instead of cached #418 = 0, servicesEnabled: true, TS container slot defined, ads rendering, and the first ad request fully targeted.

Refs #938

@aram356 aram356 self-assigned this Jul 21, 2026
@aram356 aram356 changed the title Fix React #418 hydration mismatch from ad-slot injection on Next.js App Router Fix React 418 hydration mismatch from ad-slot injection on Next.js App Router Jul 21, 2026
aram356 added a commit that referenced this pull request Jul 21, 2026
Brings the three hydration-safety changes: defer the initial adInit() until
after hydration, append the tsjs head bundle at the end of <head>, and rewrite
integration hosts in the streamed RSC flight. Merged cleanly with no conflicts.
On a Next.js App Router publisher, `adInit()` defines GPT slots on the
publisher's `-container` wrappers, mutating those ad-slot subtrees. The
`</body>` bids bootstrap called it synchronously at parse time, landing that
mutation inside React's hydration window, so React threw #418 and re-rendered
the affected subtrees (visible flashes/reflow).

A live A/B — toggling TS on the same page via the tester cookie — isolated the
trigger: the pure publisher throws 0 #418, TS activation introduces it, and the
count tracks whether adInit processes ad slots (not the injection position).

adInit is now deferred to after hydration: gated on window `load`, then a double
`requestAnimationFrame`. Run-once, no retry timer.

Deferring opens a window in which an SPA navigation can commit a new route (and
run its own adInit via the SPA auction hook) before the callback fires, so the
callback captures the route it was scheduled for and no-ops when the route has
changed — otherwise it would re-run adInit against the newer route's live
slots/bids, destroying and redefining that route's TS slots and refreshing it
twice. Browser globals are window-qualified so a page-level lexical binding
cannot shadow them.

Verified end to end through the dev proxy against a live App Router publisher:
#418 goes from 1-2 to 0 with TS still defining its container slots and ads
rendering.

Refs #938
@aram356
aram356 force-pushed the fix/react418-hydration-safety branch from 6a1db09 to 3ebcf8f Compare July 21, 2026 18:05
@aram356 aram356 changed the title Fix React 418 hydration mismatch from ad-slot injection on Next.js App Router Defer initial adInit until after React hydration on Next.js App Router Jul 21, 2026
aram356 added a commit that referenced this pull request Jul 21, 2026
…uly"

This reverts commit eaa8fed, reversing
changes made to 41345fc.
@aram356
aram356 requested review from ChristianPavilonis and prk-Jr and removed request for ChristianPavilonis July 21, 2026 20:49
@ChristianPavilonis
ChristianPavilonis self-requested a review July 21, 2026 21:49

@ChristianPavilonis ChristianPavilonis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review summary

🔧 Requesting changes. The diff is narrow, but it changes initial ad initialization for every matched-slot publisher and introduces high-risk navigation and GPT request-ordering regressions. CI is green, but the current automated coverage does not exercise these lifecycle paths.

"<script>(window.tsjs=window.tsjs||{{}}).bids=JSON.parse(\"{}\");(function(){{var f=window.tsjs.adInit;if(typeof f===\"function\")f();}})();</script>",
"<script>(window.tsjs=window.tsjs||{{}}).bids=JSON.parse(\"{}\");\
(function(){{\
var p=location.pathname+location.search;\

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 P1 — URL equality is not a safe navigation identity

The guard compares pathname + search, while the SPA auction hook intentionally identifies routes using only pathname (crates/trusted-server-js/lib/src/integrations/gpt/index.ts:715-724). This creates two failures:

  • A query-only pushState/replaceState before load makes this callback abort, but the SPA hook also declines to auction because the pathname is unchanged. Initial adInit() is therefore never called.
  • An /a → /b → /a transition before this callback defeats the stale-route check because the URL equals /a again. The SPA hook auctions the new /a route and calls adInit(), after which this initial callback can call it again, destroying/redefining slots and issuing duplicate requests.

This can leave all initial ads uninitialized after query normalization or double-refresh inventory after a rapid round trip. Please use a shared monotonic navigation generation maintained synchronously by the SPA hook, capture that generation here, and run only if it is unchanged. Keep query-only changes ignored unless /__ts/page-bids and the SPA hook are deliberately changed to treat them as auction-relevant.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented in 6f9dbff. The SPA auction hook now maintains tsjs.navGeneration — a monotonic counter incremented synchronously the moment a pathname navigation is accepted (before the page-bids fetch) — and the deferred initial-adInit callback captures it at schedule time and stands down if it has moved.

Query-only history changes remain ignored by both the hook and the guard, so the initial adInit() still runs after a query-only replaceState; an /a → /b → /a round trip advances the counter twice, so the stale callback no-ops. Both scenarios have executable coverage in schedule_initial_ad_init.test.ts, plus a counter-accounting test in spa_hook.test.ts.

var f=function(){{\
if(location.pathname+location.search!==p)return;\
var a=window.tsjs.adInit;if(typeof a===\"function\")a();}};\
var d=function(){{window.requestAnimationFrame(function(){{window.requestAnimationFrame(f);}});}};\

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 P1 — The global load gate moves TS targeting behind normal publisher GPT requests

This generic bootstrap now waits for every load-blocking resource and then two animation frames. A publisher that defines/displays GPT before or during window.load sends its first request before TS targeting is applied. adInit() subsequently finds the publisher-owned slot (crates/trusted-server-js/lib/src/integrations/gpt/index.ts:512-518) and refreshes it (index.ts:621-633), turning the TS-targeted request into a second impression. This behavior applies to every creative-opportunity page, not only the tested Next.js App Router publisher.

That can lose the server-auction bid on the first impression, create a duplicate request, delay TS-owned slots behind unrelated resources, and age the auction result. Testing one publisher whose first request happened after this callback does not establish compatibility with other supported initialization patterns.

Please preserve pre-request targeting rather than delaying the entire operation: defer only the hydration-unsafe TS-owned slot definition/display work while intercepting or targeting existing publisher slots before their first display(). If that cannot be done safely here, make delayed initialization an explicit App Router/publisher opt-in and retain synchronous behavior elsewhere.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two separable points here.

Refreshing a publisher-owned slot to deliver TS targeting is pre-existing behavior, not introduced by this PR — before this change adInit() ran synchronously at body-parse time and refreshed publisher-owned slots the same way whenever the publisher's initial load had already fired. This PR changes only how long that window stays open. On the live App Router publisher this was measured against, the TS slot's first GPT request (adindex 0) carries ts_initial=1 + hb_*, and the wire investigation for #958 confirmed the publisher's own slots are distinct from TS's slots (the early pre-SSAT fetch belonged to the publisher).

The gate timing is addressed in the agreed stacked follow-up (958-adinit-hydration-chunk-gate, spec at docs/superpowers/specs/2026-07-24-adinit-hydration-gate-design.md): wait for the async /_next/static/chunks/ hydration scripts (a few seconds) instead of window.load (~52s on heavy pages), with load kept as the can't-hang fallback — effectively the App Router-specific fast path this comment asks for, with no new config surface. That spec's PR strategy is to land #945 with the conservative gate and make the timing change in its own PR with its own #418 A/B evidence, so the gate here is unchanged. A sync-targeting/deferred-DOM split of adInit() is the spec's explicit non-goal (the "hydration-safe adInit rewrite" — a larger change, separate effort).

What did land here for this finding's test ask: the deferral now lives in the GPT bundle (tsjs.scheduleInitialAdInit, commit 6f9dbff), and an executable test pins that a publisher-displayed slot receives setTargeting before the refresh that delivers it.

}

#[test]
fn bids_script_defers_ad_init_until_after_hydration() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 P2 — This test does not execute the lifecycle being changed

The assertions only check for substrings such as requestAnimationFrame, "load", and location.pathname. The test would still pass if adInit() ran synchronously, if only one frame were used, or with both navigation bugs noted above. The central guarantees—post-load ordering, exactly-once invocation, stale-navigation cancellation, and interaction with the SPA hook—remain unprotected.

Please add an executable browser/JavaScript test covering loading and already-complete documents, two-frame ordering, query-only history changes, /a → /b → /a, and a publisher GPT display() during load. Assert both the adInit() count and first-request targeting.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 6f9dbff. The bootstrap moved out of the Rust-emitted inline string into the GPT bundle (scheduleInitialAdInit in gpt/index.ts); the </body> bids script now only assigns tsjs.bids and delegates to it.

The lifecycle now runs for real under Vitest/jsdom in test/integrations/gpt/schedule_initial_ad_init.test.ts — the real scheduler plus, where relevant, the real adInit() and SPA auction hook: load + two-frame ordering, already-complete documents, exactly-once invocation across duplicate load events, query-only history changes, /a → /b → /a cancellation, and a publisher-displayed slot receiving setTargeting before the refresh that delivers it. The Rust tests now pin only the inline script's delegation and XSS safety.

Real first-request GPT targeting stays verified by the live A/B in the PR description — an actual GPT ad request is not reachable from jsdom.

@prk-Jr prk-Jr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

The hydration mitigation is narrow, but it changes initial ad initialization for every matched-slot publisher and leaves navigation and GPT request-ordering regressions. CI is green; the current automated coverage does not execute these lifecycle paths.

Blocking

  • 🔧 Use a real navigation generation instead of URL equality.
  • 🔧 Preserve first-request GPT targeting instead of globally delaying all initialization.
  • 🔧 Add executable lifecycle coverage for the deferred bootstrap.

CI Status

  • fmt: PASS
  • clippy/checks: PASS
  • rust tests: PASS
  • JavaScript tests: PASS
  • browser/integration tests: PASS

"<script>(window.tsjs=window.tsjs||{{}}).bids=JSON.parse(\"{}\");(function(){{var f=window.tsjs.adInit;if(typeof f===\"function\")f();}})();</script>",
"<script>(window.tsjs=window.tsjs||{{}}).bids=JSON.parse(\"{}\");\
(function(){{\
var p=location.pathname+location.search;\

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 wrench — URL equality is not a safe navigation identity

This guard uses pathname + search, while the SPA auction hook intentionally identifies routes using only pathname (crates/trusted-server-js/lib/src/integrations/gpt/index.ts:715-724). That produces two failures:

  • A query-only pushState/replaceState before load makes this callback abort, but the SPA hook also declines to auction because the pathname is unchanged. Initial adInit() is never called.
  • An /a → /b → /a transition before this callback defeats the stale-route check because the URL equals /a again. After the SPA hook applies the new /a state and calls adInit(), this callback can call it again, destroying/redefining slots and issuing duplicate requests.

Please use a shared monotonic navigation generation maintained synchronously by the SPA hook, capture it here, and run only if it remains unchanged. Query-only changes should remain ignored unless the SPA auction path is deliberately changed to treat them as auction-relevant.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented in 6f9dbff. The SPA auction hook now maintains tsjs.navGeneration — a monotonic counter incremented synchronously the moment a pathname navigation is accepted (before the page-bids fetch) — and the deferred initial-adInit callback captures it at schedule time and stands down if it has moved.

Query-only history changes remain ignored by both the hook and the guard, so the initial adInit() still runs after a query-only replaceState; an /a → /b → /a round trip advances the counter twice, so the stale callback no-ops. Both scenarios have executable coverage in schedule_initial_ad_init.test.ts, plus a counter-accounting test in spa_hook.test.ts.

var f=function(){{\
if(location.pathname+location.search!==p)return;\
var a=window.tsjs.adInit;if(typeof a===\"function\")a();}};\
var d=function(){{window.requestAnimationFrame(function(){{window.requestAnimationFrame(f);}});}};\

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 wrench — The global load gate can move targeting behind the first GPT request

This generic bootstrap now waits for every load-blocking resource and two animation frames. A publisher that defines/displays GPT before or during window.load can send its first request before TS targeting is applied. The later adInit() finds the publisher-owned slot (crates/trusted-server-js/lib/src/integrations/gpt/index.ts:512-518) and refreshes it (index.ts:621-633), making the TS-targeted request a second impression.

That can lose the server-auction bid on the first impression, create a duplicate request, delay TS-owned slots behind unrelated resources, and age the auction result. Please preserve pre-request targeting—e.g. defer only hydration-unsafe TS-owned slot definition/display work—or make delayed initialization an explicit App Router/publisher opt-in while retaining synchronous behavior elsewhere.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two separable points here.

Refreshing a publisher-owned slot to deliver TS targeting is pre-existing behavior, not introduced by this PR — before this change adInit() ran synchronously at body-parse time and refreshed publisher-owned slots the same way whenever the publisher's initial load had already fired. This PR changes only how long that window stays open. On the live App Router publisher this was measured against, the TS slot's first GPT request (adindex 0) carries ts_initial=1 + hb_*, and the wire investigation for #958 confirmed the publisher's own slots are distinct from TS's slots (the early pre-SSAT fetch belonged to the publisher).

The gate timing is addressed in the agreed stacked follow-up (958-adinit-hydration-chunk-gate, spec at docs/superpowers/specs/2026-07-24-adinit-hydration-gate-design.md): wait for the async /_next/static/chunks/ hydration scripts (a few seconds) instead of window.load (~52s on heavy pages), with load kept as the can't-hang fallback — effectively the App Router-specific fast path this comment asks for, with no new config surface. That spec's PR strategy is to land #945 with the conservative gate and make the timing change in its own PR with its own #418 A/B evidence, so the gate here is unchanged. A sync-targeting/deferred-DOM split of adInit() is the spec's explicit non-goal (the "hydration-safe adInit rewrite" — a larger change, separate effort).

What did land here for this finding's test ask: the deferral now lives in the GPT bundle (tsjs.scheduleInitialAdInit, commit 6f9dbff), and an executable test pins that a publisher-displayed slot receives setTargeting before the refresh that delivers it.

}

#[test]
fn bids_script_defers_ad_init_until_after_hydration() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 wrench — This test does not execute the lifecycle being changed

These assertions only check for substrings such as requestAnimationFrame, "load", and location.pathname. They would still pass if adInit() ran synchronously, if only one frame were used, or with both navigation failures above. The core guarantees—post-load ordering, exactly-once invocation, stale-navigation cancellation, and interaction with publisher GPT—remain unprotected.

Please add executable browser/JavaScript coverage for loading and already-complete documents, two-frame ordering, query-only history changes, /a → /b → /a, and a publisher GPT display() during load. Assert both the adInit() count and first-request targeting.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 6f9dbff. The bootstrap moved out of the Rust-emitted inline string into the GPT bundle (scheduleInitialAdInit in gpt/index.ts); the </body> bids script now only assigns tsjs.bids and delegates to it.

The lifecycle now runs for real under Vitest/jsdom in test/integrations/gpt/schedule_initial_ad_init.test.ts — the real scheduler plus, where relevant, the real adInit() and SPA auction hook: load + two-frame ordering, already-complete documents, exactly-once invocation across duplicate load events, query-only history changes, /a → /b → /a cancellation, and a publisher-displayed slot receiving setTargeting before the refresh that delivers it. The Rust tests now pin only the inline script's delegation and XSS safety.

Real first-request GPT targeting stays verified by the live A/B in the PR description — an actual GPT ad request is not reachable from jsdom.

aram356 added 3 commits July 27, 2026 09:06
Address PR #945 review findings:

- Replace the URL-equality stale-route guard with a monotonic navigation
  generation (tsjs.navGeneration) maintained synchronously by the SPA
  auction hook. URL equality diverged from the hook's pathname-only route
  identity: a query-only replaceState before load cancelled the initial
  adInit entirely, and an /a -> /b -> /a round trip defeated the guard and
  double-ran adInit against the round-tripped route's live slots.

- Move the deferral bootstrap (window load, double requestAnimationFrame,
  stale-navigation cancellation) from the Rust-emitted inline script into
  the GPT bundle module as tsjs.scheduleInitialAdInit, where the lifecycle
  is executable under Vitest and the navigation generation is shared with
  the SPA hook. The </body> bids script now only assigns tsjs.bids and
  delegates to the scheduler; the GPT module ships in the synchronous head
  bundle, so it has always run by the time the inline script executes.

- Add executable lifecycle coverage (schedule_initial_ad_init.test.ts):
  load plus two-frame ordering, already-complete documents, exactly-once
  invocation across duplicate load events, query-only history changes,
  /a -> /b -> /a cancellation, and a publisher-displayed slot receiving
  setTargeting before the refresh that delivers it. The Rust tests now pin
  only the inline script's delegation and XSS safety.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants