diff --git a/.agents/skills/webjs/references/testing.md b/.agents/skills/webjs/references/testing.md index a40bfff07..ec1b17507 100644 --- a/.agents/skills/webjs/references/testing.md +++ b/.agents/skills/webjs/references/testing.md @@ -36,7 +36,7 @@ Cancel a form on its `submit` event, not on the submit control's `click`. The fo Resolve the anchor from `e.composedPath()`, not `e.target.closest('a[href]')`. The listener is on `window`, so a click inside a shadow root (a `static shadow = true` component) arrives retargeted to the host, and `closest()` walks only light-tree ancestors and never finds the link, which fails open exactly where the router itself handles the case. A pure-fragment `href="#x"` link needs no guard at all, since it never navigates the page away. -One thing this cannot cover: the router assigns `location.href` when it degrades a soft navigation, and `preventDefault` does not cancel a script assignment. Listen for `webjs:navigation-fallback` on `document` and assert none fired; its `cause` is the diagnosis. +There is a SECOND channel a click guard cannot reach: the router assigns `location.href` when it degrades a soft navigation, and `preventDefault` does not cancel a script assignment. Do not try to intercept `location.href` itself, which is non-configurable on Chromium, Firefox, and WebKit alike, so its setter cannot be redefined on any of them. Listen for `webjs:navigation-fallback` on `document` and assert none fired; its `cause` is the diagnosis. It is a few lines, so write it in your suite's setup and detach it in teardown: @@ -51,7 +51,7 @@ window.addEventListener('click', onClick); // bubble, never capture window.addEventListener('submit', onSubmit); ``` -(The WebJs framework repo keeps its own copy of exactly this in one shared module, `test/browser-nav-guard.js`, whose `installNavGuard()` returns `{ fallbacks, remove }`. That module is framework-repo infrastructure and is not part of a scaffolded app.) +(The WebJs framework repo keeps its own copy of exactly this in one shared module, `test/browser-nav-guard.js`, whose `installNavGuard()` returns `{ fallbacks, hardNavigations, remove }`. That module is framework-repo infrastructure and is not part of a scaffolded app.) ## App runners (`webjs test`) diff --git a/packages/core/AGENTS.md b/packages/core/AGENTS.md index f4f3d2ed5..2e95589ec 100644 --- a/packages/core/AGENTS.md +++ b/packages/core/AGENTS.md @@ -238,7 +238,7 @@ Chromium, Firefox, and WebKit via web-test-runner). A browser test that clicks a real `` or submits a real `
` MUST install the shared guard from `test/browser-nav-guard.js` -(`installNavGuard()`, returning `{ fallbacks, remove }`). web-test-runner +(`installNavGuard()`, returning `{ fallbacks, hardNavigations, remove }`). web-test-runner aborts the whole session when the page navigates, so an escaped click takes down every browser file rather than failing one test. The guard listens on `window` in the BUBBLE phase, which is load-bearing: capture @@ -247,6 +247,17 @@ runs, and `onClick` returns on that flag, so every guarded router test would pass while testing nothing. Full rule in [`references/testing.md`](../../.agents/skills/webjs/references/testing.md). +The guard also covers the SECOND channel, the router's own hard navigation on +a degradation. `preventDefault` cannot cancel a script assignment, and +`location.href` is non-configurable on all three engines so its setter cannot +be redefined, so `router-client.js` routes every hard navigation through one +`hardNavigate` indirection with a `setHardNavigate(fn)` seam. The guard +installs an override that records the attempt into `hardNavigations` instead of +performing it, so a degradation fails one test with its `cause` slug rather +than aborting the whole session. `setHardNavigate` is TEST-ONLY and is +deliberately not re-exported from `index.js` / `index-browser.js`; tests reach +it through the same direct `src/router-client.js` import they already use. + Cross-package tests that exercise core through the SSR pipeline or scaffolds live at the repo root in `test/ssr/`, `test/scaffolds/`, etc. See [`references/testing.md`](../../.agents/skills/webjs/references/testing.md). diff --git a/packages/core/src/router-client.js b/packages/core/src/router-client.js index 10bcfac0a..54933fb89 100644 --- a/packages/core/src/router-client.js +++ b/packages/core/src/router-client.js @@ -377,7 +377,9 @@ export function disableClientRouter() { export async function navigate(url, opts) { const target = new URL(url, location.href); if (target.origin !== location.origin) { - location.href = url; + // Cross-origin: an intentional full-page nav, not a degradation, but it + // ends the session in a test just the same, so it rides the same seam. + hardNavigate(url); return; } await performNavigation(target.href, opts?.replace ?? false, null); @@ -475,6 +477,28 @@ export function revalidate(url) { */ const NON_HTML_EXTENSIONS = /\.(?:pdf|zip|tar|gz|7z|rar|dmg|exe|msi|deb|rpm|apk|ipa|xlsx?|docx?|pptx?|csv|odt|ods|odp|rtf|epub|mobi|xml|json|rss|atom|txt|md|wasm|mp3|mp4|mov|avi|webm|ogg|flac|wav|m4a|m4v|mkv|png|jpe?g|gif|webp|avif|bmp|ico|svg|tiff?|heic)$/i; +/** + * The one place the router hands a navigation back to the browser. + * + * Every hard navigation the router performs goes through here rather than + * assigning `location.href` inline, so a browser test can observe it. The + * default is exactly the assignment it replaces, so behaviour is unchanged + * unless something calls `setHardNavigate`. + * + * This exists because 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 a test cannot redefine its setter either (measured; the older + * `spyOnReload` helper that tried was silently a no-op on every engine). In a + * web-test-runner session a real navigation aborts the WHOLE session, so one + * degradation destroys every remaining browser test file and reports `0 failed` + * on the way out. A seam is the only thing that makes it catchable. + * + * @param {string} href + */ +let hardNavigate = (href) => { location.href = href; }; + + /** @param {MouseEvent} e */ function onClick(e) { if (!enabled) return; @@ -1181,7 +1205,7 @@ async function performNavigation(href, isPopState, frameId) { // nav carries its own boundary element. if (shouldFullLoadDuringParse(isPopState, frameId) && typeof location !== 'undefined') { reportFallback('readyState-loading', href); - location.href = href; + hardNavigate(href); return; } @@ -2082,7 +2106,7 @@ function handleNavigationError(href, status, error) { // carries no `cause` / `willReload`, so it is not a substitute. if (typeof location !== 'undefined') { reportFallback('navigation-error-unrecoverable', href); - location.href = href; + hardNavigate(href); } } @@ -2910,14 +2934,14 @@ function applySwap(doc, frameId, revalidating, href, incomingBuild, incomingSrc) } else { if (sessionStorage) sessionStorage.setItem(flag, '1'); reportFallback('deploy-mismatch', href); - location.href = href; + hardNavigate(href); return; } } catch { // sessionStorage unavailable (private mode w/ quota etc.): // fall through to a single reload like before. reportFallback('deploy-mismatch', href); - location.href = href; + hardNavigate(href); return; } } else if (!mismatch) { @@ -3055,7 +3079,7 @@ function applySwap(doc, frameId, revalidating, href, incomingBuild, incomingSrc) reportFallback(!here ? 'live-boundaries-malformed' : !there ? 'incoming-boundaries-malformed' : 'no-shared-boundary', href); - location.href = href; + hardNavigate(href); return; } @@ -4523,6 +4547,20 @@ export function _resetPrefetch() { clearPrefetchViewTimers(); } +/** + * Test-only: replace the hard-navigate action so a browser test can observe a + * navigation instead of being destroyed by it. Call with no argument to + * restore. Underscore-prefixed and kept in this block like every other + * test-only export here, so it stays out of `router-client.d.ts` and out of + * the app-facing API (the `./client-router` subpath resolves this file under + * the `source` condition, so an unprefixed name here would read as public). + * + * @param {((href: string) => void) | null} [fn] + */ +export function _setHardNavigate(fn) { + hardNavigate = fn || ((href) => { location.href = href; }); +} + /** Test-only: read the monotonic navigation-token counter. */ export function _navToken() { return currentNavigationToken; } /** Test-only: bump the navigation-token counter (simulates a fresh nav). */ diff --git a/packages/core/test/routing/browser/form-action-submit.test.js b/packages/core/test/routing/browser/form-action-submit.test.js index 1b47628be..70e8e7646 100644 --- a/packages/core/test/routing/browser/form-action-submit.test.js +++ b/packages/core/test/routing/browser/form-action-submit.test.js @@ -36,10 +36,6 @@ suite('Client router: bound form submissions (#1155)', () => { let navGuard; let container, origFetch, calls; - // When a test redefines window.location.href (to detect a full-page reload), - // it records the restore fn here so teardown reverts it even if the body - // throws. Null when no redefine is active. - let restoreHref; let bOpen, bClose; function setup(responder) { @@ -55,7 +51,6 @@ suite('Client router: bound form submissions (#1155)', () => { document.body.appendChild(container); document.body.appendChild(bClose); calls = []; - restoreHref = null; origFetch = window.fetch; window.fetch = (url, init) => { calls.push({ url: String(url), init: init || {} }); @@ -65,38 +60,11 @@ suite('Client router: bound form submissions (#1155)', () => { function teardown() { navGuard.remove(); window.fetch = origFetch; - if (restoreHref) { try { restoreHref(); } catch { /* ignore */ } restoreHref = null; } container.remove(); if (bOpen) bOpen.remove(); if (bClose) bClose.remove(); } - /** - * Replace window.location.href's setter with a spy so a full-page reload is - * observable (the router falls back to `location.href = url` only for a - * non-HTML / error response). Returns a getter for the reload count. The - * descriptor restore is registered on `restoreHref` so teardown always - * reverts it. Some browsers forbid redefining the accessor; in that case the - * spy is a no-op and the test leans on the DOM-applied assertion instead. - */ - function spyOnReload() { - let reloads = 0; - const realDescriptor = Object.getOwnPropertyDescriptor(Location.prototype, 'href') - || Object.getOwnPropertyDescriptor(window.location, 'href'); - let installed = false; - try { - Object.defineProperty(window.location, 'href', { - configurable: true, - get: () => location.toString(), - set: () => { reloads += 1; }, - }); - installed = true; - } catch { /* redefining forbidden here; rely on the DOM assertion */ } - if (installed && realDescriptor) { - restoreHref = () => Object.defineProperty(window.location, 'href', realDescriptor); - } - return { count: () => reloads, installed: () => installed }; - } test('a bound form posts to the page own url and carries the identity field', async () => { // The rendered form has NO `action` attribute (the renderer omits it so the @@ -161,7 +129,6 @@ suite('Client router: bound form submissions (#1155)', () => { '
', { status: 422, headers: { 'content-type': 'text/html', 'x-webjs-build': '' } }, )); - const reload = spyOnReload(); try { render(html`
@@ -176,7 +143,12 @@ suite('Client router: bound form submissions (#1155)', () => { await tick(); assert.ok(calls.length, 'fetch was issued'); - assert.equal(reload.count(), 0, '422 HTML must be applied in place, never a full reload'); + // The seam (#1286) records a hard navigation instead of performing it, so + // this is a real observation. The old `spyOnReload` helper could not make + // it: `location.href` is non-configurable on all three engines, so its + // redefine always threw and the count was structurally always zero. + assert.equal(navGuard.hardNavigations.length, 0, + '422 HTML must be applied in place, never a full reload'); // The 422 body was actually applied to the live DOM (the field error is // now present), which a full reload would never achieve from a fetch stub. assert.ok(document.getElementById(marker), 'the 422 re-render body was applied in place'); diff --git a/packages/core/test/routing/browser/nav-guard.test.js b/packages/core/test/routing/browser/nav-guard.test.js index c3a72c216..fa5daf4ef 100644 --- a/packages/core/test/routing/browser/nav-guard.test.js +++ b/packages/core/test/routing/browser/nav-guard.test.js @@ -137,6 +137,45 @@ suite('Browser-test nav guard (#1135)', () => { } finally { teardown(); } }); + test('a real degradation is recorded, not performed (#1286)', async () => { + setup(); + try { + // Force the router to degrade: strip the live boundary pair so the swap + // cannot find a shared boundary. That path reports a fallback and then + // hands the navigation to the browser, which before the seam existed + // aborted the whole web-test-runner session rather than failing here. + bOpen.remove(); + bClose.remove(); + render(html`go`, container); + const settled = awaitNavigation(); + container.querySelector('a').click(); + await settled; + + assert.ok(guard.hardNavigations.some((u) => u.includes('/nav-guard-degrade')), + 'the hard navigation must be RECORDED by the seam'); + // There is deliberately NO in-test assertion that the navigation was not + // PERFORMED, because none can be honest. `location` cannot serve: the + // degradation path falls through to `history.pushState`, so the pathname + // changes either way. Nor can a surviving window sentinel: a + // `location.href` assignment starts a navigation that commits on a later + // task rather than tearing the realm down synchronously, so the sentinel + // reads the same on both sides and would be a vacuous assertion, which is + // the exact defect class this seam exists to remove. + // + // What proves non-performance is the counterfactual: disable the override + // in `installNavGuard` and this file does not fail, it aborts the whole + // web-test-runner session on every engine. + // The cause slug is the diagnosis, and it only survives because the + // navigation no longer happens. + assert.ok(guard.fallbacks.length > 0, 'the degradation reported a cause'); + assert.match(String(guard.fallbacks[0].cause), /boundar|shared/, + `expected a boundary-related cause, got ${guard.fallbacks[0].cause}`); + } finally { + // teardown() removes bOpen/bClose; they are already detached here. + teardown(); + } + }); + test('does NOT suppress the router on a plain link (capture-phase regression)', async () => { setup(); try { diff --git a/test/browser-nav-guard.js b/test/browser-nav-guard.js index 85ecba8f1..8e5a06aa9 100644 --- a/test/browser-nav-guard.js +++ b/test/browser-nav-guard.js @@ -1,3 +1,5 @@ +import { _setHardNavigate } from '../packages/core/src/router-client.js'; + /** * Shared navigation guard for browser tests (#1135). Sibling of * `test/browser-assert.js` (#777) and with the same "one source of truth for @@ -37,21 +39,40 @@ * possible failure here, and it is why `nav-guard.test.js` asserts the router * still ran rather than trusting this by inspection. * - * ## What it cannot do + * ## The second channel: the router's own hard navigation + * + * `preventDefault` cancels a default action, not a script assignment, so it can + * never stop the router assigning `location.href` when it degrades. That is a + * separate channel and it needs a separate mechanism: the router routes every + * hard navigation through one `_setHardNavigate` seam (#1286), and this installs an override that + * RECORDS the attempt into `hardNavigations` instead of performing it. So a + * degradation now fails the one test with a readable message instead of + * aborting the whole session. + * + * Intercepting `location.href` directly is not an option and should not be + * attempted: it is non-configurable on all three engines, so its setter cannot + * be redefined. That is why the seam lives in the router rather than here. * - * It cannot stop the router's own `location.href` assignment on a degradation: - * `preventDefault` cancels a default action, not a script assignment. The - * `fallbacks` array is the coverage for that second channel. Every reload site - * dispatches `webjs:navigation-fallback` with a stable `cause` immediately - * beforehand, so a test asserts `fallbacks` is empty and names the cause. - * Removing the conditions that cause a degradation is the fixture work in - * #1053. + * `fallbacks` stays useful alongside it: it carries the stable `cause` slug + * that says WHY the router degraded, which the recorded href alone does not. * - * @returns {{ fallbacks: Array<{cause: string, href: string, willReload: boolean}>, remove: () => void }} + * This catches one navigation that is NOT a degradation: a cross-origin + * `navigate()`, which is an intentional full-page nav. It is recorded rather + * than performed like any other, so it is observable (assert on + * `hardNavigations`) instead of ending the session. Nothing is swallowed + * silently; a suite asserting `hardNavigations` is empty will fail on it. + * + * Note this module imports the router, which self-enables on load. Every suite + * that installs the guard is already a router suite that imports it, so this + * changes nothing in practice. + * + * @returns {{ fallbacks: Array<{cause: string, href: string, willReload: boolean}>, hardNavigations: string[], remove: () => void }} */ export function installNavGuard() { /** @type {Array<{cause: string, href: string, willReload: boolean}>} */ const fallbacks = []; + /** @type {string[]} */ + const hardNavigations = []; const onClick = (e) => { // Walk the COMPOSED path, exactly as the router's `findAnchorInPath` does, @@ -76,13 +97,18 @@ export function installNavGuard() { const onFallback = (e) => { fallbacks.push(e.detail); }; + // Record the router's own hard navigations instead of performing them. + _setHardNavigate((href) => { hardNavigations.push(String(href)); }); + window.addEventListener('click', onClick); window.addEventListener('submit', onSubmit); document.addEventListener('webjs:navigation-fallback', onFallback); return { fallbacks, + hardNavigations, remove() { + _setHardNavigate(null); window.removeEventListener('click', onClick); window.removeEventListener('submit', onSubmit); document.removeEventListener('webjs:navigation-fallback', onFallback); diff --git a/website/app/docs/testing/page.ts b/website/app/docs/testing/page.ts index 98d36e9d2..12aa4c014 100644 --- a/website/app/docs/testing/page.ts +++ b/website/app/docs/testing/page.ts @@ -231,7 +231,7 @@ const onSubmit = (e) => e.preventDefault(); // forms cancel on submit, not window.addEventListener('click', onClick); // window BUBBLE phase, never capture window.addEventListener('submit', onSubmit);

Register on window in the bubble phase. That is the last step of the propagation path, so it runs after the router's own listeners, and preventDefault() still cancels the default action. Never use the capture phase: it sets defaultPrevented before the router sees the event, and the router bows out on that flag, so every such test would pass while testing nothing. A pure-fragment href="#x" link needs no guard, since it never navigates the page away.

-

One case this cannot cover: the router assigns location.href when it degrades a soft navigation, and preventDefault does not cancel a script assignment. Listen for webjs:navigation-fallback on document and assert none fired; its cause names the reason.

+

There is a second channel a click guard cannot reach: the router assigns location.href when it degrades a soft navigation, and preventDefault does not cancel a script assignment. Do not try to intercept location.href itself, which is non-configurable on Chromium, Firefox, and WebKit alike, so its setter cannot be redefined on any of them. Listen for webjs:navigation-fallback on document and assert none fired; its cause names the reason.

Convention Validation

webjs check validates your app for correctness issues: