From cc0396b30db857eb24d6e77325bb4fefc86f6d9b Mon Sep 17 00:00:00 2001 From: Ryan Carniato Date: Wed, 16 Sep 2026 09:35:39 -0700 Subject: [PATCH] =?UTF-8?q?fix(signals,solid):=20reserve=20`=5Fparent`=20f?= =?UTF-8?q?rom=20property=20mangling=20=E2=80=94=20the=20second=20cross-pa?= =?UTF-8?q?ckage=20owner=20field;=20retire=20the=20wall-clock=20tripwires?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signals' prod/observe artifacts rename every `_` field but `_name`; the dev artifact the suites run against is unmangled. Two `_parent` reads crossed the package boundary and only worked in dev: solid-js's client hydration walked `owner._parent` to mark the snapshot root (prod marked the current owner instead), and the core's `ownerPath`/`isExcluded` walks over server owners (`ownerPath` shimmed by `located()`, now removed; `OBSERVE.exclude` a silent no-op for a server owner outside dev). `_parent` is reserved alongside `_name`; ~40 B brotli on the prod app scenarios, two caps bumped. Pinned from both ends: solid's cross-package-fields spec checks the reserved fields survive mangling and scans every built client artifact for a signals `_` field not on the list; web's server-owner-walks spec runs `ownerPath` and `OBSERVE.exclude` over server owners against the built observe and development artifacts. Timing tripwires that flaked under parallel CI load: heap-mark re-measures a round over the cap; attribution-navigation asserts against the wait actually taken rather than a timer's nominal 10ms; both shell-gating specs prove ordering by settling gates by hand (shell-gating-harness.ts) instead of wall-clock deltas against timers. RFC 08 gains "Values in records — the PII surface": every record and finding field that carries user data, `data.error` on the server error findings included, for exporters that leave the process. Co-authored-by: Claude via Cursor Co-authored-by: Cursor --- .changeset/cross-package-owner-fields.md | 15 ++ documentation/solid-2.0/08-dev-diagnostics.md | 2 + packages/signals/rollup.config.js | 2 +- packages/signals/scripts/mangle-props.mjs | 19 +- .../tests/attribution-navigation.test.ts | 5 +- .../tests/heap-mark-incremental.test.ts | 15 +- packages/solid/src/server/diagnostics.ts | 29 +-- .../solid/test/cross-package-fields.spec.ts | 115 +++++++++++ .../test/server/dynamic-shell-gating.spec.tsx | 154 ++++++++------- .../test/server/lazy-shell-gating.spec.tsx | 179 ++++++++++-------- .../server/server-owner-walks.fixture.mjs | 48 +++++ .../test/server/server-owner-walks.spec.tsx | 60 ++++++ .../web/test/server/shell-gating-harness.ts | 107 +++++++++++ scripts/size/.size-limit.js | 18 +- 14 files changed, 575 insertions(+), 193 deletions(-) create mode 100644 .changeset/cross-package-owner-fields.md create mode 100644 packages/solid/test/cross-package-fields.spec.ts create mode 100644 packages/web/test/server/server-owner-walks.fixture.mjs create mode 100644 packages/web/test/server/server-owner-walks.spec.tsx create mode 100644 packages/web/test/server/shell-gating-harness.ts diff --git a/.changeset/cross-package-owner-fields.md b/.changeset/cross-package-owner-fields.md new file mode 100644 index 000000000..28efe492b --- /dev/null +++ b/.changeset/cross-package-owner-fields.md @@ -0,0 +1,15 @@ +--- +"@solidjs/signals": patch +"solid-js": patch +--- + +`_parent` joins `_name` as a field signals' property mangling reserves — the two cross-package owner fields. + +Signals' prod and observe artifacts rename every `_`-prefixed property except a reserved list; the dev artifact (which the test suites run against) is unmangled. Two things read `_parent` across the package boundary and only worked in dev: + +- `solid-js`'s client hydration walks `owner._parent` to the root to mark the hydration snapshot scope. In the built prod and observe artifacts the walk found nothing and marked the current owner instead, so computations created outside that owner's subtree during hydration read live values rather than the server snapshot. +- The core's owner walks — `ownerPath` and `OBSERVE.exclude`/`isExcluded` — over `solid-js`'s server owners. `ownerPath` had a server-side shim (`located()`, now removed); `OBSERVE.exclude` was a silent no-op for a server owner outside dev. + +Cost: ~40 B brotli on the prod app scenarios; the observe scenarios did not grow. Pinned from both ends: `packages/solid/test/cross-package-fields.spec.ts` checks the reserved fields survive in the mangled artifacts and scans the built client artifacts of `solid-js`, `@solidjs/web` and `@solidjs/universal` for any signals `_` field that is not reserved; `packages/web/test/server/server-owner-walks.spec.tsx` runs `ownerPath` and `OBSERVE.exclude` over server owners against the built observe and development artifacts. + +Also: RFC 08 gains "Values in records — the PII surface", the complete list of record and finding fields that carry user data (value previews, interaction target text, navigation paths/params, `data.error` on the server error findings) for exporters that leave the process. diff --git a/documentation/solid-2.0/08-dev-diagnostics.md b/documentation/solid-2.0/08-dev-diagnostics.md index 700f10af8..f02b9d4c3 100644 --- a/documentation/solid-2.0/08-dev-diagnostics.md +++ b/documentation/solid-2.0/08-dev-diagnostics.md @@ -853,6 +853,8 @@ createRoot(() => { **Excluding the observer.** `OBSERVE.exclude(owner)` marks an owner subtree as the observer's own: diagnostics whose subject sits under it are built (a throwing site still throws) but never delivered or printed, and the attribution engine records no run for its computations, charges none of them to an interaction, counts no write to its signals or stores toward an interaction, and does not spend a once-per-key slot (`IMMUTABLE_UPDATE_IN_STORE`'s per-path memory) on them. An interaction whose writes all went to excluded subjects, with none of the app's work run — a click on the observer's own panel — is not recorded at all. Mark the root as it is created (a store's nodes take the owner the store was created under, recorded only once the engine is enabled — enable before creating the panel's stores), and make writes from outside the graph under it (`runWithOwner(owner, () => setPanel(…))`) so the writer's context is excluded too. `OBSERVE.isExcluded(subject)` answers the question for any owner or node. +**Values in records — the PII surface.** Records name things (owner paths, `name` options, store paths, route patterns, function ids) and are otherwise numbers, kinds and outcomes; a handful of fields carry _user data_, and an exporter that leaves the process owns scrubbing them (vendors already have the control surface — `beforeSend`, `sendDefaultPii` — and the runtime keeps producing them because they are what makes dev output readable). The complete list: `ChangeRecord.prev`/`value` and `HeldWrite.prev`/`value` — previews of the written values (`preview()`: strings quoted and cut at 40 characters, numbers/booleans verbatim, everything else a type tag such as `Array(12)` or `[Object]`), so the string case is the one to drop or hash unless opted in; `ChangeOrigin.target` (and `InteractionRef.target`) — the element hit, `tag#id "text"` with up to 30 characters of `textContent` for anything that is not an `input`/`textarea`/`select`, so a label but also whatever a `` said; `ChangeOrigin.to`/`from`/`params` and `NavigationEvent.to`/`from`/`params` (`NavigationHop` too) — concrete paths and the values a route pattern bound (`/users/42`, `{ id: "42" }`), while `name` is the pattern; `DiagnosticEvent.message` and `data` for the responsiveness findings (`SILENT_HOLD`, `LONG_HOLD`) — the verdict sentence names the interaction (`click on button#next "Next →"`) and the navigation it was under (concrete `to`/`from`/`params`), and `data.interaction.target` / `data.navigation` carry the same fields structured; no finding quotes a value preview. `data.error` on the server error findings (`SSR_RENDER_ERROR_CONTAINED`, `SSR_ERROR_SANITIZED`, `SERVER_FN_ERROR_SANITIZED`) — the error **as thrown**, message and own properties, deliberately unsanitized: the wire got the generic message so the observer could see the real one, which means a driver's connection string or a query lands here, and an exporter treats it as it treats any captured exception. Dev-only checks may put the offending value on `data` (`PRELOAD_DESCRIPTOR_INVALID`'s `data.value`, `HEAD_TAG_INVALID`'s `data.detail`) — dev tier, never exported. Everything else is safe by construction: `RerunEvent` has names and numbers only; the runtimes' records (`"call"`, `"invocation"`, `"boundary"`, `"frame"`) never put arguments, results, thrown values, requests or responses on the record — those ride the `live` argument beside it, in-process only — and carry ids, methods, addresses, statuses and timings; `ownerPath` is component and primitive names. `stacks: true` adds first-party frames to `ChangeRecord.stack` (file paths, not values) and is a dev affordance to leave off in production. + `costs()` aggregates since `enable()`: `scopes` ranked by self-time with `wastedMs` (time in runs whose value didn't change — the equality cutoff absorbed them), and `writes` ranked by the total downstream re-run time each root write caused. Overlay work (optimistic-lane and held runs — `phase: "optimistic" | "held"`) is accounted separately as `overlayMs` and never blamed as waste. ### Provenance — "who wrote this" diff --git a/packages/signals/rollup.config.js b/packages/signals/rollup.config.js index 402668ae1..1436b39f7 100644 --- a/packages/signals/rollup.config.js +++ b/packages/signals/rollup.config.js @@ -25,7 +25,7 @@ import prettier from "rollup-plugin-prettier"; // single sequential post-pass (scripts/mangle-props.mjs) with one shared // nameCache per output; per-chunk terser would mangle the same property to // different names in different modules and break every cross-module member -// access. `_name` is reserved (the cross-package label field). +// access. `_name` and `_parent` are reserved (the cross-package owner fields). // // Two entries per build: `index` (the core) and `attribution` (the engine // behind `@solidjs/signals/attribution`). The engine reads the core's live diff --git a/packages/signals/scripts/mangle-props.mjs b/packages/signals/scripts/mangle-props.mjs index 89dc6c8f0..69c13235d 100644 --- a/packages/signals/scripts/mangle-props.mjs +++ b/packages/signals/scripts/mangle-props.mjs @@ -40,12 +40,19 @@ for (const dir of process.argv.slice(2)) { keep_classnames: true, keep_fnames: true, module: false, - // `_name` is the one cross-package field: solid-js writes the - // component label onto signals' owners (`owner._name = ""`) and - // `ownerPath` reads it. Mangling it in the observe tree would put the - // write and the read on different properties. Every other `_` field - // is private to this package. - properties: { regex: /^_/, reserved: ["_name"] } + // Two cross-package owner fields, both reserved: `_name` — solid-js + // writes the component label onto signals' owners (`owner._name = + // ""`) and `ownerPath` reads it — and `_parent`, the owner-tree + // link, which solid-js walks on signals' owners (client hydration's + // root lookup) and which the core walks on solid-js's server owners + // (`ownerPath`, `OBSERVE.exclude`/`isExcluded`). Mangling either + // puts the write and the read on different properties: before + // `_parent` was reserved, the prod client marked the wrong snapshot + // scope and `OBSERVE.exclude` was a silent no-op for server owners + // in the observe tier. Every other `_` field is private to this + // package; solid's cross-package-fields spec scans the downstream + // artifacts for any new one. + properties: { regex: /^_/, reserved: ["_name", "_parent"] } }, // preserve_annotations: terser consumes /*@__PURE__*/ during parse and // only re-emits it when asked — without this the prod tree loses the diff --git a/packages/signals/tests/attribution-navigation.test.ts b/packages/signals/tests/attribution-navigation.test.ts index b0c2e7901..a86540898 100644 --- a/packages/signals/tests/attribution-navigation.test.ts +++ b/packages/signals/tests/attribution-navigation.test.ts @@ -686,6 +686,9 @@ describe("at — a router whose request predates the write it wraps", () => { // wraps, with the user's request time carried in. const requested = performance.now(); await wait(10); + // The wait actually taken on the engine's clock (a 10ms timer can fire a + // hair under 10ms of `performance.now()`); settledMs must cover it. + const waited = performance.now() - requested; OBSERVE!.attribution.withOrigin({ ...NAV, at: requested }, () => app.setLocation("/users/42")); flush(); const [nav] = attribution.navigations(); @@ -694,7 +697,7 @@ describe("at — a router whose request predates the write it wraps", () => { app.resolve("b"); await until(() => app.shown.includes("b@/users/42"), "the held page to land"); expect(nav.outcome).toBe("held"); - expect(nav.settledMs).toBeGreaterThanOrEqual(10); + expect(nav.settledMs).toBeGreaterThanOrEqual(waited); expect(nav.hold!.origin).toBe(nav.origin); }); }); diff --git a/packages/signals/tests/heap-mark-incremental.test.ts b/packages/signals/tests/heap-mark-incremental.test.ts index a3279578f..36fcf185b 100644 --- a/packages/signals/tests/heap-mark-incremental.test.ts +++ b/packages/signals/tests/heap-mark-incremental.test.ts @@ -63,16 +63,23 @@ describe("heap marking stays incremental across mid-tick pulls", () => { // the rows within one process instead — linear scaling lands near 8×, // the quadratic regime near 64×. Best-of-k tames JIT/GC noise at the // small end. Measured locally: ~10× fixed (3 → 30 ms), ~50× on next - // (17 → 850 ms). + // (17 → 850 ms). Under a loaded worker (the suite runs beside two other + // packages' suites) the one large sample can draw a GC pause the small + // ones did not, so a round over the cap is re-measured: the quadratic + // regime is over the cap every round, contention is not. const best = (N: number, k: number) => { let ms = Infinity; for (let i = 0; i < k; i++) ms = Math.min(ms, mount(N)); return ms; }; best(1000, 2); // warm - const small = best(1000, 3); - const large = best(8000, 2); - expect(large / small).toBeLessThan(24); + let ratio = Infinity; + for (let round = 0; round < 3 && ratio >= 24; round++) { + const small = best(1000, 3); + const large = best(8000, 2); + ratio = Math.min(ratio, large / small); + } + expect(ratio).toBeLessThan(24); }); it("a write landing between two mid-tick pulls is visible through a memo chain in the same pass", () => { diff --git a/packages/solid/src/server/diagnostics.ts b/packages/solid/src/server/diagnostics.ts index e96f5a199..15acac54a 100644 --- a/packages/solid/src/server/diagnostics.ts +++ b/packages/solid/src/server/diagnostics.ts @@ -23,8 +23,10 @@ export type Finding = Omit; * `in › `, the once-per-code footer). `subject` locates it; the * current owner by default, which is what the check sites want (they fire * inside the scope that misbehaved). Server owners are signals-shaped - * enough for the core's `ownerPath` walk (`_parent` + `_name`), so component - * labels (see `createComponent`) come through unchanged. No-op in prod. + * enough for the core's `ownerPath` and exclusion walks (`_parent` + + * `_name`, the two fields signals' property mangling reserves as + * cross-package), so component labels (see `createComponent`) come through + * unchanged in every tier. No-op in prod. * * Advisory (`info`) findings are structured-channel only, as in the core: * a fact worth recording that has not earned the console. @@ -36,29 +38,10 @@ export function emitFinding( if (!IS_OBSERVE) return; // `OBSERVE`/`DEV` are typed optional (undefined in the tiers below theirs); // the gates above are the same conditions that define them. - const entry = OBSERVE!.diagnostics.emit(located(finding, subject), subject); + const entry = OBSERVE!.diagnostics.emit(finding, subject); if (IS_DEV && finding.severity !== "info") DEV!.report(entry); } -/** - * The finding with its `ownerPath` — the labels up this entry's OWN owner - * chain (`createComponentOwner`'s ``) — filled in here rather than by - * the core's walk: the core reads `_parent` under its own build's property - * mangling (the observe and prod artifacts rename `_`-fields; `_name` alone - * is reserved as the cross-package label), so its walk finds nothing on a - * server owner in the observe artifact. An `ownerPath` already on the - * finding wins, as in the core. - */ -function located(finding: Finding, subject: DiagnosticSubject | null): Finding { - if (finding.ownerPath !== undefined || !subject || !("_parent" in subject)) return finding; - const path: string[] = []; - for (let owner: any = subject; owner; owner = owner._parent) { - const name = owner._name; - if (typeof name === "string" && name.length) path.push(name); - } - return path.length ? { ...finding, ownerPath: path.reverse() } : finding; -} - /** * The structured record alone, for a site that THROWS its message: the * thrown error is the console face, and the core lands the once-per-code @@ -69,7 +52,7 @@ export function recordFinding( finding: Finding, subject: DiagnosticSubject | null = getOwner() ): void { - if (IS_OBSERVE) OBSERVE!.diagnostics.emit(located(finding, subject), subject); + if (IS_OBSERVE) OBSERVE!.diagnostics.emit(finding, subject); } /** diff --git a/packages/solid/test/cross-package-fields.spec.ts b/packages/solid/test/cross-package-fields.spec.ts new file mode 100644 index 000000000..3d76bbd97 --- /dev/null +++ b/packages/solid/test/cross-package-fields.spec.ts @@ -0,0 +1,115 @@ +/** + * The cross-package `_`-field contract with `@solidjs/signals`. + * + * Signals' prod and observe artifacts mangle every `_`-prefixed property + * (scripts/mangle-props.mjs) except a reserved list; the dev artifact is + * unmangled. Any `_` field a downstream package reads or writes on a + * signals object — an `Owner`, `Computed`, `Signal` — therefore only works + * in every tier if it is on that list. The suite cannot see this: it runs + * against source, where nothing is mangled. Two things went wrong before + * this spec existed — the client's hydration root lookup walked `_parent` + * (mangled → the wrong snapshot scope in prod), and the core's owner walks + * found nothing on a server owner in the observe tier (`OBSERVE.exclude` a + * silent no-op) — so the contract is pinned from both ends here: + * + * - the reserved fields survive in the mangled signals artifacts, and a + * private one does not (so a change to the mangler's regex or list shows); + * - every `_` field the built client artifacts of solid-js and + * @solidjs/web touch is either reserved or not a signals field at all. + * + * Requires a prior `pnpm build`. + */ +import { readdirSync, readFileSync, statSync } from "node:fs"; +import { join, resolve } from "node:path"; +import { describe, expect, test } from "vitest"; + +const ROOT = resolve(import.meta.dirname, "../.."); +const SIGNALS = join(ROOT, "signals"); + +/** The mangler's reserved list, read from the script so the two cannot drift. */ +function reservedFields(): string[] { + const script = readFileSync(join(SIGNALS, "scripts/mangle-props.mjs"), "utf8"); + const match = script.match(/reserved:\s*\[([^\]]*)\]/); + if (!match) throw new Error("mangle-props.mjs: could not find the reserved list"); + return [...match[1].matchAll(/"(_\w+)"/g)].map(m => m[1]); +} + +/** Every `_` field declared on signals' node types — the ones the mangler renames. */ +function signalsFields(): Set { + const types = readFileSync(join(SIGNALS, "src/core/types.ts"), "utf8"); + return new Set([...types.matchAll(/^\s+(_\w+)\??:/gm)].map(m => m[1])); +} + +function jsFiles(dir: string): string[] { + const out: string[] = []; + for (const entry of readdirSync(dir)) { + const path = join(dir, entry); + if (statSync(path).isDirectory()) { + if (entry !== "types" && entry !== "node_modules") out.push(...jsFiles(path)); + } else if (entry.endsWith(".js")) out.push(path); + } + return out; +} + +/** `._name` member accesses in `code`, by field, with a short context for the report. */ +function fieldAccesses(code: string): Map { + const seen = new Map(); + for (const m of code.matchAll(/\.(_[A-Za-z]\w*)\b/g)) { + if (!seen.has(m[1])) seen.set(m[1], code.slice(Math.max(0, m.index! - 40), m.index! + 40)); + } + return seen; +} + +describe("cross-package _-fields", () => { + const reserved = reservedFields(); + + test("the mangler reserves the two owner fields downstream packages walk", () => { + expect(reserved).toEqual(expect.arrayContaining(["_name", "_parent"])); + }); + + test("the reserved fields survive in the mangled signals artifacts; a private one does not", () => { + for (const tier of ["prod", "observe"]) { + const core = readFileSync(join(SIGNALS, `dist/${tier}/core/core.js`), "utf8"); + for (const field of reserved) expect(core, `${tier}: ${field}`).toMatch(`.${field}`); + // `_firstChild` is the owner tree's other link and is private: its + // absence proves the mangler ran on this file at all. + expect(core, `${tier}: _firstChild should be mangled`).not.toMatch("._firstChild"); + } + // The dev artifact is the unmangled one the suite runs against (flat and + // code-split: the core sits in the chunk shared with the engine entry). + const dev = readdirSync(join(SIGNALS, "dist")) + .filter(f => /^dev.*\.js$/.test(f)) + .map(f => readFileSync(join(SIGNALS, "dist", f), "utf8")) + .join("\n"); + expect(dev).toMatch("._firstChild"); + }); + + test("the built client artifacts touch no signals field the mangler renames", () => { + const fields = signalsFields(); + expect(fields.has("_parent")).toBe(true); // the parser found the node types + const artifacts = [ + ...jsFiles(join(ROOT, "solid/dist")), + ...jsFiles(join(ROOT, "web/dist")), + ...jsFiles(join(ROOT, "web/frames/dist")), + ...jsFiles(join(ROOT, "web/server-functions/dist")), + ...jsFiles(join(ROOT, "universal/dist")) + ].filter( + // Server artifacts work on solid-js's own SSR owners, whose `_` fields + // are solid-js's and unmangled; only the client's objects are signals'. + f => !/[/\\]server[^/\\]*\.js$/.test(f) && !/[/\\]server[/\\]/.test(f) + ); + expect(artifacts.length).toBeGreaterThan(5); + const offenders: string[] = []; + for (const file of artifacts) { + for (const [field, context] of fieldAccesses(readFileSync(file, "utf8"))) { + if (fields.has(field) && !reserved.includes(field)) + offenders.push(`${file.slice(ROOT.length + 1)}: ${field} — …${context.trim()}…`); + } + } + expect( + offenders, + "a downstream artifact reads a signals `_` field the mangler renames — reserve it in " + + "packages/signals/scripts/mangle-props.mjs or stop reaching into the node" + ).toEqual([]); + }); +}); diff --git a/packages/web/test/server/dynamic-shell-gating.spec.tsx b/packages/web/test/server/dynamic-shell-gating.spec.tsx index ca2dadf98..3ee3fea3b 100644 --- a/packages/web/test/server/dynamic-shell-gating.spec.tsx +++ b/packages/web/test/server/dynamic-shell-gating.spec.tsx @@ -12,122 +12,130 @@ // duration of the server work, and the boundary's fallback was never emitted // — the shell just sat there. Nothing streamed, so every byte of the page // waited on the slowest server function. +// +// Ordering is proven by settling gates by hand — see shell-gating-harness.ts. import { describe, expect, test } from "vitest"; -import { renderToStream, Loading, dynamic } from "@solidjs/web"; +import { Loading, dynamic } from "@solidjs/web"; import { createMemo } from "solid-js"; - -const wait = (ms: number) => new Promise(r => setTimeout(r, ms)); - -/** Resolves with the shell (first chunk) and the time it took to arrive. */ -function collectTimed(code: () => any): Promise<{ shell: string; shellAt: number; html: string }> { - return new Promise(resolve => { - const t0 = Date.now(); - const chunks: string[] = []; - let shell = ""; - let shellAt = -1; - renderToStream(code).pipe({ - write: (c: string) => { - if (shellAt < 0) { - shellAt = Date.now() - t0; - shell = c; - } - chunks.push(c); - }, - end: () => resolve({ shell, shellAt, html: chunks.join("") }) - }); - }); -} +import { collect, drain, gate } from "./shell-gating-harness.js"; describe("promise-backed dynamic() under a boundary", () => { - const DELAY = 150; - test("defers to the enclosing Loading instead of gating the shell", async () => { - const Slow = dynamic(() => wait(DELAY).then(() => () => content)); - - const { shell, shellAt, html } = await collectTimed(() => ( -
- waiting…}> - - -
- )); + const source = gate<() => any>(); + const Slow = dynamic(() => source.promise); + + const r = collect( + () => ( +
+ waiting…}> + + +
+ ), + [source] + ); - // The shell must not wait on the source. - expect(shellAt).toBeLessThan(DELAY); + // The shell must not wait on the source: it flushes with the source open. + const shell = await r.shell; + expect(r.settledAtShell.has(source)).toBe(false); // The boundary — not the renderer — owns the wait. expect(shell).toContain(">waiting…"); // And the content still arrives, streamed in behind the placeholder. + source.resolve(() => content); + const { html } = await r.done; expect(html).toContain(">content"); }); test("a near-instant source still inlines with no fallback flash", async () => { const Fast = dynamic(() => Promise.resolve(() => content)); - const { html } = await collectTimed(() => ( + const { html } = await collect(() => (
waiting…}>
- )); + )).done; expect(html).toContain(">content"); expect(html).not.toContain(">waiting…"); }); test("deferStream opts the source into holding the shell", async () => { - const Slow = dynamic(() => wait(DELAY).then(() => () => content), { - deferStream: true - }); - - const { shell, shellAt, html } = await collectTimed(() => ( -
- waiting…}> - - -
- )); + const source = gate<() => any>(); + const Slow = dynamic(() => source.promise, { deferStream: true }); + + const r = collect( + () => ( +
+ waiting…}> + + +
+ ), + [source] + ); - expect(shellAt).toBeGreaterThanOrEqual(DELAY - 5); + await drain(); + expect(r.shellFlushed()).toBe(false); + source.resolve(() => content); + const shell = await r.shell; + expect(r.settledAtShell.has(source)).toBe(true); expect(shell).toContain(">content"); + const { html } = await r.done; expect(html).not.toContain(">waiting…"); }); test("deferStream holds the shell for the source only; the resolved component's data still streams", async () => { - const Slow = dynamic( - () => - wait(50).then(() => () => { - const data = createMemo(() => wait(DELAY).then(() => "data")); - return {data()}; - }), - { deferStream: true } + const source = gate<() => any>(); + const data = gate(); + const Slow = dynamic(() => source.promise, { deferStream: true }); + + const r = collect( + () => ( +
+ waiting…}> + + +
+ ), + [source, data] ); - const { shell, shellAt, html } = await collectTimed(() => ( -
- waiting…}> - - -
- )); - - expect(shellAt).toBeGreaterThanOrEqual(45); - expect(shellAt).toBeLessThan(DELAY); + await drain(); + expect(r.shellFlushed()).toBe(false); + source.resolve(() => { + const value = createMemo(() => data.promise); + return {value()}; + }); + const shell = await r.shell; + expect(r.settledAtShell.has(source)).toBe(true); + expect(r.settledAtShell.has(data)).toBe(false); expect(shell).toContain(">waiting…"); + data.resolve("data"); + const { html } = await r.done; expect(html).toContain(">data"); }); test("with no boundary to defer to, the shell still waits for the source", async () => { - const Slow = dynamic(() => wait(DELAY).then(() => () => content)); + const source = gate<() => any>(); + const Slow = dynamic(() => source.promise); - const { shell } = await collectTimed(() => ( -
- -
- )); + const r = collect( + () => ( +
+ +
+ ), + [source] + ); // Nothing to stream behind, so the root hole keeps the shell back and the // content is inline in the first chunk rather than lost. + await drain(); + expect(r.shellFlushed()).toBe(false); + source.resolve(() => content); + const shell = await r.shell; expect(shell).toContain(">content"); }); }); diff --git a/packages/web/test/server/lazy-shell-gating.spec.tsx b/packages/web/test/server/lazy-shell-gating.spec.tsx index 43aeb28ea..fc97f15a5 100644 --- a/packages/web/test/server/lazy-shell-gating.spec.tsx +++ b/packages/web/test/server/lazy-shell-gating.spec.tsx @@ -11,51 +11,28 @@ // // Asset ordering is handled separately, by `assetsPending` and its own // NotReadyError inside lazy's render memo. +// +// Every claim here is about ORDER (shell before/after a source settled) and +// is proven by settling gates by hand — see shell-gating-harness.ts for why +// the wall clock was retired. import { describe, expect, test } from "vitest"; -import { renderToStream, Loading, Errored } from "@solidjs/web"; +import { Loading, Errored } from "@solidjs/web"; import { createMemo, lazy } from "solid-js"; - -const wait = (ms: number) => new Promise(r => setTimeout(r, ms)); - -function collectTimed( - code: () => any, - options?: any -): Promise<{ shell: string; shellAt: number; html: string; chunks: number }> { - return new Promise(resolve => { - const t0 = Date.now(); - const chunks: string[] = []; - let shell = ""; - let shellAt = -1; - renderToStream(code, options).pipe({ - write: (c: string) => { - if (shellAt < 0) { - shellAt = Date.now() - t0; - shell = c; - } - chunks.push(c); - }, - end: () => resolve({ shell, shellAt, html: chunks.join(""), chunks: chunks.length }) - }); - }); -} +import { collect, drain, gate } from "./shell-gating-harness.js"; const manifest = { "./Slow.tsx": { file: "assets/slow.js" } }; -const MODULE = 50; -const DATA = 100; describe("lazy() under a boundary", () => { test("the shell waits for the module; slow data inside it still streams", async () => { + const module = gate<{ default: any }>(); + const data = gate(); const Slow = (_props: any) => { - const data = createMemo(() => wait(DATA).then(() => "async content")); - return {data()}; + const value = createMemo(() => data.promise); + return {value()}; }; - const LazySlow = lazy( - () => wait(MODULE).then(() => ({ default: Slow })), - undefined, - "./Slow.tsx" - ); + const LazySlow = lazy(() => module.promise, undefined, "./Slow.tsx"); - const { shell, shellAt, html } = await collectTimed( + const r = collect( () => (
waiting…}> @@ -63,29 +40,26 @@ describe("lazy() under a boundary", () => {
), + [module, data], { manifest } ); - // Held for the code, not for the data: the fallback covers the fetch. - expect(shellAt).toBeGreaterThanOrEqual(MODULE - 5); - expect(shellAt).toBeLessThan(DATA); + // Held for the code: nothing flushes until the module lands… + await drain(); + expect(r.shellFlushed()).toBe(false); + module.resolve({ default: Slow }); + const shell = await r.shell; + // …and not for the data: the fallback covers the fetch. + expect(r.settledAtShell.has(module)).toBe(true); + expect(r.settledAtShell.has(data)).toBe(false); expect(shell).toContain(">waiting…"); expect(shell).not.toContain("async content"); + data.resolve("async content"); + const { html } = await r.done; expect(html).toContain(">async content"); }); test("deferStream inside a lazy chunk holds the shell like the eager control (#3299)", async () => { - const Page = (_props: any) => { - const data = createMemo(() => wait(DATA).then(() => "async content"), { - deferStream: true - }); - return

{data()}

; - }; - const LazyPage = lazy( - () => wait(MODULE).then(() => ({ default: Page })), - undefined, - "./Slow.tsx" - ); const app = (Comp: any) => () => (
waiting…}> @@ -93,32 +67,54 @@ describe("lazy() under a boundary", () => {
); + const page = (data: Promise) => (_props: any) => { + const value = createMemo(() => data, { deferStream: true }); + return

{value()}

; + }; - const eager = await collectTimed(app(Page), { manifest }); - const lazied = await collectTimed(app(LazyPage), { manifest }); + // Eager control: the shell waits for the deferStream read. + const eagerData = gate(); + const eager = collect(app(page(eagerData.promise)), [eagerData], { manifest }); + await drain(); + expect(eager.shellFlushed()).toBe(false); + eagerData.resolve("async content"); + await eager.shell; + + // Lazy: the module lands, then the shell still waits for the read the + // loaded code made. + const module = gate<{ default: any }>(); + const lazyData = gate(); + const LazyPage = lazy(() => module.promise, undefined, "./Slow.tsx"); + const lazied = collect(app(LazyPage), [module, lazyData], { manifest }); + await drain(); + expect(lazied.shellFlushed()).toBe(false); + module.resolve({ default: page(lazyData.promise) }); + await drain(); + expect(lazied.shellFlushed()).toBe(false); + lazyData.resolve("async content"); + await lazied.shell; + expect(lazied.settledAtShell.has(lazyData)).toBe(true); for (const r of [eager, lazied]) { - expect(r.chunks).toBe(1); - expect(r.shell).toMatch(/]*>async content<\/h1>/); - expect(r.shell).not.toContain("waiting…"); + const { html, chunks } = await r.done; + expect(chunks).toBe(1); + expect(html).toMatch(/]*>async content<\/h1>/); + expect(html).not.toContain("waiting…"); } - expect(eager.shellAt).toBeGreaterThanOrEqual(DATA - 5); - expect(lazied.shellAt).toBeGreaterThanOrEqual(MODULE + DATA - 10); }); test("a module that resolves in microtasks is discovered before the shell flushes", async () => { // Not preloaded, but no real I/O: the boundary resumes during the flush // loop's drain. A deferStream read created there must still hold the // shell — a blocker registered after the awaited snapshot is re-awaited. + const data = gate(); const Page = (_props: any) => { - const data = createMemo(() => wait(DATA).then(() => "async content"), { - deferStream: true - }); - return

{data()}

; + const value = createMemo(() => data.promise, { deferStream: true }); + return

{value()}

; }; const LazyWarm = lazy(() => Promise.resolve({ default: Page }), undefined, "./Slow.tsx"); - const { shell, chunks } = await collectTimed( + const r = collect( () => (
waiting…}> @@ -126,11 +122,16 @@ describe("lazy() under a boundary", () => {
), + [data], { manifest } ); + await drain(); + expect(r.shellFlushed()).toBe(false); + data.resolve("async content"); + const { html, chunks } = await r.done; expect(chunks).toBe(1); - expect(shell).toMatch(/]*>async content<\/h1>/); + expect(html).toMatch(/]*>async content<\/h1>/); }); test("a preloaded module still inlines with no fallback flash", async () => { @@ -138,7 +139,7 @@ describe("lazy() under a boundary", () => { const LazyFast = lazy(() => Promise.resolve({ default: Fast }), undefined, "./Slow.tsx"); await LazyFast.preload!(); - const { html } = await collectTimed( + const { html } = await collect( () => (
waiting…}> @@ -146,8 +147,9 @@ describe("lazy() under a boundary", () => {
), + [], { manifest } - ); + ).done; expect(html).toContain(">content"); expect(html).not.toContain(">waiting…"); @@ -158,13 +160,10 @@ describe("lazy() under a boundary", () => { // errors under a boundary are the client's to render, the // server serializes the rejection. What this pins is that a rejected // block does not hold the shell (or hang the response). - const LazyBroken = lazy( - () => wait(MODULE).then(() => Promise.reject(new Error("chunk 404"))), - undefined, - "./Slow.tsx" - ); + const module = gate<{ default: any }>(); + const LazyBroken = lazy(() => module.promise, undefined, "./Slow.tsx"); - const { shell, shellAt, html } = await collectTimed( + const r = collect( () => (
{(err() as Error).message}}> @@ -174,12 +173,17 @@ describe("lazy() under a boundary", () => {
), + [module], { manifest } ); - expect(shellAt).toBeGreaterThanOrEqual(MODULE - 5); - expect(shellAt).toBeLessThan(MODULE + 50); + await drain(); + expect(r.shellFlushed()).toBe(false); + module.reject(new Error("chunk 404")); + // Released by the rejection: `shell` rejects on its own if it never comes. + const shell = await r.shell; expect(shell).toContain('new Error("chunk 404")'); + const { html } = await r.done; expect(html).not.toContain("waiting…"); }); @@ -188,20 +192,19 @@ describe("lazy() under a boundary", () => { // only then does its content mount a lazy. `block` is a no-op once the // shell has flushed, so the module (and the deferStream inside it) stream // in as fragments — by design. + const outerData = gate(); + const module = gate<{ default: any }>(); + const innerData = gate(); const Inner = (_props: any) => { - const data = createMemo(() => wait(30).then(() => "inner"), { deferStream: true }); - return {data()}; + const value = createMemo(() => innerData.promise, { deferStream: true }); + return {value()}; }; - const LazyInner = lazy( - () => wait(MODULE).then(() => ({ default: Inner })), - undefined, - "./Slow.tsx" - ); + const LazyInner = lazy(() => module.promise, undefined, "./Slow.tsx"); const Outer = () => { - const gate = createMemo(() => wait(DATA).then(() => true)); + const open = createMemo(() => outerData.promise); return (
- {gate() && ( + {open() && ( inner-waiting}> @@ -210,17 +213,27 @@ describe("lazy() under a boundary", () => { ); }; - const { shell, shellAt, html } = await collectTimed( + const r = collect( () => ( outer-waiting}> ), + [outerData, module, innerData], { manifest } ); - expect(shellAt).toBeLessThan(DATA); + // Plain data under a boundary never holds the shell. + const shell = await r.shell; + expect(r.settledAtShell.size).toBe(0); expect(shell).toContain("outer-waiting"); + // The lazy mounts after the shell closed; nothing left to hold. + outerData.resolve(true); + await drain(); + module.resolve({ default: Inner }); + await drain(); + innerData.resolve("inner"); + const { html } = await r.done; expect(html).toMatch(/]*>inner<\/b>/); }); }); diff --git a/packages/web/test/server/server-owner-walks.fixture.mjs b/packages/web/test/server/server-owner-walks.fixture.mjs new file mode 100644 index 000000000..2c6acf33b --- /dev/null +++ b/packages/web/test/server/server-owner-walks.fixture.mjs @@ -0,0 +1,48 @@ +// The core's owner walks over SERVER owners, run by server-owner-walks.spec.tsx +// in a child Node against the BUILT artifacts under `--conditions`. Signals' +// observe and prod artifacts mangle `_` fields; `_parent` and `_name` are the +// reserved cross-package pair, so the core's `ownerPath` and +// `isExcluded` walks work on solid-js's SSR owners in every tier. Before the +// reservation, `ownerPath` needed a server-side shim and `OBSERVE.exclude` was +// a silent no-op for a server owner outside dev. Prints one JSON result. +import { Errored, createComponent, escape, renderToString, ssr } from "@solidjs/web"; +import { OBSERVE, getOwner } from "solid-js"; + +const fallback = err => ssr(["

", "

"], escape(String(err().message))); + +const errored = children => + createComponent( + Errored, + { + fallback, + get children() { + return children(); + } + }, + "Errored" + ); + +/** A labelled component that throws inside an , `exclude`-ing its root first if asked. */ +const app = exclude => () => + createComponent( + () => { + if (exclude && OBSERVE) OBSERVE.exclude(getOwner()); + return errored(() => { + throw new Error("boom"); + }); + }, + {}, + "App" + ); + +async function findings(run) { + const capture = OBSERVE ? OBSERVE.diagnostics.capture() : undefined; + const html = await run(); + const events = capture ? capture.stop() : []; + return { html, findings: events.map(e => ({ code: e.code, ownerPath: e.ownerPath })) }; +} + +const results = { observe: OBSERVE !== undefined }; +results.plain = await findings(() => renderToString(app(false))); +results.excluded = await findings(() => renderToString(app(true))); +process.stdout.write(JSON.stringify(results)); diff --git a/packages/web/test/server/server-owner-walks.spec.tsx b/packages/web/test/server/server-owner-walks.spec.tsx new file mode 100644 index 000000000..354ae23bd --- /dev/null +++ b/packages/web/test/server/server-owner-walks.spec.tsx @@ -0,0 +1,60 @@ +/** + * The core's owner walks over server owners, per tier, against the BUILT + * artifacts (server-owner-walks.fixture.mjs in a child Node under Node's + * real resolver). The suite aliases to source, where nothing is mangled, so + * only a built run can show that `ownerPath` and `OBSERVE.exclude` see + * solid-js's SSR owners through signals' property mangling — the `_parent` + * + `_name` reservation. Requires a prior `pnpm build`. + */ +import { execFileSync } from "node:child_process"; +import { resolve } from "node:path"; +import { describe, expect, test } from "vitest"; + +const FIXTURE = resolve(import.meta.dirname, "server-owner-walks.fixture.mjs"); + +interface Scenario { + html: string; + findings: Array<{ code: string; ownerPath?: string[] }>; +} +interface Results { + observe: boolean; + plain: Scenario; + excluded: Scenario; +} + +function run(conditions: string[]): Results { + const stdout = execFileSync( + process.execPath, + [...conditions.map(c => `--conditions=${c}`), FIXTURE], + { cwd: process.cwd(), encoding: "utf8", stdio: ["ignore", "pipe", "ignore"] } + ); + return JSON.parse(stdout); +} + +describe.each([ + ["observe", ["observe"]], + ["development", ["development"]] +])("%s artifacts", (_name, conditions) => { + test("a server finding locates itself through the core's walk, component labels included", () => { + const { observe, plain } = run(conditions); + expect(observe).toBe(true); + expect(plain.html).toMatch(/

(boom|Internal Server Error)<\/p>/); + const contained = plain.findings.filter(f => f.code === "SSR_RENDER_ERROR_CONTAINED"); + expect(contained).toHaveLength(1); + expect(contained[0].ownerPath).toEqual(["", ""]); + }); + + test("OBSERVE.exclude on a server owner silences findings under it", () => { + const { excluded } = run(conditions); + // The boundary still renders its fallback; the observer just hears nothing. + expect(excluded.html).toMatch(/

(boom|Internal Server Error)<\/p>/); + expect(excluded.findings).toEqual([]); + }); +}); + +test("the production artifacts have no channel to walk for", () => { + const { observe, plain, excluded } = run([]); + expect(observe).toBe(false); + expect(plain.findings).toEqual([]); + expect(excluded.html).toContain("

Internal Server Error

"); +}); diff --git a/packages/web/test/server/shell-gating-harness.ts b/packages/web/test/server/shell-gating-harness.ts new file mode 100644 index 000000000..84387c5ad --- /dev/null +++ b/packages/web/test/server/shell-gating-harness.ts @@ -0,0 +1,107 @@ +// Shared harness for the shell-gating specs (lazy-shell-gating, +// dynamic-shell-gating). These specs are about ORDER — did the shell flush +// before or after this source settled — and used to prove it with wall-clock +// deltas against timers (`shellAt < DATA`). Under a loaded CI worker the +// event loop stalls past the timer and the delta lies. Here every async +// source is a gate the test settles by hand, so the claim "the shell waited +// for X" is "no shell before X was settled, shell after", with no clock in +// it. +import { renderToStream } from "@solidjs/web"; + +export interface Gate { + promise: Promise; + resolve(value: T): void; + reject(reason: unknown): void; + /** Whether the gate has been settled (either way). */ + readonly settled: boolean; +} + +/** A promise the test settles, with a flag the shell can be checked against. */ +export function gate(): Gate { + let settled = false; + let resolve!: (v: T) => void; + let reject!: (r: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = v => { + settled = true; + res(v); + }; + reject = r => { + settled = true; + rej(r); + }; + }); + return { + promise, + resolve, + reject, + get settled() { + return settled; + } + }; +} + +export interface Collected { + /** The first chunk, once it flushes. Rejects if it has not within `SHELL_TIMEOUT_MS`. */ + shell: Promise; + /** The whole response. */ + done: Promise<{ html: string; chunks: number }>; + /** Whether the first chunk has flushed yet. */ + shellFlushed(): boolean; + /** The gates that had settled at the moment the shell flushed. */ + settledAtShell: Set>; +} + +const SHELL_TIMEOUT_MS = 4000; + +/** + * Render and observe the chunk sequence. `gates` are watched so the test can + * ask which of them had settled when the shell went out — the ordering fact + * the wall clock used to stand in for. + */ +export function collect(code: () => any, gates: Gate[] = [], options?: any): Collected { + let shellFlushed = false; + const settledAtShell = new Set>(); + let resolveShell!: (s: string) => void; + let rejectShell!: (e: Error) => void; + const shell = new Promise((res, rej) => { + resolveShell = res; + rejectShell = rej; + }); + const timer = setTimeout( + () => rejectShell(new Error(`the shell did not flush within ${SHELL_TIMEOUT_MS}ms`)), + SHELL_TIMEOUT_MS + ); + const done = new Promise<{ html: string; chunks: number }>(resolve => { + const chunks: string[] = []; + renderToStream(code, options).pipe({ + write: (c: string) => { + if (!shellFlushed) { + shellFlushed = true; + for (const g of gates) if (g.settled) settledAtShell.add(g); + clearTimeout(timer); + resolveShell(c); + } + chunks.push(c); + }, + end: () => { + clearTimeout(timer); + resolve({ html: chunks.join(""), chunks: chunks.length }); + } + }); + }); + // A shell that never flushes surfaces through `shell`, not as an unhandled rejection. + shell.catch(() => {}); + return { shell, done, shellFlushed: () => shellFlushed, settledAtShell }; +} + +/** + * Let the renderer do everything it can with the gates as they are: drain + * microtasks and two macrotask turns. With no timer in the code under test, + * anything the shell was going to do without our gates has happened by now — + * so `shellFlushed()` afterwards is the ordering fact, not a race. + */ +export async function drain(): Promise { + await new Promise(r => setTimeout(r, 0)); + await new Promise(r => setTimeout(r, 0)); +} diff --git a/scripts/size/.size-limit.js b/scripts/size/.size-limit.js index c80b0a3ff..d9be74b6a 100644 --- a/scripts/size/.size-limit.js +++ b/scripts/size/.size-limit.js @@ -881,7 +881,14 @@ module.exports = [ // ~+200 B; pay-for-use, the price of a boundary that can tell a monitor // what it caught. Scenarios without a boundary did not move (`render`'s // write of `onError` onto the root owner is the only prod-floor cost). - limit: "19.80 KB", + // `_parent` reserved from property mangling (2026-09-16): 19.80 -> 19.85 KB, + // measured at 19,825 B. It is the second cross-package owner field beside + // `_name`: solid-js walks it on signals' owners (hydration's snapshot + // root — which the prod build was mis-marking while it was mangled) and + // the core walks it on solid-js's server owners (`ownerPath`, + // `OBSERVE.exclude`). ~+40 B across the prod scenarios; the observe ones + // moved by gzip noise or shrank. + limit: "19.85 KB", modifyEsbuildConfig }, { @@ -1161,7 +1168,14 @@ module.exports = [ // ~+180 B; pay-for-use, the price of a boundary that can tell a monitor // what it caught. Scenarios without a boundary did not move (`render`'s // write of `onError` onto the root owner is the only prod-floor cost). - limit: "15.15 KB", + // `_parent` reserved from property mangling (2026-09-16): 15.15 -> 15.20 KB, + // measured at 15,181 B. It is the second cross-package owner field beside + // `_name`: solid-js walks it on signals' owners (hydration's snapshot + // root — which the prod build was mis-marking while it was mangled) and + // the core walks it on solid-js's server owners (`ownerPath`, + // `OBSERVE.exclude`). ~+40 B across the prod scenarios; the observe ones + // moved by gzip noise or shrank. + limit: "15.20 KB", modifyEsbuildConfig }, {