test(e2e): gate clicks that can land before hydration commits

[NathanDrake2406]

Closes #2716.

The failure

Dev error overlay › soft-nav to a broken route still updates the URL intermittently fails expect(canary).toBe(true) with Received: undefined: the browser performed a document navigation and lost window.__vinextReloadCanary instead of the soft RSC navigation the test asserts on. Most recently on #2708 (https://github.com/cloudflare/vinext/actions/runs/30244321290/job/89907934259), failing both the initial attempt and retry #1. It is a recurrence of #1769 and the same race #2409 fixed for the app-with-src spec.

The test gated its click on an inline waitForFunction that only checked the navigation runtime exposes functions.navigate. That object is published before React commits the hydrated tree, so the predicate can pass while the anchor still has no click listener.

I swept the rest of the suite before fixing it

A one-line guard swap is worth little if the shape is everywhere, so I audited all 200 E2E specs first. It is everywhere: 80 tests across 30 files click without a hydration gate, 29 of them high-exposure (click within <=1 awaited step of goto()), 11 concentrated in tests/e2e/use-params-app-pages/. Method, measurements and the cleared false positives are in #2716 (comment).

Polling at rAF cadence, counting ticks where the gate signal is observable but the post-commit marker is not:

Gate	ticks early	click can land inside
App Router navigationRuntime.functions.navigate	3–6	yes
Pages Router __VINEXT_ROOT__ (waitForHydration)	0	no
no gate at all	—	yes, both routers

Hydration is finished neither when goto() resolves nor after one visibility assertion:

app-router:    hydratedAtGotoResolve=false  afterOneAssertion=false   (6/6 runs)
pages-router:  hydratedAtGotoResolve=false  afterOneAssertion=true    (3/4 runs)

Timing the dev-overlay page, recording when each signal first becomes observable:

navigate=2133.9ms  hydrated=2163.7ms  window=29.8ms
navigate=156.8ms   hydrated=173.4ms   window=16.6ms
navigate=150.3ms   hydrated=162.0ms   window=11.7ms

12–30ms, idle and local. Playwright's click() runs actionability checks and scrolling before dispatching, so on a loaded runner it lands inside that window — which is why the same shard also reported error-interactive.spec.ts:51 flaky.

What this fixes

The confirmed failure. dev-error-overlay.spec.ts now uses waitForAppRouterHydration, the guard its six other tests and the sibling app-with-src/dev-overlay-recovery.spec.ts already rely on. I checked the four other specs that reference Symbol.for("vinext.navigationRuntime") inline (advanced, rsc-fetch-errors, build-id-navigation, hash-popstate-scroll); all drive navigation programmatically rather than gating an anchor click, so this site was the only one.

The cluster. Every ungated click in tests/e2e/use-params-app-pages/, gated by whichever router owns the clicked element. The fixture is hybrid, so this is not a per-directory decision:

• use-params.spec.ts — 7 clicks and 1 hover on App pages (app/page.tsx, app/rerenders/[dynamic]) → waitForAppRouterHydration
• pages-form.spec.ts — 5 clicks on /form-source, which is pages/form-source/index.tsx → waitForHydration

That is 12 clicks, not the audit's 11; I enumerated this directory by hand rather than trusting the detector, which also misfiled the whole directory as Pages Router from its path string.

waitForHydration itself is untouched. It is theoretically weak — __VINEXT_ROOT__ is assigned before await hydrationCommitted — but measures a 0-tick gap, so changing it would churn 95 call sites for nothing. Applying it where there was no gate at all is still worth it.

A test that was never flaky, just vacuous. use-params.spec.ts:90 clicks #router-prefetch-pages and asserts zero RSC requests. A lost click means the handler never runs, prefetch() is never called, and the assertion passes for the wrong reason — quietly, indefinitely. A gate alone does not fix that, so the fixture's handler now also prefetches /prefetch-control and the test asserts that request was issued.

Picking that control target took some care. /account/details was the obvious choice and is wrong: it is already a <Link> on the page, and link.tsx runs a shared IntersectionObserver for viewport prefetching, so its RSC request appears after hydration whether or not anything is clicked. /prefetch-control is App-owned via the root catch-all app/[...path] and nothing links to it. Both prefetches sit in one handler invocation, so the control cannot pass while the subject call is skipped.

A second vacuity in the same assertion, unrelated to the race. Its filter was req.url().includes(".rsc") — a literal dot. This project emits query-param RSC URLs:

http://localhost:4186/prefetch-control?_rsc=4lsyqStKJzbXmimU
  rsc: 1, next-router-prefetch: 1, accept: text/x-component

"...?_rsc=abc".includes(".rsc") is false, so a real RSC prefetch to /pages-dir/foobar would have been filtered out before the assertion ever saw it. Now uses the existing isAppRouterRscRequestForPath, matching pathname + _rsc + rsc: 1 — all three verified on a captured request.

What this is not

No timeout bumps, no retry annotations, no new helper, no change to waitForHydration.

Not the rest of the audit. The other 18 high-exposure candidates span 9 files in 6 Playwright projects, several doing a full build, so validating them here would be slow and hard to review; #2716 tracks them. That list is a candidate list, not a defect list — of three I sampled, two must not be touched. server-actions.spec.ts:64 and :372 run with javaScriptEnabled: false, where a gate would hang until timeout, and catch-error.spec.ts:52 is already correctly gated with waitForHydration because it clicks a Pages Router element.

Validation

Measured:

• vp check clean.
• tests/e2e/use-params-app-pages/ scoped to its own project, --repeat-each=5: 90 passed.
• The dev-overlay test in the app-router project, --repeat-each=5: 5 passed.
• The de-vacuuming, demonstrated rather than claimed. Green runs prove nothing here — the old test was green too. I stripped the onClick off #router-prefetch-pages, rebuilt the fixture, and ran both versions of the test against a button whose handler cannot fire:

