diff --git a/.agents/skills/webjs/references/components.md b/.agents/skills/webjs/references/components.md index dd193731d..d5a24dd2d 100644 --- a/.agents/skills/webjs/references/components.md +++ b/.agents/skills/webjs/references/components.md @@ -390,7 +390,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 +- 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) - 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/.agents/skills/webjs/references/runtime.md b/.agents/skills/webjs/references/runtime.md index 1273ef945..55c1fcdea 100644 --- a/.agents/skills/webjs/references/runtime.md +++ b/.agents/skills/webjs/references/runtime.md @@ -20,7 +20,7 @@ Pick a runtime from the deploy target, not the code. Default to Node unless you Three seams pick a runtime-specific implementation, all inside the framework, none in your app: - **The listener.** `startServer` selects the `node:http` request shell on Node and a native `Bun.serve` shell on Bun. Both parse the request, run middleware, dispatch to your routes, and stream the response through the same downstream pipeline, so an SSR page, a server action RPC, and a route handler behave identically. -- **The type stripper.** WebJs serves `.ts` / `.tsx` as ES modules by erasing the types in place with no bundler. On Node that is the built-in `module.stripTypeScriptTypes`; on Bun it is `amaro` (the same engine, byte-identical and position-preserving so stack traces still point at the right line). Either way your TypeScript must be erasable (see `typescript.md`). +- **The type stripper.** WebJs serves `.ts` / `.mts` as ES modules by erasing the types in place with no bundler. Those two are the whole set: there is no JSX anywhere in the framework, so a `.tsx` file is not served. On Node that is the built-in `module.stripTypeScriptTypes`; on Bun it is `amaro` (the same engine, byte-identical and position-preserving so stack traces still point at the right line). Either way your TypeScript must be erasable (see `typescript.md`). - **A few built-ins.** SQLite, hot reload, and WebSockets each bind to the runtime's native primitive (see the table). ## Node vs Bun at a glance diff --git a/.claude/skills/webjs-doc-sync/SKILL.md b/.claude/skills/webjs-doc-sync/SKILL.md index 316077413..b28c700c2 100644 --- a/.claude/skills/webjs-doc-sync/SKILL.md +++ b/.claude/skills/webjs-doc-sync/SKILL.md @@ -43,7 +43,7 @@ applies, then update or consciously skip each. relevant `references/` file. 2. **`README.md`** (repo root). Update when a headline capability changes (the feature list, the quickstart, the runtime/template matrix). -3. **The docs site: `website/app/docs//page.tsx`.** This is the +3. **The docs site: `website/app/docs//page.ts`.** This is the user-facing documentation at webjs.dev/docs. Find the topic page(s) that cover the area (`server-actions`, `routing`, `components`, `caching`, `configuration`, `client-router`, `data-fetching`, ...) and update them. `llms.txt` / @@ -102,7 +102,7 @@ The surface checks below are independent reads, so run them fanned out: one read 4. Verify: re-run the grep and confirm each applicable surface now describes the new behaviour, and no surface still describes the old one. 5. Respect the prose-punctuation invariant (#11) and run `webjs check` if any - code-shaped doc (a `.tsx` doc page) changed. + code-shaped doc (a `.ts` doc page) changed. ## Audit-mode procedure (sweep shipped work for drift) diff --git a/packages/server/AGENTS.md b/packages/server/AGENTS.md index 33e7ba507..83caa9817 100644 --- a/packages/server/AGENTS.md +++ b/packages/server/AGENTS.md @@ -463,6 +463,27 @@ conditional leaves a CSP-off document one newline shorter than a CSP-on one). 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 limitation since the scaffold's catch-all `"#*": "./*"` is always local. + Every scan reads TYPE-ERASED source: `analyzeElision` runs a `.ts` / `.mts` / + `.tsx` module through the framework's own stripper (`eraseTypesForScan`) + before scanning it, because the scans are lexical and TS annotations are + call-shaped often enough to matter (`readonly (readonly [number, number, + number])[]` is an identifier immediately followed by `(`, which the + module-scope scan read as a top-level call, pinning every importing route + module, #1423). The stripper rather than a lexical annotation matcher, so the + analyser and the runtime agree by construction, and it is position-preserving, + so every offset the other scans depend on survives. The extension set is the + union of the TypeScript-carrying extensions in the three filters that seed + `allFiles`, NOT the servable set: `scanComponents` admits `/\.m?[jt]sx?$/`, + wider than the graph walker and the router, so a `.tsx` component reaches the + analysis and narrowing to `.ts` / `.mts` silently un-erases it. A non-TS file, + and a source the stripper rejects (non-erasable syntax, which invariant 10 + forbids, or real JSX in a `.tsx`), are scanned as authored, the conservative + direction; a missing stripper BACKEND is the one failure that warns, since it + un-erases the whole app rather than one file. Results are memoized per file + and validated against the source, so a dev rebuild re-strips only what + changed. Consequence for a DIRECT caller of `hasModuleScopeSideEffect` / + `analyzeComponentSource`: both still take source as given, so handing either + one `.ts` as authored opts back into that false positive. Import-only modules join the elision fingerprint (a verdict flip busts `?v`) and the bare-import scan exclusion (an SSR-only page import is no longer vendored), like inert modules. `collectRouteModules` (`dev.js`) feeds only diff --git a/packages/server/src/component-elision.js b/packages/server/src/component-elision.js index 32a89ea71..b780ad925 100644 --- a/packages/server/src/component-elision.js +++ b/packages/server/src/component-elision.js @@ -45,6 +45,7 @@ import { redactToPlaceholders, } from './js-scan.js'; import { transitiveDeps, expandImportAlias } from './module-graph.js'; +import { ensureStripper } from './ts-strip.js'; /** * Named imports from a `@webjsdev/core` specifier that imply the @@ -267,6 +268,23 @@ const INSTANCEOF_RE = /\binstanceof\s+([A-Z][A-Za-z0-9_$]*)/g; * `customElements.define(...)` legitimately uses it and must not force ship). */ const COMPONENT_CLIENT_GLOBAL_RE = /\b(?:window|document|navigator|localStorage|sessionStorage|matchMedia|addEventListener)\b/; +/** + * Constructors that produce inert DATA with no side effect, so a module-scope + * `export const X = new Set([...])` (a lookup table, a compiled RegExp, a + * parsed URL) is not client work and must not pin an importing page/layout + * (#623). Any constructor NOT in this set (`new WebSocket()`, `new Worker()`, + * `new EventSource()`, `new Audio()`) IS a side effect and still ships. + */ +const PURE_DATA_CONSTRUCTORS = new Set([ + 'Set', 'Map', 'WeakSet', 'WeakMap', 'Date', 'RegExp', 'Array', 'Object', + 'Number', 'String', 'Boolean', 'BigInt', 'Symbol', + 'Error', 'TypeError', 'RangeError', 'SyntaxError', + 'URL', 'URLSearchParams', 'ArrayBuffer', 'DataView', + 'Int8Array', 'Uint8Array', 'Uint8ClampedArray', 'Int16Array', 'Uint16Array', + 'Int32Array', 'Uint32Array', 'Float32Array', 'Float64Array', + 'BigInt64Array', 'BigUint64Array', +]); + /** * 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 @@ -285,13 +303,22 @@ const COMPONENT_CLIENT_GLOBAL_RE = /\b(?:window|document|navigator|localStorage| * * Scans the redacted copy (strings / templates / comments blanked, regex * literals and nested `${...}` interpolation tracked by the lexer) so template - * prose and JSDoc / TS type annotations cannot trip it; quoted-string bodies, - * which redaction keeps verbatim for other rules, are blanked here too so a - * string like `"foo()"` or `"{"` is not read as a call and cannot unbalance - * the brace scan. The unbalanced-brace and unterminated-string fallbacks below - * are defense in depth: with the lexer tracking regex literals, neither should - * trigger on valid code, but if either does the module ships rather than risk - * hiding client work. + * prose and JSDoc cannot trip it; quoted-string bodies, which redaction keeps + * verbatim for other rules, are blanked here too so a string like `"foo()"` or + * `"{"` is not read as a call and cannot unbalance the brace scan. The + * unbalanced-brace and unterminated-string fallbacks below are defense in + * depth: with the lexer tracking regex literals, neither should trigger on + * valid code, but if either does the module ships rather than risk hiding + * client work. + * + * PASS TYPE-ERASED SOURCE for a TypeScript module. Redaction blanks comments + * and literals, NOT type syntax, and TS annotations are call-shaped often + * enough to matter: `readonly (readonly [number, number, number])[]` is an + * identifier immediately followed by `(`, so a module holding nothing but a + * typed data literal reads as running code at module scope (#1423). The + * analyser erases types before it calls this (`eraseTypesForScan`), so the + * 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 @@ -299,25 +326,10 @@ const COMPONENT_CLIENT_GLOBAL_RE = /\b(?:window|document|navigator|localStorage| * top-level object / array initializer or a destructuring default, and a * side-effecting tagged-template hole evaluated at module scope. * - * @param {string} src raw module source - */ -/** - * Constructors that produce inert DATA with no side effect, so a module-scope - * `export const X = new Set([...])` (a lookup table, a compiled RegExp, a - * parsed URL) is not client work and must not pin an importing page/layout - * (#623). Any constructor NOT in this set (`new WebSocket()`, `new Worker()`, - * `new EventSource()`, `new Audio()`) IS a side effect and still ships. + * @param {string} src module source, type-erased if it came from a TypeScript file + * @param {string[]} [literals] the redaction's literal bodies, when already computed + * @returns {boolean} */ -const PURE_DATA_CONSTRUCTORS = new Set([ - 'Set', 'Map', 'WeakSet', 'WeakMap', 'Date', 'RegExp', 'Array', 'Object', - 'Number', 'String', 'Boolean', 'BigInt', 'Symbol', - 'Error', 'TypeError', 'RangeError', 'SyntaxError', - 'URL', 'URLSearchParams', 'ArrayBuffer', 'DataView', - 'Int8Array', 'Uint8Array', 'Uint8ClampedArray', 'Int16Array', 'Uint16Array', - 'Int32Array', 'Uint32Array', 'Float32Array', 'Float64Array', - 'BigInt64Array', 'BigUint64Array', -]); - export function hasModuleScopeSideEffect(src, literals) { let redacted = src; if (!literals) { @@ -871,6 +883,131 @@ export function extractRenderedTags(src) { return tags; } +/** + * The extensions that can carry TypeScript type syntax INTO this analysis. + * + * Derive it from what actually reaches `allFiles`, which is seeded from three + * places with three different filters: the component set from `scanComponents` + * (`/\.m?[jt]sx?$/`), the route modules from the router (`js|mjs|ts|mts`), and + * the module graph from its walker (`/\.(js|ts|mjs|mts)$/`). The COMPONENT set + * is the widest and it does not pass through the other two, so a `.tsx` + * component arrives here whatever the walker and the router admit. Hence + * `.tsx`, even though nothing in WebJs serves one: leaving it out is how the + * #1423 false positive comes back for exactly the file class no other filter + * would have let through. `.cts` is absent because no filter admits it. + * + * The rule to hold, if any of those three filters moves: this set is every + * TypeScript-carrying extension in their UNION. Narrowing it to the servable + * set instead is a change that looks tighter and silently un-erases files. + */ +const TS_SOURCE_RE = /\.m?tsx?$/; + +/** + * Per-file memo of {@link eraseTypesForScan}, holding the source it was derived + * from so a hit is proven rather than assumed. One entry per file, so it is + * bounded by the app's file count and needs no eviction policy; a deleted file + * leaves one dead entry until the process ends, which is cheaper than tracking + * liveness for it. + * + * @type {Map} + */ +const STRIP_CACHE = new Map(); + +/** + * Return `src` with its TYPE syntax erased, so every scan below reads the code + * that actually RUNS rather than the code as authored (#1423). + * + * The scans are lexical heuristics, and TypeScript annotations are syntax the + * heuristics were never designed for: `readonly (readonly [number, number, + * number])[]` is an identifier immediately followed by `(`, which the + * top-level-call matcher in `hasModuleScopeSideEffect` reads as a call, so a + * module holding nothing but a typed data literal was classified as running + * code at module scope and pinned every route module that imported it. Type + * syntax is erased before anything runs, so it can never be a runtime signal, + * and the only sound reading of it is none at all. + * + * This uses the framework's own stripper rather than a lexical annotation + * matcher: it is the same erasure the browser is served, so the analyser and + * the runtime agree by construction, where a hand-rolled matcher would meet + * generics, `as`, `satisfies`, and conditional types and get some of them + * wrong. It is position-preserving whitespace replacement, so every offset, + * line, and column the other scans depend on is unchanged. + * + * A non-TypeScript file is returned untouched (there is nothing to erase, and + * running a TS parser over one buys only new ways to fail). A PER-FILE strip + * failure also returns the source untouched, which is the pre-#1423 behaviour + * and therefore the conservative direction: the scans then over-detect at + * worst. Two things take that path, both silent because both are one file's + * problem. A non-erasable module, which invariant 10 forbids and `webjs check` + * catches at edit time, so it is a broken app rather than a supported one. And + * a `.tsx` holding real JSX, which the stripper parses as non-JSX TypeScript + * and rejects, so a JSX component is scanned as authored while a `.tsx` that is + * only TypeScript is erased normally. + * + * A missing stripper BACKEND is the failure that does not stay silent, and it + * is why the backend is resolved once by the caller rather than per file. It + * means no TypeScript in the app can be erased (a runtime with neither the + * built-in nor `amaro`), so every module is scanned as authored and the #1423 + * verdict comes back app-wide. Swallowing that alongside a syntax rejection + * would make an app-wide regression indistinguishable from one broken file, so + * `resolveScanStripper` warns once and this returns the source untouched. + * + * Results are memoized per file and validated against the exact source, since + * this runs on every dev rebuild and stripping costs roughly 50x the read it + * sits beside. Validating on content rather than mtime means a same-mtime + * rewrite cannot serve a stale strip. + * + * @param {import('./ts-strip.js').Stripper | null} stripper resolved once per run, null when unavailable + * @param {string} file absolute path, the cache key and the extension source + * @param {string} src + * @returns {string} + */ +function eraseTypesForScan(stripper, file, src) { + if (!stripper || !TS_SOURCE_RE.test(file)) return src; + const hit = STRIP_CACHE.get(file); + if (hit && hit.src === src) return hit.out; + let out; + try { + out = stripper.fn(src); + } catch { + out = src; + } + STRIP_CACHE.set(file, { src, out }); + return out; +} + +/** Whether {@link resolveScanStripper} has already reported a missing backend. */ +let warnedNoStripper = false; + +/** + * Resolve the TypeScript stripper backend once for a whole analysis run, or + * `null` when this runtime has none. + * + * `analyzeElision` runs outside the dev request handler too (`webjs check`, + * `webjs elision`), and `ensureStripper` is called on the dev path only, so + * this is where the no-backend case surfaces for the other entry points. It + * warns once per process rather than per file: the condition is a property of + * the runtime, so repeating it once per module would bury it. + * + * @returns {Promise} + */ +async function resolveScanStripper() { + try { + return await ensureStripper(); + } catch (e) { + if (!warnedNoStripper) { + warnedNoStripper = true; + console.warn( + '[webjs] elision analysis could not resolve a TypeScript stripper, so type syntax ' + + 'is not erased before the scans read a module. A type annotation can then read as ' + + 'module-scope work, which ships modules that would otherwise be elided. Underlying: ' + + (e && e.message ? e.message : String(e)), + ); + } + return null; + } +} + /** * Compute the set of component FILES whose browser download can be * elided. A file is elidable only when every component it defines is @@ -978,6 +1115,11 @@ export async function analyzeElision(components, routeModules, moduleGraph, read for (const v of vs) if (!appDir || v.startsWith(appDir)) allFiles.add(v); } + // Resolve the stripper ONCE for the run: the backend is a property of the + // runtime, not of a file, and the no-backend case must be reported rather + // than swallowed per module (see `eraseTypesForScan`). + const scanStripper = await resolveScanStripper(); + for (const file of allFiles) { if (SERVER_FILE_RE.test(file)) { serverFiles.add(file); continue; } let src; @@ -989,6 +1131,11 @@ export async function analyzeElision(components, routeModules, moduleGraph, read continue; } if (typeof src !== 'string') continue; + // Erase type syntax first, so every scan below reads the code that RUNS + // (#1423). This is the single point where the analyser reads a file, so + // doing it here covers the module-scope, template, import, and component + // scans at once. + src = eraseTypesForScan(scanStripper, file, src); // Mask comments once for every signal scan below (#179): a ``, an // `@event`, a browser global, an `import`, or a `whenDefined` written in a // comment must not register as a real signal. String and template content diff --git a/packages/server/test/elision/type-annotations.test.js b/packages/server/test/elision/type-annotations.test.js new file mode 100644 index 000000000..3afb1ba12 --- /dev/null +++ b/packages/server/test/elision/type-annotations.test.js @@ -0,0 +1,245 @@ +/** + * A TypeScript TYPE ANNOTATION must never be read as runtime behaviour by the + * elision analyser (#1423). + * + * The scans in `component-elision.js` are lexical heuristics over source that + * has had comments and literals redacted, and type syntax was not on that list. + * `readonly (readonly [number, number, number])[]` is an identifier immediately + * followed by `(`, so `hasModuleScopeSideEffect`'s top-level-call matcher read + * it as a call and classified a module of pure typed data as running code at + * module scope. The cost was invisible: no error, no warning, identical + * behaviour, just every importing route module downgraded from inert to + * shipping whole, and any component carrying such an annotation forced to ship + * along with the display-only children it renders. + * + * These tests drive the REAL pipeline over a REAL `.ts` app on disk, because + * that is the only level the fix lives at: the analyser erases types before it + * scans, so a unit call on a source string would prove nothing about the path + * that decides what the browser downloads. The first three are the issue's + * acceptance criteria, and they are the discriminating ones: reverting the + * erasure reds exactly those three (proven at 3b3cb6a5). The rest are the + * counterweight that keeps them honest, since erasing too much would satisfy + * the first three by simply never detecting anything, and they stay green in + * both directions on purpose. + */ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { mkdtemp, mkdir, writeFile, readFile, rm } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { buildModuleGraph } from '../../src/module-graph.js'; +import { scanComponents } from '../../src/component-scanner.js'; +import { analyzeElision } from '../../src/component-elision.js'; + +/** The tic-tac-toe declaration from the report, verbatim in shape. */ +const LINES_TS = ` +export type Cell = 'X' | 'O' | null; +export interface Board { cells: Cell[]; } +export const LINES: readonly (readonly [number, number, number])[] = [ + [0, 1, 2], [3, 4, 5], [6, 7, 8], + [0, 3, 6], [1, 4, 7], [2, 5, 8], + [0, 4, 8], [2, 4, 6], +]; +export function winner(cells: Cell[]): Cell { + for (const [a, b, c] of LINES) { + if (cells[a] && cells[a] === cells[b] && cells[b] === cells[c]) return cells[a]; + } + return null; +} +`; + +/** + * Write a throwaway TypeScript app, run the real pipeline over it, and return + * the verdict plus the paths the assertions key on. + * + * `direct: true` has the PAGE import the util and render no component, which is + * the shape the report hit: there the util is the page's whole client closure, + * so a wrong verdict on it shows up directly as the page's blocker instead of + * being laundered through a component that ships for its own reason. + * + * @param {{ utilSrc: string, direct?: boolean }} spec + */ +async function analyseApp(spec) { + const dir = await mkdtemp(join(tmpdir(), 'webjs-type-annotations-')); + try { + await mkdir(join(dir, 'app'), { recursive: true }); + await mkdir(join(dir, 'components'), { recursive: true }); + await mkdir(join(dir, 'modules/game/utils'), { recursive: true }); + await writeFile(join(dir, 'modules/game/utils/game.ts'), spec.utilSrc); + if (!spec.direct) await writeFile(join(dir, 'components/badge.ts'), ` +import { WebComponent, html } from '@webjsdev/core'; +import { LINES } from '../modules/game/utils/game.ts'; +export class Badge extends WebComponent { + render() { return html\`\${LINES.length}\`; } +} +Badge.register('my-badge'); +`); + await writeFile(join(dir, 'app/page.ts'), spec.direct ? ` +import { html } from '@webjsdev/core'; +import { LINES } from '../modules/game/utils/game.ts'; +export default () => html\`

\${LINES.length}

\`; +` : ` +import { html } from '@webjsdev/core'; +import '../components/badge.ts'; +export default () => html\`\`; +`); + + const graph = await buildModuleGraph(dir); + const components = await scanComponents(dir); + const pageFile = join(dir, 'app/page.ts'); + const badgeFile = join(dir, 'components/badge.ts'); + const verdict = await analyzeElision( + components, [pageFile], graph, (f) => readFile(f, 'utf8'), dir, + ); + return { verdict, pageFile, badgeFile }; + } finally { + await rm(dir, { recursive: true, force: true }); + } +} + +test('a parenthesised type annotation does not mark its module as running code at module scope', async () => { + const { verdict, pageFile } = await analyseApp({ utilSrc: LINES_TS, direct: true }); + const shipped = verdict.shippedRouteModules.get(pageFile); + assert.equal(shipped, undefined, + `the page must not ship; it did, blocked by ${shipped && shipped.blocker} (${shipped && shipped.reason})`); +}); + +test('a page importing only such a module is inert', async () => { + const { verdict, pageFile } = await analyseApp({ utilSrc: LINES_TS }); + assert.ok(verdict.inertRouteModules.has(pageFile), + 'the whole client closure is display-only, so the page ships zero JavaScript'); +}); + +test('a component whose only module-scope construct is a parenthesised annotation is elided', async () => { + const { verdict, badgeFile } = await analyseApp({ utilSrc: LINES_TS }); + assert.ok(verdict.elidableComponents.has(badgeFile), + 'the badge renders static markup and the annotation is not a signal'); + assert.equal(verdict.componentVerdicts.get(badgeFile).shipped, false); +}); + +test('the pin is the ANNOTATION, not the data: an `as const` spelling was already inert', async () => { + // The report's own workaround, kept as a control. Both spellings describe the + // same runtime value, so a fix that only moved one of them would be reading + // something other than the annotation. + const asConst = LINES_TS.replace( + /export const LINES: readonly \(readonly \[number, number, number\]\)\[\] = \[/, + 'export const LINES = [', + ).replace(/\n\];/, '\n] as const;'); + assert.notEqual(asConst, LINES_TS, 'the control rewrite must actually apply'); + const { verdict, pageFile } = await analyseApp({ utilSrc: asConst, direct: true }); + assert.ok(verdict.inertRouteModules.has(pageFile)); +}); + +test('real module-scope work in a .ts module still ships the page', async () => { + // The conservative direction, asserted per construct. Type erasure must not + // become a way for genuine client work to slip past: each of these is real + // code in the file the browser would run. + const cases = { + 'a top-level call': 'export const boot = init();\nfunction init() { return 1; }', + 'a non-data new': 'export const ws = new WebSocket("wss://x");', + 'a dynamic import': 'export const mod = import("./other.ts");', + 'a top-level await': 'export const v = await Promise.resolve(1);', + 'a browser global': 'export const w = window.innerWidth;', + }; + for (const [label, stmt] of Object.entries(cases)) { + const { verdict, pageFile } = await analyseApp({ + utilSrc: `export const LINES: readonly (readonly [number, number, number])[] = [[0, 1, 2]];\n${stmt}\n`, + direct: true, + }); + assert.ok(!verdict.inertRouteModules.has(pageFile), `${label} must keep the page shipping`); + } +}); + +test('a genuine top-level call named like a type keyword still ships', async () => { + // `readonly` is a legal function name in JavaScript, so exempting the bare + // identifier would have been a false NEGATIVE, the one direction this + // analyser may not take. Erasing types instead leaves the real call visible. + const { verdict, pageFile } = await analyseApp({ + utilSrc: 'function readonly(x: number) { return x; }\nexport const LINES = readonly(1);\n', + direct: true, + }); + assert.ok(!verdict.inertRouteModules.has(pageFile), + 'a call to a function named `readonly` is a call'); +}); + +test('a .tsx component is erased too, because it reaches the analysis by a wider filter', async () => { + // `allFiles` is seeded from the COMPONENT set before the module graph, and + // `scanComponents` admits `/\.m?[jt]sx?$/`, which is wider than both the + // graph walker's file filter and the router's. So a `.tsx` component lands + // in the analysis whatever those two admit, and an erasure set narrowed to + // the SERVABLE extensions would leave exactly this file class un-erased, + // with the #1423 verdict intact and nothing else covering it. + const dir = await mkdtemp(join(tmpdir(), 'webjs-type-annotations-tsx-')); + try { + await mkdir(join(dir, 'components'), { recursive: true }); + await writeFile(join(dir, 'components/badge.tsx'), ` +import { WebComponent, html } from '@webjsdev/core'; +export const LINES: readonly (readonly [number, number, number])[] = [[0, 1, 2]]; +export class Badge extends WebComponent { + render() { return html\`\${LINES.length}\`; } +} +Badge.register('my-badge'); +`); + const graph = await buildModuleGraph(dir); + const components = await scanComponents(dir); + const badgeFile = join(dir, 'components/badge.tsx'); + assert.ok(components.some((c) => c.file === badgeFile), + 'precondition: the component scanner admits a .tsx file'); + const verdict = await analyzeElision(components, [], graph, (f) => readFile(f, 'utf8'), dir); + assert.ok(verdict.elidableComponents.has(badgeFile), + 'the annotation is not a signal in a .tsx file either'); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +test('the strip memo is validated against the source, so an edit is re-erased', async () => { + // The memo is keyed by PATH and holds the source it was derived from, because + // this runs on every dev rebuild and stripping costs roughly 50x the read it + // sits beside. Keying by path alone would serve a stale strip after an edit, + // and mtime would miss a same-mtime rewrite, so the check is on content. Same + // path, two different sources, two different verdicts is what proves it. + const dir = await mkdtemp(join(tmpdir(), 'webjs-type-annotations-memo-')); + try { + await mkdir(join(dir, 'app'), { recursive: true }); + await mkdir(join(dir, 'modules/game/utils'), { recursive: true }); + const util = join(dir, 'modules/game/utils/game.ts'); + const pageFile = join(dir, 'app/page.ts'); + await writeFile(pageFile, ` +import { html } from '@webjsdev/core'; +import { LINES } from '../modules/game/utils/game.ts'; +export default () => html\`

\${LINES.length}

\`; +`); + const analyse = async () => { + const graph = await buildModuleGraph(dir); + return analyzeElision(await scanComponents(dir), [pageFile], graph, (f) => readFile(f, 'utf8'), dir); + }; + + await writeFile(util, 'export const LINES: readonly (readonly [number, number, number])[] = [[0, 1, 2]];\n'); + assert.ok((await analyse()).inertRouteModules.has(pageFile), 'pure typed data is inert'); + + // Same path, new content that DOES do module-scope work. A memo keyed only + // by path would hand back the erased first version and call this inert too. + await writeFile(util, 'export const LINES = init();\nfunction init() { return [[0, 1, 2]]; }\n'); + assert.ok(!(await analyse()).inertRouteModules.has(pageFile), + 'the edit must be re-read and re-scanned, not served from the memo'); + } finally { + await rm(dir, { recursive: true, force: true }); + } +}); + +test('a .ts module with non-erasable syntax falls back to scanning it as authored', async () => { + // The stripper throws on non-erasable TypeScript (invariant 10 forbids it and + // `webjs check` catches it at edit time, so this is a broken app rather than a + // supported one). The analyser must not throw with it: it keeps the source as + // authored, which is the pre-#1423 behaviour and therefore the conservative + // direction. The `init()` call is the proof it still SCANNED rather than + // silently gave up and called the module clean. + const { verdict, pageFile } = await analyseApp({ + utilSrc: 'export enum E { A, B }\nexport const LINES = init();\nfunction init() { return []; }\n', + direct: true, + }); + assert.ok(!verdict.inertRouteModules.has(pageFile), + 'an unstrippable module is still scanned, and its real call still ships the page'); +}); diff --git a/test/bun/elision-report.mjs b/test/bun/elision-report.mjs index c6a093c38..ce3056e50 100644 --- a/test/bun/elision-report.mjs +++ b/test/bun/elision-report.mjs @@ -8,9 +8,13 @@ * WebJs runs on Node 24+ AND Bun (#508), and an app scaffolded with `--runtime * bun` runs `webjs elision` against a Bun-served app, so a verdict that drifted * between runtimes would mean the report told a Bun author something untrue - * about their own app. The analysis is filesystem reads plus regular - * expressions with no runtime-specific API, so there is nothing legitimate to - * skip and this file carries no DENYLIST entry. + * about their own app. The analysis is filesystem reads and regular expressions + * over source the framework's TypeScript stripper has erased (#1423), and that + * stripper IS runtime-specific: Node's built-in `module.stripTypeScriptTypes` + * on one side, `amaro` on the other. The two are meant to be byte-identical, + * and the `.ts` route below is what holds them to it, since a divergence there + * would silently change what a Bun-served app downloads. Nothing here is + * legitimate to skip, so this file carries no DENYLIST entry. * * The fixture covers one component of each verdict and one route module of each * class, so a divergence in ANY of the projection's moving parts (the tag sort, @@ -57,6 +61,15 @@ Counter.register('my-counter'); // Import-only: the page itself does no client work, and the only client work // its closure reaches is a shipping component. write('app/page.js', "import { html } from '@webjsdev/core';\nimport '../components/counter.js';\nexport default () => html``;"); + // Inert, and the row that exercises the STRIPPER seam: a parenthesised type + // annotation is call-shaped, so a runtime whose stripper left it behind would + // read this util as running code at module scope and report the page as + // shipping whole (#1423). + write('modules/game/utils/game.ts', ` +export type Cell = 'X' | 'O' | null; +export const LINES: readonly (readonly [number, number, number])[] = [[0, 1, 2], [3, 4, 5]]; +`); + write('app/typed/page.ts', "import { html } from '@webjsdev/core';\nimport { LINES } from '../../modules/game/utils/game.ts';\nexport default () => html`

${LINES.length}

`;"); const r = await analyzeAppElision(dir); @@ -81,13 +94,14 @@ Counter.register('my-counter'); [ ['app/about/page.js', 'inert', ''], ['app/page.js', 'import-only', 'components/counter.js'], + ['app/typed/page.ts', 'inert', ''], ], ); assert.deepEqual(r.orphans, []); assert.deepEqual(r.summary, { components: 2, elided: 1, shipped: 1, - routeModules: 2, inert: 1, importOnly: 1, shippedWhole: 0, + routeModules: 3, inert: 2, importOnly: 1, shippedWhole: 0, orphans: 0, }); diff --git a/website/app/docs/elision/page.ts b/website/app/docs/elision/page.ts index 6fd0b55cc..e38c2158a 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.
    • +
  • 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.
  • 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.