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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/effect-mainline-ownership-3412.md
Original file line number Diff line number Diff line change
@@ -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).
11 changes: 8 additions & 3 deletions packages/signals/src/core/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -653,10 +653,15 @@ export function recompute(el: Computed<any>, 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,
Expand Down
110 changes: 110 additions & 0 deletions packages/signals/tests/effect-mainline-ownership-3412.test.ts
Original file line number Diff line number Diff line change
@@ -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 = <T>(ms: number, value: T) => new Promise<T>(r => setTimeout(r, ms, value));

function setup(opts: { boundary: boolean; on: boolean; unconditional: boolean }) {
const out: Record<string, unknown> = {};
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");
});
}
});
77 changes: 77 additions & 0 deletions packages/web/test/loading-on-outside-reader-3412.spec.tsx
Original file line number Diff line number Diff line change
@@ -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 = <T = void,>(ms: number, value?: T) => new Promise<T>(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 (
<>
<p>Show: {String(show())}</p>
{opts.unconditional ? <p>Details: {details()}</p> : null}
<p>Panel: {show() ? details() : "hidden"}</p>
<p>
Copy:{" "}
{opts.boundary ? (
opts.on ? (
<Loading on={count()} fallback="Loading...">
{copy()}
</Loading>
) : (
<Loading fallback="Loading...">{copy()}</Loading>
)
) : (
copy()
)}
</p>
</>
);
}, 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();
});
}
});
6 changes: 5 additions & 1 deletion scripts/size/.size-limit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
{
Expand Down
Loading