Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .changeset/cross-package-owner-fields.md
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 2 additions & 0 deletions documentation/solid-2.0/08-dev-diagnostics.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<td>` 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"
Expand Down
2 changes: 1 addition & 1 deletion packages/signals/rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 13 additions & 6 deletions packages/signals/scripts/mangle-props.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "<App>"`) 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 =
// "<App>"`) 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
Expand Down
5 changes: 4 additions & 1 deletion packages/signals/tests/attribution-navigation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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);
});
});
15 changes: 11 additions & 4 deletions packages/signals/tests/heap-mark-incremental.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
29 changes: 6 additions & 23 deletions packages/solid/src/server/diagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ export type Finding = Omit<DiagnosticEvent, "sequence">;
* `in <App> › <Page>`, 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.
Expand All @@ -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 `<Name>`) — 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
Expand All @@ -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);
}

/**
Expand Down
115 changes: 115 additions & 0 deletions packages/solid/test/cross-package-fields.spec.ts
Original file line number Diff line number Diff line change
@@ -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<string> {
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<string, string> {
const seen = new Map<string, string>();
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([]);
});
});
Loading
Loading