From 56c6fbfb31016e22a6edbeedf428e93afd7d56e6 Mon Sep 17 00:00:00 2001 From: Vivek Date: Wed, 5 Aug 2026 16:03:25 +0530 Subject: [PATCH] fix: hint the core runtime in the head and measure pre-boot clicks Links are clickable from first paint, but the boot is a module script, which the HTML spec defers until parsing finishes. A click inside that window is a plain browser navigation. The window cannot be closed from inside the router (an inline capture shim has no prior art, forks the eligibility rules in onClick, and adds an inline script to every page), so it is shrunk and then measured. Shrinking it: every module the boot imports pulls @webjsdev/core, but the boot script names only page and component URLs, so the browser discovered core one full round trip late. The head now hints it, with the href taken straight from the importmap target rather than through fp(), since the map's targets are already base-path-prefixed and content-hashed and a differing href would make the browser fetch core twice. Gated on the page actually shipping a boot module, so a fully elided page is not handed a preload for a runtime it never loads. vendorPreloadTargets still excludes core: the hint comes from the head builder, not the vendor path. Measuring what is left: a document that arrived by a same-origin navigation the router did not soft-navigate now reports pre-boot-navigation through the existing webjs:navigation-fallback channel, with willReload false because the load already happened. A marker in sessionStorage keeps a full load the router itself chose from being counted twice. A reload, a back/forward restore and an external entry are excluded. The check is deliberately imprecise (a data-no-router link and a cross-document form post land here too) and the docs say so: the signal is the rate, not any single event. --- .../references/client-router-and-streaming.md | 4 +- packages/core/src/router-client.js | 121 +++++++++++++++++ .../core/test/routing/router-client.test.js | 108 +++++++++++++++ packages/server/src/ssr.js | 32 ++++- .../test/importmap/vendor-preload.test.js | 69 ++++++++++ test/bun/core-modulepreload.mjs | 123 ++++++++++++++++++ test/bun/core-modulepreload.test.mjs | 11 ++ test/e2e/e2e.test.mjs | 89 +++++++++++++ website/app/docs/client-router/page.ts | 4 +- 9 files changed, 558 insertions(+), 3 deletions(-) create mode 100644 test/bun/core-modulepreload.mjs create mode 100644 test/bun/core-modulepreload.test.mjs diff --git a/.agents/skills/webjs/references/client-router-and-streaming.md b/.agents/skills/webjs/references/client-router-and-streaming.md index 87c9dda66..c9d111ef1 100644 --- a/.agents/skills/webjs/references/client-router-and-streaming.md +++ b/.agents/skills/webjs/references/client-router-and-streaming.md @@ -65,7 +65,7 @@ document.addEventListener('webjs:navigation-error', (e) => { }); ``` -**Observing a degradation.** Some conditions make a soft nav impossible, and the router then degrades to a full page load rather than risk a corrupt DOM (the #1015 integrity model). Every such path dispatches `webjs:navigation-fallback` on `document`, in ALL environments including production, with `detail { cause, href, willReload }`. Causes: `no-shared-boundary`, `live-boundaries-malformed`, `incoming-boundaries-malformed`, `readyState-loading`, `deploy-mismatch`, `deploy-mismatch-reload-suppressed`, `navigation-error-unrecoverable`, `revalidation-discarded`. `willReload` is false for a degradation that does NOT reload (a dropped background revalidation), so a listener can tell "this click became a document load" from "a background op was skipped". Not cancelable: by the time it fires the degradation is the only safe option. In dev a deduped console warning also prints. +**Observing a degradation.** Some conditions make a soft nav impossible, and the router then degrades to a full page load rather than risk a corrupt DOM (the #1015 integrity model). Every such path dispatches `webjs:navigation-fallback` on `document`, in ALL environments including production, with `detail { cause, href, willReload }`. Causes: `no-shared-boundary`, `live-boundaries-malformed`, `incoming-boundaries-malformed`, `readyState-loading`, `deploy-mismatch`, `deploy-mismatch-reload-suppressed`, `navigation-error-unrecoverable`, `revalidation-discarded`, `pre-boot-navigation`. `willReload` is false for a degradation that does NOT reload (a dropped background revalidation), so a listener can tell "this click became a document load" from "a background op was skipped". Not cancelable: by the time it fires the degradation is the only safe option. In dev a deduped console warning also prints. ```ts document.addEventListener('webjs:navigation-fallback', (e) => { @@ -74,6 +74,8 @@ document.addEventListener('webjs:navigation-fallback', (e) => { }); ``` +**`pre-boot-navigation` reports ABOUT a load, not during one (#1118).** The boot is a module script, which the HTML spec defers until parsing finishes, while the links it will intercept are clickable from first paint. A click in that window is a plain browser navigation, and the ARRIVING document reports it with `willReload: false` (the load already happened). The window is a few tens of milliseconds warm and network-sized on a cold, throttled first visit, which is why `@webjsdev/core` is hinted in the head with `` (emitted only when the page actually ships a boot module) instead of being discovered a round trip later. Read the cause as a RATE: the check knows only that this document arrived by a same-origin navigation that was not a soft nav, so a `data-no-router` link, a `target="_blank"` open, a cross-document form post, and a `clientRouter: false` app all land here too. Excluded: a reload, a back/forward restore, an external or typed entry, and a full load the router itself chose (already reported under its own cause). The report rides the router's own boot, so a fully elided page that ships no client runtime reports nothing. + **Form state.** A form submitting through the router gets `aria-busy="true"` for the in-flight duration, plus bubbling `webjs:submit-start` and `webjs:submit-end` (detail `{ form, url, ok }`) events. Style `form[aria-busy="true"]` in pure CSS or listen for the events. **Inline scripts in a swapped range re-execute, so write them to be re-runnable (#1102).** A script the swap brings in runs again on every navigation that swaps its range, whether it sits inside the swapped content or is a top-level node of the range itself (a layout emitting its enhancement script as a sibling of `${children}`). A script parsed out of the response carries the HTML spec's already-started flag and is inert, so the router replaces it with a fresh clone, and the clone is what runs; the clone carries the page-load CSP nonce rather than the one the response was rendered with. Giving the script an `id` does NOT make it run once: the keyed differ reuses the live element and the router still re-emits it. So a script that installs a listener or a `MutationObserver` must be idempotent or guard on a flag it sets the first time. The alternative default, running once and then never again, is the failure this replaced (a progressive-enhancement highlighter that stopped working after the first soft nav). When work genuinely must happen once, put it in the ROOT layout, whose markup is never swapped. `data-webjs-permanent` splits into two cases (#1252). A script that IS the marked element is re-emitted like any other, so the attribute is not an escape hatch for a script itself: its regraft only fires when the node exists on both sides, so exempting it would leave a script that runs on a cold load and never on a soft nav. A script INSIDE a marked element the swap actually preserved is left alone, because the attribute is SUBTREE-scoped and that node survived by identity. The exemption is conditional on real preservation, so a permanent element arriving for the first time, or one with no `id` (which can never be regrafted), still runs its scripts. diff --git a/packages/core/src/router-client.js b/packages/core/src/router-client.js index 2dc465ed0..a03c1cff4 100644 --- a/packages/core/src/router-client.js +++ b/packages/core/src/router-client.js @@ -345,6 +345,11 @@ export function enableClientRouter() { // Seed the "current page" tracker so the first navigation can // snapshot the page the user is leaving. if (typeof location !== 'undefined') currentPageUrl = location.href; + // Last, once the listeners are on: report whether the load that got us here + // was a same-origin navigation the router never saw (#1118). Running it after + // the listeners means a throw inside a diagnostic can never leave the router + // half-installed. + reportPreBootNavigation(); } /** Disable the client router. */ @@ -791,6 +796,105 @@ function shouldFullLoadDuringParse(isPopState, frameId) { ); } +/** + * `sessionStorage` key holding the destination of a full load the ROUTER + * itself chose (#1118). Written by `reportFallback` when `willReload` is true, + * consumed once by the next document's boot. Per-tab and cleared with the tab, + * which is the right lifetime for a marker about one navigation. + */ +const FALLBACK_MARKER_KEY = 'webjs:nav-fallback'; + +/** + * Has the pre-boot check already run for THIS document (#1118)? Module scope, + * so it resets with the document, which is the lifetime the report is about. + */ +let reportedPreBoot = false; + +/** + * Was THIS document load a same-origin navigation the client router never saw? + * + * Pure so the branch logic is testable without driving a real navigation + * (#1118). Every argument is read from the environment by the one caller. + * + * @param {string} navType `performance.getEntriesByType('navigation')[0].type`. + * Only `'navigate'` qualifies: a `'reload'` and a `'back_forward'` restore are + * things the browser does, not clicks the router could have intercepted. + * @param {string} referrer `document.referrer`. Must parse to the same origin as + * `href`: an empty referrer means a typed URL or an external entry (no router + * was running to miss the click), and a cross-origin one means the previous + * page was not ours. + * @param {string} href `location.href` of the document that just loaded. + * @param {string | null} marker the consumed `FALLBACK_MARKER_KEY` value. When + * it equals `href` the router already reported this load under its own cause, + * so counting it again would double-count a known degradation as an unknown. + * @returns {boolean} + */ +function isPreBootNavigation(navType, referrer, href, marker) { + if (navType !== 'navigate') return false; + if (!referrer) return false; + if (marker && marker === href) return false; + try { + return new URL(referrer).origin === new URL(href).origin; + } catch { + return false; + } +} + +/** + * Report a document load that reached us by a same-origin navigation the router + * did not soft-navigate (#1118). + * + * A module script is deferred by spec, so it runs only after HTML parsing + * completes, while the links it will intercept are clickable from first paint. + * That window cannot be closed from inside the router (see #1118 for why an + * inline capture shim was rejected), so it is MEASURED instead: this turns the + * frequency into a production number a deployed app can read off the existing + * `webjs:navigation-fallback` channel, rather than folklore. + * + * Deliberately imprecise, and the docs say so: a `data-no-router` link, a + * cross-document form post, and an app that opted out of the client router all + * land here too. The signal is the RATE, not any single event. + * + * `willReload` is false because the document load has already happened. That is + * exactly the distinction the flag was added for. + */ +function reportPreBootNavigation() { + // Same guard the scroll/current-page seeding above uses: a DOM shim without a + // `location` (linkedom under the unit runner) is not a document load to + // report on, and reading through would throw inside the boot. + if (typeof location === 'undefined') return; + // Once per DOCUMENT, not once per enable. `enableClientRouter` is re-callable + // after `disableClientRouter()` (the documented per-moment opt-out), and this + // reports on the load that produced the document, which does not happen again + // when the router is toggled back on. Without this, an app that toggles would + // emit a duplicate for a single load and inflate the very rate the report + // exists to measure. The marker is already consumed by then, so it cannot + // suppress the duplicate on its own. + if (reportedPreBoot) return; + reportedPreBoot = true; + /** @type {string | null} */ + let marker = null; + try { + marker = sessionStorage.getItem(FALLBACK_MARKER_KEY); + // Consume unconditionally, even when it does not match: a stale marker left + // by an earlier navigation must never suppress a later real one. + sessionStorage.removeItem(FALLBACK_MARKER_KEY); + } catch { + // No marker available. Treated as absent, which can only over-report. + } + let navType = ''; + try { + const nav = performance.getEntriesByType('navigation')[0]; + navType = nav ? /** @type {PerformanceNavigationTiming} */ (nav).type : ''; + } catch { + // No Navigation Timing Level 2 entry. Without a nav type the check cannot + // exclude a reload, so it reports nothing rather than guessing. + } + if (isPreBootNavigation(navType, document.referrer, location.href, marker)) { + reportFallback('pre-boot-navigation', location.href, false); + } +} + /** * The client router degraded a soft navigation. Records WHY (the `cause`), so * "why did my SPA nav do a full reload?" is answerable instead of guessed at. @@ -824,6 +928,21 @@ function shouldFullLoadDuringParse(isPopState, frameId) { * dropped", which are very different user-visible events. */ function reportFallback(cause, href, willReload = true) { + if (willReload) { + // Leave a marker naming the destination this full load is going to + // (#1118). The next document's boot reads it to tell "the router itself + // chose this full load, and already reported it under its own cause" from + // "a same-origin navigation the router never saw", which is the pre-boot + // click window. Best-effort: `sessionStorage` throws in some privacy modes + // and partitioned contexts, and a diagnostic must never break a navigation. + try { + sessionStorage.setItem(FALLBACK_MARKER_KEY, href); + } catch { + // Without the marker the next boot may attribute this load to the + // pre-boot window. That is a false positive in a diagnostic, which is + // strictly better than a thrown navigation. + } + } if (typeof document !== 'undefined' && typeof CustomEvent !== 'undefined') { try { document.dispatchEvent(new CustomEvent('webjs:navigation-fallback', { @@ -4542,6 +4661,8 @@ export { addNewHeadElements as _addNewHeadElements, mergeHead as _mergeHead, reactivateScripts as _reactivateScripts, + isPreBootNavigation as _isPreBootNavigation, + FALLBACK_MARKER_KEY as _FALLBACK_MARKER_KEY, activateSwappedRange as _activateSwappedRange, findAnchorInPath as _findAnchorInPath, activeFrameId as _activeFrameId, diff --git a/packages/core/test/routing/router-client.test.js b/packages/core/test/routing/router-client.test.js index d76da9669..2ebcf3047 100644 --- a/packages/core/test/routing/router-client.test.js +++ b/packages/core/test/routing/router-client.test.js @@ -37,6 +37,7 @@ let _collect, _plan, _keyOf, _diffEl, _reconcile, _prefetchSaysSaveData, _prefetchPeek, _prefetchInflightSize, _resetPrefetch, _viewTransitionsEnabled, _runWithTransition, _regraftPermanentElements, _regraftPermanentInSlice, _applyStreamedResolve, + _isPreBootNavigation, _FALLBACK_MARKER_KEY, enableClientRouter, disableClientRouter, revalidate, WebComponent, html; @@ -84,6 +85,8 @@ before(async () => { _mergeHead: _merge, _isNonHtmlPath, _reactivateScripts, + _isPreBootNavigation, + _FALLBACK_MARKER_KEY, _activateSwappedRange, _findAnchorInPath, _activeFrameId, @@ -4417,3 +4420,108 @@ test('a prefetch that reveals a NEW app-source id evicts stale caches, no build _prefetchCache.clear(); } }); + +/* ========================================================================== + * Pre-boot navigation reporting (#1118) + * + * A module script is deferred by spec, so links are clickable before the router + * listens. The window cannot be closed from inside the router, so it is + * measured: a same-origin document load the router never soft-navigated is + * reported through the existing `webjs:navigation-fallback` channel. These pin + * the branch logic; the headline behaviour is the e2e assertion. + * ========================================================================== */ + +test('#1118: a same-origin navigate with no router marker is a pre-boot navigation', () => { + assert.equal( + _isPreBootNavigation('navigate', 'https://app.test/from', 'https://app.test/to', null), + true, + ); +}); + +test('#1118: a reload and a back/forward restore are NOT pre-boot navigations', () => { + // Neither is a click the router could have intercepted, so counting them + // would make the production number meaningless. + for (const navType of ['reload', 'back_forward', 'prerender', '']) { + assert.equal( + _isPreBootNavigation(navType, 'https://app.test/from', 'https://app.test/to', null), + false, + `${navType || '(empty)'} must not report`, + ); + } +}); + +test('#1118: a cross-origin or absent referrer is NOT a pre-boot navigation', () => { + // An external entry or a typed URL had no router running to miss the click. + assert.equal( + _isPreBootNavigation('navigate', 'https://other.test/x', 'https://app.test/to', null), + false, + 'cross-origin referrer', + ); + assert.equal(_isPreBootNavigation('navigate', '', 'https://app.test/to', null), false, 'empty referrer'); + assert.equal( + _isPreBootNavigation('navigate', 'not a url', 'https://app.test/to', null), + false, + 'an unparseable referrer reports nothing rather than throwing', + ); +}); + +test('#1118: a marker matching this href means the ROUTER chose the load, so it does not double-count', () => { + // `reportFallback` already dispatched its own cause for this load. + assert.equal( + _isPreBootNavigation('navigate', 'https://app.test/from', 'https://app.test/to', 'https://app.test/to'), + false, + 'the router-chosen full load is not re-reported as pre-boot', + ); + // A STALE marker naming some other destination must not suppress a real one. + assert.equal( + _isPreBootNavigation('navigate', 'https://app.test/from', 'https://app.test/to', 'https://app.test/elsewhere'), + true, + 'a marker for a different href does not suppress the report', + ); +}); + +test('#1118: the marker key is a stable literal', () => { + // The write and the read are in different documents, so the key cannot be + // derived or renamed on one side only. + assert.equal(_FALLBACK_MARKER_KEY, 'webjs:nav-fallback'); +}); + +test('#1118: the report is once per DOCUMENT, not once per enable', () => { + // `enableClientRouter` is re-callable after `disableClientRouter()`, the + // documented per-moment opt-out. The report describes the load that produced + // this document, which does not happen again when the router is toggled back + // on, so a toggling app must not inflate the rate the report exists to + // measure. The consumed marker cannot prevent this on its own: it is gone + // after the first read, so the second enable would see a clean slate. + const savedLocation = globalThis.location; + const savedGet = globalThis.performance.getEntriesByType; + const savedReferrer = Object.getOwnPropertyDescriptor(globalThis.document, 'referrer'); + /** @type {any[]} */ + const seen = []; + const onFallback = (e) => { if (e.detail.cause === 'pre-boot-navigation') seen.push(e.detail); }; + document.addEventListener('webjs:navigation-fallback', onFallback); + try { + globalThis.location = /** @type any */ ({ href: 'http://x/to', origin: 'http://x' }); + Object.defineProperty(globalThis.document, 'referrer', { + configurable: true, get: () => 'http://x/from', + }); + globalThis.performance.getEntriesByType = (t) => (t === 'navigation' ? [{ type: 'navigate' }] : []); + globalThis.sessionStorage.clear(); + + disableClientRouter(); + enableClientRouter(); + assert.equal(seen.length, 1, 'the first enable of this document reports once'); + + disableClientRouter(); + enableClientRouter(); + assert.equal(seen.length, 1, 'a re-enable does not re-report the same document load'); + } finally { + disableClientRouter(); + document.removeEventListener('webjs:navigation-fallback', onFallback); + globalThis.performance.getEntriesByType = savedGet; + if (savedReferrer) Object.defineProperty(globalThis.document, 'referrer', savedReferrer); + else delete (/** @type any */ (globalThis.document)).referrer; + globalThis.location = savedLocation; + globalThis.sessionStorage.clear(); + } +}); diff --git a/packages/server/src/ssr.js b/packages/server/src/ssr.js index 69886e794..e12e1a5c8 100644 --- a/packages/server/src/ssr.js +++ b/packages/server/src/ssr.js @@ -1,7 +1,7 @@ import { pathToFileURL, fileURLToPath } from 'node:url'; import { resolve } from 'node:path'; import { renderToString, isNotFound, isRedirect, isForbidden, isUnauthorized, lookupModuleUrl, isLazy, cspNonce } from '@webjsdev/core'; -import { importMapTag, vendorIntegrityFor, publishedBuildId, appSourceId, basePath, vendorPreconnectOrigins, vendorPreloadTargets } from './importmap.js'; +import { importMapTag, vendorIntegrityFor, publishedBuildId, appSourceId, basePath, vendorPreconnectOrigins, vendorPreloadTargets, buildImportMap } from './importmap.js'; import { withBasePath } from './base-path.js'; import { withAssetHash } from './asset-hash.js'; import { jsonForScriptTag } from './script-tag-json.js'; @@ -1660,6 +1660,36 @@ function wrapHead(opts) { // importmap-rails) applies nonce on every modulepreload tag for // the same reason. const noncePreload = opts.nonce ? ` nonce="${escapeAttr(opts.nonce)}"` : ''; + // Core runtime modulepreload (#1118). Every module the boot imports pulls + // `@webjsdev/core`, but the boot script names only page and component URLs, + // so without this hint the browser discovers core only after one of those is + // fetched AND parsed: one full round trip into the load, which is exactly + // where the pre-boot click window lives on a cold, throttled connection. + // + // The href comes STRAIGHT from the importmap target (no `fp()` rewrite: the + // map's targets are already base-path-prefixed and content-hashed), so it is + // byte-identical to what the import resolves to. A differing href makes the + // browser treat the preload and the import as two resources and fetch core + // twice. That is why this copies the vendor loop below, not the module loop + // above. `vendorPreloadTargets` still excludes core deliberately; this hint + // is emitted from the head builder, not the vendor path. + // + // Gated on the boot actually shipping something. A fully elided page ships no + // boot module and must not be handed a preload for a runtime it never loads + // (#780), which also keeps it off `global-error.{js,ts}`, whose document is + // returned verbatim with no importmap and no boot script. + if (opts.moduleUrls.length || lazyEntries) { + const coreMap = buildImportMap(); + const coreHref = coreMap.imports['@webjsdev/core']; + if (coreHref) { + const raw = coreMap.integrity ? coreMap.integrity[coreHref] : undefined; + const coreIntegrity = raw ? ` integrity="${escapeAttr(raw)}"` : ''; + linkTags.push( + ``, + ); + } + } // Sub-path deployment (issue #256): the modulepreload href is prefixed with // the base path (a no-op when empty), but `crossorigin` / `integrity` are // decided on the ORIGINAL url, so the integrity lookup still keys on the diff --git a/packages/server/test/importmap/vendor-preload.test.js b/packages/server/test/importmap/vendor-preload.test.js index 493301692..de8fe3ea0 100644 --- a/packages/server/test/importmap/vendor-preload.test.js +++ b/packages/server/test/importmap/vendor-preload.test.js @@ -12,6 +12,10 @@ * cross-origin vendor emits `` for the reached vendor URL; an elided/unused vendor is NOT * preloaded; the modulepreload href is byte-identical to the importmap target. + * + * Also covers the CORE runtime hint (#1118), which lives in the same head + * builder but deliberately NOT in `vendorPreloadTargets` (core is same-origin, + * not a CDN-waterfall vendor). Same byte-identity rule, same over-fetch rule. */ import { test, before, after } from 'node:test'; import assert from 'node:assert/strict'; @@ -352,3 +356,68 @@ test('import-only page: skipping the dropped page module does NOT drop its shipp 'the shipping component\'s vendor IS preloaded (skipping the dropped page did not drop it)'); await setCoreInstall(CORE_DIR, true); }); + +/* ---------------- core runtime modulepreload (#1118) ---------------- */ + +test('a page shipping a boot module preloads @webjsdev/core, byte-identical to the importmap target', async () => { + // The boot script names only page and component URLs, so without this hint the + // browser discovers core one round trip later (fetch a component -> parse -> + // fetch core). That round trip is the pre-boot click window on a cold link. + const appDir = makeApp(); + writeVendorWidget(appDir); + writeFileSync( + join(appDir, 'app', 'page.js'), + `import { html } from ${JSON.stringify(HTML_URL)};\n` + + `import './widget.js';\n` + + `export default () => html\`\`;\n`, + ); + const app = await createRequestHandler({ appDir, dev: false }); + await app.warmup(); + const html = await (await app.handle(new Request('http://x/'))).text(); + + const target = importmapTarget(html, '@webjsdev/core'); + assert.ok(target, '@webjsdev/core is in the served importmap'); + const core = modulepreloadLinks(html).filter((l) => l.includes(`href="${target}"`)); + assert.equal(core.length, 1, 'exactly one modulepreload for the core runtime'); + // Byte-identity is the whole correctness condition: a differing href makes the + // browser treat the preload and the import as two resources and fetch core + // twice, silently (the page still works, it just pays double). + assert.ok(core[0].includes(`href="${target}"`), 'preload href === importmap target (no double fetch)'); + + // The hint is emitted FIRST, ahead of the page/component preloads, since every + // one of them imports it. + const links = modulepreloadLinks(html); + assert.ok(links[0].includes(`href="${target}"`), 'the core hint is the first modulepreload'); + assert.ok(links.some((l) => l.includes('/app/widget.js')), 'the component preloads are still emitted'); +}); + +test('a fully elided page (no boot module) does NOT preload core (#780 no over-fetch)', async () => { + // A display-only page ships nothing, so a preload for the runtime it never + // loads is pure over-fetch. Same rule that keeps a core hint off + // `global-error.{js,ts}`, whose document is returned verbatim with no boot. + const appDir = makeApp(); + writeFileSync( + join(appDir, 'app', 'page.js'), + `import { html } from ${JSON.stringify(HTML_URL)};\n` + + `export default () => html\`
static
\`;\n`, + ); + const app = await createRequestHandler({ appDir, dev: false }); + await app.warmup(); + const html = await (await app.handle(new Request('http://x/'))).text(); + + // Precondition: nothing ships, so the assertion below cannot pass vacuously + // for the wrong reason (a rendered page that simply had no modules at all). + assert.ok(!/