diff --git a/.changeset/optimistic-reverted.md b/.changeset/optimistic-reverted.md new file mode 100644 index 000000000..2cec07b2e --- /dev/null +++ b/.changeset/optimistic-reverted.md @@ -0,0 +1,5 @@ +--- +"@solidjs/signals": patch +--- + +`OPTIMISTIC_REVERTED` (info, responsiveness): an optimistic value the screen showed was replaced by a different one — reverted at settle, or superseded by the truth — with the source and both values. `AttributionHooks.optimisticReverted` is the seam; `optimisticReverts: false` disables it. The runtime's own optimistic nodes (`isPending`/`latest` companions, derived overrides) are never judged. diff --git a/documentation/plans/responsiveness-findings-plan.md b/documentation/plans/responsiveness-findings-plan.md index c1e087c8e..8d51c8aa2 100644 --- a/documentation/plans/responsiveness-findings-plan.md +++ b/documentation/plans/responsiveness-findings-plan.md @@ -92,7 +92,14 @@ any pre-await write opens the hold path, which `SILENT_HOLD` judges. once and its interaction record's `settledMs` covers the await; the same handler with `isPending(source)` read in JSX fires nothing. -### 2. Stamp the browser's `interactionId` on the interaction +### 2. Stamp the browser's `interactionId` on the interaction — SATISFIED BY #3580 + +`interactionId` lives on the `PerformanceEventTiming` entry, delivered by a +`PerformanceObserver` after the fact; it is not readable during dispatch, so +a stamp was never available. #3580 made `InteractionEvent.at` the event's +`timeStamp`, which equals the entry's `startTime`: the join is by equality +on `at`. Nothing further to do in the runtime; the adapter recipe belongs +in the docs. - **Known:** the web runtime is inside the event dispatch when it opens the frame; Event Timing (`PerformanceEventTiming.interactionId`) assigns @@ -112,7 +119,15 @@ any pre-await write opens the hold path, which `SILENT_HOLD` judges. - **Proof:** a click whose Event Timing entry reports 480 ms produces an interaction record with the same id and a hold that names the blocker. -### 3. Optimistic reverts +### 3. Optimistic reverts — LANDED (signals; stores follow) + +Shipped as `AttributionHooks.optimisticReverted` (two sites in +`optimistic.ts`: supersession, and the drop at settle) and +`OPTIMISTIC_REVERTED` (see RFC 08). Left for a follow-up: optimistic +_stores_ (the overlay folds off per path in `_clearOptimisticStores`), and +naming the interaction that wrote the guess — optimistic writes bypass the +`write` hook today, so the node carries no origin stamp; stamping them +touches the interaction accounting and is its own change. - **Known:** the optimistic lane knows the shown value and the settled value; `asyncEnd`'s `prev`/`value` and the lane commit see both. diff --git a/documentation/solid-2.0/08-dev-diagnostics.md b/documentation/solid-2.0/08-dev-diagnostics.md index 56e580929..be0a4dfad 100644 --- a/documentation/solid-2.0/08-dev-diagnostics.md +++ b/documentation/solid-2.0/08-dev-diagnostics.md @@ -566,6 +566,12 @@ The other end of the `SILENT_HOLD` spectrum: too much feedback for too little wa When a hold commits, `stackedHolds.count` or more interactions (default 3) were waiting in it — the person clicked or typed again while the first answer was still in the air, and the runtime folded every repeat into the same wait. The pile is the symptom; the hold's own verdict is the cause, so the repair is the acknowledgement plus a control that does not accept the repeat. `warn`, once per hold. `data`: the hold's data (`holdMs`, `blockers`, `heldWrites`, `interaction`, `navigation`) plus `interactions`, the count. Subject: the first held write's node. `false` disables. +#### `OPTIMISTIC_REVERTED` + +**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). + ### Server rendering (`ssr`, `head`) The server runtime reports on the same channel. Two groups, two tiers. **Findings** are facts about a render whichever tier is running — an error a boundary contained, work the stream threw away, an error the server-function wire replaced. They ride `OBSERVE.diagnostics` in observe and dev builds (a production observability consumer subscribes to them; in dev they also print) and fold out of prod entirely. **Checks** are guidance for a developer at a console — a write on the server, an invalid preload descriptor — and exist only in the dev build, where they print like any client warning. Every entry carries `ownerPath` when it fired inside a component: on the server the component wrapper labels its owner `` exactly as the client's does, so `in › ` reads the same on both sides. Codes that already exist on the client (`ASYNC_OUTSIDE_LOADING_BOUNDARY`, `UNRECOGNIZED_INSERT_VALUE`) are shared, not duplicated; `data.side` or the message tells the platforms apart where it matters. @@ -854,68 +860,69 @@ The runtime derives a request's trace itself in every tier — the W3C `tracepar ## Diagnostic codes (quick reference) -| Code | Severity | Category | Trigger | -| ---------------------------------- | --------- | -------------- | -------------------------------------------------------------------------------------------------------------------------- | -| `REACTIVE_WRITE_IN_OWNED_SCOPE` | error | write | Reactive write/invalidation inside component/computation | -| `ASYNC_STORE_SETTER` | error | write | Store setter callback returned a Promise (setters are synchronous transactions) | -| `PENDING_ASYNC_UNTRACKED_READ` | error | async | Reading pending async outside tracking scope | -| `ASYNC_OUTSIDE_LOADING_BOUNDARY` | warn | async | Async computation outside Loading boundary (non-halting; root mount is deferred) | -| `CLEANUP_IN_FORBIDDEN_SCOPE` | error | lifecycle | `onCleanup` inside trackedEffect/onSettled | -| `SETTLED_CLEANUP_UNOWNED` | error | lifecycle | `onSettled` returned a cleanup in an unowned (out-of-band) scope | -| `PRIMITIVE_IN_FORBIDDEN_SCOPE` | error | lifecycle | Reactive primitive created inside trackedEffect/onSettled | -| `ACTION_CALLED_IN_OWNED_SCOPE` | error | write | `action()` invoked from a component body or computation | -| `MISSING_EFFECT_FN` | error | lifecycle | `createEffect` called without the effect function | -| `SYNC_NODE_RECEIVED_ASYNC` | error | lifecycle | `sync: true` computation returned a Promise / AsyncIterable | -| `INVALID_REFRESH_TARGET` | error | write | `refresh()` target is not a source accessor or refreshable store | -| `INVALID_AFFECTS_TARGET` | error | write | `affects()` given a key path, or a key on an accessor | -| `REACTIVITY_HALTED` | error | error | Uncaught error escaped every boundary; scheduling stopped (reported, cause to `reportError`) | -| `INVARIANT_VIOLATION` | error | error | Internal consistency check failed (throws under `__TEST__`, reported in dev) | -| `SETTLE_WALK_UNINITIALIZED_SOURCE` | error | lifecycle | Internal: settle walk reached a source that never produced a value (reported) | -| `STRICT_READ_UNTRACKED` | warn | strict-read | Untracked reactive read in component/effect body | -| `UNTRACKED_READ_AFTER_AWAIT` | warn | async | Async computation first read a signal/memo/store key after an `await`; never a dependency (dev; V8 engines only) | -| `PENDING_ASYNC_FORBIDDEN_SCOPE` | warn | async | Pending async read in trackedEffect/onSettled | -| `LOADING_ON_OUTSIDE_HOLD` | warn | async | `` changed but its data is also read outside the boundary and holds the frame: the fallback can never be seen | -| `NO_OWNER_EFFECT` | warn | lifecycle | Effect created without reactive owner | -| `NO_OWNER_CLEANUP` | warn | lifecycle | `onCleanup` called without owner | -| `NO_OWNER_BOUNDARY` | warn | lifecycle | Boundary created without owner | -| `RUN_WITH_DISPOSED_OWNER` | warn | owner | `runWithOwner` with disposed owner | -| `FLUSH_IN_EFFECT_CALLBACK` | warn | lifecycle | `flush()` from an effect callback (no-op; the drain is already running) | -| `HUGE_FAN_OUT` | warn | graph | One change reached 2000 live subscribers (always on) | -| `HUGE_FAN_IN` | warn | graph | One recompute tracked 2000 sources (always on) | -| `GRAPH_GROWTH` | warn | perf | The live owner count at the same route's settle climbed on 3 consecutive visits to 1.25× — something each visit leaves behind (attribution enabled) | -| `HOT_SCOPE_RERUNS` | warn | perf | 120+ re-runs of one scope in 1s (attribution enabled) | -| `HOT_SCOPE_FANOUT` | warn | perf | 5+/50+/500+ scopes hot from one root cause (attribution enabled) | -| `HOT_SCOPE_TIME` | warn | perf | 8ms+ self-time in one scope in 1s (attribution enabled) | -| `WIDE_SCOPE_DEPS` | warn | perf | Scope subscribed to 30+ sources (attribution enabled) | -| `WIDE_WRITE` | warn | perf | Committed write reached 250+ subscribers (attribution enabled) | -| `ASYNC_WATERFALL` | info/warn | perf | 2+/3+ sequential async flights: origin-proven (attribution enabled), or a `` boundary's passes (server, dev) | -| `UNSTABLE_MEMO_OUTPUT` | warn | perf | Memo returned a new-but-equivalent container 4+ runs running (attribution enabled) | -| `WASTED_RECOMPUTE` | warn | perf | 80%+ of a scope's 5+ runs in a second produced an unchanged value for 2ms+ of compute — inputs change, result does not (attribution enabled) | -| `EFFECT_WRITES_OWN_SOURCE` | info/warn | perf | Effect's write provably feeds back into its own inputs; `info` for multi-effect rings (attribution enabled) | -| `EFFECT_RELAY_TEAR` | info/warn | perf | Reader ran twice for one root change because an effect relayed it; `warn` when derivable or repeated (attribution enabled) | -| `IMMUTABLE_UPDATE_IN_STORE` | warn | perf | Store container replaced by a mostly-identical copy (attribution enabled) | -| `UNSTABLE_LIST_IDENTITY` | warn | perf | `mapArray`/`For` recreated rows for equivalent items (attribution enabled) | -| `SILENT_HOLD` | info/warn | responsiveness | Write held 100ms+/200ms+ by pending async with no on-screen acknowledgement (attribution enabled) | -| `LONG_HOLD` | info/warn | responsiveness | Acknowledged hold whose tail (last input → commit) ran 500ms+/1000ms+ (attribution enabled) | +| Code | Severity | Category | Trigger | +| ---------------------------------- | --------- | -------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `REACTIVE_WRITE_IN_OWNED_SCOPE` | error | write | Reactive write/invalidation inside component/computation | +| `ASYNC_STORE_SETTER` | error | write | Store setter callback returned a Promise (setters are synchronous transactions) | +| `PENDING_ASYNC_UNTRACKED_READ` | error | async | Reading pending async outside tracking scope | +| `ASYNC_OUTSIDE_LOADING_BOUNDARY` | warn | async | Async computation outside Loading boundary (non-halting; root mount is deferred) | +| `CLEANUP_IN_FORBIDDEN_SCOPE` | error | lifecycle | `onCleanup` inside trackedEffect/onSettled | +| `SETTLED_CLEANUP_UNOWNED` | error | lifecycle | `onSettled` returned a cleanup in an unowned (out-of-band) scope | +| `PRIMITIVE_IN_FORBIDDEN_SCOPE` | error | lifecycle | Reactive primitive created inside trackedEffect/onSettled | +| `ACTION_CALLED_IN_OWNED_SCOPE` | error | write | `action()` invoked from a component body or computation | +| `MISSING_EFFECT_FN` | error | lifecycle | `createEffect` called without the effect function | +| `SYNC_NODE_RECEIVED_ASYNC` | error | lifecycle | `sync: true` computation returned a Promise / AsyncIterable | +| `INVALID_REFRESH_TARGET` | error | write | `refresh()` target is not a source accessor or refreshable store | +| `INVALID_AFFECTS_TARGET` | error | write | `affects()` given a key path, or a key on an accessor | +| `REACTIVITY_HALTED` | error | error | Uncaught error escaped every boundary; scheduling stopped (reported, cause to `reportError`) | +| `INVARIANT_VIOLATION` | error | error | Internal consistency check failed (throws under `__TEST__`, reported in dev) | +| `SETTLE_WALK_UNINITIALIZED_SOURCE` | error | lifecycle | Internal: settle walk reached a source that never produced a value (reported) | +| `STRICT_READ_UNTRACKED` | warn | strict-read | Untracked reactive read in component/effect body | +| `UNTRACKED_READ_AFTER_AWAIT` | warn | async | Async computation first read a signal/memo/store key after an `await`; never a dependency (dev; V8 engines only) | +| `PENDING_ASYNC_FORBIDDEN_SCOPE` | warn | async | Pending async read in trackedEffect/onSettled | +| `LOADING_ON_OUTSIDE_HOLD` | warn | async | `` changed but its data is also read outside the boundary and holds the frame: the fallback can never be seen | +| `NO_OWNER_EFFECT` | warn | lifecycle | Effect created without reactive owner | +| `NO_OWNER_CLEANUP` | warn | lifecycle | `onCleanup` called without owner | +| `NO_OWNER_BOUNDARY` | warn | lifecycle | Boundary created without owner | +| `RUN_WITH_DISPOSED_OWNER` | warn | owner | `runWithOwner` with disposed owner | +| `FLUSH_IN_EFFECT_CALLBACK` | warn | lifecycle | `flush()` from an effect callback (no-op; the drain is already running) | +| `HUGE_FAN_OUT` | warn | graph | One change reached 2000 live subscribers (always on) | +| `HUGE_FAN_IN` | warn | graph | One recompute tracked 2000 sources (always on) | +| `GRAPH_GROWTH` | warn | perf | The live owner count at the same route's settle climbed on 3 consecutive visits to 1.25× — something each visit leaves behind (attribution enabled) | +| `HOT_SCOPE_RERUNS` | warn | perf | 120+ re-runs of one scope in 1s (attribution enabled) | +| `HOT_SCOPE_FANOUT` | warn | perf | 5+/50+/500+ scopes hot from one root cause (attribution enabled) | +| `HOT_SCOPE_TIME` | warn | perf | 8ms+ self-time in one scope in 1s (attribution enabled) | +| `WIDE_SCOPE_DEPS` | warn | perf | Scope subscribed to 30+ sources (attribution enabled) | +| `WIDE_WRITE` | warn | perf | Committed write reached 250+ subscribers (attribution enabled) | +| `ASYNC_WATERFALL` | info/warn | perf | 2+/3+ sequential async flights: origin-proven (attribution enabled), or a `` boundary's passes (server, dev) | +| `UNSTABLE_MEMO_OUTPUT` | warn | perf | Memo returned a new-but-equivalent container 4+ runs running (attribution enabled) | +| `WASTED_RECOMPUTE` | warn | perf | 80%+ of a scope's 5+ runs in a second produced an unchanged value for 2ms+ of compute — inputs change, result does not (attribution enabled) | +| `EFFECT_WRITES_OWN_SOURCE` | info/warn | perf | Effect's write provably feeds back into its own inputs; `info` for multi-effect rings (attribution enabled) | +| `EFFECT_RELAY_TEAR` | info/warn | perf | Reader ran twice for one root change because an effect relayed it; `warn` when derivable or repeated (attribution enabled) | +| `IMMUTABLE_UPDATE_IN_STORE` | warn | perf | Store container replaced by a mostly-identical copy (attribution enabled) | +| `UNSTABLE_LIST_IDENTITY` | warn | perf | `mapArray`/`For` recreated rows for equivalent items (attribution enabled) | +| `SILENT_HOLD` | info/warn | responsiveness | Write held 100ms+/200ms+ by pending async with no on-screen acknowledgement (attribution enabled) | +| `LONG_HOLD` | info/warn | responsiveness | Acknowledged hold whose tail (last input → commit) ran 500ms+/1000ms+ (attribution enabled) | | `UNTRACKED_ASYNC_HANDLER` | info/warn | responsiveness | Handler awaited 100ms+/200ms+ past its frame with no write before the `await` and no `action()`: a dead click no hold could judge (attribution enabled) | -| `ABANDONED_FLIGHTS` | warn | responsiveness | One async source abandoned 3+ flights in 1s, each superseded before landing — the request-per-keystroke signature (attribution enabled) | -| `FALLBACK_FLASH` | info | responsiveness | A `Loading` fallback showed for under 150ms — feedback for a wait too short to need it (attribution enabled) | -| `STACKED_HOLDS` | warn | responsiveness | 3+ interactions were waiting in one hold when it committed — repeats piled behind the same source (attribution enabled) | -| `SSR_RENDER_ERROR_CONTAINED` | error | ssr | Server render error routed by a boundary: `data.handling` fallback / client / failed (observe + dev) | -| `SSR_SUBTREE_ABANDONED` | warn | ssr | A failed fragment's pending descendants were discarded (observe + dev) | -| `SSR_STREAM_ABANDONED` | warn | ssr | Response stream cancelled or sink failed with fragments pending (observe + dev) | -| `LATE_HEADER_WRITE` | error | head | Response header written after the head was sent; dropped (observe + dev; dev throws) | -| `SERVER_FN_ERROR_SANITIZED` | error | ssr | Server-function error replaced with the generic Error on the wire; `data.error` is the original (observe + dev) | -| `FRAME_MARKER_CORRUPTED` | error | ssr | Frame slot range missing its end marker on the client — nesting or an HTML rewriter (observe + dev) | -| `SERVER_WRITE` | warn | write | Signal/store/optimistic setter ran during a server render; inert, will throw (dev; once per category) | -| `REVEAL_IN_RENDER_TO_STRING` | warn | ssr | Nested `` with collapsed/together under `renderToString` (dev) | -| `SSR_CLIENT_CONTENT_MASKED` | warn | ssr | Client-only content in a `` surfaced only after a server wait; the server's work was discarded (dev) | -| `LAZY_ASSET_UNMAPPED` | warn | ssr | `lazy()` component's client assets could not be resolved for the page (dev) | -| `PRELOAD_DESCRIPTOR_INVALID` | warn | head | `registerAsset("preload")` descriptor broke a field rule; link dropped or field ignored (dev) | -| `HEAD_TAG_INVALID` | warn | head | `useHead` registration the render could not honor; `data.reason` names the rule (dev) | -| `UNRECOGNIZED_INSERT_VALUE` | warn | render | Value at an insert position the renderer cannot render; skipped (dev; server and client) | -| `UNSCOPED_HOLE_ALLOCATED_IDS` | warn | render | Unscoped hole was handed a function whose content took ids at a position the other side does not share; keys permute (dev) | -| `BEHAVIOR_CLAIM_DROPPED` | warn | ssr | Behavior position on a server-rendered element got a spread prop or a server-local function (dev) | +| `ABANDONED_FLIGHTS` | warn | responsiveness | One async source abandoned 3+ flights in 1s, each superseded before landing — the request-per-keystroke signature (attribution enabled) | +| `FALLBACK_FLASH` | info | responsiveness | A `Loading` fallback showed for under 150ms — feedback for a wait too short to need it (attribution enabled) | +| `STACKED_HOLDS` | warn | responsiveness | 3+ interactions were waiting in one hold when it committed — repeats piled behind the same source (attribution enabled) | +| `OPTIMISTIC_REVERTED` | info | responsiveness | An optimistic value the screen showed was replaced by a different one — reverted at settle, or superseded by the truth (attribution enabled) | +| `SSR_RENDER_ERROR_CONTAINED` | error | ssr | Server render error routed by a boundary: `data.handling` fallback / client / failed (observe + dev) | +| `SSR_SUBTREE_ABANDONED` | warn | ssr | A failed fragment's pending descendants were discarded (observe + dev) | +| `SSR_STREAM_ABANDONED` | warn | ssr | Response stream cancelled or sink failed with fragments pending (observe + dev) | +| `LATE_HEADER_WRITE` | error | head | Response header written after the head was sent; dropped (observe + dev; dev throws) | +| `SERVER_FN_ERROR_SANITIZED` | error | ssr | Server-function error replaced with the generic Error on the wire; `data.error` is the original (observe + dev) | +| `FRAME_MARKER_CORRUPTED` | error | ssr | Frame slot range missing its end marker on the client — nesting or an HTML rewriter (observe + dev) | +| `SERVER_WRITE` | warn | write | Signal/store/optimistic setter ran during a server render; inert, will throw (dev; once per category) | +| `REVEAL_IN_RENDER_TO_STRING` | warn | ssr | Nested `` with collapsed/together under `renderToString` (dev) | +| `SSR_CLIENT_CONTENT_MASKED` | warn | ssr | Client-only content in a `` surfaced only after a server wait; the server's work was discarded (dev) | +| `LAZY_ASSET_UNMAPPED` | warn | ssr | `lazy()` component's client assets could not be resolved for the page (dev) | +| `PRELOAD_DESCRIPTOR_INVALID` | warn | head | `registerAsset("preload")` descriptor broke a field rule; link dropped or field ignored (dev) | +| `HEAD_TAG_INVALID` | warn | head | `useHead` registration the render could not honor; `data.reason` names the rule (dev) | +| `UNRECOGNIZED_INSERT_VALUE` | warn | render | Value at an insert position the renderer cannot render; skipped (dev; server and client) | +| `UNSCOPED_HOLE_ALLOCATED_IDS` | warn | render | Unscoped hole was handed a function whose content took ids at a position the other side does not share; keys permute (dev) | +| `BEHAVIOR_CLAIM_DROPPED` | warn | ssr | Behavior position on a server-rendered element got a spread prop or a server-local function (dev) | ## Run attribution — "why did this run" diff --git a/packages/signals/src/core/attribution-hooks.ts b/packages/signals/src/core/attribution-hooks.ts index c921a7d02..79b7164b6 100644 --- a/packages/signals/src/core/attribution-hooks.ts +++ b/packages/signals/src/core/attribution-hooks.ts @@ -199,6 +199,21 @@ 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 77770d381..5338f29e9 100644 --- a/packages/signals/src/core/attribution.ts +++ b/packages/signals/src/core/attribution.ts @@ -496,6 +496,15 @@ export interface AttributionOptions { * and every one of them waited on the same source. `false` disables. */ stackedHolds?: { count: number } | false; + /** + * Optimistic-revert finding: emit OPTIMISTIC_REVERTED (`info`) when an + * optimistic value the screen showed is replaced by a different one — + * reverted at settle, or superseded by the truth (default true). The + * runtime's own optimistic nodes — `isPending`/`latest` companions and + * derived overrides — are never judged: they are the acknowledgement + * machinery, not a guess the person saw. `false` disables. + */ + optimisticReverts?: boolean; } /** A fallback shown for less than this is a flash: feedback for a wait too short to need it. */ @@ -572,6 +581,7 @@ const defaultOptions = { graphGrowth: { visits: 3, ratio: 1.25 } as { visits: number; ratio: number } | false, abandonedFlights: { count: 3, windowMs: 1000 } as { count: number; windowMs: number } | false, fallbackFlashes: true, + optimisticReverts: true, stackedHolds: { count: 3 } as { count: number } | false }; let options: typeof defaultOptions = { ...defaultOptions }; @@ -2908,6 +2918,52 @@ function checkLongHold(event: HoldEvent, subject: Signal): void { if (severity === "warn") reportDiagnostic(entry); } +/** + * The person saw the guess, then the correction. An optimistic value is a + * promise the UI makes about the outcome; when the outcome differs — the + * action failed and the override lifted back to the old value, or the + * source answered with something else — the screen changes twice for one + * intent. Expected on failure and correct by construction (the override + * reverts; that is the feature), so `info`: a count that grows for one + * source is what says the guess, or the failure rate, is wrong. Judged by + * the node's own equality, so a structurally equal replacement is not a + * revert. + */ +function checkOptimisticRevert( + el: Signal | Computed, + shown: unknown, + truth: unknown, + how: "superseded" | "reverted" +): void { + // The runtime's own optimistic nodes are not guesses the person saw: an + // `isPending()` companion 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. Same predicate the hold + // census uses to skip them. + if (!options.optimisticReverts || isCompanion(el)) return; + const equals = (el as { _equals?: false | ((a: unknown, b: unknown) => boolean) })._equals; + if (equals && equals(shown, truth)) return; + const source = nodeName(el); + const message = + `[OPTIMISTIC_REVERTED] the optimistic value of ${source} showed ${preview(shown)}; it ` + + `${how === "superseded" ? "settled to" : "reverted to"} ${preview(truth)}. 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.`; + emitDiagnostic( + { + code: "OPTIMISTIC_REVERTED", + kind: "responsiveness", + severity: "info", + message, + nodeName: source, + data: { source, shown: preview(shown), truth: preview(truth), how } + }, + el + ); +} + // --- Graph growth ----------------------------------------------------------------- /** Per route: the graph's size at its last `visits` settles, oldest first. */ @@ -4129,6 +4185,9 @@ 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); + }, currentOrigin() { return ambientOrigin(); } diff --git a/packages/signals/src/core/dev.ts b/packages/signals/src/core/dev.ts index 43029a481..8f553fc1a 100644 --- a/packages/signals/src/core/dev.ts +++ b/packages/signals/src/core/dev.ts @@ -79,6 +79,7 @@ export type DiagnosticCode = | "ABANDONED_FLIGHTS" | "FALLBACK_FLASH" | "STACKED_HOLDS" + | "OPTIMISTIC_REVERTED" | "EFFECT_WRITES_OWN_SOURCE" | "EFFECT_RELAY_TEAR" | "IMMUTABLE_UPDATE_IN_STORE" diff --git a/packages/signals/src/core/optimistic.ts b/packages/signals/src/core/optimistic.ts index c6fab1964..53c681ae9 100644 --- a/packages/signals/src/core/optimistic.ts +++ b/packages/signals/src/core/optimistic.ts @@ -256,7 +256,13 @@ function resolveOptimisticNodes(nodes: OptimisticNode[]): void { node._value !== unwrapOverride(prevOverride) ) { if (derived) node._value = unwrapOverride(prevOverride); - else insertSubs(node, true); + else { + // The guess lifts and what was beneath it differs: the screen + // changes from the override to the committed value. + if (__OBSERVE__ && attrHooks !== null) + attrHooks.optimisticReverted(node, unwrapOverride(prevOverride), node._value, "reverted"); + insertSubs(node, true); + } } node._transition = null; if (node._x !== null) node._x._overrideOwner = null; @@ -323,6 +329,16 @@ function supersedeOverride(el: OptimisticNode, value: unknown): void { // 0 is mainline (no action): always the current question. if (origin && origin < el._x!._overrideStamp) return; el._config |= CONFIG_OVERRIDE_SUPERSEDED; + // A fresh landing is staged in `_pendingValue` (landOnOverride) or is the + // truth endOptimism read from it; the committed value with nothing staged + // is the value the guess covered — the screen goes back, not forward. + if (__OBSERVE__ && attrHooks !== null) + attrHooks.optimisticReverted( + el, + unwrapOverride(el._x!._overrideValue), + value, + el._pendingValue === NOT_PENDING && value === el._value ? "reverted" : "superseded" + ); const lane = el._x?._optimisticLane; if (lane) { const root = findLane(lane); diff --git a/packages/signals/tests/attribution-optimistic-revert.test.ts b/packages/signals/tests/attribution-optimistic-revert.test.ts new file mode 100644 index 000000000..13a5318a4 --- /dev/null +++ b/packages/signals/tests/attribution-optimistic-revert.test.ts @@ -0,0 +1,235 @@ +/** + * OPTIMISTIC_REVERTED — the person saw the guess, then the correction. + * + * Claim under test: when an optimistic override the screen displayed is + * replaced by a different value — the source answered with something else + * (`superseded`), or the override lifted at settle back to a differing + * committed value (`dropped`) — the engine emits one `info` finding naming + * the source, both values, and the interaction that wrote the guess. A guess + * the truth confirms, by identity or by the node's own equality, is silent. + */ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { attribution } from "../src/attribution.js"; +import { + action, + createMemo, + createOptimistic, + createRenderEffect, + createRoot, + createSignal, + flush, + isPending, + latest, + OBSERVE +} from "../src/index.js"; +import type { DiagnosticEvent } from "../src/core/dev.js"; + +afterEach(() => { + attribution.disable(); + flush(); + vi.restoreAllMocks(); +}); + +function deferred() { + let resolve!: (v: T) => void; + const promise = new Promise(r => (resolve = r)); + return { promise, resolve }; +} + +function arm(opts: { optimisticReverts?: boolean } = {}) { + vi.spyOn(console, "warn").mockImplementation(() => {}); + vi.spyOn(console, "info").mockImplementation(() => {}); + attribution.enable({ log: false, hotRuns: false, hotTime: false, waterfalls: false, ...opts }); + const findings: DiagnosticEvent[] = []; + OBSERVE!.diagnostics.subscribe(e => { + if (e.code === "OPTIMISTIC_REVERTED") findings.push(e); + }); + return { findings }; +} + +describe("OPTIMISTIC_REVERTED", () => { + it("reverted: the action ends without the guess coming true, and the value snaps back", async () => { + const { findings } = arm(); + const gate = deferred(); + const [status, setStatus] = createOptimistic("idle", { name: "status" }); + createRoot(() => createRenderEffect(status, () => {}, { name: "badge" })); + flush(); + const save = action(function* save() { + setStatus("saved"); + yield gate.promise; + // The server said no; nothing writes the truth. The override lifts. + }); + const p = save(); + flush(); + expect(status()).toBe("saved"); + expect(findings).toHaveLength(0); + gate.resolve(); + await p; + flush(); + expect(status()).toBe("idle"); + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ + code: "OPTIMISTIC_REVERTED", + kind: "responsiveness", + severity: "info", + nodeName: "status", + data: { source: "status", shown: '"saved"', truth: '"idle"', how: "reverted" } + }); + expect(findings[0].message).toContain('showed "saved"; it reverted to "idle"'); + }); + + it("superseded: the source answers with a different value while the guess is showing", async () => { + const { findings } = arm(); + const gate = deferred(); + const [count, setCount] = createSignal(0, { name: "count" }); + const [shown, setShown] = createOptimistic(() => count(), { name: "shownCount" }); + createRoot(() => createRenderEffect(shown, () => {}, { name: "counter" })); + flush(); + const increment = action(function* increment() { + setShown(1); + yield gate.promise; + setCount(2); // the server counted someone else's click too + }); + const p = increment(); + flush(); + expect(shown()).toBe(1); + gate.resolve(); + await p; + flush(); + expect(shown()).toBe(2); + expect(findings).toHaveLength(1); + expect(findings[0].data).toMatchObject({ + source: "shownCount", + shown: "1", + truth: "2", + how: "superseded" + }); + expect(findings[0].message).toContain("settled to 2"); + }); + + it("a guess the truth confirms is silent", async () => { + const { findings } = arm(); + const gate = deferred(); + const [count, setCount] = createSignal(0, { name: "count" }); + const [shown, setShown] = createOptimistic(() => count(), { name: "shownCount" }); + createRoot(() => createRenderEffect(shown, () => {}, { name: "counter" })); + flush(); + const increment = action(function* increment() { + setShown(1); + yield gate.promise; + setCount(1); + }); + const p = increment(); + flush(); + gate.resolve(); + await p; + flush(); + expect(shown()).toBe(1); + expect(findings).toHaveLength(0); + }); + + it("the node's own equality decides: a structurally equal correction is not a revert", async () => { + const { findings } = arm(); + const gate = deferred(); + const [user, setUser] = createSignal({ name: "Ada" }, { name: "user" }); + const [shownUser, setShownUser] = createOptimistic(() => user(), { + name: "shownUser", + equals: (a, b) => a.name === b.name + }); + createRoot(() => createRenderEffect(shownUser, () => {}, { name: "profile" })); + flush(); + const rename = action(function* rename() { + setShownUser({ name: "Grace" }); + yield gate.promise; + setUser({ name: "Grace" }); // a fresh object, the same name + }); + const p = rename(); + flush(); + gate.resolve(); + await p; + flush(); + expect(shownUser().name).toBe("Grace"); + expect(findings).toHaveLength(0); + }); + + it("an acknowledged hold — isPending and latest doing their job — is not a revert", async () => { + // The regression: isPending()'s companion is an optimistic signal that + // goes true while pending and back to false at commit, by design. It is + // the acknowledgement SILENT_HOLD asks for, not a guess the person saw. + const { findings } = arm(); + const [page, setPage] = createSignal(1, { name: "page" }); + let resolve!: (v: string) => void; + const posts = createMemo( + () => { + const p = page(); + return new Promise(r => (resolve = v => r(`${v}-p${p}`))); + }, + { name: "posts" } + ); + const shown: string[] = []; + createRoot(() => { + createRenderEffect(posts, v => void shown.push(String(v)), { name: "feed" }); + createRenderEffect( + () => isPending(() => posts()), + () => {}, + { name: "spinner" } + ); + createRenderEffect( + () => latest(page), + () => {}, + { name: "pageLabel" } + ); + }); + flush(); + resolve("a"); + await new Promise(r => setTimeout(r, 10)); + flush(); + expect(shown).toContain("a-p1"); + // A held write: the companions flip on, then off at commit. + setPage(2); + flush(); + await new Promise(r => setTimeout(r, 10)); + resolve("b"); + await new Promise(r => setTimeout(r, 10)); + flush(); + expect(shown).toContain("b-p2"); + expect(findings).toHaveLength(0); + }); + + it("`optimisticReverts: false` disables the finding", async () => { + const { findings } = arm({ optimisticReverts: false }); + const gate = deferred(); + const [status, setStatus] = createOptimistic("idle", { name: "status" }); + createRoot(() => createRenderEffect(status, () => {}, { name: "badge" })); + flush(); + const save = action(function* save() { + setStatus("saved"); + yield gate.promise; + }); + const p = save(); + flush(); + gate.resolve(); + await p; + flush(); + expect(status()).toBe("idle"); + expect(findings).toHaveLength(0); + }); + + it("with no engine, the sites are inert", async () => { + const gate = deferred(); + const [status, setStatus] = createOptimistic("idle", { name: "status" }); + createRoot(() => createRenderEffect(status, () => {}, { name: "badge" })); + flush(); + const save = action(function* save() { + setStatus("saved"); + yield gate.promise; + }); + const p = save(); + flush(); + gate.resolve(); + await p; + flush(); + expect(status()).toBe("idle"); + expect(attribution.history()).toEqual([]); + }); +}); diff --git a/packages/solid/skills/reactivity-diagnostics/SKILL.md b/packages/solid/skills/reactivity-diagnostics/SKILL.md index 475cb77bf..a3f5d81d4 100644 --- a/packages/solid/skills/reactivity-diagnostics/SKILL.md +++ b/packages/solid/skills/reactivity-diagnostics/SKILL.md @@ -558,6 +558,33 @@ Do NOT block input by removing the hold or making the write synchronous; the repeats are a reaction to silence, and the hold is what keeps the screen consistent while the answer arrives. +### OPTIMISTIC_REVERTED + +An optimistic value the screen showed was replaced by a different one: the +person saw the guess, then the correction. `data.how` says which road: +`reverted` — the action ended without the guess coming true and the value +snapped back to what it covered; `superseded` — the real value landed and +differed from the guess (the server counted differently, rejected part of +the input). Both are the runtime doing exactly what optimistic UI promises, +which is why this is `info` and never reaches the console; it is a count to +read, not a warning to silence. Read it two ways: + +- One source, many reverts: the failure is common and the UI is hiding it. + Show the failure where the value renders — catch in the action and write + an error the UI reads, or put the reader behind an `Errored` boundary — so + the snap-back is explained rather than silent. +- One source, many supersessions: the guess is systematically wrong for this + input (it ignores what the server adds or normalizes). Guess less: write + only the part of the value the client can know, or none, and read + `isPending()` for the rest. + +Do NOT "fix" a revert by writing the guess to the authoritative signal so it +cannot revert — that is not optimistic UI, it is lying to the user about what +happened. The runtime's own optimistic nodes — the `isPending()`/`latest()` +companions that acknowledge a hold — are never reported; only values you +wrote optimistically are. Optimistic stores are not covered yet; a store-path revert shows +up only as the reader's re-run. + ### Where to start: `feedback()` Before chasing individual `SILENT_HOLD` events, read the ranked tables — the diff --git a/scripts/size/.size-limit.js b/scripts/size/.size-limit.js index 6666fb8e8..ec56013ba 100644 --- a/scripts/size/.size-limit.js +++ b/scripts/size/.size-limit.js @@ -2206,7 +2206,22 @@ module.exports = [ // 30,649 (+719 B; the PR's own +769 was against the pre-#3604 28,999 — // the repair texts share more back-references with four more findings // in the bundle). The tier did not move: 17,618 either way. - limit: "31.45 KB", + // OPTIMISTIC_REVERTED (2026-09-23, rebased): 29.05 -> 29.35 KB, measured at + // 29,262 B on a full build (+263 B over next's 28,999). The `optimisticReverted` + // hook, the node-equality gate, and the finding with its two-road text + // (reverted / superseded). The tier moved 38 B for the two hook sites in + // optimistic.ts; under its cap. + // Re-measured at landing (#3607 rebased over #3604/#3623/#3613/#3619/#3608, + // 2026-09-23): 31.45 -> 31.65 KB, measured at 31,590 B against `next`'s + // 31,368 (+222 B; the PR's own +263 was against the pre-#3604 28,999). + // The tier did not move on this base: 17,618 either way — the 38 B the + // note above saw was nameCache drift, the hook sites fold to the same + // minified shape. The companion guard (2026-09-24: `optimisticReverts` + // option + the `isCompanion` skip in checkOptimisticRevert, so isPending/ + // latest companions and derived overrides are never judged) is +35 B: + // 31,625 B, under the same cap (25 B of headroom; the next finding moves + // it). Tier unchanged at 17,618. + limit: "31.65 KB", modifyEsbuildConfig: observeEsbuildConfig }, {