diff --git a/.changeset/equals-netting-4483.md b/.changeset/equals-netting-4483.md new file mode 100644 index 0000000000..abaa123caf --- /dev/null +++ b/.changeset/equals-netting-4483.md @@ -0,0 +1,5 @@ +--- +"@solidjs/signals": patch +--- + +A write that a custom `equals` calls equal to the committed value now nets to it (D-8) while another write to the signal is staged, so it pends nothing (#4483). diff --git a/packages/signals/src/core/core.ts b/packages/signals/src/core/core.ts index bd34b91e96..31c6d4f89d 100644 --- a/packages/signals/src/core/core.ts +++ b/packages/signals/src/core/core.ts @@ -2543,10 +2543,17 @@ export function setSignal(el: Signal | Computed, v: T | ((prev: T) => T // user comparator must not run against `undefined` (matches recompute). // D-41: a write over a shown error is a change whatever the last good // value was — the error leaves at its commit (`commitPendingNode`). + // `custom`: with the verdict engine, whether `el` carries a comparator of + // its own — `isSame`'s fast-path test, taken once for this gate and D-8's + // netting below, so the default comparator's `===` runs inline and a + // rewrite loop pays nothing for the netting (CodSpeed `propagation: + // avoidable`). Without the engine it is `null`: the gate folds to the + // plain `isSame` call and the netting away, byte for byte as before. + const custom = observeFlightHook ? el._equals !== isEqual : null; const valueChanged = !!((el as Computed)._statusFlags & STATUS_UNINITIALIZED) || !!el._x?._shownError || - !isSame(el, currentValue, v); + (custom === false ? currentValue !== v : !isSame(el, currentValue, v)); if (!valueChanged) return v; // Attribution hook: this committed write is where a re-run chain begins. @@ -2568,7 +2575,22 @@ export function setSignal(el: Signal | Computed, v: T | ((prev: T) => T // A28: a held node rewritten outside a flush keeps its flushed staging // for every reader until the flush that carries the rewrite. else if (el._config & CONFIG_HELD && !globalQueue._running) stashFlushed(el); - el._pendingValue = v; + // D-8 (#4483): a staged rewrite the comparator calls equal to the + // committed value nets to it; staging the committed value itself lets + // every netted test (`_pendingValue === _value`) see writes that made no + // proposal. (Unstaged, the gate above already compared it.) + // The netted test reads it only with the verdict engine (`isPending`, + // `latest`): a build without it stages `v` as written. Under the default + // comparator only the committed object itself is equal, and `v` is already + // it, so a rewrite loop pays one local test (`custom`, from the gate). + // (`custom` is truthy only with the engine installed.) + el._pendingValue = + custom && + wasStaged && + !((el as Computed)._statusFlags & STATUS_UNINITIALIZED || el._x?._shownError) && + isSame(el, el._value, v) + ? el._value + : v; // A28 arm, gated on the load the write already pays for (`context`); the // arm itself is a cold helper so setSignal stays within every setter's // inlining budget, ~300 B bytecode. diff --git a/packages/signals/tests/netted-equals-4483.test.ts b/packages/signals/tests/netted-equals-4483.test.ts new file mode 100644 index 0000000000..fe90ddcbbe --- /dev/null +++ b/packages/signals/tests/netted-equals-4483.test.ts @@ -0,0 +1,66 @@ +// #4483 (D-8: a tick whose writes net to the slot's committed value made no +// proposal). An action rewrites a signal back to its committed value while a +// mainline write to it is still staged. Under a custom `equals`, a new object +// the comparator calls equal nets exactly like the committed object itself: +// the slot pends nothing, so `isPending` of a memo over it stays `false`. +// Without a custom `equals`, only identity nets; a new object is a proposal. +import { afterEach, describe, expect, it } from "vitest"; +import { + action, + createMemo, + createRenderEffect, + createRoot, + createSignal, + flush, + isPending, + resetErrorHalt +} from "../src/index.js"; + +afterEach(() => resetErrorHalt()); +const tick = async () => { + await new Promise(r => setTimeout(r, 0)); + flush(); +}; + +const run = async (custom: boolean, write: "committed" | "copy") => { + const one = { k: 1 }; + const [s, setS] = createSignal(one, custom ? { equals: (a, b) => a.k === b.k } : undefined); + const seen: boolean[] = []; + const dispose = createRoot(d => { + const m0 = createMemo(() => { + const v = s().k + 1; + if (v % 4 === 2) throw new Error(`m0:${v}`); + return v; + }); + createRenderEffect( + () => isPending(m0), + v => void seen.push(v) + ); + return d; + }); + flush(); + const act = action(function* () { + setS(write === "committed" ? one : { k: 1 }); + yield new Promise(() => {}); + }); + setS({ k: 2 }); + act(); + await tick(); + dispose(); + return seen; +}; + +describe("#4483: writes that net under a custom equals pend nothing (D-8)", () => { + it("nets the committed object itself (identity)", async () => { + expect(await run(true, "committed")).toEqual([false]); + }); + + it("nets a new object the comparator calls equal", async () => { + expect(await run(true, "copy")).toEqual([false]); + }); + + it("without a custom equals, nets only the committed object", async () => { + expect(await run(false, "committed")).toEqual([false]); + expect(await run(false, "copy")).toEqual([false, true]); + }); +});