diff --git a/.changeset/effect-mainline-ownership-3412.md b/.changeset/effect-mainline-ownership-3412.md new file mode 100644 index 000000000..5a0784eb7 --- /dev/null +++ b/.changeset/effect-mainline-ownership-3412.md @@ -0,0 +1,5 @@ +--- +"@solidjs/signals": patch +--- + +Keep a mainline-computed effect value out of a parked transaction. When an effect stamped by a held transaction recomputed on an unrelated write and no longer read the held source, the forced re-run inside that transaction re-claimed ownership of the value it had just published. A finalize-time re-entry (a `Loading` boundary's `on` reset flipping its fallback state) then parked the effect with the transaction, leaving a `show() ? details() : "hidden"` reader stale until the unrelated async settled (#3412). diff --git a/packages/signals/src/core/core.ts b/packages/signals/src/core/core.ts index 0567cc92c..a50c5f46a 100644 --- a/packages/signals/src/core/core.ts +++ b/packages/signals/src/core/core.ts @@ -653,10 +653,15 @@ export function recompute(el: Computed, create: boolean = false): void { (!create || el._statusFlags & STATUS_PENDING) && (!el._transition || hasOverride) && queuePendingNode(el); - el._transition && - isEffect && - activeTransition !== el._transition && + if (el._transition && isEffect && activeTransition !== el._transition) { + // The re-run refreshes the transaction's STAGED view (_pendingValue); the + // value this pass published in _value belongs to the run that just + // finished. Keep that ownership, or the effect phase parks a + // mainline-computed value with the transaction (#3412). + const owner = (el as any)._valueTransition; runInTransition(el._transition, () => recompute(el)); + (el as any)._valueTransition = owner; + } // Missed-wake reschedule (see the finally above): values this pass read // before the nested commit are stale, so run again now that the heap will // accept the node. Equality gates stop same-value landings from cascading, diff --git a/packages/signals/tests/effect-mainline-ownership-3412.test.ts b/packages/signals/tests/effect-mainline-ownership-3412.test.ts new file mode 100644 index 000000000..7a7d7c5f6 --- /dev/null +++ b/packages/signals/tests/effect-mainline-ownership-3412.test.ts @@ -0,0 +1,110 @@ +/** + * #3412: an effect stamped by a parked transaction (it was pending on a held + * async source) recomputes mainline on an unrelated write and no longer reads + * the held source. The forced re-run inside its own transaction refreshes the + * staged view but must not claim ownership of the mainline value; if a + * finalize-time re-entry (a Loading `on` reset's `_disabled` write) makes the + * transaction active before the effect phase, the mainline value is otherwise + * parked with it. + */ +import { describe, expect, test, beforeEach, afterEach, vi } from "vitest"; +import { + createLoadingBoundary, + createMemo, + createRenderEffect, + createRoot, + createSignal, + flush +} from "../src/index.js"; + +beforeEach(() => vi.useFakeTimers()); +afterEach(() => vi.useRealTimers()); + +const delay = (ms: number, value: T) => new Promise(r => setTimeout(r, ms, value)); + +function setup(opts: { boundary: boolean; on: boolean; unconditional: boolean }) { + const out: Record = {}; + let setCount!: (v: number) => void; + let setShow!: (v: boolean) => void; + createRoot(() => { + const [count, _setCount] = createSignal(0); + const [show, _setShow] = createSignal(true); + setCount = _setCount; + setShow = _setShow; + const copy = createMemo(async () => count(), undefined, { name: "copy" }); + const details = createMemo(() => delay(1500, copy()), undefined, { name: "details" }); + createRenderEffect( + () => String(show()), + v => { + out.show = v; + }, + { name: "E:show" } + ); + if (opts.unconditional) + createRenderEffect( + () => details(), + v => { + out.details = v; + }, + { name: "E:details" } + ); + createRenderEffect( + () => (show() ? details() : "hidden"), + v => { + out.panel = v; + }, + { name: "E:panel" } + ); + if (opts.boundary) { + const b = createLoadingBoundary( + () => copy(), + () => "Loading...", + opts.on ? { on: () => count() } : undefined + ); + createRenderEffect( + () => b(), + v => { + out.copy = v; + }, + { name: "E:copy" } + ); + } else { + createRenderEffect( + () => copy(), + v => { + out.copy = v; + }, + { name: "E:copy" } + ); + } + }); + return { out, setCount, setShow }; +} + +describe("effect mainline ownership (#3412)", () => { + for (const opts of [ + { boundary: true, on: true, unconditional: true }, + { boundary: true, on: false, unconditional: true }, + { boundary: false, on: false, unconditional: true }, + { boundary: true, on: true, unconditional: false } + ]) { + test(`panel hides when show flips ${JSON.stringify(opts)}`, async () => { + const { out, setCount, setShow } = setup(opts); + flush(); + await vi.advanceTimersByTimeAsync(1500); + flush(); + setCount(1); + flush(); + await vi.advanceTimersByTimeAsync(500); + flush(); + setShow(false); + flush(); + await Promise.resolve(); + flush(); + const afterShow = { ...out }; + await vi.advanceTimersByTimeAsync(1500); + flush(); + expect(afterShow.panel).toBe("hidden"); + }); + } +}); diff --git a/packages/web/test/loading-on-outside-reader-3412.spec.tsx b/packages/web/test/loading-on-outside-reader-3412.spec.tsx new file mode 100644 index 000000000..097003ed2 --- /dev/null +++ b/packages/web/test/loading-on-outside-reader-3412.spec.tsx @@ -0,0 +1,77 @@ +/** + * @jsxImportSource @solidjs/web + * @vitest-environment jsdom + */ +import { describe, expect, test, beforeEach, afterEach, vi } from "vitest"; +import { createSignal, createMemo, Loading, flush } from "solid-js"; +import { render } from "../src/index.js"; + +beforeEach(() => vi.useFakeTimers()); +afterEach(() => vi.useRealTimers()); + +const delay = (ms: number, value?: T) => new Promise(r => setTimeout(r, ms, value)); + +function setup(opts: { boundary: boolean; on: boolean; unconditional: boolean }) { + const div = document.createElement("div"); + let setCount!: (v: number) => void; + let setShow!: (v: boolean) => void; + const dispose = render(() => { + const [count, _setCount] = createSignal(0); + const [show, _setShow] = createSignal(true); + setCount = _setCount; + setShow = _setShow; + const copy = createMemo(async () => count()); + const details = createMemo(() => delay(1500, copy())); + return ( + <> +

Show: {String(show())}

+ {opts.unconditional ?

Details: {details()}

: null} +

Panel: {show() ? details() : "hidden"}

+

+ Copy:{" "} + {opts.boundary ? ( + opts.on ? ( + + {copy()} + + ) : ( + {copy()} + ) + ) : ( + copy() + )} +

+ + ); + }, div); + return { div, setCount, setShow, dispose }; +} + +describe("Loading on reset with an outside conditional reader (#3412)", () => { + for (const opts of [ + { boundary: true, on: true, unconditional: true }, + { boundary: true, on: false, unconditional: true }, + { boundary: false, on: false, unconditional: true }, + { boundary: true, on: true, unconditional: false } + ]) { + test(`panel hides when show flips ${JSON.stringify(opts)}`, async () => { + const { div, setCount, setShow, dispose } = setup(opts); + flush(); + await vi.advanceTimersByTimeAsync(1500); + flush(); + setCount(1); + flush(); + await vi.advanceTimersByTimeAsync(500); + flush(); + setShow(false); + flush(); + await Promise.resolve(); + flush(); + const afterShow = div.textContent; + await vi.advanceTimersByTimeAsync(1500); + flush(); + expect(afterShow).toContain("Panel: hidden"); + dispose(); + }); + } +}); diff --git a/scripts/size/.size-limit.js b/scripts/size/.size-limit.js index 3fb9ef302..50afb9b49 100644 --- a/scripts/size/.size-limit.js +++ b/scripts/size/.size-limit.js @@ -859,7 +859,11 @@ module.exports = [ // 28268 (+32 — the core seams and the collecting boundary). // Boundary reset ends the hold (#3375 ruling, 2026-09-12): 28.35 -> 28.40 KB, // measured at 28380 B against 28300 (+80); see the core floor note. - limit: "28.40 KB", + // Mainline effect ownership across the forced in-transaction re-run + // (#3412, 2026-09-13): 28.40 -> 28.45 KB, measured at 28408 B on the + // merge with `next` — the save/restore of `_valueTransition` in + // recompute, on top of #3413's companion-gate change. + limit: "28.45 KB", modifyEsbuildConfig }, {