From bdfc47b56cccd6bc7b1c97047d58b24ecd69fa24 Mon Sep 17 00:00:00 2001 From: Dev Agrawal Date: Sat, 7 Nov 2026 08:08:30 -0600 Subject: [PATCH 01/11] feat(signals): list what each drain showed on the flush record Co-authored-by: tembo[bot] <208362400+tembo-io[bot]@users.noreply.github.com> --- .changeset/flush-record-changes.md | 10 + documentation/solid-2.0/08-dev-diagnostics.md | 6 +- packages/signals/src/attribution.prod.ts | 2 + packages/signals/src/attribution.ts | 2 + .../signals/src/core/attribution-hooks.ts | 25 +-- packages/signals/src/core/attribution.ts | 159 ++++++++++++- packages/signals/src/core/dev.ts | 9 +- packages/signals/src/core/lanes.ts | 29 +-- packages/signals/src/core/scheduler.ts | 4 +- .../tests/attribution-flush-changes.test.ts | 208 ++++++++++++++++++ packages/signals/tests/dev.test.ts | 47 ---- packages/solid/test/dev.spec.ts | 22 -- 12 files changed, 401 insertions(+), 122 deletions(-) create mode 100644 .changeset/flush-record-changes.md create mode 100644 packages/signals/tests/attribution-flush-changes.test.ts diff --git a/.changeset/flush-record-changes.md b/.changeset/flush-record-changes.md new file mode 100644 index 0000000000..b34ef2939d --- /dev/null +++ b/.changeset/flush-record-changes.md @@ -0,0 +1,10 @@ +--- +"@solidjs/signals": patch +"solid-js": patch +--- + +The `flush` record lists what each drain changed on screen; `OPTIMISTIC_REVERTED` is judged from it; `DEV.hooks.onUpdate` removed + +- **`FlushEvent.changes`**: one entry per signal, memo or store property whose shown value the drain changed — by a commit, a held transition's landing, or an optimistic value appearing or lifting — with `nodeId`, `nodeName`, `key` (store properties) and `origin` (the action step, effect, interaction or navigation that staged the value; a held write keeps its origin, so a landing names its action). First values and effects are not listed. New types `FlushChange` and `FlushLive`: the record's `live` is now `{ changes: [{ node, prev?, value? }] }`, index for index with `event.changes`, the values only for a listener that subscribed with `{ bodies: true }`. +- **`AttributionHooks.staged(el)`** replaces `optimisticReverted`: core reports where a value was staged (`queuePendingNode`, `list`), never where it became visible; the engine compares what each staged node showed before and after the drain. `OPTIMISTIC_REVERTED` is that comparison's: a shown guess replaced by its covered value (`reverted`) or by a different one (`superseded`), judged at the end of the drain that shows the correction. +- **Removed**: `DEV.hooks.onUpdate`. Subscribe to `OBSERVE.records` `"flush"` instead (observe and dev builds; requires `attribution.enable()`). diff --git a/documentation/solid-2.0/08-dev-diagnostics.md b/documentation/solid-2.0/08-dev-diagnostics.md index 9f24bce003..af64ed78ba 100644 --- a/documentation/solid-2.0/08-dev-diagnostics.md +++ b/documentation/solid-2.0/08-dev-diagnostics.md @@ -651,7 +651,7 @@ When a hold commits, `stackedHolds.count` or more interactions (default 3) were **Message:** "the optimistic value of `status` showed "saved"; it reverted to "idle". The person saw the guess, then the correction. A revert on failure is the feature; one that recurs says the guess is wrong for this input or the action fails often — show the failure where the value renders (the action's catch, an `Errored` boundary) rather than letting the value snap back on its own." -An optimistic override (`createOptimistic`) the screen displayed was replaced by a different value. Two roads, named in `data.how`: `reverted` — nothing new landed and the override lifted back to the committed value it covered (the action failed, or never wrote what it promised); `superseded` — an authoritative value landed that differs from the guess (the server counted differently), and tracked readers re-derived to it. Both are correct by construction — the override reverting _is_ the feature — so the finding is `info`, structured channel only, always on while the engine is enabled: a count that grows for one source is what says the guess or the failure rate is wrong. Judged by the node's own `equals`, so a structurally equal correction is silent. The runtime's own optimistic nodes are never judged: an `isPending()`/`latest()` companion is an optimistic signal that goes `true` while pending and back to `false` at commit by design — the acknowledgement `SILENT_HOLD` asks for — and a derived override promotes rather than reverts; the check skips them by the same predicate the hold census uses. `optimisticReverts: false` disables. `data`: `source`, `shown`, `truth` (previews, 40 characters), `how`. Subject: the node, so `ownerPath` locates it. Not yet covered: optimistic _stores_ (`createOptimisticStore`), whose overlay folds off per path; and the interaction that wrote the guess (optimistic writes are not stamped with an origin today). +An optimistic override (`createOptimistic`) the screen displayed was replaced by a different value. Two roads, named in `data.how`: `reverted` — nothing new landed and the override lifted back to the committed value it covered (the action failed, or never wrote what it promised); `superseded` — an authoritative value landed that differs from the guess (the server counted differently), and tracked readers re-derived to it. Both are correct by construction — the override reverting _is_ the feature — so the finding is `info`, structured channel only, always on while the engine is enabled: a count that grows for one source is what says the guess or the failure rate is wrong. Judged at the end of the drain that shows the correction, by the node's own `equals`, so a structurally equal correction is silent. The runtime's own optimistic nodes are never judged: an `isPending()`/`latest()` companion is an optimistic signal that goes `true` while pending and back to `false` at commit by design — the acknowledgement `SILENT_HOLD` asks for — and a derived override promotes rather than reverts; the check skips them by the same predicate the hold census uses. `optimisticReverts: false` disables. `data`: `source`, `shown`, `truth` (previews, 40 characters), `how`. Subject: the node, so `ownerPath` locates it. Not yet covered: optimistic _stores_ (`createOptimisticStore`), whose overlay folds off per path; and the interaction that wrote the guess (optimistic writes are not stamped with an origin today). ### Server rendering (`ssr`, `head`) @@ -865,7 +865,7 @@ Each `DiagnosticEvent` has: | `nodeName` | `string?` | Debug name of the signal/node involved | | `data` | `object?` | Additional context | -An event is a serializable record and never carries the node it is about. The live node arrives **beside** it instead, as the listener's second argument — `OBSERVE.diagnostics.subscribe((event, subject) => …)` for findings, `OBSERVE.records.subscribe(type, (event, live) => …)` for records — for a consumer that runs in-process: the console reporter uses it to print the DOM element a binding effect writes; devtools use it to go from a record to the scope. `subject` is `undefined` for a finding with no location (an interaction has no node) or a host finding whose owners are not signals' owners; `live` is what the record type declares (the computation for a re-run, the held signal for a hold, nothing for a flush). A copy of the event that left the process and came back has no subject: the handle was never on it. +An event is a serializable record and never carries the node it is about. The live node arrives **beside** it instead, as the listener's second argument — `OBSERVE.diagnostics.subscribe((event, subject) => …)` for findings, `OBSERVE.records.subscribe(type, (event, live) => …)` for records — for a consumer that runs in-process: the console reporter uses it to print the DOM element a binding effect writes; devtools use it to go from a record to the scope. `subject` is `undefined` for a finding with no location (an interaction has no node) or a host finding whose owners are not signals' owners; `live` is what the record type declares (the computation for a re-run, the held signal for a hold, the changed nodes for a flush). A copy of the event that left the process and came back has no subject: the handle was never on it. ### `OBSERVE.records` — the runtimes' records channel @@ -1221,7 +1221,7 @@ createRoot(() => { **Records and clocks.** Everything the engine hands out — `RerunEvent`, `InteractionEvent`, `HoldEvent`, `NavigationEvent` — is a record with an absolute `at` on the `performance.now()` clock (`RerunEvent.at` the run's start, `HoldEvent.at` the start of the wait, `NavigationEvent.at`/`InteractionEvent.at` the request/dispatch) plus durations from it (`holdMs`, `settledMs`, `selfMs`). Epoch time for an exporter is `performance.timeOrigin + at` (milliseconds). Without cross-origin isolation the browser quantizes `performance.now()` to 100µs, so a single run's `selfMs` is often `0`; the per-interaction `settledMs` is the wall-clock number to report. Records are serializable as emitted: none carries a live graph reference — a re-run names its scope by `nodeId` (the engine's per-node id, stable across the scope's runs in the process, distinct between scopes; in-process consumers get the node itself as the listener's second argument, `live`) — while the frame objects that join records (`origin`, `interaction`) are the same object across records in-process, so join by identity there and by `ChangeOrigin.run`/`at`/`name` once they have left it. `OBSERVE.records.subscribe(type, listener)` delivers each record synchronously at the moment it is complete (a re-run at recompute end; an interaction, hold or navigation when it settles), bottom-up: a hold before the navigation it held, before the interaction that performed it — the same objects `history(type)` keeps. A listener runs inside the engine and must not write signals; hand work off to a microtask. A `RerunEvent` is built — dep diff, previews, cause list, ring-buffer push — only while something wants it: a `rerun` listener, an imported fold (`costs`/`feedback`) or the console log. Without one the engine still keeps its per-node facts and runs every check (hot runs, hot time, wasted recompute, dep width) from them, allocates no record, and `history("rerun")` stays empty. -**Timeline records.** Five further record types describe the work itself rather than its outcome, shaped for a profiler track (`@solidjs/web/performance-tracks`, below) and for the responsiveness findings; they enter no ring buffer and are built only while something is subscribed to them (`OBSERVE.records.observed(type)`), so an app that does not listen pays nothing beyond the hook null-check. `flush` — one per scheduler drain: `at`, `durationMs`, `runs` and `created` inside it, `held` (a transition parked), `interaction`. `create` — a computation's creation run (`RerunEvent`'s shape without causes: `nodeId`, `nodeName`, `nodeKind`, `at`, `selfMs`, `totalMs`, `depCount`, `phase`, `held`, `interaction`) — the mount work that no `RerunEvent` describes, and what `InteractionEvent.created` counts. `effect` — an effect's callback, the imperative half that writes the DOM: `nodeId`, `nodeName`, `at`, `durationMs`, `run`, `interaction`. `flight` — an async node's flight, kickoff to landing: `nodeId`, `nodeName`, `ownerPath`, `at`, `durationMs`, `outcome: "landed" | "abandoned"` (superseded before it landed), `interaction`. `fallback` — a `` boundary's fallback, displayed to hidden: `ownerPath`, `at`, `shownMs`, `interaction`. `at` is the end of the drain that rendered the swap — the boundary's swap is a staged write that lands with its frame ([RFC 05](05-async-data.md#loading-on-prop-dependencies-that-show-the-fallback-again)), and a swap cleared before that (the content landed before the frame did, or the commit's own sweep cleared it ahead of any effect — the `LOADING_ON_OUTSIDE_HOLD` shape) was never on screen and is no record. Every derived `ChangeRecord` on a re-run's cause chain also carries the `nodeId` of the memo it came from, so a consumer can walk the propagation of one write node by node. +**Timeline records.** Five further record types describe the work itself rather than its outcome, shaped for a profiler track (`@solidjs/web/performance-tracks`, below) and for the responsiveness findings; they enter no ring buffer and are built only while something is subscribed to them (`OBSERVE.records.observed(type)`), so an app that does not listen pays nothing beyond the hook null-check. `flush` — one per scheduler drain: `at`, `durationMs`, `runs` and `created` inside it, `held` (a transition parked), `interaction`, and `changes` — what the drain changed on screen: one entry per signal, memo or store property whose shown value differs from what it showed before the drain (`nodeId`, `nodeName`, `key` for a store property, and `origin`, the action step, effect, interaction or navigation that staged the value), whether it got there by a commit, a held transition's landing, or an optimistic value appearing or lifting. A held write is listed by the drain that lands it and keeps the origin it was staged under, so a landing names its action; a node's first value and effects are not listed. The record stays serializable: the nodes travel beside it (`live.changes`, index for index), with their values before and after only for a listener that subscribed with `{ bodies: true }` — live values, which a listener that keeps them copies on delivery. Core reports where a value was staged (`AttributionHooks.staged`); the engine compares what each staged node showed before and after the drain, which is also how `OPTIMISTIC_REVERTED` is judged. `create` — a computation's creation run (`RerunEvent`'s shape without causes: `nodeId`, `nodeName`, `nodeKind`, `at`, `selfMs`, `totalMs`, `depCount`, `phase`, `held`, `interaction`) — the mount work that no `RerunEvent` describes, and what `InteractionEvent.created` counts. `effect` — an effect's callback, the imperative half that writes the DOM: `nodeId`, `nodeName`, `at`, `durationMs`, `run`, `interaction`. `flight` — an async node's flight, kickoff to landing: `nodeId`, `nodeName`, `ownerPath`, `at`, `durationMs`, `outcome: "landed" | "abandoned"` (superseded before it landed), `interaction`. `fallback` — a `` boundary's fallback, displayed to hidden: `ownerPath`, `at`, `shownMs`, `interaction`. `at` is the end of the drain that rendered the swap — the boundary's swap is a staged write that lands with its frame ([RFC 05](05-async-data.md#loading-on-prop-dependencies-that-show-the-fallback-again)), and a swap cleared before that (the content landed before the frame did, or the commit's own sweep cleared it ahead of any effect — the `LOADING_ON_OUTSIDE_HOLD` shape) was never on screen and is no record. Every derived `ChangeRecord` on a re-run's cause chain also carries the `nodeId` of the memo it came from, so a consumer can walk the propagation of one write node by node. **Joining an interaction to the browser's INP entry.** An `InteractionEvent.at` is the DOM event's `timeStamp` when the runtime dispatched it (compiled event bindings pass it; `OBSERVE.attribution.withInteraction({ …, at: e.timeStamp }, fn)` for a custom dispatcher), and `inputDelayMs` is the queueing from that moment to the handler's entry — the browser's input delay. The `PerformanceEventTiming` entry for the same event (a `PerformanceObserver` on `"event"`) has `startTime === interaction.at` on the same clock and carries `interactionId`, so the join is `entry.startTime === interaction.at`: `inputDelayMs` accounts for the entry's `processingStart − startTime`, `handlerMs` for its processing, and `settledMs` extends past its `duration` to when Solid had the screen right. diff --git a/packages/signals/src/attribution.prod.ts b/packages/signals/src/attribution.prod.ts index 2a6a85334b..d5b0bfb712 100644 --- a/packages/signals/src/attribution.prod.ts +++ b/packages/signals/src/attribution.prod.ts @@ -57,7 +57,9 @@ export type { FallbackEvent, FlightEvent, FlightLink, + FlushChange, FlushEvent, + FlushLive, GraphEvent, GraphSize, HeldWrite, diff --git a/packages/signals/src/attribution.ts b/packages/signals/src/attribution.ts index eb80a229e5..f9376e4218 100644 --- a/packages/signals/src/attribution.ts +++ b/packages/signals/src/attribution.ts @@ -31,7 +31,9 @@ export type { FallbackEvent, FlightEvent, FlightLink, + FlushChange, FlushEvent, + FlushLive, GraphEvent, GraphSize, HeldWrite, diff --git a/packages/signals/src/core/attribution-hooks.ts b/packages/signals/src/core/attribution-hooks.ts index 792a8f64dc..4c58b2599f 100644 --- a/packages/signals/src/core/attribution-hooks.ts +++ b/packages/signals/src/core/attribution-hooks.ts @@ -94,6 +94,16 @@ export interface AttributionHooks { derivedChanged(el: Computed): void; /** A signal write committed (value passed the equality gate). */ write(el: Signal | Computed, prev: unknown, value: unknown): void; + /** + * A value for `el` was staged: queued for this flush's commit + * (`queuePendingNode`) or put on a transaction's or lane's list (`list`). + * What the screen shows of `el` may change at the end of this drain, or of + * a later one if the value is held. The engine compares what `el` showed + * before with what it shows at `flushEnd` — the one place a drain's visible + * changes are known — so core never has to say where a value became visible + * (a commit, a landing, a lane's reveal or lift). + */ + staged(el: Signal | Computed): void; /** refresh() invalidated this node (self-invalidation, no dep changed). */ refreshed(el: Computed): void; /** @@ -209,21 +219,6 @@ export interface AttributionHooks { shown: boolean, transition?: Transition | null ): void; - /** - * An optimistic override the screen displayed is being replaced by a - * different value. `"superseded"`: a new authoritative value landed that - * differs from the guess (tracked readers re-derive to it). `"reverted"`: - * nothing new landed and the guess lifts back to the committed value it - * covered (the action failed, or never wrote what it promised). `shown` - * is the override as displayed. Fired when the two differ by identity; - * the engine applies the node's own equality before judging. - */ - optimisticReverted( - el: Signal | Computed, - shown: unknown, - truth: unknown, - how: "superseded" | "reverted" - ): void; /** * The one query on the surface: the provenance a root write performed at * this moment would be stamped with — the innermost open frame (an effect diff --git a/packages/signals/src/core/attribution.ts b/packages/signals/src/core/attribution.ts index 1e6c06874c..fd9b366aa1 100644 --- a/packages/signals/src/core/attribution.ts +++ b/packages/signals/src/core/attribution.ts @@ -7,13 +7,17 @@ import { } from "./attribution-hooks.js"; import { CONFIG_GUESS, + CONFIG_HELD, CONFIG_OVERRIDE, + CONFIG_SLOT_NODE, CONFIG_KEY_BAG, CONFIG_PLUMBING, CONFIG_WIDE, CONFIG_SUPERSEDES, CONFIG_VERDICT, - NOT_PENDING + NOT_PENDING, + REACTIVE_DISPOSED, + STATUS_UNINITIALIZED } from "./constants.js"; import { anyExcluded, @@ -291,6 +295,40 @@ export interface FlushEvent { * interactions shared the drain. */ interaction?: ChangeOrigin; + /** + * What the drain changed on screen: each signal, memo or store property + * whose shown value differs from what it showed before the drain — a + * commit, a held transition's landing, an optimistic value appearing or + * lifting. One entry per node, in no promised order. Nodes that got their + * first value, and effects, are not listed. + */ + changes: FlushChange[]; +} + +/** One node whose shown value a drain changed (`FlushEvent.changes`). */ +export interface FlushChange { + /** The node, in the id space of `RerunEvent.nodeId` and `ChangeRecord.nodeId`. */ + nodeId: number; + nodeName: string; + /** A store property's node: the property, as a string. */ + key?: string; + /** + * Who staged the value that is now shown — the action step, effect, + * interaction or navigation — when one was open. A held write keeps the + * origin it was staged under, so a landing's changes name their action. + */ + origin?: ChangeOrigin; +} + +/** + * Beside a `flush` record (`OBSERVE.records.subscribe("flush", (event, live) => …)`): + * the live side of `event.changes`, index for index — the node, and, for a + * listener that subscribed with `{ bodies: true }`, the values before and + * after. The values are live, not copies: a store's objects keep changing + * after the record, so a listener that keeps them copies them on delivery. + */ +export interface FlushLive { + changes: { node: Signal | Computed; prev?: unknown; value?: unknown }[]; } /** @@ -3754,6 +3792,113 @@ function noteFlushRun( else if (flush.interaction !== interaction) flush.interaction = null; } +// --- What each drain showed --------------------------------------------------- +// +// Core says where a value was staged (`staged`: queued for a commit, or put +// on a transaction's or a lane's list), never where it became visible: a +// commit, a held transition's landing, a lane's reveal or its lift. The +// engine keeps what the screen showed of each staged node and compares it +// with what it shows at `flushEnd`. A difference is a change the drain +// showed. A node stays watched while its shown value can still change +// without a new staging (held, or on a lane), and is dropped once settled. +// +// The same comparison is OPTIMISTIC_REVERTED's: a guess the screen showed +// that is replaced by something else. Its truth is the value it covered +// (reverted: nothing came true) or a different one (superseded). +interface Watched { + /** What the screen showed of the node at the last drain end (or before its staging). */ + shown: unknown; + /** `shown` is an optimistic guess. */ + guess: boolean; + /** The committed value beneath the guess, while `guess`. */ + covered: unknown; + /** Who staged the value the drain will show, when an origin was open. */ + origin: ChangeOrigin | undefined; +} +const watched = new Map | Computed, Watched>(); + +/** What the screen shows of `n`: a revealed lane's value, else the committed one (NOT_PENDING: none yet). */ +function shownOf(n: Signal | Computed): unknown { + const x = n._x; + if (n._config & CONFIG_OVERRIDE && x != null && x._transaction?._shown && x._lane !== NOT_PENDING) + return x._lane; + return (n as Computed)._statusFlags & STATUS_UNINITIALIZED ? NOT_PENDING : n._value; +} + +/** The screen shows `n`'s optimistic guess (not a derived override or a companion). */ +function showsGuess(n: Signal | Computed): boolean { + return ( + (n._config & (CONFIG_OVERRIDE | CONFIG_GUESS)) === (CONFIG_OVERRIDE | CONFIG_GUESS) && + !!n._x!._transaction?._shown && + n._x!._lane !== NOT_PENDING + ); +} + +/** Its shown value can still change without a new staging: staged, held, or on a lane. */ +function unsettled(n: Signal | Computed): boolean { + return ( + n._pendingValue !== NOT_PENDING || + (n._config & (CONFIG_HELD | CONFIG_OVERRIDE)) !== 0 || + (n._x != null && n._x._transaction !== null) + ); +} + +function trackStaged(el: Signal | Computed): void { + // Effects are not state; their values are what they rendered. + if ((el as { _type?: number })._type) return; + const origin = ambientOrigin(); + const w = watched.get(el); + if (w !== undefined) { + if (origin !== undefined) w.origin = origin; + return; + } + // Watched for the flush record's audience, or for the revert finding (a + // guess enters a lane through `list`). + if (!records.observed("flush") && !(options.optimisticReverts && el._x?._transaction?._lane)) + return; + const guess = showsGuess(el); + watched.set(el, { shown: shownOf(el), guess, covered: el._value, origin }); +} + +/** flushEnd: compare, judge the reverts, and list the changes for the record (`live` beside it). */ +function drainChanges(record: boolean, live: FlushLive["changes"]): FlushChange[] { + const changes: FlushChange[] = []; + if (watched.size === 0) return changes; + const bodies = record && records.observed("flush", "bodies"); + for (const [n, w] of watched) { + if ((n as Computed)._flags & REACTIVE_DISPOSED) { + watched.delete(n); + continue; + } + const now = shownOf(n); + if (!Object.is(now, w.shown)) { + const guess = showsGuess(n); + if (w.guess && !guess && now !== NOT_PENDING && w.shown !== NOT_PENDING) + checkOptimisticRevert( + n, + w.shown, + now, + Object.is(now, w.covered) ? "reverted" : "superseded" + ); + if (record && w.shown !== NOT_PENDING && now !== NOT_PENDING && !excludedNode(n)) { + const change: FlushChange = { nodeId: devId(n), nodeName: nodeName(n) }; + if (n._config & CONFIG_SLOT_NODE) change.key = String((n as { _key?: PropertyKey })._key); + if (w.origin !== undefined) change.origin = w.origin; + changes.push(change); + live.push(bodies ? { node: n, prev: w.shown, value: now } : { node: n }); + } + w.shown = now; + w.origin = undefined; + // A guess's truth is what it covered when it appeared; a new guess over + // a shown guess keeps covering the same truth. + if (guess && !w.guess) w.covered = n._value; + w.guess = guess; + } + if (!unsettled(n)) watched.delete(n); + } + return changes; +} + function trackFlushStart(): void { if (!records.observed("flush")) return; openFlush = { at: now(), runs: 0, created: 0, held: false, interaction: undefined }; @@ -3885,6 +4030,8 @@ function rebaseFallbacks(from: Transition, into: Transition | typeof DRAIN): voi function trackFlushEnd(): void { // The drain's own record first: the interactions it settles below waited on it. const flush = openFlush; + const live: FlushLive = { changes: [] }; + const changes = drainChanges(flush !== null, live.changes); if (flush !== null) { openFlush = null; const event: FlushEvent = { @@ -3892,10 +4039,11 @@ function trackFlushEnd(): void { durationMs: now() - flush.at, runs: flush.runs, created: flush.created, - held: flush.held + held: flush.held, + changes }; if (flush.interaction != null) event.interaction = flush.interaction; - records.emit("flush", event, undefined); + records.emit("flush", event, live); } drainSeq++; // The swaps this drain committed have rendered. @@ -4509,8 +4657,8 @@ const engineHooks: AttributionHooks = { // The folds hear the show at its display (see trackFallback), not here. trackFallback(boundary, tree, shown, transition ?? null); }, - optimisticReverted(el, shown, truth, how) { - checkOptimisticRevert(el, shown, truth, how); + staged(el) { + trackStaged(el); }, currentOrigin() { return ambientOrigin(); @@ -4552,6 +4700,7 @@ function resetWindows(): void { /** The live tracking state — reset only when the engine is (un)installed. */ function resetTracking(): void { frames.length = 0; + watched.clear(); activeHold = null; openFlush = null; stagedFallbacks.delete(DRAIN); diff --git a/packages/signals/src/core/dev.ts b/packages/signals/src/core/dev.ts index c8732dcb02..88b5faf6c9 100644 --- a/packages/signals/src/core/dev.ts +++ b/packages/signals/src/core/dev.ts @@ -13,6 +13,7 @@ import type { FallbackEvent, FlightEvent, FlushEvent, + FlushLive, GraphEvent, HoldEvent, InteractionEvent, @@ -38,7 +39,6 @@ import type { Computed, Link, Owner, Root, Signal } from "./types.js"; export interface DevHooks { onOwner?: (owner: Owner) => void; onGraph?: (value: any, owner: Owner | null) => void; - onUpdate?: () => void; onStoreNodeUpdate?: (state: any, property: PropertyKey, value: any, prev: any) => void; /** @internal Every node read (the frames client's identity-drift check; `setReadWatch`). */ onRead?: (node: object) => void; @@ -302,8 +302,9 @@ export interface AttributionSlot { * about where there is one — the node that ran for `rerun`, `create` and * `effect`, the async node for `flight`, the boundary's subtree for * `fallback` (when the boundary reported one), the first held root signal - * for `hold` (the subject the SILENT_HOLD finding names) — and `undefined` - * for the records with no single subject (`flush`, `interaction`, + * for `hold` (the subject the SILENT_HOLD finding names), the changed + * nodes for `flush` (`FlushLive`, with their values under `bodies`) — and + * `undefined` for the records with no single subject (`interaction`, * `navigation`, `graph`). The records are the same objects the engine's * ring buffers hold (`attribution.history(type)`), delivered synchronously * the moment each is complete — a re-run at recompute end, a hold, a @@ -319,7 +320,7 @@ export interface RecordTypes extends HostRecordTypes { rerun: { event: RerunEvent; live: Computed }; create: { event: CreateEvent; live: Computed }; effect: { event: EffectRunEvent; live: Computed }; - flush: { event: FlushEvent; live: undefined }; + flush: { event: FlushEvent; live: FlushLive }; flight: { event: FlightEvent; live: Computed }; fallback: { event: FallbackEvent; live: Computed | undefined }; interaction: { event: InteractionEvent; live: undefined }; diff --git a/packages/signals/src/core/lanes.ts b/packages/signals/src/core/lanes.ts index 9d34da57d5..73725e4a46 100644 --- a/packages/signals/src/core/lanes.ts +++ b/packages/signals/src/core/lanes.ts @@ -60,7 +60,6 @@ import { STATUS_PENDING, STATUS_UNINITIALIZED } from "./constants.js"; -import { attrHooks } from "./attribution-hooks.js"; import { bornInto, creatorReads, ext, stagedRead, tracking } from "./core.js"; import { NotReadyError, type StatusError } from "./error.js"; import { @@ -509,8 +508,8 @@ export function laneStage( * rows) must not tie them together; a pass that derives from the held * truth joins the parent on its own (`joinPass`). */ /** The correction round is replacing a guess with the value it covered (no - * landing beneath it): observe's "reverted"; the lane gives back the - * guesses it took (#4336, `endLanes`). */ + * landing beneath it): the lane gives back the guesses it took (#4336, + * `endLanes`). */ let reverting = false; export function supersede(n: Signal | Computed, value: unknown, changed: boolean): void { @@ -525,11 +524,6 @@ export function supersede(n: Signal | Computed, value: unknown, change if (changed && n._config & CONFIG_SLOT_NODE && GlobalQueue._echoes?.(n._x!._lane, value)) changed = false; if (changed) { - // Observe: a displayed guess is being replaced by a differing truth — a - // landing's (superseded), or the value it covered (reverted: the body - // ended with nothing coming true). - if (__OBSERVE__ && l._shown && attrHooks !== null) - attrHooks.optimisticReverted(n, n._x!._lane, value, reverting ? "reverted" : "superseded"); dissolveLane(l, parent, n); if (guessed && reverting) marked.add(l); // What the screen showed stays the screen until the parent lands (A18 @@ -591,20 +585,7 @@ function dissolveLane(l: Transaction, into: Transaction | null, except?: Signal< if (into === null) { // The parent landed: a guess lands the truth beneath it or reverts to // the one it covered; a derivation's latest commits (its frame with - // it); what the screen showed and changes notifies. Observe: a - // displayed guess that lifts to something else — the truth staged - // beneath it (superseded by a landing held there) or the value it - // covered (reverted: nothing came true). - if (__OBSERVE__ && guess && l._shown && attrHooks !== null) { - const truth = n._pendingValue !== NOT_PENDING ? n._pendingValue : n._value; - if (truth !== slot) - attrHooks.optimisticReverted( - n, - slot, - truth, - n._pendingValue !== NOT_PENDING ? "superseded" : "reverted" - ); - } + // it); what the screen showed and changes notifies. // A derivation the revert re-derives (dirtied by the guess's // notification — listed before it), or one still in flight, shows the // lane's answer beside inputs that are the truth now: a fresh reader @@ -866,9 +847,9 @@ function laneCorrections(): boolean { const n = l._nodes[i]; if (n._config & CONFIG_GUESS && n._x!._transaction === l) { const truth = n._pendingValue !== NOT_PENDING ? n._pendingValue : covered(n); - if (__OBSERVE__ || guessed) reverting = n._pendingValue === NOT_PENDING; + if (guessed) reverting = n._pendingValue === NOT_PENDING; supersede(n, truth, !n._equals || !n._equals(n._x!._lane, truth)); - if (__OBSERVE__ || guessed) reverting = false; + if (guessed) reverting = false; } } } diff --git a/packages/signals/src/core/scheduler.ts b/packages/signals/src/core/scheduler.ts index 99351a9e75..78983aba41 100644 --- a/packages/signals/src/core/scheduler.ts +++ b/packages/signals/src/core/scheduler.ts @@ -41,7 +41,6 @@ import { resyncUnflushedCompanions } from "./core.js"; import { - DEV, emitDiagnostic, GRAPH_SIZE_WARN_AT, locationLines, @@ -626,6 +625,7 @@ export function list(n: Signal, t: Transaction): void { if (x._transaction === t) return; x._transaction = t; t._nodes.push(n); + if (__OBSERVE__ && attrHooks !== null) attrHooks.staged(n); } /** Hold the (uncommitted) frame under a held pass: every computed in the @@ -966,7 +966,6 @@ export class GlobalQueue implements IQueue { n => pendingNodes.includes(n) || (n._config & (CONFIG_HELD | CONFIG_OVERRIDE)) !== 0 ); } - if (__DEV__) DEV.hooks.onUpdate?.(); } finally { this._running = false; passTx = null; @@ -1359,6 +1358,7 @@ export function queuePendingNode(node: Signal): void { if (__DEV__) lastStagedNodeName = (node as any)._name ?? null; pendingNodes.push(node); if (!globalQueue._running) markUnflushedStaged(); // A28 + if (__OBSERVE__ && attrHooks !== null) attrHooks.staged(node); } // Dev-only attribution for the flush loop guard (#3140): when the guard diff --git a/packages/signals/tests/attribution-flush-changes.test.ts b/packages/signals/tests/attribution-flush-changes.test.ts new file mode 100644 index 0000000000..8b2c4e3402 --- /dev/null +++ b/packages/signals/tests/attribution-flush-changes.test.ts @@ -0,0 +1,208 @@ +/** + * `FlushEvent.changes` — what each drain changed on screen. + * + * Claim under test: a `flush` record lists every signal, memo and store + * property whose shown value the drain changed — once per node, with the + * origin that staged the value — and nothing else: not a write that is held, + * not a node's first value, not an effect. A held write is listed by the + * drain that lands it, still naming the action that wrote it; an optimistic + * value is listed when it appears and again when it lifts. Values travel only + * to a listener that asked for bodies. + */ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { attribution, type FlushEvent, type FlushLive } from "../src/attribution.js"; +import { + action, + createMemo, + createOptimistic, + createRenderEffect, + createRoot, + createSignal, + createStore, + flush, + OBSERVE +} from "../src/index.js"; +import type { RecordSubscribeOptions } from "../src/core/dev.js"; + +const offs: (() => void)[] = []; +afterEach(() => { + for (const off of offs.splice(0)) off(); + attribution.disable(); + flush(); + vi.restoreAllMocks(); +}); + +type Flush = FlushEvent & { live: FlushLive }; +function arm(options?: RecordSubscribeOptions): Flush[] { + vi.spyOn(console, "warn").mockImplementation(() => {}); + vi.spyOn(console, "info").mockImplementation(() => {}); + attribution.enable({ log: false, checks: false, waterfalls: false }); + const flushes: Flush[] = []; + offs.push( + OBSERVE!.records.subscribe("flush", (e, live) => flushes.push({ ...e, live }), options) + ); + return flushes; +} +/** Each change with its live side: name, key, and the values when bodies were asked for. */ +const seen = (flushes: Flush[]) => + flushes.flatMap(f => f.changes.map((c, i) => ({ ...c, ...f.live.changes[i], node: undefined }))); + +const CLICK = { type: "click", target: "button#save" }; +const click = (fn: () => T) => OBSERVE!.attribution.withInteraction(CLICK, fn); +const names = (e: FlushEvent) => e.changes.map(c => c.nodeName).sort(); + +function deferred() { + let resolve!: () => void; + const promise = new Promise(r => (resolve = r)); + return { promise, resolve }; +} + +describe("FlushEvent.changes", () => { + it("lists the drain's changed signals and memos, with the origin that staged them", () => { + const flushes = arm(); + const [count, setCount] = createSignal(0, { name: "count" }); + createRoot(() => { + const double = createMemo(() => count() * 2, { name: "double" }); + createRenderEffect(double, () => {}, { name: "paint" }); + }); + flush(); + // First values are not changes. + expect(flushes.flatMap(f => f.changes)).toEqual([]); + flushes.length = 0; + + click(() => setCount(1)); + flush(); + expect(flushes).toHaveLength(1); + expect(names(flushes[0])).toEqual(["count", "double"]); + const change = flushes[0].changes.find(c => c.nodeName === "count")!; + expect(change.origin).toMatchObject({ kind: "interaction", name: "click" }); + expect(change.nodeId).toEqual(expect.any(Number)); + // The node travels beside the record; without bodies, no values. + expect(flushes[0].live.changes.map(c => Object.keys(c))).toEqual([["node"], ["node"]]); + }); + + it("gives one record per drain: two synchronous flushes are two", () => { + const flushes = arm(); + const [a, setA] = createSignal(0, { name: "a" }); + const [b, setB] = createSignal(0, { name: "b" }); + createRoot(() => + createRenderEffect( + () => a() + b(), + () => {}, + { name: "paint" } + ) + ); + flush(); + flushes.length = 0; + setA(1); + flush(); + setB(1); + flush(); + expect(flushes.map(names)).toEqual([["a"], ["b"]]); + }); + + it("carries the values before and after only for a listener that asked for bodies", () => { + const flushes = arm({ bodies: true }); + const [count, setCount] = createSignal(1, { name: "count" }); + createRoot(() => createRenderEffect(count, () => {}, { name: "paint" })); + flush(); + setCount(2); + flush(); + expect(seen(flushes.slice(-1))).toEqual([ + expect.objectContaining({ nodeName: "count", prev: 1, value: 2 }) + ]); + // The event itself carries no values: it stays serializable. + expect("value" in flushes.at(-1)!.changes[0]).toBe(false); + }); + + it("names a store property by its key", () => { + const flushes = arm({ bodies: true }); + const [state, setState] = createStore({ title: "a", done: false }); + createRoot(() => + createRenderEffect( + () => state.title, + () => {}, + { name: "paint" } + ) + ); + flush(); + flushes.length = 0; + setState(s => { + s.title = "b"; + }); + flush(); + expect(seen(flushes)).toEqual([ + expect.objectContaining({ key: "title", prev: "a", value: "b" }) + ]); + }); + + it("lists a held write in the drain that lands it, naming the action that wrote it", async () => { + const flushes = arm(); + const gate = deferred(); + const [saved, setSaved] = createSignal("draft", { name: "saved" }); + createRoot(() => createRenderEffect(saved, () => {}, { name: "paint" })); + flush(); + flushes.length = 0; + const submit = action(function* save() { + setSaved("final"); + yield gate.promise; + }); + const done = submit(); + flush(); + // Held: the screen still shows "draft". + expect(saved()).toBe("draft"); + expect(flushes.flatMap(f => f.changes)).toEqual([]); + gate.resolve(); + await done; + flush(); + const landed = flushes.flatMap(f => f.changes); + expect(landed).toEqual([expect.objectContaining({ nodeName: "saved" })]); + expect(landed[0].origin).toMatchObject({ kind: "action", name: "save" }); + }); + + it("lists an optimistic value when it appears and again when it lifts", async () => { + const flushes = arm({ bodies: true }); + const gate = deferred(); + const [status, setStatus] = createOptimistic("idle", { name: "status" }); + createRoot(() => createRenderEffect(status, () => {}, { name: "badge" })); + flush(); + flushes.length = 0; + const save = action(function* save() { + setStatus("saving"); + yield gate.promise; + }); + const done = save(); + flush(); + expect(status()).toBe("saving"); + expect(seen(flushes)).toEqual([ + expect.objectContaining({ nodeName: "status", prev: "idle", value: "saving" }) + ]); + flushes.length = 0; + gate.resolve(); + await done; + flush(); + expect(status()).toBe("idle"); + expect(seen(flushes)).toEqual([ + expect.objectContaining({ nodeName: "status", prev: "saving", value: "idle" }) + ]); + }); + + it("is empty for a drain that changed nothing on screen", () => { + const flushes = arm(); + const [count, setCount] = createSignal(0, { name: "count" }); + createRoot(() => + createRenderEffect( + () => count() % 2, + () => {}, + { name: "parity" } + ) + ); + flush(); + flushes.length = 0; + setCount(0); + setCount(2); + flush(); + // `count` changed (0 → 2); the effect's output did not, and effects are never listed. + expect(flushes.map(names)).toEqual([["count"]]); + }); +}); diff --git a/packages/signals/tests/dev.test.ts b/packages/signals/tests/dev.test.ts index 37eb369c5f..c3b5519135 100644 --- a/packages/signals/tests/dev.test.ts +++ b/packages/signals/tests/dev.test.ts @@ -19,7 +19,6 @@ const { getChildren, getSignals, getParent, getSources, getObservers } = DEV!; afterEach(() => { DEV!.hooks.onOwner = undefined; DEV!.hooks.onGraph = undefined; - DEV!.hooks.onUpdate = undefined; DEV!.hooks.onStoreNodeUpdate = undefined; }); @@ -182,52 +181,6 @@ describe("Phase 1b: Notification hooks", () => { }); }); - describe("onUpdate", () => { - it("fires once per flush", () => { - const calls: number[] = []; - DEV!.hooks.onUpdate = () => calls.push(1); - let setCount: (v: number) => void; - createRoot(() => { - const [count, _setCount] = createSignal(0); - setCount = _setCount; - createEffect( - () => count(), - () => {} - ); - flush(); - expect(calls.length).toBe(1); - }); - - setCount!(1); - flush(); - expect(calls.length).toBe(2); - }); - - it("fires once even with multiple signal writes", () => { - const calls: number[] = []; - DEV!.hooks.onUpdate = () => calls.push(1); - let setA: (v: number) => void; - let setB: (v: number) => void; - createRoot(() => { - const [a, _setA] = createSignal(0); - const [b, _setB] = createSignal(0); - setA = _setA; - setB = _setB; - createEffect( - () => a() + b(), - () => {} - ); - flush(); - }); - const before = calls.length; - - setA!(1); - setB!(2); - flush(); - expect(calls.length - before).toBe(1); - }); - }); - describe("onStoreNodeUpdate", () => { it("fires on store property mutation with old and new values", () => { const updates: any[] = []; diff --git a/packages/solid/test/dev.spec.ts b/packages/solid/test/dev.spec.ts index d91cfec7c4..c4d39867f8 100644 --- a/packages/solid/test/dev.spec.ts +++ b/packages/solid/test/dev.spec.ts @@ -19,7 +19,6 @@ afterEach(() => { if (DEV) { DEV.hooks.onOwner = undefined; DEV.hooks.onGraph = undefined; - DEV.hooks.onUpdate = undefined; DEV.hooks.onStoreNodeUpdate = undefined; } }); @@ -330,27 +329,6 @@ describe("DEV.hooks", () => { expect(entries.length).toBeGreaterThanOrEqual(1); }); - test("onUpdate fires after flush", () => { - const calls: number[] = []; - let setCount!: (value: number) => number; - DEV!.hooks.onUpdate = () => calls.push(1); - - createRoot(() => { - const [count, _setCount] = createSignal(0); - setCount = _setCount; - createEffect( - () => count(), - () => {} - ); - flush(); - }); - - const initial = calls.length; - setCount(1); - flush(); - expect(calls.length).toBe(initial + 1); - }); - test("onStoreNodeUpdate fires on store mutation with old/new values", () => { const updates: any[] = []; DEV!.hooks.onStoreNodeUpdate = (state: any, property: PropertyKey, value: any, prev: any) => From 69a10f2e2bc55320c89961fa6e83421d8595725e Mon Sep 17 00:00:00 2001 From: Dev Agrawal Date: Sat, 7 Nov 2026 16:42:55 -0600 Subject: [PATCH 02/11] refactor(signals): fold derivedChanged into recomputeEnd with phase Co-authored-by: tembo[bot] <208362400+tembo-io[bot]@users.noreply.github.com> --- .../signals/src/core/attribution-hooks.ts | 15 ++++------ packages/signals/src/core/attribution.ts | 30 +++++-------------- packages/signals/src/core/core.ts | 17 ++++++----- 3 files changed, 23 insertions(+), 39 deletions(-) diff --git a/packages/signals/src/core/attribution-hooks.ts b/packages/signals/src/core/attribution-hooks.ts index 4c58b2599f..1b7ccc84fe 100644 --- a/packages/signals/src/core/attribution-hooks.ts +++ b/packages/signals/src/core/attribution-hooks.ts @@ -76,22 +76,19 @@ export interface AttributionHooks { recomputeStart(el: Computed, create: boolean): void; /** * The recompute finished. `changed` = committed a changed value (false for - * errored runs); `optimistic` = ran under an optimistic lane / lane-dirty - * posture; `transition` = a transition was active or owns this node; - * `held` = the value went to `_pendingValue` (a transition hold) rather - * than committing directly — its reveal happens later on the transition's - * own schedule. + * errored runs) — for a memo's re-run, a derived change its subscribers + * will name as their cause; `phase` = the posture it ran under: a lane's + * work (`"optimistic"`), a hold open or owning the node (`"held"`), or + * neither (`"plain"`); `held` = the value went to `_pendingValue` rather + * than committing directly — its reveal happens later on its own schedule. */ recomputeEnd( el: Computed, create: boolean, changed: boolean, - optimistic: boolean, - transition: boolean, + phase: "plain" | "held" | "optimistic", held: boolean ): void; - /** A non-effect computed committed a changed value during a re-run. */ - derivedChanged(el: Computed): void; /** A signal write committed (value passed the equality gate). */ write(el: Signal | Computed, prev: unknown, value: unknown): void; /** diff --git a/packages/signals/src/core/attribution.ts b/packages/signals/src/core/attribution.ts index fd9b366aa1..9e4f35248c 100644 --- a/packages/signals/src/core/attribution.ts +++ b/packages/signals/src/core/attribution.ts @@ -4467,12 +4467,12 @@ const engineHooks: AttributionHooks = { : enclosingInteraction() }); }, - derivedChanged(el) { - const frame = frames[frames.length - 1]; - stampDerived(el, frame !== undefined && frame.causes !== null ? frame.causes : []); - }, - recomputeEnd(el, _create, changed, optimistic, transition, held) { + recomputeEnd(el, create, changed, phase, held) { const frame = frames.pop(); + // A memo's re-run that changed its value is a derived change: its + // subscribers name it as their cause, chaining back to the root write. + if (changed && !create && !(el as { _type?: number })._type) + stampDerived(el, frame !== undefined && frame.causes !== null ? frame.causes : []); // enable() can land mid-recompute: no opening frame, nothing to report. if (frame === undefined) return; const totalMs = now() - frame.start; @@ -4495,13 +4495,7 @@ const engineHooks: AttributionHooks = { // plain-flush memo commits and in `_value` for direct ones. Overlay runs // are excluded — an optimistic re-derive legitimately produces fresh // equivalents while the lane settles. - if ( - frame.causes !== null && - changed && - !optimistic && - !transition && - !(el as { _type?: number })._type - ) + if (frame.causes !== null && changed && phase === "plain" && !(el as { _type?: number })._type) checkUnstableOutput( el, frame.prevValue, @@ -4513,15 +4507,7 @@ const engineHooks: AttributionHooks = { // proves nothing. Reported as changed, so no consumer counts it as waste. if (!changed && (el._pendingValue !== NOT_PENDING ? el._pendingValue : el._value) === undefined) changed = true; - if (frame.causes !== null) - recordRerun( - el, - frame, - { selfMs, totalMs }, - changed, - optimistic ? "optimistic" : transition ? "held" : "plain", - held - ); + if (frame.causes !== null) recordRerun(el, frame, { selfMs, totalMs }, changed, phase, held); else if (!excludedNode(el)) { // Creation runs still get the wide-scope check: a memo can be born with // its coarse-read problem already in place — and their time is charged @@ -4541,7 +4527,7 @@ const engineHooks: AttributionHooks = { depCount: captureDeps(el).length, selfMs, totalMs, - phase: optimistic ? "optimistic" : transition ? "held" : "plain", + phase, held }; if (frame.interaction !== undefined) event.interaction = frame.interaction; diff --git a/packages/signals/src/core/core.ts b/packages/signals/src/core/core.ts index 82dc36ac19..593c39b7d7 100644 --- a/packages/signals/src/core/core.ts +++ b/packages/signals/src/core/core.ts @@ -785,7 +785,7 @@ export function recompute(el: Computed, create: boolean = false): void { clearDeps(el); clearInFlight(el); if (__OBSERVE__ && attrHooks !== null) - attrHooks.recomputeEnd(el, create, false, false, false, false); + attrHooks.recomputeEnd(el, create, false, "plain", false); if (laneStageHook) setPassLane(prevLane); return; } @@ -845,11 +845,9 @@ export function recompute(el: Computed, create: boolean = false): void { !isSame(el, compareValue, value); // A committed derived change becomes a cause for this node's subscribers, - // chaining their attribution through this node to the root write. - if (__OBSERVE__ && attrHooks !== null) { - devChanged = !!valueChanged && !el._x?._error; - if (devChanged && !isEffect && !create) attrHooks.derivedChanged(el); - } + // chaining their attribution through this node to the root write + // (`recomputeEnd`'s `changed`). + if (__OBSERVE__ && attrHooks !== null) devChanged = !!valueChanged && !el._x?._error; // Effects use `_equals: false` (no per-effect closure). The side effects that // the equals closure used to perform — flagging the effect dirty and enqueueing @@ -1048,8 +1046,11 @@ export function recompute(el: Computed, create: boolean = false): void { el, create, devChanged, - lane !== null, - flushTransaction !== null || passTx !== null || (el._config & CONFIG_HELD) !== 0, + lane !== null + ? "optimistic" + : flushTransaction !== null || passTx !== null || (el._config & CONFIG_HELD) !== 0 + ? "held" + : "plain", el._pendingValue !== NOT_PENDING ); // A staged value, a parked frame (L2: the commit retires it), or status the From 5cf357b47fdb3457a2615db989b95dd0a5ece94e Mon Sep 17 00:00:00 2001 From: Dev Agrawal Date: Sat, 7 Nov 2026 16:46:18 -0600 Subject: [PATCH 03/11] refactor(signals): report async landings once, with whether they changed Co-authored-by: tembo[bot] <208362400+tembo-io[bot]@users.noreply.github.com> --- packages/signals/src/core/async.ts | 44 +++++++++++-------- .../signals/src/core/attribution-hooks.ts | 14 +++--- packages/signals/src/core/attribution.ts | 36 ++------------- packages/signals/src/core/core.ts | 29 +++++++++++- 4 files changed, 62 insertions(+), 61 deletions(-) diff --git a/packages/signals/src/core/async.ts b/packages/signals/src/core/async.ts index ce55348187..e20d263e39 100644 --- a/packages/signals/src/core/async.ts +++ b/packages/signals/src/core/async.ts @@ -28,7 +28,14 @@ import { NewSet } from "./constants.js"; import { attrHooks } from "./attribution-hooks.js"; -import { context, setSignal, untrack, ext, statusNotifierOf } from "./core.js"; +import { + context, + setSignal, + untrack, + ext, + statusNotifierOf, + landThroughSetSignal +} from "./core.js"; import { devTrackHeldPending } from "./invariants.js"; import { caughtReadFlightSettled, @@ -612,10 +619,10 @@ export function handleAsync( const carried = holdWorkHook && pendingNodes.some(n => n._pendingValue !== NOT_PENDING); const wasUninitialized = el._statusFlags & STATUS_UNINITIALIZED; landStatus(el); - // Attribution hook: lets the engine snapshot state before the landing - // branches, so it can tell whether the plain path's setSignal committed a - // change (and only then classify it as an async landing). - if (__OBSERVE__ && attrHooks !== null) attrHooks.asyncStart(el); + // Observe: what a setter landing moves on the node, to say whether it committed. + const observed = __OBSERVE__ && attrHooks !== null; + const before = observed ? el._value : undefined; + const beforeTime = observed ? el._time : 0; if (setter) { try { setter(value); @@ -635,14 +642,20 @@ export function handleAsync( if (wasUninitialized) landStatus(el, true); // Attribution hook: this path landed through the setter (derived // stores, projections), which does not go through setSignal, so the - // plain branch's asyncEnd never ran and the flight stayed open — the - // next flight counted a committed answer as abandoned (#3947). - // direct=true: asyncEnd finalizes either way and stamps an async write - // only if the commit moved `_value`/`_time` or parked `_pendingValue`. - // A store setter does not move this node's `_value`, so no stamp is - // recorded and waterfall chaining through derived stores stays - // unmeasured. Outside the try (#2883 — see attribution-hooks.ts). - if (__OBSERVE__ && attrHooks !== null) attrHooks.asyncEnd(el, undefined, value, true); + // flight is closed here (#3947). It committed if the commit moved + // `_value`/`_time` or parked `_pendingValue`; a store setter does not + // move this node's `_value`, so waterfall chaining through derived + // stores stays unmeasured. Outside the try (#2883 — see attribution-hooks.ts). + if (observed) + attrHooks!.asyncLanded( + el, + value, + el._value !== before || el._time !== beforeTime || el._pendingValue === value + ); + } else if (observed) { + // The plain landing below, reported once: as an async landing, not a write. + const changed = landThroughSetSignal(el, value); + attrHooks!.asyncLanded(el, value, changed); } else { // CARVE 2: the override-covered landing (hold + A18 supersession) and the // lane-routed landing (derived override, lane effect queue) went with @@ -650,11 +663,6 @@ export function handleAsync( // already held stages its answer into that hold (`setSignal`). Its // comparator throw is contained there (`isSame`, A3). setSignal(el, () => value); - // Attribution hook: this path landed through setSignal, whose write - // hook already saw any committed change — direct=false lets the engine - // reclassify that write as an async landing iff it actually committed. - // Outside the try (#2883 — see attribution-hooks.ts). - if (__OBSERVE__ && attrHooks !== null) attrHooks.asyncEnd(el, undefined, value, false); } // First real answer landing: the window closes when the answer becomes // OBSERVABLE. A direct commit is observable now; a staged write diff --git a/packages/signals/src/core/attribution-hooks.ts b/packages/signals/src/core/attribution-hooks.ts index 1b7ccc84fe..cd45cc9a1a 100644 --- a/packages/signals/src/core/attribution-hooks.ts +++ b/packages/signals/src/core/attribution-hooks.ts @@ -114,17 +114,13 @@ export interface AttributionHooks { * carry their true start time instead of the moment the graph saw them. */ flightStart(el: Computed, flight: object): void; - /** An async landing is about to apply its value (before any branch). */ - asyncStart(el: Computed): void; /** - * The async landing finished. `direct` = the landing applies the value - * itself (lane/override paths); false = it went through setSignal, whose - * own `write` hook already saw any committed change. Fired whether or not - * the landing committed — call sites cannot carry that fact out of their - * try blocks (see the try rule above), so the engine derives committed-ness - * from the node's state against its asyncStart snapshot. + * An async landing applied its value. `changed`: it committed or staged a + * change (it passed the equality gate) — a write the engine stamps as an + * async landing; false, the flight is over with nothing changed. A landing + * through `setSignal` is reported here only, never also as a `write`. */ - asyncEnd(el: Computed, prev: unknown, value: unknown, direct: boolean): void; + asyncLanded(el: Computed, value: unknown, changed: boolean): void; /** * An effect's imperative half (its effect callback) is about to run / * has run. Both fire outside the run's try; `effectRunEnd` fires whether diff --git a/packages/signals/src/core/attribution.ts b/packages/signals/src/core/attribution.ts index 9e4f35248c..4778d2f3f8 100644 --- a/packages/signals/src/core/attribution.ts +++ b/packages/signals/src/core/attribution.ts @@ -4430,9 +4430,6 @@ function navigationData(origin: ChangeOrigin): Record { // The engine's implementation of the core's dev hook points. Installed by // enable(), uninstalled by disable() — while uninstalled the core pays one // null check per site and nothing else. -let asyncStartSeq = 0; -let asyncStartTime = 0; -let asyncStartValue: unknown; const engineHooks: AttributionHooks = { interactionStart, interactionEnd, @@ -4545,35 +4542,10 @@ const engineHooks: AttributionHooks = { flightStart(el, flight) { trackFlightStart(el, flight); }, - asyncStart(el) { - asyncStartSeq = (el as AttributedNode)._devChange?.seq ?? 0; - asyncStartTime = el._time; - asyncStartValue = el._value; - }, - asyncEnd(el, prev, value, direct) { - if (direct) { - // Core calls this unconditionally (hook calls cannot live inside its - // try blocks — see attribution-hooks.ts), so committed-ness is detected - // here against the asyncStart snapshot: a direct commit moves `_value` - // (or `_time`, for a same-reference commit under `equals: false`), and - // a transition hold parks the value in `_pendingValue`. A landing the - // equality gate swallowed moves none of them and must leave no stamp. - const committed = - el._value !== asyncStartValue || - el._time !== asyncStartTime || - (el as Computed)._pendingValue === value; - if (committed) stampWrite(el, "async", prev === undefined ? NO_VALUES : prev, value); - // Flight over either way — an equality-swallowed landing still spent - // the wall time (finalizeFlight only chains through a fresh stamp). - finalizeFlight(el); - return; - } - // Landed through setSignal: reclassify its "write" stamp as an async - // landing — but only if it actually stamped (the value changed) since - // asyncStart; a no-change landing must leave no fresh stamp behind. - const change = (el as AttributedNode)._devChange; - if (change !== undefined && change.seq > asyncStartSeq && change.kind === "write") - stampWrite(el, "async", NO_VALUES, value); + asyncLanded(el, value, changed) { + if (changed) stampWrite(el, "async", NO_VALUES, value); + // Flight over either way — an equality-swallowed landing still spent + // the wall time (finalizeFlight only chains through a fresh stamp). finalizeFlight(el); }, effectRunStart(el) { diff --git a/packages/signals/src/core/core.ts b/packages/signals/src/core/core.ts index 593c39b7d7..4c3239f41a 100644 --- a/packages/signals/src/core/core.ts +++ b/packages/signals/src/core/core.ts @@ -2692,6 +2692,26 @@ export function isSame(el: Signal, a: unknown, b: unknown): unknown { } } +/** Observe: the async node `landThroughSetSignal` is landing, and whether its write passed the equality gate. */ +let landing: object | null = null; +let landed = false; + +/** + * Observe: an async landing through `setSignal` (async.ts). Its write is not + * reported as a `write` — the landing reports it, as `asyncLanded` — and the + * answer is whether the value changed (passed the equality gate). + */ +export function landThroughSetSignal(el: Computed, value: unknown): boolean { + landing = el; + landed = false; + try { + setSignal(el, () => value); + } finally { + landing = null; + } + return landed; +} + export function setSignal(el: Signal | Computed, v: T | ((prev: T) => T)): T { if (writeHook !== null) writeHook(el); if ( @@ -2744,8 +2764,13 @@ export function setSignal(el: Signal | Computed, v: T | ((prev: T) => T (custom === false ? currentValue !== v : !isSame(el, currentValue, v)); if (!valueChanged) return v; - // Attribution hook: this committed write is where a re-run chain begins. - if (__OBSERVE__ && attrHooks !== null) attrHooks.write(el, currentValue, v); + // Attribution hook: this committed write is where a re-run chain begins — + // unless it is an async landing (`landThroughSetSignal`), which the + // landing reports itself, once. + if (__OBSERVE__ && attrHooks !== null) { + if (el === landing) landed = true; + else attrHooks.write(el, currentValue, v); + } // A write during hydration's snapshot capture to a source that has no // snapshot — created BEFORE capture began (module-level state: an identity From 0f4d6123b0beca730e781b0f98b9b6825df15417 Mon Sep 17 00:00:00 2001 From: Dev Agrawal Date: Sat, 7 Nov 2026 16:49:56 -0600 Subject: [PATCH 04/11] refactor(signals): move the store census into the engine via storeWrite Co-authored-by: tembo[bot] <208362400+tembo-io[bot]@users.noreply.github.com> --- .../signals/src/core/attribution-hooks.ts | 31 ++---- packages/signals/src/core/attribution.ts | 91 ++++++++++++++- packages/signals/src/core/dev.ts | 3 +- packages/signals/src/store/store.ts | 104 +----------------- packages/signals/tests/dev.test.ts | 37 +------ packages/solid/test/dev.spec.ts | 23 ---- 6 files changed, 109 insertions(+), 180 deletions(-) diff --git a/packages/signals/src/core/attribution-hooks.ts b/packages/signals/src/core/attribution-hooks.ts index cd45cc9a1a..029fb611e7 100644 --- a/packages/signals/src/core/attribution-hooks.ts +++ b/packages/signals/src/core/attribution-hooks.ts @@ -155,27 +155,18 @@ export interface AttributionHooks { /** `outgoing` was folded into `target` (`outgoing._done = target`). */ transitionMerged(target: Transition, outgoing: Transition): void; /** - * A store setter batch replaced the container at `path` (e.g. `store.user`) - * with a different one (both non-null, same array-ness, not the same - * logical slot), and this is the leaf census of the new container against - * the old: `total` leaves (own keys, or items) in the new one, `unchanged` - * of which are the same value as before (identity, judged on unwrapped - * values — object keys compared by key, array items by membership), and - * `prevTotal` leaves in the old one. Containers above 64 leaves are not - * announced. Fired per written key from the write channel's notify. The - * engine decides whether the replacement was a spread-copy worth a - * diagnostic. `owner` is the owner the store root was created under - * (undefined when unrecorded): the finding is about the store, not about - * whoever wrote it, so an `OBSERVE.exclude`d panel's store stays silent - * however its writes arrive. + * A store setter batch is writing `target`'s container: `old` the + * committed backing, `written` the new one, `keys` the keys this setter + * wrote (`null`: any of them — an array length write, accessors, a + * non-plain prototype). Fired once per setter exit, before its nodes are + * told. The engine judges what it needs from the two containers (a spread + * copy where a draft mutation would do). */ - storeReplaced( - path: string, - isArray: boolean, - total: number, - unchanged: number, - prevTotal: number, - owner: Owner | null | undefined + storeWrite( + target: object, + old: Record, + written: Record, + keys: Iterable | null ): void; /** * A `mapArray` update both disposed and created rows: `removed` are the diff --git a/packages/signals/src/core/attribution.ts b/packages/signals/src/core/attribution.ts index 4778d2f3f8..f6f66bb3e2 100644 --- a/packages/signals/src/core/attribution.ts +++ b/packages/signals/src/core/attribution.ts @@ -32,6 +32,9 @@ import { reportDiagnostic } from "./dev.js"; import { blockersOf, GlobalQueue } from "./scheduler.js"; +import { storeRootOwner, targetsEqual, unwrapValue } from "../store/store.js"; +import { $OWNER, type StoreTarget } from "../store/target.js"; +import { isRawValue, isWrappable } from "../store/types.js"; import type { Computed, Owner, Signal } from "./types.js"; /** @@ -2250,7 +2253,8 @@ function checkRelayTear( // a fresh container is pure cost: every reader of `user` (any path below it) // re-runs for the one leaf that moved, where a draft mutation would re-run // only the readers of `name`. The store's notify sees both containers at the -// write and reports a leaf census (identity on unwrapped values, capped); the +// write (`storeWrite`), and the engine takes a leaf census (identity on +// unwrapped values, capped); the // verdict is the whole detector: a replacement whose leaves are mostly the // SAME values is a spread-copy, and one whose leaves are mostly different is // new data (reconcile's job — and UNSTABLE_LIST_IDENTITY's, downstream). Once @@ -2258,6 +2262,87 @@ function checkRelayTear( const immutableReported = new Set(); +/** A store target's path from its root: `store.user.address`. */ +function storePathOf(t: StoreTarget): string { + let path = ""; + for (let cur: StoreTarget | null = t; cur !== null; cur = cur.u) + path = cur.pk === null ? "store" + path : "." + String(cur.pk) + path; + return path; +} +const REPLACED_CENSUS_MAX = 64; +function sameLeaf(a: unknown, b: unknown): boolean { + if (a === b) return true; + if (a === null || b === null || typeof a !== "object" || typeof b !== "object") return false; + return unwrapValue(a) === unwrapValue(b) || targetsEqual(a, b); +} +/** storeWrite: the containers a setter batch replaced wholesale, each with its leaf census. */ +function censusReplacedContainers( + t: StoreTarget, + old: Record, + pb: Record, + writtenKeys: Iterable | null +): void { + // Shallow stores (and raw / unwrappable leaves) invalidate the whole + // replaced value as one unit — leaf draft assignment does not narrow + // who re-runs (#4686). The spread-copy diagnostic only helps deep stores. + if (t.s) return; + const keys = writtenKeys ?? Reflect.ownKeys(pb); + const isArray = Array.isArray(pb); + const owner = storeRootOwner(t); + // Items the array held before the write: an index that now holds one of + // them was moved into place (an in-place splice / shift), not replaced. + let oldArrayItems: Set | null = null; + for (const key of keys) { + if ((isArray && key === "length") || key === $OWNER) continue; + if (t.del?.has(key)) continue; + const ov = unwrapValue(old[key as any]); + const nv = unwrapValue(pb[key as any]); + if ( + ov === null || + nv === null || + typeof ov !== "object" || + typeof nv !== "object" || + ov === nv || + targetsEqual(ov, nv) + ) + continue; + // A raw / unwrappable previous value is itself a leaf: replacing it is + // the write, not a container copy the store could have mutated in place. + if (isRawValue(ov) || !isWrappable(ov)) continue; + if (isArray) { + if (oldArrayItems === null) { + oldArrayItems = new Set(); + for (const item of old as unknown[]) oldArrayItems.add(unwrapValue(item)); + } + if (oldArrayItems.has(nv)) continue; + } + const isArr = Array.isArray(nv); + if (isArr !== Array.isArray(ov)) continue; + let total: number; + let unchanged = 0; + if (isArr) { + total = (nv as unknown[]).length; + if (total > REPLACED_CENSUS_MAX) continue; + const oldItems = new Set(); + for (const item of ov as unknown[]) oldItems.add(unwrapValue(item)); + for (const item of nv as unknown[]) if (oldItems.has(unwrapValue(item))) unchanged++; + } else { + const nkeys = Object.keys(nv); + total = nkeys.length; + if (total > REPLACED_CENSUS_MAX) continue; + for (const k of nkeys) if (sameLeaf((ov as any)[k], (nv as any)[k])) unchanged++; + } + checkImmutableUpdate( + storePathOf(t) + "." + String(key), + isArr, + total, + unchanged, + isArr ? (ov as unknown[]).length : Object.keys(ov).length, + owner + ); + } +} + function checkImmutableUpdate( path: string, isArray: boolean, @@ -4605,8 +4690,8 @@ const engineHooks: AttributionHooks = { trackHoldMerge(target, outgoing); rebaseFallbacks(outgoing, target); }, - storeReplaced(path, isArray, total, unchanged, prevTotal, owner) { - checkImmutableUpdate(path, isArray, total, unchanged, prevTotal, owner); + storeWrite(target, old, written, keys) { + censusReplacedContainers(target as StoreTarget, old, written, keys); }, listChurn(el, removed, created, newLen, keyed) { checkListIdentity(el, removed, created, newLen, keyed); diff --git a/packages/signals/src/core/dev.ts b/packages/signals/src/core/dev.ts index 88b5faf6c9..77061fe990 100644 --- a/packages/signals/src/core/dev.ts +++ b/packages/signals/src/core/dev.ts @@ -39,7 +39,6 @@ import type { Computed, Link, Owner, Root, Signal } from "./types.js"; export interface DevHooks { onOwner?: (owner: Owner) => void; onGraph?: (value: any, owner: Owner | null) => void; - onStoreNodeUpdate?: (state: any, property: PropertyKey, value: any, prev: any) => void; /** @internal Every node read (the frames client's identity-drift check; `setReadWatch`). */ onRead?: (node: object) => void; } @@ -527,7 +526,7 @@ export interface Dev { * for example from a devtools state editor that only holds the node from * `getSignals` / `getSources`. Runs as an unowned write (devtools UI * events are not owned scopes). Store property slots go through the - * store setter path so `onStoreNodeUpdate` and backing state stay in sync. + * store setter path so the backing state and its readers stay in sync. * * @see https://github.com/solidjs/solid/issues/4679 */ diff --git a/packages/signals/src/store/store.ts b/packages/signals/src/store/store.ts index 29bef12d46..40e91032a8 100644 --- a/packages/signals/src/store/store.ts +++ b/packages/signals/src/store/store.ts @@ -75,7 +75,6 @@ import { import { asyncTailFlights, checkPostAwaitRead, - DEV, emitDiagnostic, registerDevStoreSlotWriter, registerGraph, @@ -547,7 +546,7 @@ function storeRoot(target: StoreTarget): StoreTarget { while (root.u !== null) root = root.u; return root; } -function storeRootOwner(target: StoreTarget): Owner | null | undefined { +export function storeRootOwner(target: StoreTarget): Owner | null | undefined { return storeOwners!.get(storeRoot(target)); } function stampNodeOwner(created: Signal, target: StoreTarget): void { @@ -1052,86 +1051,6 @@ function foldTarget(t: StoreTarget, old: Record): void { // --------------------------------------------------------------------------- // write-time notification (setter exit) -/** Devtools and attribution: the containers a setter replaced wholesale. */ -function storePathOf(t: StoreTarget): string { - let path = ""; - for (let cur: StoreTarget | null = t; cur !== null; cur = cur.u) - path = cur.pk === null ? "store" + path : "." + String(cur.pk) + path; - return path; -} -const REPLACED_CENSUS_MAX = 64; -function sameLeaf(a: unknown, b: unknown): boolean { - if (a === b) return true; - if (a === null || b === null || typeof a !== "object" || typeof b !== "object") return false; - return unwrapValue(a) === unwrapValue(b) || targetsEqual(a, b); -} -function reportReplacedContainers( - t: StoreTarget, - old: Record, - pb: Record, - writtenKeys: Iterable | null -): void { - // Shallow stores (and raw / unwrappable leaves) invalidate the whole - // replaced value as one unit — leaf draft assignment does not narrow - // who re-runs (#4686). The spread-copy diagnostic only helps deep stores. - if (t.s) return; - const keys = writtenKeys ?? Reflect.ownKeys(pb); - const isArray = Array.isArray(pb); - const owner = storeRootOwner(t); - // Items the array held before the write: an index that now holds one of - // them was moved into place (an in-place splice / shift), not replaced. - let oldArrayItems: Set | null = null; - for (const key of keys) { - if ((isArray && key === "length") || key === $OWNER) continue; - if (t.del?.has(key)) continue; - const ov = unwrapValue(old[key as any]); - const nv = unwrapValue(pb[key as any]); - if ( - ov === null || - nv === null || - typeof ov !== "object" || - typeof nv !== "object" || - ov === nv || - targetsEqual(ov, nv) - ) - continue; - // A raw / unwrappable previous value is itself a leaf: replacing it is - // the write, not a container copy the store could have mutated in place. - if (isRawValue(ov) || !isWrappable(ov)) continue; - if (isArray) { - if (oldArrayItems === null) { - oldArrayItems = new Set(); - for (const item of old as unknown[]) oldArrayItems.add(unwrapValue(item)); - } - if (oldArrayItems.has(nv)) continue; - } - const isArr = Array.isArray(nv); - if (isArr !== Array.isArray(ov)) continue; - let total: number; - let unchanged = 0; - if (isArr) { - total = (nv as unknown[]).length; - if (total > REPLACED_CENSUS_MAX) continue; - const oldItems = new Set(); - for (const item of ov as unknown[]) oldItems.add(unwrapValue(item)); - for (const item of nv as unknown[]) if (oldItems.has(unwrapValue(item))) unchanged++; - } else { - const nkeys = Object.keys(nv); - total = nkeys.length; - if (total > REPLACED_CENSUS_MAX) continue; - for (const k of nkeys) if (sameLeaf((ov as any)[k], (nv as any)[k])) unchanged++; - } - attrHooks!.storeReplaced( - storePathOf(t) + "." + String(key), - isArr, - total, - unchanged, - isArr ? (ov as unknown[]).length : Object.keys(ov).length, - owner - ); - } -} - /** A user setter wrote a key whose staging is a derivation another * transaction holds (A34 (3), #3612): the hold re-derives over the write. */ let heldDerivationHit = false; @@ -1182,23 +1101,6 @@ function notifyWrites(t: StoreTarget): void { optHooks.pair(t, pb); } const old = t.v; - // Devtools mutation hook: full-key diff (dev-only cost) so unobserved - // writes report too. Overlay backings materialize first so the diff walks - // a real container. - if (__DEV__ && DEV.hooks.onStoreNodeUpdate) { - if (t.ovl) materializePB(t); - pb = t.pb!; - for (const key of Reflect.ownKeys(pb)) { - if ((Array.isArray(pb) && key === "length") || key === $OWNER) continue; - const ov = old[key as any]; - const nv = pb[key as any]; - if (!isEqual(ov, nv)) DEV.hooks.onStoreNodeUpdate(t.px, key, nv, ov); - } - for (const key of Reflect.ownKeys(old)) { - if (key in pb || key === $OWNER) continue; - DEV.hooks.onStoreNodeUpdate(t.px, key, undefined, old[key as any]); - } - } const nodes = t.n; // Written-keys bound: trap writes record their keys, so the notify visits // O(written) nodes instead of every subscription on the record (a selection @@ -1216,7 +1118,7 @@ function notifyWrites(t: StoreTarget): void { const wk0 = t.wk; const writtenKeys = wk0 === WK_ALL || t.a === true || !plainProto(t.ovl ? (t.v as object) : pb) ? null : wk0; - if (__OBSERVE__ && attrHooks !== null) reportReplacedContainers(t, old, pb, writtenKeys); + if (__OBSERVE__ && attrHooks !== null) attrHooks.storeWrite(t, old, pb, writtenKeys); if (t._fam !== null && !heldDerivationHit && writtenKeys !== null) for (const key of writtenKeys) if (heldDerivation(t, nodes?.[key as any], key)) { @@ -2884,7 +2786,7 @@ function snapshotRecord( } // Devtools `DEV.setSignal` for store property slots (#4679): go through the -// store setter so backing state and `onStoreNodeUpdate` stay consistent. +// store setter so the backing state and its readers stay consistent. if (__DEV__) { registerDevStoreSlotWriter((node, value) => { const host = (node as any)._host as StoreTarget; diff --git a/packages/signals/tests/dev.test.ts b/packages/signals/tests/dev.test.ts index c3b5519135..9b0fb7f46b 100644 --- a/packages/signals/tests/dev.test.ts +++ b/packages/signals/tests/dev.test.ts @@ -19,7 +19,6 @@ const { getChildren, getSignals, getParent, getSources, getObservers } = DEV!; afterEach(() => { DEV!.hooks.onOwner = undefined; DEV!.hooks.onGraph = undefined; - DEV!.hooks.onStoreNodeUpdate = undefined; }); describe("Phase 1a: Signal-to-owner tracking", () => { @@ -180,29 +179,6 @@ describe("Phase 1b: Notification hooks", () => { expect(entries.length).toBe(1); }); }); - - describe("onStoreNodeUpdate", () => { - it("fires on store property mutation with old and new values", () => { - const updates: any[] = []; - DEV!.hooks.onStoreNodeUpdate = (state, property, value, prev) => - updates.push({ state, property, value, prev }); - - let setStore!: (fn: (s: { count: number; name: string }) => void) => void; - createRoot(() => { - const [, _setStore] = createStore({ count: 0, name: "test" }); - setStore = _setStore; - }); - - setStore(s => { - s.count = 5; - }); - flush(); - expect(updates.length).toBe(1); - expect(updates[0].property).toBe("count"); - expect(updates[0].value).toBe(5); - expect(updates[0].prev).toBe(0); - }); - }); }); describe("Phase 1c: Graph traversal helpers", () => { @@ -357,17 +333,16 @@ describe("Phase 1c: Graph traversal helpers", () => { }); }); - it("writes a store property slot and fires onStoreNodeUpdate", () => { - const updates: any[] = []; - DEV!.hooks.onStoreNodeUpdate = (state, property, value, prev) => { - updates.push({ state, property, value, prev }); - }; + it("writes a store property slot through the store setter", () => { + const updates: number[] = []; createRoot(() => { const [store] = createStore({ count: 0 }); // Touch the key so a slot node exists for getSources/devtools. createEffect( () => store.count, - () => {} + v => { + updates.push(v); + } ); flush(); const sources = getSources(getChildren(getOwner()!)[0] as any); @@ -376,7 +351,7 @@ describe("Phase 1c: Graph traversal helpers", () => { DEV!.setSignal(slot!, 9); flush(); expect(store.count).toBe(9); - expect(updates.some(u => u.property === "count" && u.value === 9)).toBe(true); + expect(updates).toEqual([0, 9]); }); }); diff --git a/packages/solid/test/dev.spec.ts b/packages/solid/test/dev.spec.ts index c4d39867f8..3bc3b3abbe 100644 --- a/packages/solid/test/dev.spec.ts +++ b/packages/solid/test/dev.spec.ts @@ -19,7 +19,6 @@ afterEach(() => { if (DEV) { DEV.hooks.onOwner = undefined; DEV.hooks.onGraph = undefined; - DEV.hooks.onStoreNodeUpdate = undefined; } }); @@ -328,28 +327,6 @@ describe("DEV.hooks", () => { expect(entries.length).toBeGreaterThanOrEqual(1); }); - - test("onStoreNodeUpdate fires on store mutation with old/new values", () => { - const updates: any[] = []; - DEV!.hooks.onStoreNodeUpdate = (state: any, property: PropertyKey, value: any, prev: any) => - updates.push({ state, property, value, prev }); - - let setStore!: (fn: (s: { count: number; name: string }) => void) => void; - createRoot(() => { - const [, _setStore] = createStore({ count: 0, name: "test" }); - setStore = _setStore; - }); - - setStore(s => { - s.count = 5; - }); - flush(); - - expect(updates.length).toBe(1); - expect(updates[0].property).toBe("count"); - expect(updates[0].value).toBe(5); - expect(updates[0].prev).toBe(0); - }); }); describe("DEV graph traversal helpers", () => { From 878ab2f17008def52c0767375bddbec5052db877 Mon Sep 17 00:00:00 2001 From: Dev Agrawal Date: Sat, 7 Nov 2026 16:59:18 -0600 Subject: [PATCH 05/11] refactor(signals): judge fallback display from its output, not a copy Co-authored-by: tembo[bot] <208362400+tembo-io[bot]@users.noreply.github.com> --- packages/signals/src/boundaries.ts | 20 +-- .../signals/src/core/attribution-hooks.ts | 12 +- packages/signals/src/core/attribution.ts | 117 ++++++++---------- 3 files changed, 64 insertions(+), 85 deletions(-) diff --git a/packages/signals/src/boundaries.ts b/packages/signals/src/boundaries.ts index 8836e8f2ec..74c993da32 100644 --- a/packages/signals/src/boundaries.ts +++ b/packages/signals/src/boundaries.ts @@ -77,8 +77,6 @@ import { heldUnder, joinFuture, passLane, - passTx, - resolveTx, schedule, setPassLane, txOf, @@ -941,19 +939,11 @@ function fallback(b: Boundary): T { out._flags |= REACTIVE_LANE_READ; b._lane = null; } - // Observe: a loading boundary starts showing its fallback — the swap lands - // with the frame's transaction, or this drain commits it (lane work, or no - // transaction). A mount's synchronous first show has no drain of its own: - // one is scheduled so the engine's display instant (`flushEnd`) comes. - if (__OBSERVE__ && !b._fallback && b._type & STATUS_PENDING && attrHooks !== null) { - attrHooks.boundaryFallback( - b, - b._tree, - true, - passLane !== null ? null : flushTransaction !== null ? resolveTx(flushTransaction) : passTx - ); - if (!globalQueue._running) schedule(); - } + // Observe: a loading boundary starts showing its fallback. The swap is the + // output's value; the engine watches it and counts the fallback displayed + // when the screen shows it (a commit, a landing, a lane's reveal). + if (__OBSERVE__ && !b._fallback && b._type & STATUS_PENDING && attrHooks !== null) + attrHooks.boundaryFallback(b, b._tree, true, out); b._fallback = true; // Built in its own scope, like `Show`'s function child, once per show // (D-39: the fallback is the display until a new answer replaces the diff --git a/packages/signals/src/core/attribution-hooks.ts b/packages/signals/src/core/attribution-hooks.ts index 029fb611e7..0469e99094 100644 --- a/packages/signals/src/core/attribution-hooks.ts +++ b/packages/signals/src/core/attribution-hooks.ts @@ -190,18 +190,18 @@ export interface AttributionHooks { * the boundary. Fired at the source-set transitions (first pending source * registers / last one clears), not per flush. * - * A show is the boundary's SWAP, a staged write: `transition` is the one - * it lands with (`transitionSettled` is its display instant), or `null` - * when this drain commits it (`flushEnd`) — the lane swap included, whose - * readers run in this drain. A hide before that commit means the fallback - * was never displayed — the content landed first and the sweep cleared the + * A show is the boundary's SWAP: `output`, the boundary's value, is + * computing the fallback. The fallback is displayed when the screen shows + * the output's new value (`staged` and the drain-end comparison: a commit, + * a landing, a lane's reveal). A hide before that means the fallback was + * never displayed — the content landed first and the sweep cleared the * swap ahead of the frame (#3540). */ boundaryFallback( boundary: object, tree: Computed | undefined, shown: boolean, - transition?: Transition | null + output?: Computed ): void; /** * The one query on the surface: the provenance a root write performed at diff --git a/packages/signals/src/core/attribution.ts b/packages/signals/src/core/attribution.ts index f6f66bb3e2..e6f556cefe 100644 --- a/packages/signals/src/core/attribution.ts +++ b/packages/signals/src/core/attribution.ts @@ -3928,6 +3928,12 @@ function unsettled(n: Signal | Computed): boolean { ); } +/** Watch `n` from here even with no flush listener (a fallback's output). */ +function watch(n: Computed): void { + if (!watched.has(n)) + watched.set(n, { shown: shownOf(n), guess: false, covered: n._value, origin: undefined }); +} + function trackStaged(el: Signal | Computed): void { // Effects are not state; their values are what they rendered. if ((el as { _type?: number })._type) return; @@ -3957,6 +3963,8 @@ function drainChanges(record: boolean, live: FlushLive["changes"]): FlushChange[ } const now = shownOf(n); if (!Object.is(now, w.shown)) { + const fallback = pendingFallbacks.get(n as Computed); + if (fallback !== undefined) displayFallback(fallback); const guess = showsGuess(n); if (w.guess && !guess && now !== NOT_PENDING && w.shown !== NOT_PENDING) checkOptimisticRevert( @@ -3979,7 +3987,7 @@ function drainChanges(record: boolean, live: FlushLive["changes"]): FlushChange[ if (guess && !w.guess) w.covered = n._value; w.guess = guess; } - if (!unsettled(n)) watched.delete(n); + if (!unsettled(n) && !pendingFallbacks.has(n as Computed)) watched.delete(n); } return changes; } @@ -3996,19 +4004,18 @@ function trackFlushStart(): void { // its boundary); `gen` stamps the show with the tracking generation so a show // that predates a reinstall cannot emit against the new window. // -// The show the boundary reports is the SWAP — a staged write, which lands -// with its transaction's commit (#3540: `on` follows the frame) and is on -// screen once the drain that committed it has run its effects. Until then -// the fallback is not displayed: an open is `staged` — under the transaction -// it lands with (`transitionSettled` moves it to the settling drain; -// `transitionMerged` follows a fold), or under the current drain when no -// transaction carries it, the lane swap included (its readers run in this -// drain). `flushEnd` — the "committed, screen updated" instant — stamps the -// drain's opens with their `at`. A hide that finds the open still staged was -// never displayed: the content landed before the frame did and the sweep -// cleared the swap ahead of the commit, or the commit's own sweep cleared it -// before any effect ran (the LOADING_ON_OUTSIDE_HOLD shape). It drops the -// open and makes no record, and the folds hear neither show nor hide. +// The show the boundary reports is the SWAP: the boundary's output computing +// the fallback, a value that lands with its frame (#3540: `on` follows the +// frame) — committed this drain, held until its transaction lands, or shown +// with a lane. Until the screen shows it the fallback is not displayed: the +// open is pending on its output, which the drain-end comparison watches +// (`drainChanges`); the drain that shows the output's new value stamps the +// open's `at`. A mount outside any drain shows it as the mount returns. A +// hide that finds the open still pending was never displayed: the content +// landed before the frame did and the sweep cleared the swap ahead of the +// commit, or the commit's own sweep cleared it before any effect ran (the +// LOADING_ON_OUTSIDE_HOLD shape). It drops the open and makes no record, and +// the folds hear neither show nor hide. interface OpenFallback { at: number; gen: number; @@ -4017,23 +4024,22 @@ interface OpenFallback { boundary: object; tree: Computed | undefined; interaction: ChangeOrigin | undefined; - /** Pending display: the transaction it lands with, or `DRAIN` for this flush's end. */ - staged: Transition | typeof DRAIN | null; + /** The output it waits on to be displayed; `null` once displayed. */ + pending: Computed | null; } -/** The key for swaps no transaction carries — an object, so the map below can be weak. */ -const DRAIN: { readonly drain: true } = { drain: true }; const openFallbacks = new WeakMap(); -// Weak on the transaction: one that is dropped without settling or merging -// (its boundary disposed, its queues discarded) takes its staged opens with it. -const stagedFallbacks = new WeakMap(); +/** Opens waiting for their output's display, by output. */ +const pendingFallbacks = new Map, OpenFallback>(); /** Bumped by `resetTracking` — the engine's install generation. */ let trackingGen = 0; +/** Between `flushStart` and `flushEnd`. */ +let draining = false; function trackFallback( boundary: object, tree: Computed | undefined, shown: boolean, - transition: Transition | null + output: Computed | undefined ): void { if (shown) { // Folds count showings too, and the flash finding needs the show's @@ -4043,7 +4049,6 @@ function trackFallback( // The wait is the enclosing recompute's cause's interaction — the read // that registered the pending source runs inside one — else the ambient. const causes = enclosingCauses(); - const staged = transition ?? DRAIN; const open: OpenFallback = { at: now(), gen: trackingGen, @@ -4051,22 +4056,26 @@ function trackFallback( boundary, tree, interaction: causes !== null ? interactionIn(causes) : (currentInteraction ?? undefined), - staged + pending: null }; openFallbacks.set(boundary, open); - const list = stagedFallbacks.get(staged); - if (list === undefined) stagedFallbacks.set(staged, [open]); - else list.push(open); + // A mount outside any drain: its first value is published as the mount + // returns, with no drain to show it in. + if (output === undefined || (!draining && shownOf(output) === NOT_PENDING)) + displayFallback(open); + else { + open.pending = output; + pendingFallbacks.set(output, open); + watch(output); + } return; } const open = openFallbacks.get(boundary); if (open === undefined) return; openFallbacks.delete(boundary); - if (open.staged !== null) { - // Cleared before its commit: never on screen. - const list = stagedFallbacks.get(open.staged)!; - list.splice(list.indexOf(open), 1); - if (list.length === 0) stagedFallbacks.delete(open.staged); + if (open.pending !== null) { + // Cleared before the screen showed it: never displayed. + pendingFallbacks.delete(open.pending); return; } if (open.gen !== trackingGen) return; @@ -4084,31 +4093,12 @@ function trackFallback( records.emit("fallback", event, subtree); } -/** flushEnd: the drain's committed swaps have rendered — those fallbacks are on screen from here. */ -function displayFallbacks(): void { - const list = stagedFallbacks.get(DRAIN); - if (list === undefined) return; - stagedFallbacks.delete(DRAIN); - const at = now(); - for (const open of list) { - open.staged = null; - open.at = at; - if (open.gen === trackingGen) - for (const f of folds) f.fallback?.(open.boundary, open.tree, true); - } -} - -/** transitionSettled (`from` a transaction, into this drain) and - * transitionMerged (`from` the outgoing, into the surviving transaction): - * the swaps staged under `from` now land with `into`. */ -function rebaseFallbacks(from: Transition, into: Transition | typeof DRAIN): void { - const list = stagedFallbacks.get(from); - if (list === undefined) return; - stagedFallbacks.delete(from); - for (const open of list) open.staged = into; - const target = stagedFallbacks.get(into); - if (target === undefined) stagedFallbacks.set(into, list); - else target.push(...list); +/** The screen shows the fallback from here. */ +function displayFallback(open: OpenFallback): void { + if (open.pending !== null) pendingFallbacks.delete(open.pending); + open.pending = null; + open.at = now(); + if (open.gen === trackingGen) for (const f of folds) f.fallback?.(open.boundary, open.tree, true); } /** flushEnd: every open, closed, unheld navigation's (and interaction's) writes just committed. */ @@ -4131,8 +4121,6 @@ function trackFlushEnd(): void { records.emit("flush", event, live); } drainSeq++; - // The swaps this drain committed have rendered. - displayFallbacks(); // Landed first: they settled before anything this drain settles below. for (let i = 0; i < landedGraphs.length; i++) trackGraph(landedGraphs[i]); landedGraphs.length = 0; @@ -4521,9 +4509,11 @@ const engineHooks: AttributionHooks = { originStart, originEnd, flushStart() { + draining = true; trackFlushStart(); }, flushEnd() { + draining = false; trackFlushEnd(); }, recomputeStart(el, create) { @@ -4684,11 +4674,9 @@ const engineHooks: AttributionHooks = { }, transitionSettled(t) { trackHoldSettled(t); - rebaseFallbacks(t, DRAIN); }, transitionMerged(target, outgoing) { trackHoldMerge(target, outgoing); - rebaseFallbacks(outgoing, target); }, storeWrite(target, old, written, keys) { censusReplacedContainers(target as StoreTarget, old, written, keys); @@ -4696,9 +4684,9 @@ const engineHooks: AttributionHooks = { listChurn(el, removed, created, newLen, keyed) { checkListIdentity(el, removed, created, newLen, keyed); }, - boundaryFallback(boundary, tree, shown, transition) { + boundaryFallback(boundary, tree, shown, output) { // The folds hear the show at its display (see trackFallback), not here. - trackFallback(boundary, tree, shown, transition ?? null); + trackFallback(boundary, tree, shown, output); }, staged(el) { trackStaged(el); @@ -4746,7 +4734,8 @@ function resetTracking(): void { watched.clear(); activeHold = null; openFlush = null; - stagedFallbacks.delete(DRAIN); + pendingFallbacks.clear(); + draining = false; trackingGen++; openNavs.clear(); landedGraphs.length = 0; From 115ff725903f1ed7ac710ddb59b3e92c511e78b9 Mon Sep 17 00:00:00 2001 From: Dev Agrawal Date: Sat, 7 Nov 2026 17:00:12 -0600 Subject: [PATCH 06/11] docs(signals): describe the slimmer attribution hooks in the changeset Co-authored-by: tembo[bot] <208362400+tembo-io[bot]@users.noreply.github.com> --- .changeset/flush-record-changes.md | 7 ++++--- documentation/solid-2.0/08-dev-diagnostics.md | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.changeset/flush-record-changes.md b/.changeset/flush-record-changes.md index b34ef2939d..e570b2149b 100644 --- a/.changeset/flush-record-changes.md +++ b/.changeset/flush-record-changes.md @@ -3,8 +3,9 @@ "solid-js": patch --- -The `flush` record lists what each drain changed on screen; `OPTIMISTIC_REVERTED` is judged from it; `DEV.hooks.onUpdate` removed +The `flush` record lists what each drain changed on screen; the attribution hooks shrink around it; `DEV.hooks.onUpdate` and `DEV.hooks.onStoreNodeUpdate` removed - **`FlushEvent.changes`**: one entry per signal, memo or store property whose shown value the drain changed — by a commit, a held transition's landing, or an optimistic value appearing or lifting — with `nodeId`, `nodeName`, `key` (store properties) and `origin` (the action step, effect, interaction or navigation that staged the value; a held write keeps its origin, so a landing names its action). First values and effects are not listed. New types `FlushChange` and `FlushLive`: the record's `live` is now `{ changes: [{ node, prev?, value? }] }`, index for index with `event.changes`, the values only for a listener that subscribed with `{ bodies: true }`. -- **`AttributionHooks.staged(el)`** replaces `optimisticReverted`: core reports where a value was staged (`queuePendingNode`, `list`), never where it became visible; the engine compares what each staged node showed before and after the drain. `OPTIMISTIC_REVERTED` is that comparison's: a shown guess replaced by its covered value (`reverted`) or by a different one (`superseded`), judged at the end of the drain that shows the correction. -- **Removed**: `DEV.hooks.onUpdate`. Subscribe to `OBSERVE.records` `"flush"` instead (observe and dev builds; requires `attribution.enable()`). +- **Visibility is judged once, by the engine.** Core reports where a value was staged (`AttributionHooks.staged`, from `queuePendingNode` and `list`), never where it became visible; the engine compares what each staged node showed before and after the drain. `OPTIMISTIC_REVERTED` and the `fallback` record's display instant are judged from that comparison. Removed: the `optimisticReverted` hook, the engine's per-transaction copy of staged fallbacks (`stagedFallbacks` and its rebasing on settle/merge), and the `schedule()` a mounting boundary made so the engine got a drain end. +- **Hook inputs reduced to what core knows** (internal `AttributionHooks`): `recomputeEnd(el, create, changed, phase, held)` replaces the `optimistic`/`transition` pair with the `phase` the engine derived from them, and absorbs `derivedChanged`; `asyncLanded(el, value, changed)` replaces `asyncStart` + `asyncEnd(direct)` — an async landing through `setSignal` is reported once, not first as a `write` (the engine no longer snapshots the node to tell whether it committed); `storeWrite(target, old, written, keys)` replaces `storeReplaced`, and the immutable-update census moves from the store into the engine; `boundaryFallback` takes the boundary's output instead of the transaction its swap is staged under. +- **Removed**: `DEV.hooks.onUpdate` (subscribe to `OBSERVE.records` `"flush"`) and `DEV.hooks.onStoreNodeUpdate` (store property changes are in `flush.changes`; properties nobody reads have no node and are not listed). Both records require `attribution.enable()` (observe and dev builds). diff --git a/documentation/solid-2.0/08-dev-diagnostics.md b/documentation/solid-2.0/08-dev-diagnostics.md index af64ed78ba..59b3288008 100644 --- a/documentation/solid-2.0/08-dev-diagnostics.md +++ b/documentation/solid-2.0/08-dev-diagnostics.md @@ -1221,7 +1221,7 @@ createRoot(() => { **Records and clocks.** Everything the engine hands out — `RerunEvent`, `InteractionEvent`, `HoldEvent`, `NavigationEvent` — is a record with an absolute `at` on the `performance.now()` clock (`RerunEvent.at` the run's start, `HoldEvent.at` the start of the wait, `NavigationEvent.at`/`InteractionEvent.at` the request/dispatch) plus durations from it (`holdMs`, `settledMs`, `selfMs`). Epoch time for an exporter is `performance.timeOrigin + at` (milliseconds). Without cross-origin isolation the browser quantizes `performance.now()` to 100µs, so a single run's `selfMs` is often `0`; the per-interaction `settledMs` is the wall-clock number to report. Records are serializable as emitted: none carries a live graph reference — a re-run names its scope by `nodeId` (the engine's per-node id, stable across the scope's runs in the process, distinct between scopes; in-process consumers get the node itself as the listener's second argument, `live`) — while the frame objects that join records (`origin`, `interaction`) are the same object across records in-process, so join by identity there and by `ChangeOrigin.run`/`at`/`name` once they have left it. `OBSERVE.records.subscribe(type, listener)` delivers each record synchronously at the moment it is complete (a re-run at recompute end; an interaction, hold or navigation when it settles), bottom-up: a hold before the navigation it held, before the interaction that performed it — the same objects `history(type)` keeps. A listener runs inside the engine and must not write signals; hand work off to a microtask. A `RerunEvent` is built — dep diff, previews, cause list, ring-buffer push — only while something wants it: a `rerun` listener, an imported fold (`costs`/`feedback`) or the console log. Without one the engine still keeps its per-node facts and runs every check (hot runs, hot time, wasted recompute, dep width) from them, allocates no record, and `history("rerun")` stays empty. -**Timeline records.** Five further record types describe the work itself rather than its outcome, shaped for a profiler track (`@solidjs/web/performance-tracks`, below) and for the responsiveness findings; they enter no ring buffer and are built only while something is subscribed to them (`OBSERVE.records.observed(type)`), so an app that does not listen pays nothing beyond the hook null-check. `flush` — one per scheduler drain: `at`, `durationMs`, `runs` and `created` inside it, `held` (a transition parked), `interaction`, and `changes` — what the drain changed on screen: one entry per signal, memo or store property whose shown value differs from what it showed before the drain (`nodeId`, `nodeName`, `key` for a store property, and `origin`, the action step, effect, interaction or navigation that staged the value), whether it got there by a commit, a held transition's landing, or an optimistic value appearing or lifting. A held write is listed by the drain that lands it and keeps the origin it was staged under, so a landing names its action; a node's first value and effects are not listed. The record stays serializable: the nodes travel beside it (`live.changes`, index for index), with their values before and after only for a listener that subscribed with `{ bodies: true }` — live values, which a listener that keeps them copies on delivery. Core reports where a value was staged (`AttributionHooks.staged`); the engine compares what each staged node showed before and after the drain, which is also how `OPTIMISTIC_REVERTED` is judged. `create` — a computation's creation run (`RerunEvent`'s shape without causes: `nodeId`, `nodeName`, `nodeKind`, `at`, `selfMs`, `totalMs`, `depCount`, `phase`, `held`, `interaction`) — the mount work that no `RerunEvent` describes, and what `InteractionEvent.created` counts. `effect` — an effect's callback, the imperative half that writes the DOM: `nodeId`, `nodeName`, `at`, `durationMs`, `run`, `interaction`. `flight` — an async node's flight, kickoff to landing: `nodeId`, `nodeName`, `ownerPath`, `at`, `durationMs`, `outcome: "landed" | "abandoned"` (superseded before it landed), `interaction`. `fallback` — a `` boundary's fallback, displayed to hidden: `ownerPath`, `at`, `shownMs`, `interaction`. `at` is the end of the drain that rendered the swap — the boundary's swap is a staged write that lands with its frame ([RFC 05](05-async-data.md#loading-on-prop-dependencies-that-show-the-fallback-again)), and a swap cleared before that (the content landed before the frame did, or the commit's own sweep cleared it ahead of any effect — the `LOADING_ON_OUTSIDE_HOLD` shape) was never on screen and is no record. Every derived `ChangeRecord` on a re-run's cause chain also carries the `nodeId` of the memo it came from, so a consumer can walk the propagation of one write node by node. +**Timeline records.** Five further record types describe the work itself rather than its outcome, shaped for a profiler track (`@solidjs/web/performance-tracks`, below) and for the responsiveness findings; they enter no ring buffer and are built only while something is subscribed to them (`OBSERVE.records.observed(type)`), so an app that does not listen pays nothing beyond the hook null-check. `flush` — one per scheduler drain: `at`, `durationMs`, `runs` and `created` inside it, `held` (a transition parked), `interaction`, and `changes` — what the drain changed on screen: one entry per signal, memo or store property whose shown value differs from what it showed before the drain (`nodeId`, `nodeName`, `key` for a store property, and `origin`, the action step, effect, interaction or navigation that staged the value), whether it got there by a commit, a held transition's landing, or an optimistic value appearing or lifting. A held write is listed by the drain that lands it and keeps the origin it was staged under, so a landing names its action; a node's first value and effects are not listed. The record stays serializable: the nodes travel beside it (`live.changes`, index for index), with their values before and after only for a listener that subscribed with `{ bodies: true }` — live values, which a listener that keeps them copies on delivery. Core reports where a value was staged (`AttributionHooks.staged`); the engine compares what each staged node showed before and after the drain, which is also how `OPTIMISTIC_REVERTED` is judged. `create` — a computation's creation run (`RerunEvent`'s shape without causes: `nodeId`, `nodeName`, `nodeKind`, `at`, `selfMs`, `totalMs`, `depCount`, `phase`, `held`, `interaction`) — the mount work that no `RerunEvent` describes, and what `InteractionEvent.created` counts. `effect` — an effect's callback, the imperative half that writes the DOM: `nodeId`, `nodeName`, `at`, `durationMs`, `run`, `interaction`. `flight` — an async node's flight, kickoff to landing: `nodeId`, `nodeName`, `ownerPath`, `at`, `durationMs`, `outcome: "landed" | "abandoned"` (superseded before it landed), `interaction`. `fallback` — a `` boundary's fallback, displayed to hidden: `ownerPath`, `at`, `shownMs`, `interaction`. `at` is the end of the drain that rendered the swap — the boundary's swap is its output's value, which lands with its frame ([RFC 05](05-async-data.md#loading-on-prop-dependencies-that-show-the-fallback-again)) and is judged displayed by the same drain-end comparison as `flush.changes` (a mount outside any drain displays it as the mount returns) — and a swap cleared before that (the content landed before the frame did, or the commit's own sweep cleared it ahead of any effect — the `LOADING_ON_OUTSIDE_HOLD` shape) was never on screen and is no record. Every derived `ChangeRecord` on a re-run's cause chain also carries the `nodeId` of the memo it came from, so a consumer can walk the propagation of one write node by node. **Joining an interaction to the browser's INP entry.** An `InteractionEvent.at` is the DOM event's `timeStamp` when the runtime dispatched it (compiled event bindings pass it; `OBSERVE.attribution.withInteraction({ …, at: e.timeStamp }, fn)` for a custom dispatcher), and `inputDelayMs` is the queueing from that moment to the handler's entry — the browser's input delay. The `PerformanceEventTiming` entry for the same event (a `PerformanceObserver` on `"event"`) has `startTime === interaction.at` on the same clock and carries `interactionId`, so the join is `entry.startTime === interaction.at`: `inputDelayMs` accounts for the entry's `processingStart − startTime`, `handlerMs` for its processing, and `settledMs` extends past its `duration` to when Solid had the screen right. From 590920e38106efff00c8530304377728c2dac052 Mon Sep 17 00:00:00 2001 From: Dev Agrawal Date: Sat, 7 Nov 2026 18:08:44 -0600 Subject: [PATCH 07/11] fix(signals): stage optimistic writes where written, with their origin Co-authored-by: tembo[bot] <208362400+tembo-io[bot]@users.noreply.github.com> --- packages/signals/src/core/attribution-hooks.ts | 3 ++- packages/signals/src/core/lanes.ts | 3 +++ packages/signals/tests/attribution-flush-changes.test.ts | 6 ++++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/signals/src/core/attribution-hooks.ts b/packages/signals/src/core/attribution-hooks.ts index 0469e99094..018c91e729 100644 --- a/packages/signals/src/core/attribution-hooks.ts +++ b/packages/signals/src/core/attribution-hooks.ts @@ -93,7 +93,8 @@ export interface AttributionHooks { write(el: Signal | Computed, prev: unknown, value: unknown): void; /** * A value for `el` was staged: queued for this flush's commit - * (`queuePendingNode`) or put on a transaction's or lane's list (`list`). + * (`queuePendingNode`), written as an optimistic guess (`optimisticWrite`), + * or put on a transaction's or lane's list (`list`). * What the screen shows of `el` may change at the end of this drain, or of * a later one if the value is held. The engine compares what `el` showed * before with what it shows at `flushEnd` — the one place a drain's visible diff --git a/packages/signals/src/core/lanes.ts b/packages/signals/src/core/lanes.ts index 73725e4a46..169dc3fb57 100644 --- a/packages/signals/src/core/lanes.ts +++ b/packages/signals/src/core/lanes.ts @@ -60,6 +60,7 @@ import { STATUS_PENDING, STATUS_UNINITIALIZED } from "./constants.js"; +import { attrHooks } from "./attribution-hooks.js"; import { bornInto, creatorReads, ext, stagedRead, tracking } from "./core.js"; import { NotReadyError, type StatusError } from "./error.js"; import { @@ -198,6 +199,8 @@ export function optimisticWrite(n: Signal | Computed, v: T | ((prev: T) if (typeof v === "function") v = (v as (prev: T) => T)(prev); if (n._equals && n._equals(prev, v) && !(n._config & CONFIG_GUESS)) return v; pendingGuesses.push(n, v, question || nextQuestion()); + // Observe: staged where it was written, so the guess carries its writer's origin. + if (__OBSERVE__ && attrHooks !== null) attrHooks.staged(n); armGuessing(); guessed = true; schedule(); diff --git a/packages/signals/tests/attribution-flush-changes.test.ts b/packages/signals/tests/attribution-flush-changes.test.ts index 8b2c4e3402..dbc7090dd3 100644 --- a/packages/signals/tests/attribution-flush-changes.test.ts +++ b/packages/signals/tests/attribution-flush-changes.test.ts @@ -167,16 +167,18 @@ describe("FlushEvent.changes", () => { createRoot(() => createRenderEffect(status, () => {}, { name: "badge" })); flush(); flushes.length = 0; - const save = action(function* save() { + const submit = action(function* save() { setStatus("saving"); yield gate.promise; }); - const done = save(); + const done = submit(); flush(); expect(status()).toBe("saving"); expect(seen(flushes)).toEqual([ expect.objectContaining({ nodeName: "status", prev: "idle", value: "saving" }) ]); + // The guess carries the origin it was written under. + expect(flushes[0].changes[0].origin).toMatchObject({ kind: "action", name: "save" }); flushes.length = 0; gate.resolve(); await done; From ee6da4df2403ed0b349ce949a25326890346e00b Mon Sep 17 00:00:00 2001 From: Dev Agrawal Date: Sun, 8 Nov 2026 15:01:32 -0600 Subject: [PATCH 08/11] fix(signals): engine reads store census through registered tools Co-authored-by: tembo[bot] <208362400+tembo-io[bot]@users.noreply.github.com> --- .../signals/src/core/attribution-hooks.ts | 33 +++++++++++- packages/signals/src/core/attribution.ts | 52 +++++++++---------- packages/signals/src/store/store.ts | 27 +++++++++- 3 files changed, 81 insertions(+), 31 deletions(-) diff --git a/packages/signals/src/core/attribution-hooks.ts b/packages/signals/src/core/attribution-hooks.ts index 018c91e729..56748a6cff 100644 --- a/packages/signals/src/core/attribution-hooks.ts +++ b/packages/signals/src/core/attribution-hooks.ts @@ -161,7 +161,8 @@ export interface AttributionHooks { * wrote (`null`: any of them — an array length write, accessors, a * non-plain prototype). Fired once per setter exit, before its nodes are * told. The engine judges what it needs from the two containers (a spread - * copy where a draft mutation would do). + * copy where a draft mutation would do), through the `StoreTools` the + * store registered. */ storeWrite( target: object, @@ -338,6 +339,36 @@ export function setAttributionHooks(hooks: AttributionHooks | null): void { (globalThis as { [INSTALLED]?: AttributionHooks })[INSTALLED] = hooks ?? undefined; } +/** + * What the engine needs of the store to judge a `storeWrite` (the + * spread-copy census), registered by the store module at its init. The + * engine reads these instead of importing the store: an app without stores + * then pays nothing for the store when it turns attribution on (the size + * scenario "CSR, observe tier + attribution engine enabled"), and core keeps + * its layering — nothing under `core/` imports `store/`. A `storeWrite` + * can only arrive once the store module has loaded and registered. + */ +export interface StoreTools { + /** The target's path from its root: `store.user.address`. */ + pathOf(target: object): string; + /** A shallow store: a replaced value invalidates as one unit (#4686). */ + isShallow(target: object): boolean; + /** The keys this batch deleted on the target, if any. */ + deleted(target: object): Set | null; + /** The owner the store root was created under (undefined when unrecorded). */ + rootOwner(target: object): Owner | null | undefined; + /** The backing's owner slot, not a leaf. */ + ownerKey: PropertyKey; + unwrapValue(v: unknown): unknown; + targetsEqual(a: unknown, b: unknown): boolean; + isRawValue(v: unknown): boolean; + isWrappable(v: unknown): boolean; +} +export let storeTools: StoreTools | null = null; +export function registerStoreTools(tools: StoreTools): void { + storeTools = tools; +} + /** * Run `fn` as the handler of a user interaction: every root write it performs * (and every action step, effect or flight the write causes) is attributed to diff --git a/packages/signals/src/core/attribution.ts b/packages/signals/src/core/attribution.ts index 459c32916f..01f20823b1 100644 --- a/packages/signals/src/core/attribution.ts +++ b/packages/signals/src/core/attribution.ts @@ -1,6 +1,8 @@ import { setAttributionHooks, + storeTools, type AttributionHooks, + type StoreTools, type InteractionRef, type NavigationRef, type Transition @@ -32,9 +34,6 @@ import { reportDiagnostic } from "./dev.js"; import { blockersOf, GlobalQueue } from "./scheduler.js"; -import { storeRootOwner, targetsEqual, unwrapValue } from "../store/store.js"; -import { $OWNER, type StoreTarget } from "../store/target.js"; -import { isRawValue, isWrappable } from "../store/types.js"; import type { Computed, Owner, Signal } from "./types.js"; /** @@ -2262,57 +2261,54 @@ function checkRelayTear( const immutableReported = new Set(); -/** A store target's path from its root: `store.user.address`. */ -function storePathOf(t: StoreTarget): string { - let path = ""; - for (let cur: StoreTarget | null = t; cur !== null; cur = cur.u) - path = cur.pk === null ? "store" + path : "." + String(cur.pk) + path; - return path; -} const REPLACED_CENSUS_MAX = 64; -function sameLeaf(a: unknown, b: unknown): boolean { +function sameLeaf(tools: StoreTools, a: unknown, b: unknown): boolean { if (a === b) return true; if (a === null || b === null || typeof a !== "object" || typeof b !== "object") return false; - return unwrapValue(a) === unwrapValue(b) || targetsEqual(a, b); + return tools.unwrapValue(a) === tools.unwrapValue(b) || tools.targetsEqual(a, b); } -/** storeWrite: the containers a setter batch replaced wholesale, each with its leaf census. */ +/** storeWrite: the containers a setter batch replaced wholesale, each with its + * leaf census — through the `StoreTools` the store registered (the engine + * does not import the store; see attribution-hooks.ts). */ function censusReplacedContainers( - t: StoreTarget, + t: object, old: Record, pb: Record, writtenKeys: Iterable | null ): void { + const tools = storeTools; // Shallow stores (and raw / unwrappable leaves) invalidate the whole // replaced value as one unit — leaf draft assignment does not narrow // who re-runs (#4686). The spread-copy diagnostic only helps deep stores. - if (t.s) return; + if (tools === null || tools.isShallow(t)) return; const keys = writtenKeys ?? Reflect.ownKeys(pb); const isArray = Array.isArray(pb); - const owner = storeRootOwner(t); + const owner = tools.rootOwner(t); + const deleted = tools.deleted(t); // Items the array held before the write: an index that now holds one of // them was moved into place (an in-place splice / shift), not replaced. let oldArrayItems: Set | null = null; for (const key of keys) { - if ((isArray && key === "length") || key === $OWNER) continue; - if (t.del?.has(key)) continue; - const ov = unwrapValue(old[key as any]); - const nv = unwrapValue(pb[key as any]); + if ((isArray && key === "length") || key === tools.ownerKey) continue; + if (deleted?.has(key)) continue; + const ov = tools.unwrapValue(old[key as any]); + const nv = tools.unwrapValue(pb[key as any]); if ( ov === null || nv === null || typeof ov !== "object" || typeof nv !== "object" || ov === nv || - targetsEqual(ov, nv) + tools.targetsEqual(ov, nv) ) continue; // A raw / unwrappable previous value is itself a leaf: replacing it is // the write, not a container copy the store could have mutated in place. - if (isRawValue(ov) || !isWrappable(ov)) continue; + if (tools.isRawValue(ov) || !tools.isWrappable(ov)) continue; if (isArray) { if (oldArrayItems === null) { oldArrayItems = new Set(); - for (const item of old as unknown[]) oldArrayItems.add(unwrapValue(item)); + for (const item of old as unknown[]) oldArrayItems.add(tools.unwrapValue(item)); } if (oldArrayItems.has(nv)) continue; } @@ -2324,16 +2320,16 @@ function censusReplacedContainers( total = (nv as unknown[]).length; if (total > REPLACED_CENSUS_MAX) continue; const oldItems = new Set(); - for (const item of ov as unknown[]) oldItems.add(unwrapValue(item)); - for (const item of nv as unknown[]) if (oldItems.has(unwrapValue(item))) unchanged++; + for (const item of ov as unknown[]) oldItems.add(tools.unwrapValue(item)); + for (const item of nv as unknown[]) if (oldItems.has(tools.unwrapValue(item))) unchanged++; } else { const nkeys = Object.keys(nv); total = nkeys.length; if (total > REPLACED_CENSUS_MAX) continue; - for (const k of nkeys) if (sameLeaf((ov as any)[k], (nv as any)[k])) unchanged++; + for (const k of nkeys) if (sameLeaf(tools, (ov as any)[k], (nv as any)[k])) unchanged++; } checkImmutableUpdate( - storePathOf(t) + "." + String(key), + tools.pathOf(t) + "." + String(key), isArr, total, unchanged, @@ -4672,7 +4668,7 @@ const engineHooks: AttributionHooks = { trackHoldMerge(target, outgoing); }, storeWrite(target, old, written, keys) { - censusReplacedContainers(target as StoreTarget, old, written, keys); + censusReplacedContainers(target, old, written, keys); }, listChurn(el, removed, created, newLen, keyed) { checkListIdentity(el, removed, created, newLen, keyed); diff --git a/packages/signals/src/store/store.ts b/packages/signals/src/store/store.ts index ff0c43e39f..05bc22bc24 100644 --- a/packages/signals/src/store/store.ts +++ b/packages/signals/src/store/store.ts @@ -28,7 +28,7 @@ * setter (`wk`); overlay drafts for large plain records; zero allocation * for adopted-but-unread objects. */ -import { attrHooks } from "../core/attribution-hooks.js"; +import { attrHooks, registerStoreTools } from "../core/attribution-hooks.js"; import { $REFRESH, CONFIG_CHILDREN_FORBIDDEN, @@ -553,7 +553,7 @@ function storeRoot(target: StoreTarget): StoreTarget { while (root.u !== null) root = root.u; return root; } -export function storeRootOwner(target: StoreTarget): Owner | null | undefined { +function storeRootOwner(target: StoreTarget): Owner | null | undefined { return storeOwners!.get(storeRoot(target)); } function stampNodeOwner(created: Signal, target: StoreTarget): void { @@ -2790,6 +2790,29 @@ function snapshotRecord( return copy ?? src; } +// Observe: what the attribution engine needs of a store to judge a +// `storeWrite` (its spread-copy census). Registered here so the engine never +// imports this module — an app without stores does not load it for turning +// attribution on. +if (__OBSERVE__) { + registerStoreTools({ + pathOf(target) { + let path = ""; + for (let cur: StoreTarget | null = target as StoreTarget; cur !== null; cur = cur.u) + path = cur.pk === null ? "store" + path : "." + String(cur.pk) + path; + return path; + }, + isShallow: target => (target as StoreTarget).s, + deleted: target => (target as StoreTarget).del, + rootOwner: target => storeRootOwner(target as StoreTarget), + ownerKey: $OWNER, + unwrapValue, + targetsEqual, + isRawValue, + isWrappable + }); +} + // Devtools `DEV.setSignal` for store property slots (#4679): go through the // store setter so the backing state and its readers stay consistent. if (__DEV__) { From 99ddc35676481ca7e9dd9db5ccd04c4ec8aacafc Mon Sep 17 00:00:00 2001 From: Dev Agrawal Date: Sun, 8 Nov 2026 15:19:00 -0600 Subject: [PATCH 09/11] fix(signals): guard async landing re-entrancy, skip origin walk unwatched Co-authored-by: tembo[bot] <208362400+tembo-io[bot]@users.noreply.github.com> --- documentation/solid-2.0/08-dev-diagnostics.md | 2 +- packages/signals/src/core/attribution.ts | 11 ++++++++--- packages/signals/src/core/core.ts | 9 +++++++-- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/documentation/solid-2.0/08-dev-diagnostics.md b/documentation/solid-2.0/08-dev-diagnostics.md index 59b3288008..2a7c982cf8 100644 --- a/documentation/solid-2.0/08-dev-diagnostics.md +++ b/documentation/solid-2.0/08-dev-diagnostics.md @@ -1221,7 +1221,7 @@ createRoot(() => { **Records and clocks.** Everything the engine hands out — `RerunEvent`, `InteractionEvent`, `HoldEvent`, `NavigationEvent` — is a record with an absolute `at` on the `performance.now()` clock (`RerunEvent.at` the run's start, `HoldEvent.at` the start of the wait, `NavigationEvent.at`/`InteractionEvent.at` the request/dispatch) plus durations from it (`holdMs`, `settledMs`, `selfMs`). Epoch time for an exporter is `performance.timeOrigin + at` (milliseconds). Without cross-origin isolation the browser quantizes `performance.now()` to 100µs, so a single run's `selfMs` is often `0`; the per-interaction `settledMs` is the wall-clock number to report. Records are serializable as emitted: none carries a live graph reference — a re-run names its scope by `nodeId` (the engine's per-node id, stable across the scope's runs in the process, distinct between scopes; in-process consumers get the node itself as the listener's second argument, `live`) — while the frame objects that join records (`origin`, `interaction`) are the same object across records in-process, so join by identity there and by `ChangeOrigin.run`/`at`/`name` once they have left it. `OBSERVE.records.subscribe(type, listener)` delivers each record synchronously at the moment it is complete (a re-run at recompute end; an interaction, hold or navigation when it settles), bottom-up: a hold before the navigation it held, before the interaction that performed it — the same objects `history(type)` keeps. A listener runs inside the engine and must not write signals; hand work off to a microtask. A `RerunEvent` is built — dep diff, previews, cause list, ring-buffer push — only while something wants it: a `rerun` listener, an imported fold (`costs`/`feedback`) or the console log. Without one the engine still keeps its per-node facts and runs every check (hot runs, hot time, wasted recompute, dep width) from them, allocates no record, and `history("rerun")` stays empty. -**Timeline records.** Five further record types describe the work itself rather than its outcome, shaped for a profiler track (`@solidjs/web/performance-tracks`, below) and for the responsiveness findings; they enter no ring buffer and are built only while something is subscribed to them (`OBSERVE.records.observed(type)`), so an app that does not listen pays nothing beyond the hook null-check. `flush` — one per scheduler drain: `at`, `durationMs`, `runs` and `created` inside it, `held` (a transition parked), `interaction`, and `changes` — what the drain changed on screen: one entry per signal, memo or store property whose shown value differs from what it showed before the drain (`nodeId`, `nodeName`, `key` for a store property, and `origin`, the action step, effect, interaction or navigation that staged the value), whether it got there by a commit, a held transition's landing, or an optimistic value appearing or lifting. A held write is listed by the drain that lands it and keeps the origin it was staged under, so a landing names its action; a node's first value and effects are not listed. The record stays serializable: the nodes travel beside it (`live.changes`, index for index), with their values before and after only for a listener that subscribed with `{ bodies: true }` — live values, which a listener that keeps them copies on delivery. Core reports where a value was staged (`AttributionHooks.staged`); the engine compares what each staged node showed before and after the drain, which is also how `OPTIMISTIC_REVERTED` is judged. `create` — a computation's creation run (`RerunEvent`'s shape without causes: `nodeId`, `nodeName`, `nodeKind`, `at`, `selfMs`, `totalMs`, `depCount`, `phase`, `held`, `interaction`) — the mount work that no `RerunEvent` describes, and what `InteractionEvent.created` counts. `effect` — an effect's callback, the imperative half that writes the DOM: `nodeId`, `nodeName`, `at`, `durationMs`, `run`, `interaction`. `flight` — an async node's flight, kickoff to landing: `nodeId`, `nodeName`, `ownerPath`, `at`, `durationMs`, `outcome: "landed" | "abandoned"` (superseded before it landed), `interaction`. `fallback` — a `` boundary's fallback, displayed to hidden: `ownerPath`, `at`, `shownMs`, `interaction`. `at` is the end of the drain that rendered the swap — the boundary's swap is its output's value, which lands with its frame ([RFC 05](05-async-data.md#loading-on-prop-dependencies-that-show-the-fallback-again)) and is judged displayed by the same drain-end comparison as `flush.changes` (a mount outside any drain displays it as the mount returns) — and a swap cleared before that (the content landed before the frame did, or the commit's own sweep cleared it ahead of any effect — the `LOADING_ON_OUTSIDE_HOLD` shape) was never on screen and is no record. Every derived `ChangeRecord` on a re-run's cause chain also carries the `nodeId` of the memo it came from, so a consumer can walk the propagation of one write node by node. +**Timeline records.** Five further record types describe the work itself rather than its outcome, shaped for a profiler track (`@solidjs/web/performance-tracks`, below) and for the responsiveness findings; they enter no ring buffer and are built only while something is subscribed to them (`OBSERVE.records.observed(type)`), so an app that does not listen pays nothing beyond the hook null-check. `flush` — one per scheduler drain: `at`, `durationMs`, `runs` and `created` inside it, `held` (a transition parked), `interaction`, and `changes` — what the drain changed on screen: one entry per signal, memo or store property whose shown value differs from what it showed before the drain (`nodeId`, `nodeName`, `key` for a store property, and `origin`, the action step, effect, interaction or navigation that staged the value), whether it got there by a commit, a held transition's landing, or an optimistic value appearing or lifting. A held write is listed by the drain that lands it and keeps the origin it was staged under, so a landing names its action; a node's first value and effects are not listed. The comparison is by identity (`Object.is`) of the shown value, not the node's `equals`: a signal with `equals: false` re-set to the same object re-runs its readers but is no change here, and a store object mutated in place is listed through the property nodes whose values moved. The record stays serializable: the nodes travel beside it (`live.changes`, index for index), with their values before and after only for a listener that subscribed with `{ bodies: true }` — live values, which a listener that keeps them copies on delivery. Core reports where a value was staged (`AttributionHooks.staged`); the engine compares what each staged node showed before and after the drain, which is also how `OPTIMISTIC_REVERTED` is judged. `create` — a computation's creation run (`RerunEvent`'s shape without causes: `nodeId`, `nodeName`, `nodeKind`, `at`, `selfMs`, `totalMs`, `depCount`, `phase`, `held`, `interaction`) — the mount work that no `RerunEvent` describes, and what `InteractionEvent.created` counts. `effect` — an effect's callback, the imperative half that writes the DOM: `nodeId`, `nodeName`, `at`, `durationMs`, `run`, `interaction`. `flight` — an async node's flight, kickoff to landing: `nodeId`, `nodeName`, `ownerPath`, `at`, `durationMs`, `outcome: "landed" | "abandoned"` (superseded before it landed), `interaction`. `fallback` — a `` boundary's fallback, displayed to hidden: `ownerPath`, `at`, `shownMs`, `interaction`. `at` is the end of the drain that rendered the swap — the boundary's swap is its output's value, which lands with its frame ([RFC 05](05-async-data.md#loading-on-prop-dependencies-that-show-the-fallback-again)) and is judged displayed by the same drain-end comparison as `flush.changes` (a mount outside any drain displays it as the mount returns; a fallback that renders the same value the output already showed — `null` over nothing — is no change, so no record) — and a swap cleared before that (the content landed before the frame did, or the commit's own sweep cleared it ahead of any effect — the `LOADING_ON_OUTSIDE_HOLD` shape) was never on screen and is no record. Every derived `ChangeRecord` on a re-run's cause chain also carries the `nodeId` of the memo it came from, so a consumer can walk the propagation of one write node by node. **Joining an interaction to the browser's INP entry.** An `InteractionEvent.at` is the DOM event's `timeStamp` when the runtime dispatched it (compiled event bindings pass it; `OBSERVE.attribution.withInteraction({ …, at: e.timeStamp }, fn)` for a custom dispatcher), and `inputDelayMs` is the queueing from that moment to the handler's entry — the browser's input delay. The `PerformanceEventTiming` entry for the same event (a `PerformanceObserver` on `"event"`) has `startTime === interaction.at` on the same clock and carries `interactionId`, so the join is `entry.startTime === interaction.at`: `inputDelayMs` accounts for the entry's `processingStart − startTime`, `handlerMs` for its processing, and `settledMs` extends past its `duration` to when Solid had the screen right. diff --git a/packages/signals/src/core/attribution.ts b/packages/signals/src/core/attribution.ts index 01f20823b1..7cd48c036d 100644 --- a/packages/signals/src/core/attribution.ts +++ b/packages/signals/src/core/attribution.ts @@ -3896,6 +3896,10 @@ interface Watched { /** Who staged the value the drain will show, when an origin was open. */ origin: ChangeOrigin | undefined; } +// Strong on the node, walked at every drain end: a node leaves when it +// settles (`unsettled`) or is disposed, and a held or lane node always +// settles — its transaction lands, merges or dissolves (`unhold`, `land`, +// `dissolveLane`); nothing drops one without clearing `_transaction`. const watched = new Map | Computed, Watched>(); /** What the screen shows of `n` — a revealed guess included (NOT_PENDING: none yet). */ @@ -3926,18 +3930,19 @@ function watch(n: Computed): void { function trackStaged(el: Signal | Computed): void { // Effects are not state; their values are what they rendered. if ((el as { _type?: number })._type) return; - const origin = ambientOrigin(); const w = watched.get(el); if (w !== undefined) { + const origin = ambientOrigin(); if (origin !== undefined) w.origin = origin; return; } // Watched for the flush record's audience, or for the revert finding (a - // guess enters a lane through `list`). + // guess enters a lane through `list`). Nothing wants it: no origin walk — + // this runs on every write while the engine is enabled. if (!records.observed("flush") && !(options.optimisticReverts && el._x?._transaction?._lane)) return; const guess = showsGuess(el); - watched.set(el, { shown: shownOf(el), guess, covered: el._value, origin }); + watched.set(el, { shown: shownOf(el), guess, covered: el._value, origin: ambientOrigin() }); } /** flushEnd: compare, judge the reverts, and list the changes for the record (`live` beside it). */ diff --git a/packages/signals/src/core/core.ts b/packages/signals/src/core/core.ts index dc050be196..04f4403900 100644 --- a/packages/signals/src/core/core.ts +++ b/packages/signals/src/core/core.ts @@ -2749,14 +2749,19 @@ let landed = false; * answer is whether the value changed (passed the equality gate). */ export function landThroughSetSignal(el: Computed, value: unknown): boolean { + // Saved and restored, not reset: a landing that reaches another landing + // synchronously (a write hook, a comparator) must not clear the outer's. + const outerLanding = landing; + const outerLanded = landed; landing = el; landed = false; try { setSignal(el, () => value); + return landed; } finally { - landing = null; + landing = outerLanding; + landed = outerLanded; } - return landed; } export function setSignal(el: Signal | Computed, v: T | ((prev: T) => T)): T { From 09462b3174cb9b45c883578a9b1c964710f80608 Mon Sep 17 00:00:00 2001 From: Dev Agrawal Date: Sun, 8 Nov 2026 16:03:39 -0600 Subject: [PATCH 10/11] fix(signals): keep DEV.hooks.onUpdate for devtools panels Co-authored-by: tembo[bot] <208362400+tembo-io[bot]@users.noreply.github.com> --- .changeset/flush-record-changes.md | 4 +-- packages/signals/src/core/dev.ts | 1 + packages/signals/src/core/scheduler.ts | 2 ++ packages/signals/tests/dev.test.ts | 47 ++++++++++++++++++++++++++ packages/solid/test/dev.spec.ts | 22 ++++++++++++ 5 files changed, 74 insertions(+), 2 deletions(-) diff --git a/.changeset/flush-record-changes.md b/.changeset/flush-record-changes.md index e570b2149b..bd9ce22a9c 100644 --- a/.changeset/flush-record-changes.md +++ b/.changeset/flush-record-changes.md @@ -3,9 +3,9 @@ "solid-js": patch --- -The `flush` record lists what each drain changed on screen; the attribution hooks shrink around it; `DEV.hooks.onUpdate` and `DEV.hooks.onStoreNodeUpdate` removed +The `flush` record lists what each drain changed on screen; the attribution hooks shrink around it; `DEV.hooks.onStoreNodeUpdate` removed - **`FlushEvent.changes`**: one entry per signal, memo or store property whose shown value the drain changed — by a commit, a held transition's landing, or an optimistic value appearing or lifting — with `nodeId`, `nodeName`, `key` (store properties) and `origin` (the action step, effect, interaction or navigation that staged the value; a held write keeps its origin, so a landing names its action). First values and effects are not listed. New types `FlushChange` and `FlushLive`: the record's `live` is now `{ changes: [{ node, prev?, value? }] }`, index for index with `event.changes`, the values only for a listener that subscribed with `{ bodies: true }`. - **Visibility is judged once, by the engine.** Core reports where a value was staged (`AttributionHooks.staged`, from `queuePendingNode` and `list`), never where it became visible; the engine compares what each staged node showed before and after the drain. `OPTIMISTIC_REVERTED` and the `fallback` record's display instant are judged from that comparison. Removed: the `optimisticReverted` hook, the engine's per-transaction copy of staged fallbacks (`stagedFallbacks` and its rebasing on settle/merge), and the `schedule()` a mounting boundary made so the engine got a drain end. - **Hook inputs reduced to what core knows** (internal `AttributionHooks`): `recomputeEnd(el, create, changed, phase, held)` replaces the `optimistic`/`transition` pair with the `phase` the engine derived from them, and absorbs `derivedChanged`; `asyncLanded(el, value, changed)` replaces `asyncStart` + `asyncEnd(direct)` — an async landing through `setSignal` is reported once, not first as a `write` (the engine no longer snapshots the node to tell whether it committed); `storeWrite(target, old, written, keys)` replaces `storeReplaced`, and the immutable-update census moves from the store into the engine; `boundaryFallback` takes the boundary's output instead of the transaction its swap is staged under. -- **Removed**: `DEV.hooks.onUpdate` (subscribe to `OBSERVE.records` `"flush"`) and `DEV.hooks.onStoreNodeUpdate` (store property changes are in `flush.changes`; properties nobody reads have no node and are not listed). Both records require `attribution.enable()` (observe and dev builds). +- **Removed**: `DEV.hooks.onStoreNodeUpdate` (store property changes are in `flush.changes`, which requires `attribution.enable()`; properties nobody reads have no node and are not listed). `DEV.hooks.onUpdate` stays: devtools refresh their panels from it without turning the engine on. diff --git a/packages/signals/src/core/dev.ts b/packages/signals/src/core/dev.ts index 8e1f7596ab..4e5b4eae47 100644 --- a/packages/signals/src/core/dev.ts +++ b/packages/signals/src/core/dev.ts @@ -39,6 +39,7 @@ import type { Computed, Link, Owner, Root, Signal } from "./types.js"; export interface DevHooks { onOwner?: (owner: Owner) => void; onGraph?: (value: any, owner: Owner | null) => void; + onUpdate?: () => void; /** @internal Every node read (the frames client's identity-drift check; `setReadWatch`). */ onRead?: (node: object) => void; } diff --git a/packages/signals/src/core/scheduler.ts b/packages/signals/src/core/scheduler.ts index a0ef5d946e..4b126b7b7b 100644 --- a/packages/signals/src/core/scheduler.ts +++ b/packages/signals/src/core/scheduler.ts @@ -44,6 +44,7 @@ import { resyncUnflushedCompanions } from "./core.js"; import { + DEV, emitDiagnostic, GRAPH_SIZE_WARN_AT, locationLines, @@ -984,6 +985,7 @@ export class GlobalQueue implements IQueue { n => pendingNodes.includes(n) || (n._config & (CONFIG_HELD | CONFIG_OVERRIDE)) !== 0 ); } + if (__DEV__) DEV.hooks.onUpdate?.(); } finally { this._running = false; passTx = null; diff --git a/packages/signals/tests/dev.test.ts b/packages/signals/tests/dev.test.ts index 9b0fb7f46b..270b0378f2 100644 --- a/packages/signals/tests/dev.test.ts +++ b/packages/signals/tests/dev.test.ts @@ -19,6 +19,7 @@ const { getChildren, getSignals, getParent, getSources, getObservers } = DEV!; afterEach(() => { DEV!.hooks.onOwner = undefined; DEV!.hooks.onGraph = undefined; + DEV!.hooks.onUpdate = undefined; }); describe("Phase 1a: Signal-to-owner tracking", () => { @@ -179,6 +180,52 @@ describe("Phase 1b: Notification hooks", () => { expect(entries.length).toBe(1); }); }); + + describe("onUpdate", () => { + it("fires once per flush", () => { + const calls: number[] = []; + DEV!.hooks.onUpdate = () => calls.push(1); + let setCount: (v: number) => void; + createRoot(() => { + const [count, _setCount] = createSignal(0); + setCount = _setCount; + createEffect( + () => count(), + () => {} + ); + flush(); + expect(calls.length).toBe(1); + }); + + setCount!(1); + flush(); + expect(calls.length).toBe(2); + }); + + it("fires once even with multiple signal writes", () => { + const calls: number[] = []; + DEV!.hooks.onUpdate = () => calls.push(1); + let setA: (v: number) => void; + let setB: (v: number) => void; + createRoot(() => { + const [a, _setA] = createSignal(0); + const [b, _setB] = createSignal(0); + setA = _setA; + setB = _setB; + createEffect( + () => a() + b(), + () => {} + ); + flush(); + }); + const before = calls.length; + + setA!(1); + setB!(2); + flush(); + expect(calls.length - before).toBe(1); + }); + }); }); describe("Phase 1c: Graph traversal helpers", () => { diff --git a/packages/solid/test/dev.spec.ts b/packages/solid/test/dev.spec.ts index 3bc3b3abbe..0edc2e9c98 100644 --- a/packages/solid/test/dev.spec.ts +++ b/packages/solid/test/dev.spec.ts @@ -19,6 +19,7 @@ afterEach(() => { if (DEV) { DEV.hooks.onOwner = undefined; DEV.hooks.onGraph = undefined; + DEV.hooks.onUpdate = undefined; } }); @@ -327,6 +328,27 @@ describe("DEV.hooks", () => { expect(entries.length).toBeGreaterThanOrEqual(1); }); + + test("onUpdate fires after flush", () => { + const calls: number[] = []; + let setCount!: (value: number) => number; + DEV!.hooks.onUpdate = () => calls.push(1); + + createRoot(() => { + const [count, _setCount] = createSignal(0); + setCount = _setCount; + createEffect( + () => count(), + () => {} + ); + flush(); + }); + + const initial = calls.length; + setCount(1); + flush(); + expect(calls.length).toBe(initial + 1); + }); }); describe("DEV graph traversal helpers", () => { From 9a26cbce3153f3caf2fc4ebdae2e2235873401a7 Mon Sep 17 00:00:00 2001 From: Dev Agrawal Date: Thu, 12 Nov 2026 01:03:42 +0000 Subject: [PATCH 11/11] fix(signals): keep DEV.hooks.onStoreNodeUpdate Devtools see every store write, including properties nothing reads, in any dev build, without the attribution engine. Co-Authored-By: Claude Opus 5.5 --- .changeset/flush-record-changes.md | 3 +-- packages/signals/src/core/dev.ts | 3 ++- packages/signals/src/store/store.ts | 20 +++++++++++++++- packages/signals/tests/dev.test.ts | 37 ++++++++++++++++++++++++----- packages/solid/test/dev.spec.ts | 23 ++++++++++++++++++ 5 files changed, 76 insertions(+), 10 deletions(-) diff --git a/.changeset/flush-record-changes.md b/.changeset/flush-record-changes.md index bd9ce22a9c..af09ecb858 100644 --- a/.changeset/flush-record-changes.md +++ b/.changeset/flush-record-changes.md @@ -3,9 +3,8 @@ "solid-js": patch --- -The `flush` record lists what each drain changed on screen; the attribution hooks shrink around it; `DEV.hooks.onStoreNodeUpdate` removed +The `flush` record lists what each drain changed on screen; the attribution hooks shrink around it - **`FlushEvent.changes`**: one entry per signal, memo or store property whose shown value the drain changed — by a commit, a held transition's landing, or an optimistic value appearing or lifting — with `nodeId`, `nodeName`, `key` (store properties) and `origin` (the action step, effect, interaction or navigation that staged the value; a held write keeps its origin, so a landing names its action). First values and effects are not listed. New types `FlushChange` and `FlushLive`: the record's `live` is now `{ changes: [{ node, prev?, value? }] }`, index for index with `event.changes`, the values only for a listener that subscribed with `{ bodies: true }`. - **Visibility is judged once, by the engine.** Core reports where a value was staged (`AttributionHooks.staged`, from `queuePendingNode` and `list`), never where it became visible; the engine compares what each staged node showed before and after the drain. `OPTIMISTIC_REVERTED` and the `fallback` record's display instant are judged from that comparison. Removed: the `optimisticReverted` hook, the engine's per-transaction copy of staged fallbacks (`stagedFallbacks` and its rebasing on settle/merge), and the `schedule()` a mounting boundary made so the engine got a drain end. - **Hook inputs reduced to what core knows** (internal `AttributionHooks`): `recomputeEnd(el, create, changed, phase, held)` replaces the `optimistic`/`transition` pair with the `phase` the engine derived from them, and absorbs `derivedChanged`; `asyncLanded(el, value, changed)` replaces `asyncStart` + `asyncEnd(direct)` — an async landing through `setSignal` is reported once, not first as a `write` (the engine no longer snapshots the node to tell whether it committed); `storeWrite(target, old, written, keys)` replaces `storeReplaced`, and the immutable-update census moves from the store into the engine; `boundaryFallback` takes the boundary's output instead of the transaction its swap is staged under. -- **Removed**: `DEV.hooks.onStoreNodeUpdate` (store property changes are in `flush.changes`, which requires `attribution.enable()`; properties nobody reads have no node and are not listed). `DEV.hooks.onUpdate` stays: devtools refresh their panels from it without turning the engine on. diff --git a/packages/signals/src/core/dev.ts b/packages/signals/src/core/dev.ts index 4e5b4eae47..20680f6676 100644 --- a/packages/signals/src/core/dev.ts +++ b/packages/signals/src/core/dev.ts @@ -40,6 +40,7 @@ export interface DevHooks { onOwner?: (owner: Owner) => void; onGraph?: (value: any, owner: Owner | null) => void; onUpdate?: () => void; + onStoreNodeUpdate?: (state: any, property: PropertyKey, value: any, prev: any) => void; /** @internal Every node read (the frames client's identity-drift check; `setReadWatch`). */ onRead?: (node: object) => void; } @@ -527,7 +528,7 @@ export interface Dev { * for example from a devtools state editor that only holds the node from * `getSignals` / `getSources`. Runs as an unowned write (devtools UI * events are not owned scopes). Store property slots go through the - * store setter path so the backing state and its readers stay in sync. + * store setter path so `onStoreNodeUpdate` and backing state stay in sync. * * @see https://github.com/solidjs/solid/issues/4679 */ diff --git a/packages/signals/src/store/store.ts b/packages/signals/src/store/store.ts index 05bc22bc24..77c386577e 100644 --- a/packages/signals/src/store/store.ts +++ b/packages/signals/src/store/store.ts @@ -75,6 +75,7 @@ import { import { asyncTailFlights, checkPostAwaitRead, + DEV, emitDiagnostic, registerDevStoreSlotWriter, registerGraph, @@ -1108,6 +1109,23 @@ function notifyWrites(t: StoreTarget): void { optHooks.pair(t, pb); } const old = t.v; + // Devtools mutation hook: full-key diff (dev-only cost) so unobserved + // writes report too. Overlay backings materialize first so the diff walks + // a real container. + if (__DEV__ && DEV.hooks.onStoreNodeUpdate) { + if (t.ovl) materializePB(t); + pb = t.pb!; + for (const key of Reflect.ownKeys(pb)) { + if ((Array.isArray(pb) && key === "length") || key === $OWNER) continue; + const ov = old[key as any]; + const nv = pb[key as any]; + if (!isEqual(ov, nv)) DEV.hooks.onStoreNodeUpdate(t.px, key, nv, ov); + } + for (const key of Reflect.ownKeys(old)) { + if (key in pb || key === $OWNER) continue; + DEV.hooks.onStoreNodeUpdate(t.px, key, undefined, old[key as any]); + } + } const nodes = t.n; // Written-keys bound: trap writes record their keys, so the notify visits // O(written) nodes instead of every subscription on the record (a selection @@ -2814,7 +2832,7 @@ if (__OBSERVE__) { } // Devtools `DEV.setSignal` for store property slots (#4679): go through the -// store setter so the backing state and its readers stay consistent. +// store setter so backing state and `onStoreNodeUpdate` stay consistent. if (__DEV__) { registerDevStoreSlotWriter((node, value) => { const host = (node as any)._host as StoreTarget; diff --git a/packages/signals/tests/dev.test.ts b/packages/signals/tests/dev.test.ts index 270b0378f2..37eb369c5f 100644 --- a/packages/signals/tests/dev.test.ts +++ b/packages/signals/tests/dev.test.ts @@ -20,6 +20,7 @@ afterEach(() => { DEV!.hooks.onOwner = undefined; DEV!.hooks.onGraph = undefined; DEV!.hooks.onUpdate = undefined; + DEV!.hooks.onStoreNodeUpdate = undefined; }); describe("Phase 1a: Signal-to-owner tracking", () => { @@ -226,6 +227,29 @@ describe("Phase 1b: Notification hooks", () => { expect(calls.length - before).toBe(1); }); }); + + describe("onStoreNodeUpdate", () => { + it("fires on store property mutation with old and new values", () => { + const updates: any[] = []; + DEV!.hooks.onStoreNodeUpdate = (state, property, value, prev) => + updates.push({ state, property, value, prev }); + + let setStore!: (fn: (s: { count: number; name: string }) => void) => void; + createRoot(() => { + const [, _setStore] = createStore({ count: 0, name: "test" }); + setStore = _setStore; + }); + + setStore(s => { + s.count = 5; + }); + flush(); + expect(updates.length).toBe(1); + expect(updates[0].property).toBe("count"); + expect(updates[0].value).toBe(5); + expect(updates[0].prev).toBe(0); + }); + }); }); describe("Phase 1c: Graph traversal helpers", () => { @@ -380,16 +404,17 @@ describe("Phase 1c: Graph traversal helpers", () => { }); }); - it("writes a store property slot through the store setter", () => { - const updates: number[] = []; + it("writes a store property slot and fires onStoreNodeUpdate", () => { + const updates: any[] = []; + DEV!.hooks.onStoreNodeUpdate = (state, property, value, prev) => { + updates.push({ state, property, value, prev }); + }; createRoot(() => { const [store] = createStore({ count: 0 }); // Touch the key so a slot node exists for getSources/devtools. createEffect( () => store.count, - v => { - updates.push(v); - } + () => {} ); flush(); const sources = getSources(getChildren(getOwner()!)[0] as any); @@ -398,7 +423,7 @@ describe("Phase 1c: Graph traversal helpers", () => { DEV!.setSignal(slot!, 9); flush(); expect(store.count).toBe(9); - expect(updates).toEqual([0, 9]); + expect(updates.some(u => u.property === "count" && u.value === 9)).toBe(true); }); }); diff --git a/packages/solid/test/dev.spec.ts b/packages/solid/test/dev.spec.ts index 0edc2e9c98..d91cfec7c4 100644 --- a/packages/solid/test/dev.spec.ts +++ b/packages/solid/test/dev.spec.ts @@ -20,6 +20,7 @@ afterEach(() => { DEV.hooks.onOwner = undefined; DEV.hooks.onGraph = undefined; DEV.hooks.onUpdate = undefined; + DEV.hooks.onStoreNodeUpdate = undefined; } }); @@ -349,6 +350,28 @@ describe("DEV.hooks", () => { flush(); expect(calls.length).toBe(initial + 1); }); + + test("onStoreNodeUpdate fires on store mutation with old/new values", () => { + const updates: any[] = []; + DEV!.hooks.onStoreNodeUpdate = (state: any, property: PropertyKey, value: any, prev: any) => + updates.push({ state, property, value, prev }); + + let setStore!: (fn: (s: { count: number; name: string }) => void) => void; + createRoot(() => { + const [, _setStore] = createStore({ count: 0, name: "test" }); + setStore = _setStore; + }); + + setStore(s => { + s.count = 5; + }); + flush(); + + expect(updates.length).toBe(1); + expect(updates[0].property).toBe("count"); + expect(updates[0].value).toBe(5); + expect(updates[0].prev).toBe(0); + }); }); describe("DEV graph traversal helpers", () => {