feat: route the router's hard navigations through one test seam - #1290
Conversation
vivek7405
left a comment
vivek7405
left a comment
There was a problem hiding this comment.
Read the seam against every hard-navigate site and the guard that consumes it. The mechanism is right: no raw location.href assignment is left in packages/core/src, the default is byte-identical to what it replaced, and the counterfactual does what it claims (disable the override and the file does not fail, it takes the whole session down on every engine).
The bad one is mine and it is the same mistake this PR exists to remove. My "the realm survived" sentinel assertion was vacuous: with the seam disabled the preceding assertion throws first so it is never reached, and a location.href assignment does not tear the realm down synchronously anyway, so it read 'alive' either way. A vacuous assertion, in the PR about vacuous assertions. Removed rather than reworded, because there is no honest in-test assertion available here and the counterfactual is the real proof.
Related, and worse for being knowingly left: the PR body called out spyOnReload as permanently dead and then left its consumer asserting on a count that is structurally always zero. That assertion now reads the seam's hardNavigations, which the degradation test proves non-empty elsewhere in the same run.
Last, the setter was unprefixed and mid-file, which reads as app-facing next to setCspNonceProvider and setAssetUrlProvider, and ./client-router does resolve this file under the source condition, so it really was reachable. Renamed and moved in with the other underscore-prefixed test-only exports.
A hard navigation is unobservable and unpreventable from outside: preventDefault cancels a default action, not a script assignment, and location.href is non-configurable on Chromium, Firefox, and WebKit alike, so its setter cannot be redefined either. Measured, and it means the older spyOnReload helper that tried was silently a no-op on every engine. In web-test-runner a real navigation aborts the whole SESSION, so one degradation destroys every remaining browser test file and reports 0 failed on the way out. The nav guard from #1135 closes the click channel but cannot touch this one. Every hard navigation now goes through a single indirection whose default is byte-identical to the assignment it replaces, so production behaviour is unchanged unless setHardNavigate is called. The shared guard installs an override that records the attempt, so a degradation fails one test with its cause slug instead of killing the run.
…tercepted The second channel needs saying out loud, because the obvious workaround is impossible and someone will try it: location.href is non-configurable on all three engines, so its setter cannot be redefined, which is why the seam lives in the router rather than in the test guard.
The sentinel assertion added with the seam could not fail on either side of its own counterfactual: with the seam disabled the preceding assertion throws first, and a location.href assignment starts a navigation that commits on a later task rather than tearing the realm down synchronously, so the sentinel read the same either way. That is the exact defect class this PR exists to remove, so it is gone. What proves non-performance is the counterfactual itself, and the comment now says so instead of pretending an assertion does. form-action-submit kept spyOnReload and still asserted on its count. That count is structurally always zero, since href is non-configurable everywhere and the redefine always threw into a swallowing catch, so the assertion could never fail. It now reads the seam's hardNavigations, which the degradation test proves non-empty elsewhere in the same run. setHardNavigate is renamed _setHardNavigate and moved into the test-only block with the other underscore-prefixed exports. Unprefixed and mid-file it read as app-facing, and the ./client-router subpath resolves this file under the source condition, so the name was reachable as public API.
A cross-origin navigate() is an intentional full-page nav rather than a degradation, and it rides the same seam. Worth saying that it is therefore observable through hardNavigations rather than silently dropped, so nobody has to re-derive it from the router source.
0f6dc21 to
f16515b
Compare
1 participant
Closes #1286
Summary
When the client router degrades a soft navigation it assigns
location.href, which performs a real document load. In web-test-runner that aborts the entire session, not one file, so a single degradation destroys every remaining browser test file and the run reports0 failedbefore exiting non-zero. That reads as an infrastructure blip rather than a test problem. The nav guard from #1135 closes the click channel but cannot touch this one.Every hard navigation now goes through one
hardNavigateindirection inrouter-client.js, with asetHardNavigate(fn)seam. The shared guard installs an override that records the attempt intohardNavigationsinstead of performing it, so a degradation fails one test with itscauseslug instead of killing the run.A seam is the only thing that works here, and the obvious alternative is impossible.
preventDefaultcancels a default action, not a script assignment, andlocation.hrefis non-configurable on Chromium, Firefox, and WebKit alike, so its setter cannot be redefined on any of them. Measured, on all three:That also means the
spyOnReload()helper inform-action-submit.test.js, which tried exactly this, has always been a silent no-op: itsinstalledflag is never true and its reload count is structurally always zero.Production behaviour is unchanged
The default is byte-identical to the assignment it replaces, so nothing changes unless something calls
setHardNavigate.setHardNavigateis TEST-ONLY and deliberately not re-exported fromindex.js/index-browser.js, so the package's public surface is untouched; tests reach it through the same directsrc/router-client.jsimport they already use. Both.d.tsguards (dts-export-coverage,dts-no-phantom-exports) stay green.The cross-origin
navigate()callnavigate()hands a cross-origin URL straight to the browser. That is an intentional full-page nav, not a degradation, but it ends a test session just the same, so it rides the same seam. A test that cares can assert on the recorded href, which beats a dead session.Test plan
npm run test:browsergreen, 67/67 files on Chromium, Firefox, WebKit. New case innav-guard.test.jsforces a real degradation (strips the live boundary pair) and asserts the navigation is recorded and NOT performed, 6/6 on all three.0 passed, 0 failed. With the seam, 6/6 pass.windowsentinel, notlocation: the degradation path still falls through tohistory.pushState, so the pathname changes either way and would have made that assertion lie.npm testfailing-file set is identical to the primary checkout's; no regression. Type guards green./,/docs/testing,/ui,/ui/buttonin prod mode, no broken modulepreload hints.router-client.jsis browser-only client code with no runtime-sensitive server surface (no serializer, listener, SSR / action / CSRF dispatch, stream,node:crypto, or stripper path).mainitself fails 36 e2e assertions locally while CI is green on both e2e jobs. Deferring to CI rather than reporting a meaningless local delta.packages/core/AGENTS.md,.agents/skills/webjs/references/testing.md,website/app/docs/testing/page.ts.