Skip to content

Commit 65da5de

Browse files
ryansolidclaude
andauthored
fix(signals): defer skips a held mount's first effect run (#4543) (#4547)
* fix(signals): defer skips a held mount's first effect run (#4543) An effect created by a held pass is born held (A29): its first run is the commit's replay, not its creation's. effect() skipped only the synchronous first run for defer: true, so the replay called the effect with its initial value. The replay of a deferred effect born held (or a lane's node) now applies the value without calling back; later runs call it as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hPxMkC3jg3Uivh65gvaf2 * fix(signals): a deferred born-held effect drops its staging instead of stubbing Rework for size and creation cost: the one-shot callback stub cost +53 B minified on every core row and touched effect creation. A deferred effect now drops the staging of a first pass born held, so the commit has nothing to replay; the plain path keeps its shape (one ternary instead of the leading !defer). A source change during the hold still runs the effect once, at the commit; the spec covers it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hPxMkC3jg3Uivh65gvaf2 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 50dec1b commit 65da5de

3 files changed

Lines changed: 148 additions & 7 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@solidjs/signals": patch
3+
---
4+
5+
fix(signals): `defer: true` skips an effect's first run when a held update mounts it (#4543)

‎packages/signals/src/core/effect.ts‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,16 @@ export function effect<T>(
7979
// staged); the flush's commit replays this effect. Its first run is not this
8080
// creation's (A29). A lane's node likewise: its run is the lane's, queued
8181
// by the pass and released at the reveal (lanes.ts).
82-
!options?.defer &&
83-
node._pendingValue === NOT_PENDING &&
84-
!node._x?._transaction?._lane &&
85-
(node._type === EFFECT_USER || options?.schedule
86-
? globalQueue.enqueue(node._type, runEffect.bind(null, node))
87-
: // Type 0: the synchronous first render, not a queued run (D-19).
88-
runEffect(node, 0));
82+
// `defer` skips the first run, the commit's replay of a node born staged
83+
// included (#4543): it drops the staging, so the commit replays nothing.
84+
options?.defer
85+
? (node._pendingValue = NOT_PENDING)
86+
: node._pendingValue === NOT_PENDING &&
87+
!node._x?._transaction?._lane &&
88+
(node._type === EFFECT_USER || options?.schedule
89+
? globalQueue.enqueue(node._type, runEffect.bind(null, node))
90+
: // Type 0: the synchronous first render, not a queued run (D-19).
91+
runEffect(node, 0));
8992
if (__DEV__ && !node._parent) {
9093
const message =
9194
"[NO_OWNER_EFFECT] Effects created outside a reactive context will never be disposed";
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
/**
2+
* @jsxImportSource @solidjs/web
3+
* @vitest-environment jsdom
4+
*/
5+
import { expect, it } from "vitest";
6+
import {
7+
createEffect,
8+
createMemo,
9+
createRenderEffect,
10+
createSignal,
11+
flush,
12+
Loading,
13+
Show
14+
} from "solid-js";
15+
import { render } from "@solidjs/web";
16+
17+
// #4543: `defer: true` skips the effect's first run. An effect created in a
18+
// subtree that a held update mounts is still in its first run when the hold
19+
// commits; its source never changed, so the commit must not run it.
20+
const wait = (ms: number) => new Promise(r => setTimeout(r, ms));
21+
22+
async function mountChild(held: boolean) {
23+
const runs: unknown[] = [];
24+
const renders: unknown[] = [];
25+
const [path, setPath] = createSignal("a");
26+
const matched = createMemo(() => {
27+
const p = path();
28+
return p === "a" || !held ? p : wait(10).then(() => p);
29+
});
30+
function Child() {
31+
const [local, setLocal] = createSignal(1);
32+
const keys = createMemo(() => [local()]);
33+
createEffect(
34+
() => keys(),
35+
k => void runs.push(k),
36+
{ defer: true }
37+
);
38+
createRenderEffect(
39+
() => keys(),
40+
k => void renders.push(k),
41+
{ defer: true }
42+
);
43+
bump = setLocal;
44+
return <p>B</p>;
45+
}
46+
let bump!: (v: number) => void;
47+
const el = document.createElement("div");
48+
const dispose = render(
49+
() => (
50+
<Loading fallback="root">
51+
<Show when={matched() === "b"} fallback={<p>A</p>}>
52+
<Child />
53+
</Show>
54+
</Loading>
55+
),
56+
el
57+
);
58+
setPath("b");
59+
flush();
60+
await wait(80);
61+
flush();
62+
const text = el.textContent ?? "";
63+
const before = [...runs, ...renders];
64+
bump(2);
65+
flush();
66+
dispose();
67+
return { text, runs: before, after: [...runs, ...renders] };
68+
}
69+
70+
it("control: defer skips the first run when the mount is not held", async () => {
71+
expect(await mountChild(false)).toEqual({ text: "B", runs: [], after: [[2], [2]] });
72+
});
73+
74+
it("defer skips the first run when the mount is held by an async memo (#4543)", async () => {
75+
expect(await mountChild(true)).toEqual({ text: "B", runs: [], after: [[2], [2]] });
76+
});
77+
78+
it("a change during the hold runs a deferred effect once, at the commit (#4543)", async () => {
79+
const runs: unknown[] = [];
80+
const renders: unknown[] = [];
81+
let answerPath!: () => void;
82+
let answerData!: () => void;
83+
const [path, setPath] = createSignal("a");
84+
const matched = createMemo(() => {
85+
const p = path();
86+
return p === "a" ? p : new Promise<string>(r => (answerPath = () => r(p)));
87+
});
88+
let bump: ((v: number) => void) | undefined;
89+
function Child() {
90+
const [local, setLocal] = createSignal(1);
91+
const keys = createMemo(() => [local()]);
92+
createEffect(
93+
() => keys(),
94+
k => void runs.push(k),
95+
{ defer: true }
96+
);
97+
createRenderEffect(
98+
() => keys(),
99+
k => void renders.push(k),
100+
{ defer: true }
101+
);
102+
bump = setLocal;
103+
// A second async read keeps the mount held after `matched` answers.
104+
const data = createMemo(() => new Promise<string>(r => (answerData = () => r("d"))));
105+
return <p>B{data()}</p>;
106+
}
107+
const el = document.createElement("div");
108+
const dispose = render(
109+
() => (
110+
<Loading fallback="root">
111+
<Show when={matched() === "b"} fallback={<p>A</p>}>
112+
<Child />
113+
</Show>
114+
</Loading>
115+
),
116+
el
117+
);
118+
setPath("b");
119+
flush();
120+
answerPath();
121+
await wait(0);
122+
flush();
123+
expect([el.textContent, !!bump]).toEqual(["A", true]);
124+
bump!(3);
125+
flush();
126+
expect([runs, renders]).toEqual([[], []]);
127+
answerData();
128+
await wait(0);
129+
flush();
130+
expect(el.textContent).toBe("Bd");
131+
expect([runs, renders]).toEqual([[[3]], [[3]]]);
132+
dispose();
133+
});

0 commit comments

Comments
 (0)