test version	against a dead button
upstream/main	1 passed (3.9s) — vacuous
this PR	1 failed — the prefetch handler never ran, so the assertion below would pass vacuously

The fixture was restored before committing.

Reasoned, not measured:

• That any of the newly gated clicks would in fact have failed. The audit shows the window is open and that these clicks land in it; I did not force a failure. Exposed, not broken — except the prefetch test, measured above.
• The 12–30ms figures come from the dev app-router project. use-params-app-pages is a prod build and was not separately probed.

Review path

Two commits: 76bdd20 is the one-line guard swap, 0432586 the cluster. Worth checking:

1. __VINEXT_HYDRATED_AT is written post-commit — the useEffect in packages/vinext/src/server/app-browser-entry.ts that also calls hydrationCachePublication.complete().
2. Router ownership per gated test: / and /rerenders/foobar are app/, /form-source is pages/.
3. Nothing in app/page.tsx links to /prefetch-control. If a future edit adds one, the positive control goes soft.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@2717

npm i https://pkg.pr.new/create-vinext-app@2717

npm i https://pkg.pr.new/@vinext/types@2717

npm i https://pkg.pr.new/vinext@2717

commit: 0432586

[github-actions]

Performance benchmarks

Compared 0432586 against base 05eee9f using alternating same-runner rounds. Next.js was unchanged and skipped.

0 improved · 0 regressed · 6 within ±1.5%

Scenario	Framework	Baseline	Current	Change
Client bundle size (gzip)	vinext	132.4 KB	132.4 KB	⚫ 0.0%
Client entry size (gzip)	vinext	119.7 KB	119.7 KB	⚫ -0.0%
Dev server cold start	vinext	2.85 s	2.84 s	⚫ -0.3%
Production build time	vinext	3.13 s	3.10 s	⚫ -0.8%
RSC entry closure size (gzip)	vinext	103.3 KB	103.3 KB	⚫ -0.0%
Server bundle size (gzip)	vinext	177.8 KB	177.8 KB	⚫ -0.0%

View detailed results and traces

_{🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head}

[james-elicx]

/bigbonk review for issues

[ask-bonk]

Review

Test-only change, well-reasoned, and every factual claim in the description holds up against the code. Approving in spirit — a couple of minor observations below, none blocking.

What I verified

• Post-commit hydration marker. packages/vinext/src/server/app-browser-entry.ts:1079-1086 writes window.__VINEXT_HYDRATED_AT inside a passive useEffect that also calls hydrationCachePublication.complete(). waitForAppRouterHydration (tests/e2e/helpers.ts:35-63) additionally requires that marker plus a two-rAF settle, so it genuinely gates past commit rather than past runtime publication. The guard swap in dev-error-overlay.spec.ts is correct and the helper was already imported.
• Router ownership per gated test. / and /rerenders/[dynamic] are app/; /form-source is pages/form-source/index.tsx; #basic-form et al. live on that Pages page. Each click is gated by the router that owns the clicked element (waitForAppRouterHydration vs waitForHydration).
• Positive control isolation. /prefetch-control resolves only through app/[...path]/page.tsx (App root catch-all); there is no pages/prefetch-control, and the only reference in the fixture tree is the router.prefetch call added in app/page.tsx. No <Link> targets it, so its RSC request can only originate from the handler. The /account/details rejection rationale is sound — it is a <Link> (#to-app-priority) subject to viewport prefetch.
• De-vacuumed .rsc filter. Switching from u.includes(".rsc") to isAppRouterRscRequestForPath (pathname + _rsc param + rsc: 1 header) is correct given this project emits ?_rsc= query URLs where the literal-dot substring never matched.

Minor observations (non-blocking)

1. Negative assertion vs. viewport prefetch of #to-pages. The requests listener is registered after waitForAppRouterHydration, and the negative target /pages-dir/foobar is also a viewport <Link> (#to-pages). If a regression ever made the hybrid short-circuit emit an RSC request, viewport prefetch of that Link (not just the button handler) could trip the assertion — which is arguably desirable (still catches the bug) but means the assertion is not strictly attributable to the button click. Worth a one-line note if you want the test's failure signal to stay unambiguous.

2. review path item 3 is a live footgun. The positive control silently degrades to vacuous if a future edit adds a <Link href="/prefetch-control"> to app/page.tsx. The inline comment at app/page.tsx:40-44 captures this, which is good; no action needed, just flagging that the invariant is only enforced by comment.

No timeout bumps, no waitForHydration churn, no production code touched. LGTM.

[ask-bonk]

Review posted. Summary of my findings:

PR #2717 is a test-only change that fixes an intermittent E2E flake (a click landing before React commits the hydrated tree, causing a document navigation instead of a soft RSC nav). It swaps a weak inline gate (functions.navigate presence, published pre-commit) for waitForAppRouterHydration, and sweeps the same pattern across the use-params-app-pages fixture.

I verified every factual claim against the code:

• __VINEXT_HYDRATED_AT is genuinely written from a post-commit useEffect (app-browser-entry.ts:1079-1086).
• Router ownership per gated test is correct (app/ → waitForAppRouterHydration, pages/ → waitForHydration).
• /prefetch-control is App-owned via the catch-all with no <Link> pointing at it — a valid positive control.
• The .rsc → isAppRouterRscRequestForPath fix correctly addresses the vacuous substring filter (project emits ?_rsc= query URLs).

Posted as a COMMENT review with two non-blocking observations: the negative assertion target is also a viewport <Link> (so failure attribution isn't strictly click-only), and the positive control's isolation invariant is only enforced by a code comment. No blocking issues.

github run