diff --git a/.agents/skills/webjs/references/components.md b/.agents/skills/webjs/references/components.md index ae7402801..2885edeb3 100644 --- a/.agents/skills/webjs/references/components.md +++ b/.agents/skills/webjs/references/components.md @@ -402,7 +402,7 @@ A component that does no client-side work renders the same SSR'd HTML with or wi - a factory-declared reactive property that is not `{ state: true }` - an overridden lifecycle hook (including `renderFallback` / `renderError`) - an imported `signal` / `computed` / `watch` / `Task` / `ref` / streaming directive, or `addController` / `requestUpdate` -- code that runs at module load (a top-level call, non-data `new`, dynamic `import(...)`, top-level `await`); only declarations and `X.register(...)` are allowed. TypeScript types are erased before the analyser reads a module, so an annotation can never be a blocker however call-shaped it looks (`readonly (readonly [number, number, number])[]` is fine) +- code that runs at module load (a top-level call, non-data `new`, dynamic `import(...)`, top-level `await`); only declarations and `X.register(...)` are allowed. A call inside a declaration does not run at load, so it is not a blocker: an arrow's expression body (`export const usd = (m) => Math.round(m) / MICROS`) and a function parameter default (`function relativeTime(when, now = Date.now())`) are both fine. TypeScript types are erased before the analyser reads a module, so an annotation can never be a blocker however call-shaped it looks (`readonly (readonly [number, number, number])[]` is fine) - the dynamic slot READ surface (`slotchange`, `assignedNodes` / `assignedElements` / `assignedSlot`); merely RENDERING a `` does not ship (the SSR output carries the placed children, so a display-only slotted wrapper is byte-identical without its JS; native-write liveness is consumer-driven and the consumer's tag reference forces the ship) - being rendered by a component that itself ships diff --git a/packages/server/AGENTS.md b/packages/server/AGENTS.md index a2ebf5115..f6721ee7e 100644 --- a/packages/server/AGENTS.md +++ b/packages/server/AGENTS.md @@ -475,7 +475,13 @@ conditional leaves a CSP-off document one newline shorter than a CSP-on one). if it does real client work). A module-scope pure-data constructor (`new Set([...])` / `Map` / `Date` / `RegExp` / typed array / `URL`) is inert data, not a side effect; any other constructor (`new WebSocket()` / `Worker()`) - still ships. The vendor bare-import scan (`extractPackageName`) treats a + still ships. A `function` declaration's parameter list and the expression + body of an arrow initializing a `const` / `let` / `var` declarator are + dropped from the depth-0 frame before the call scan + (`blankDeclarationOnlyCode`, #1567), so `now = Date.now()` and + `(m) => Math.round(m) / MICROS` are declarations, not load-time calls; the + arrow body ends early (at a newline) whenever continuation is not provable, + because ending late could swallow a real statement. The vendor bare-import scan (`extractPackageName`) treats a `#`-alias specifier as local and skips it outright (it does not expand the map), so a `#` import is never sent to the resolver; the rare alias mapped to a real package (`"#x": "some-pkg"`) is consequently not vendored, an accepted diff --git a/packages/server/src/component-elision.js b/packages/server/src/component-elision.js index bac0ed454..b3b5b45ad 100644 --- a/packages/server/src/component-elision.js +++ b/packages/server/src/component-elision.js @@ -285,6 +285,118 @@ const PURE_DATA_CONSTRUCTORS = new Set([ 'BigInt64Array', 'BigUint64Array', ]); +/** + * Marks where `hasModuleScopeSideEffect` dropped a top-level `{...}` body from + * its depth-0 frame, so an arrow followed by a block body is not mistaken for + * an expression body that runs on into the next statement. Removed again + * before the call scan. + */ +const BLOCK = '\x01'; + +/** `function`, `function*`, `async function name` up to its parameter list's `(`. */ +const FUNCTION_PARAMS_RE = /(?` when that arrow is a declarator initializer + * (`const usd = (m) =>`, `let f = async x =>`, `, g = (a) =>`). The parameter + * list must hold no parens, so a call in a parameter default + * (`(a = init()) =>`) keeps its arrow unrecognised and still ships. + */ +const ARROW_DECLARATOR_RE = + /(?:(?`). Postfix `++` / `--` and a regex + * literal's closing `/` end a statement, so neither is listed. + */ +const BODY_CONTINUES_RE = /(?:=>|&&|\|\||\?\?|[?:=*%|&^<>]|(?)|\|)/; + +/** + * Removes the two depth-0 shapes that look like calls and run nothing at module + * load: a `function` declaration's parameter list (a default such as + * `now = Date.now()` runs per call) and an arrow's expression body when the + * arrow initializes a declarator (`const usd = (m) => Math.round(m) / MICROS`). + * + * The arrow body ends at a `,` or `;` at its own paren/bracket depth 0, at a + * `)` / `]` closing something outside it, or at a newline once the body has + * content, unless `BODY_CONTINUES_RE` / `LINE_CONTINUES_RE` prove the + * expression carries on. Ending early is always safe, because whatever is left + * stays in the frame and is still scanned; ending late could swallow a real + * statement, so every rule errs towards ending. + * + * @param {string} frame depth-0 text, with `BLOCK` where a `{...}` was dropped + * @returns {string | null} the frame without those parts, or null when a + * parameter list does not close (the caller then ships) + */ +function blankDeclarationOnlyCode(frame) { + let out = ''; + let last = 0; + for (const m of frame.matchAll(FUNCTION_PARAMS_RE)) { + if (m.index < last) continue; + const open = m.index + m[0].length - 1; + let depth = 0; + let close = -1; + for (let j = open; j < frame.length; j++) { + if (frame[j] === '(') depth++; + else if (frame[j] === ')' && --depth === 0) { close = j; break; } + } + if (close < 0) return null; + out += frame.slice(last, open + 1); + last = close; + } + frame = out + frame.slice(last); + + out = ''; + let depth = 0; + for (let i = 0; i < frame.length; i++) { + const c = frame[i]; + if (c === '(' || c === '[') depth++; + else if ((c === ')' || c === ']') && depth > 0) depth--; + if (depth === 0 && c === '=' && frame[i + 1] === '>' && ARROW_DECLARATOR_RE.test(out)) { + out += '=>'; + i = arrowBodyEnd(frame, i + 2) - 1; + continue; + } + out += c; + } + return out; +} + +/** + * Index where an arrow's expression body starting at `start` ends (see + * `blankDeclarationOnlyCode`). + * + * @param {string} frame + * @param {number} start index just past the `=>` + * @returns {number} + */ +function arrowBodyEnd(frame, start) { + let depth = 0; + let i = start; + for (; i < frame.length; i++) { + const c = frame[i]; + if (c === '(' || c === '[') { depth++; continue; } + if (c === ')' || c === ']') { + if (depth === 0) return i; + depth--; + continue; + } + if (depth > 0) continue; + if (c === ',' || c === ';') return i; + if (c === '\n') { + const body = frame.slice(start, i); + if (!body.trim() || BODY_CONTINUES_RE.test(body)) continue; + if (LINE_CONTINUES_RE.test(frame.slice(i + 1))) continue; + return i; + } + } + return i; +} + /** * Module-scope client work, detected by an ALLOWLIST of safe top-level forms * rather than a denylist of browser globals. A module that runs ANY code when @@ -320,8 +432,13 @@ const PURE_DATA_CONSTRUCTORS = new Set([ * request path is covered; a DIRECT caller handing it `.ts` source as authored * is opting into that false positive. * - * Over-detection is safe (a top-level arrow whose body calls something, or a - * pure top-level helper call, only ships). The accepted residual misses, all + * A `function` declaration's parameter list and the expression body of an + * arrow that initializes a `const` / `let` / `var` declarator are dropped from + * the frame first (`blankDeclarationOnlyCode`): `now = Date.now()` and + * `(m) => Math.round(m) / MICROS` declare code, they do not run it. + * + * Over-detection is safe (a pure top-level helper call, or an arrow outside a + * declarator whose body calls something, only ships). The accepted residual misses, all * contrived and structural (so they do not rot), are a call buried inside a * top-level object / array initializer or a destructuring default, and a * side-effecting tagged-template hole evaluated at module scope. @@ -361,7 +478,7 @@ export function hasModuleScopeSideEffect(src, literals) { if (depth === 0) frame += "''"; continue; } - if (c === '{') { depth++; continue; } + if (c === '{') { if (depth === 0) frame += BLOCK; depth++; continue; } if (c === '}') { if (depth > 0) depth--; continue; } if (depth === 0) frame += c; } @@ -369,6 +486,11 @@ export function hasModuleScopeSideEffect(src, literals) { // literal with a stray brace is the common case). Ship rather than risk a // hidden top-level statement. if (depth !== 0) return true; + // Parameter lists and declarator arrow bodies are declarations, not code + // that runs at load. + const declared = blankDeclarationOnlyCode(frame); + if (declared === null) return true; + frame = declared.replaceAll(BLOCK, ''); // Optional-chaining call/index: `foo?.()`, `x?.[i]()` (the `?.` defeats the // identifier-before-paren match below). if (/\?\.\s*[([]/.test(frame)) return true; diff --git a/packages/server/test/elision/declaration-shapes.test.js b/packages/server/test/elision/declaration-shapes.test.js new file mode 100644 index 000000000..3abcc7854 --- /dev/null +++ b/packages/server/test/elision/declaration-shapes.test.js @@ -0,0 +1,91 @@ +/** + * Declarations that LOOK like top-level calls must not read as module-load + * work (#1567). + * + * `hasModuleScopeSideEffect` keeps only the brace-depth-0 text of a module and + * treats any `ident(` left there as a call that runs when the module loads. Two + * declaration shapes leave such text behind without running anything: the + * expression body of an arrow that initializes a declarator + * (`export const usd = (m) => Math.round(m) / MICROS`) and a call in a + * `function` declaration's parameter default (`now = Date.now()`). Read as + * calls, they shipped every page that imported a formatting helper, along with + * that page's whole import graph. + * + * Every declaration shape is paired with real top-level calls that must keep + * shipping, because the cheap way to make the first half pass is to stop + * seeing calls at all. Reverting the frame blanking in `component-elision.js` + * reds every declaration test except the block-bodied arrow (which the + * depth-0 frame already dropped), plus the route test, and nothing else + * (proven at 015faf5c). + */ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; + +import { analyzeElision, hasModuleScopeSideEffect } from '../../src/component-elision.js'; + +const DECLARATIONS = { + 'expression-bodied arrow': 'const MICROS = 1e6;\nexport const usd = (m) => Math.round(m) / MICROS;', + 'parenthesised arrow body': 'const one = (v) => (Array.isArray(v) ? v[0] : v);', + 'arrow body on the next line': 'export const one = (v) =>\n Array.isArray(v) ? v[0] : v;\nexport const two = 2;', + 'leading-operator ternary body': 'const f = (v) =>\n isX(v)\n ? fmt(v)\n : other(v);\n', + 'trailing-operator body': 'const ok = (v) => v.ok &&\n check(v);', + 'async arrow and a second declarator': 'const a = async (v) => await load(v), b = (x) => g(x);', + 'curried arrow': 'const add = (a) => (b) => sum(a, b);', + 'arrow returning a constructor': 'const mk = (u) => new WebSocket(u);', + 'type-erased arrow': 'export const usd = (m ) => Math.round(m);', + 'function parameter default': 'export function relativeTime(when, now = Date.now()) { return now - when; }', + 'parameter default calling a helper': 'export function buildSearchIndex(pages = livePages()) { return pages; }', + 'async generator parameter default': 'async function* gen(a = x()) { yield a; }', + 'block-bodied arrow': 'const f = (v) => {\n return g(v);\n}\nconst z = 1;', +}; + +const CALLS = { + 'a declarator calling a helper': "const x = planById('free');", + 'a method call on data': 'export const A = LIST.filter((p) => p.ok);', + 'a bare call': 'foo();', + 'a call after an arrow statement': 'const f = () => x; f();', + 'a call on the line after an arrow (no semicolon)': 'const f = () => x\nf()', + 'a call after a block-bodied arrow': 'const f = (v) => {\n return g(v);\n}\nfoo();', + 'an arrow IIFE': '(() => { go(); })();', + 'an immediately invoked arrow initializer': 'export const y = (() => 1)();', + 'a call in the declarator after an arrow': 'const g = (a) => a, h = init();', + 'a call in an arrow parameter default': 'const f = (a = init()) => a;', + 'a call after a postfix increment ends the body': 'const f = (v) => v++\ninit();', + 'a call after a regex literal ends the body': 'const r = () => /x/\ninit();', + 'a call after a function declaration': 'function f() {}\ninit();', + 'an invoked function expression': 'const x = function (a = 1) { return a; }();', + 'a function IIFE with a parameter default': '(function (a = b()) {})();', + 'a call after a conditional arrow': 'const k = cond ? (a) => f(a) : null;\nfoo();', +}; + +for (const [name, src] of Object.entries(DECLARATIONS)) { + test(`declaration, not a load-time call: ${name}`, () => { + assert.equal(hasModuleScopeSideEffect(src), false, src); + }); +} + +for (const [name, src] of Object.entries(CALLS)) { + test(`still a load-time call: ${name}`, () => { + assert.equal(hasModuleScopeSideEffect(src), true, src); + }); +} + +test('a page importing a formatting helper module does not ship for it', async () => { + const format = `const MICROS = 1_000_000; +export const usd = (m) => '$' + (Math.round(m / 10_000) / 100).toFixed(2); +export function relativeTime(when, now = Date.now()) { + return Math.round((now - when) / 1000) + 's ago'; +}`; + const page = `import { html } from '@webjsdev/core'; +import { usd, relativeTime } from './format.js'; +export default ({ cost, at }) => html\`

\${usd(cost)} \${relativeTime(at)}

\`;`; + const files = { '/app/page.js': page, '/app/format.js': format }; + const r = await analyzeElision( + [], + ['/app/page.js'], + new Map([['/app/page.js', new Set(['/app/format.js'])]]), + async (f) => files[f], + '/app', + ); + assert.ok(r.inertRouteModules.has('/app/page.js'), 'the page must be inert'); +}); diff --git a/packages/server/test/ssr/dropped-page-app-preload.test.js b/packages/server/test/ssr/dropped-page-app-preload.test.js index 70b1f05a2..9fcafc454 100644 --- a/packages/server/test/ssr/dropped-page-app-preload.test.js +++ b/packages/server/test/ssr/dropped-page-app-preload.test.js @@ -102,8 +102,9 @@ test('a SHIPPED page still preloads its transitive relative-helper app modules ( // The under-fetch guard for the roots change: this is what would break if the // moduleUrls -> absolute-path round-trip failed to match the graph's node keys // and the walk from `shippedRoots` silently dropped a genuinely-shipped dep. - // A page that imports a pure relative helper `./fmt.js` (used as a value) SHIPS, - // so `page.js` is a boot module. `fmt.js` and its own helper `./deep.js` are + // A page that runs code at module load (`Date.now()` at top level) SHIPS, so + // `page.js` is a boot module, while its pure relative helper `./fmt.js` does + // nothing at load (an arrow body is a declaration, #1567). `fmt.js` and its own helper `./deep.js` are // NOT boot modules; their preloads come ONLY from `deduplicatedPreloads` walking // the transitive closure from `shippedRoots` (the boot-module preload loop // covers `page.js`, not its deps, and `deduplicatedPreloads` excludes @@ -116,7 +117,8 @@ test('a SHIPPED page still preloads its transitive relative-helper app modules ( 'app/page.js': `import { html } from ${JSON.stringify(HTML_URL)};\n` + `import { fmt } from './fmt.js';\n` + - `export default () => html\`

\${fmt(1)}

\`;\n`, + `const LOADED_AT = Date.now();\n` + + `export default () => html\`

\${fmt(LOADED_AT)}

\`;\n`, }); const app = await createRequestHandler({ appDir, dev: false }); await app.warmup(); diff --git a/website/app/docs/elision/page.ts b/website/app/docs/elision/page.ts index e38c2158a..6038705dc 100644 --- a/website/app/docs/elision/page.ts +++ b/website/app/docs/elision/page.ts @@ -33,7 +33,7 @@ export default function Elision() {
  • A factory-declared reactive property that is not { state: true }.
  • An overridden lifecycle hook, renderFallback() and renderError() included.
  • An imported signal / computed / watch / Task / ref or a streaming directive, or a call to addController / requestUpdate.
    • -
  • Code that runs at module load: a top-level call, a non-data new, a dynamic import(...), a top-level await. Only declarations and the register(...) call are inert. TypeScript types are erased before the analyser reads a module, so an annotation is never a signal however call-shaped it looks: readonly (readonly [number, number, number])[] is inert data.
    • +
  • Code that runs at module load: a top-level call, a non-data new, a dynamic import(...), a top-level await. Only declarations and the register(...) call are inert. A call inside a declaration does not run at load, so an arrow's expression body (export const usd = (m) => Math.round(m) / MICROS) and a function parameter default (function relativeTime(when, now = Date.now())) are inert too. TypeScript types are erased before the analyser reads a module, so an annotation is never a signal however call-shaped it looks: readonly (readonly [number, number, number])[] is inert data.
  • A browser global at module scope, or a side-effect import of an npm package.
  • The dynamic slot READ surface (slotchange, assignedNodes / assignedElements / assignedSlot). Merely rendering a <slot> does not ship, because the SSR output already carries the placed children.
  • Being rendered or imported by a component that itself ships.