Skip to content

Commit 50dec1b

Browse files
authored
fix(signals): preserve signed zero in staged signal reads (#4538)
* fix(signals): preserve signed zero in staged reads Split the netted-read return from the committed-screen return introduced by #4498. Strict equality identifies opposite signed zeros, so a write accepted by Object.is or equals: false must serve the staged payload itself. Keep the existing netting order and comparator behavior. Add observed reciprocal regressions for both signed-zero directions and both comparator options, plus a default-comparator control. * fix(signals): distinguish signed zero throughout staged netting Guard netting with SameValue as well as strict equality. Apply it to fast and strict reads, verdict routing and the seat oracle, preserving accepted NaN rewrites. Cover pending/reveal atomicity with plain and latest readers across action write locations.
1 parent 215c365 commit 50dec1b

5 files changed

Lines changed: 201 additions & 9 deletions

File tree

‎.changeset/staged-signed-zero.md‎

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+
Preserve signed zero and pending-state atomicity when a derivation reads a staged signal write accepted by a custom comparator.

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

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2091,16 +2091,19 @@ export function read<T>(el: Signal<T> | Computed<T>): T {
20912091
}
20922092
// A staging equal to the committed value is writes that netted (D-8): no
20932093
// proposal, so not a read of the frame's future — nor, for a verdict
2094-
// reader, a stale read of the tick's hold re-run at its landing (#4485):
2095-
// the committed value is the staging. (A plain signal is never
2096-
// uninitialized — the carve-out is `serve`'s.) Lane work over a folded
2097-
// lane (D-34), once the frame has a transaction, reads the screen too.
2094+
// reader, a stale read of the tick's hold re-run at its landing (#4485).
2095+
// (A plain signal is never uninitialized — the carve-out is `serve`'s.)
2096+
// `===` alone identifies opposite signed zeros. Require SameValue too;
2097+
// keep the strict check so an accepted NaN rewrite remains a proposal.
2098+
const netted = el._pendingValue === el._value && Object.is(el._pendingValue, el._value);
2099+
// Lane work over a folded lane (D-34), once the frame has a transaction,
2100+
// reads the screen too.
20982101
if (
2099-
el._pendingValue === el._value ||
2102+
netted ||
21002103
(((observeFlightHook && c._config & CONFIG_VERDICT) || (laneStageHook && passLane?.folded)) &&
21012104
stagedScreen(c as Computed<any>, el))
21022105
) {
2103-
if (__TEST__) shadowActual(el._pendingValue === el._value ? SLOT_P : SLOT_C);
2106+
if (__TEST__) shadowActual(netted ? SLOT_P : SLOT_C);
21042107
return el._value as T;
21052108
}
21062109
stagedRead(c as Computed<any>);
@@ -2397,7 +2400,12 @@ export function serve(el: Signal<any> | Computed<any>, c: Computed<any> | null):
23972400
// frame's future. Only the strict `untrack` read skips the fast block in
23982401
// dev (its `strictRead` test); a hydration snapshot or a promoted write
23992402
// reaches here in dev and prod alike, and keeps the mark.
2400-
if (!(__DEV__ && strictRead) || (el as Computed<any>)._fn || el._pendingValue !== el._value)
2403+
if (
2404+
!(__DEV__ && strictRead) ||
2405+
(el as Computed<any>)._fn ||
2406+
el._pendingValue !== el._value ||
2407+
!Object.is(el._pendingValue, el._value)
2408+
)
24012409
stagedRead(c!, el);
24022410
// D-41: the staged answer is an error (`stageError`).
24032411
if (el._config & CONFIG_STAGED_ERROR) {

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,12 @@ export function slot(
508508
* that netted, no proposal (`read`'s plain arm; #4452, #4485). A window's
509509
* read of it is the seam's verdict's rows below (`provisionalVerdict`
510510
* answers it final and routes it as they say). */
511-
const netted = staged && !isFn && !laneNode && el._pendingValue === el._value;
511+
const netted =
512+
staged &&
513+
!isFn &&
514+
!laneNode &&
515+
el._pendingValue === el._value &&
516+
Object.is(el._pendingValue, el._value);
512517
/** The staging as a slot: its value, or the error it stages (D-41). */
513518
const stagedSlot = stagedError ? SLOT_E : SLOT_P;
514519
/** The screen as a slot: the committed value, or the error still shown

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,10 @@ function verdictValue(el: Signal<any> | Computed<any>, c: Computed<any> | null):
572572
// reader is still the tick's verdict reader, shown now (D-25: the
573573
// posture is the pass's). A signal's staging only, as `read`'s arm.
574574
else if (tracked && globalQueue._running)
575-
flushTransaction && !(el as any)._fn && el._pendingValue === el._value
575+
flushTransaction &&
576+
!(el as any)._fn &&
577+
el._pendingValue === el._value &&
578+
Object.is(el._pendingValue, el._value)
576579
? route(c!, flushTransaction)
577580
: provisionalVerdict(c!, !latestActive && !uninitialized);
578581
// Outside a flush, the staging is an unflushed write (A28): the probe
Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
import { describe, expect, it } from "vitest";
2+
import {
3+
action,
4+
createMemo,
5+
createRenderEffect,
6+
createRoot,
7+
createSignal,
8+
flush,
9+
isPending,
10+
latest,
11+
untrack
12+
} from "../src/index.js";
13+
14+
describe.each([
15+
["Object.is", Object.is],
16+
["false", false]
17+
] as const)("staged signed zero with equals: %s", (_, equals) => {
18+
it.each([
19+
["-0 to +0", -0, 0],
20+
["+0 to -0", 0, -0]
21+
] as const)("updates an observed derivation from %s", (_, initial, next) => {
22+
const { dispose, value, setValue, reciprocal, seen } = createRoot(dispose => {
23+
const [value, setValue] = createSignal(initial, { equals });
24+
const reciprocal = createMemo(() => 1 / value());
25+
const seen: number[] = [];
26+
createRenderEffect(reciprocal, value => void seen.push(value));
27+
return { dispose, value, setValue, reciprocal, seen };
28+
});
29+
try {
30+
flush();
31+
expect(seen).toEqual([1 / initial]);
32+
33+
setValue(next);
34+
flush();
35+
36+
expect(value()).toBe(next);
37+
expect(reciprocal()).toBe(1 / next);
38+
expect(seen).toEqual([1 / initial, 1 / next]);
39+
} finally {
40+
dispose();
41+
}
42+
});
43+
44+
for (const where of ["in-action", "before-action"] as const)
45+
for (const strict of [false, true])
46+
it.each([
47+
["-0 to +0", -0, 0],
48+
["+0 to -0", 0, -0]
49+
] as const)(
50+
`keeps %s pending until reveal (${where}, strict: ${strict})`,
51+
async (_, initial, next) => {
52+
const gate = Promise.withResolvers<void>();
53+
const state = createRoot(dispose => {
54+
const [value, setValue] = createSignal(initial, { equals });
55+
const [other, setOther] = createSignal(0);
56+
const plain: [number, boolean][] = [];
57+
const ahead: [boolean, number][] = [];
58+
createRenderEffect(other, () => {});
59+
createRenderEffect(
60+
() => {
61+
value();
62+
const v = strict ? untrack(value, "signed-zero") : value();
63+
return [1 / v, isPending(value)] as [number, boolean];
64+
},
65+
v => void plain.push(v)
66+
);
67+
createRenderEffect(
68+
() => [isPending(value), 1 / latest(value)] as [boolean, number],
69+
v => void ahead.push(v)
70+
);
71+
return { dispose, value, setValue, setOther, plain, ahead };
72+
});
73+
try {
74+
flush();
75+
if (where === "before-action") state.setValue(next);
76+
action(function* () {
77+
state.setOther(1);
78+
if (where === "in-action") state.setValue(next);
79+
yield gate.promise;
80+
})();
81+
flush();
82+
83+
expect(state.value()).toBe(initial);
84+
expect(isPending(state.value)).toBe(true);
85+
expect(state.plain).toEqual([
86+
[1 / initial, false],
87+
[1 / initial, true]
88+
]);
89+
expect(state.ahead).toEqual([
90+
[false, 1 / initial],
91+
[true, 1 / next]
92+
]);
93+
94+
gate.resolve();
95+
await new Promise<void>(resolve => setTimeout(resolve, 0));
96+
flush();
97+
98+
expect(state.value()).toBe(next);
99+
expect(isPending(state.value)).toBe(false);
100+
expect(state.plain).toEqual([
101+
[1 / initial, false],
102+
[1 / initial, true],
103+
[1 / next, false]
104+
]);
105+
expect(state.ahead).toEqual([
106+
[false, 1 / initial],
107+
[true, 1 / next],
108+
[false, 1 / next]
109+
]);
110+
} finally {
111+
gate.resolve();
112+
state.dispose();
113+
flush();
114+
}
115+
}
116+
);
117+
});
118+
119+
it("keeps the default comparator's signed-zero equality", () => {
120+
const { dispose, value, setValue, reciprocal, seen } = createRoot(dispose => {
121+
const [value, setValue] = createSignal(-0);
122+
const reciprocal = createMemo(() => 1 / value());
123+
const seen: number[] = [];
124+
createRenderEffect(reciprocal, value => void seen.push(value));
125+
return { dispose, value, setValue, reciprocal, seen };
126+
});
127+
try {
128+
flush();
129+
130+
setValue(0);
131+
flush();
132+
133+
expect(value()).toBe(-0);
134+
expect(reciprocal()).toBe(-Infinity);
135+
expect(seen).toEqual([-Infinity]);
136+
} finally {
137+
dispose();
138+
}
139+
});
140+
141+
it("keeps an accepted NaN rewrite as a proposal", async () => {
142+
const gate = Promise.withResolvers<void>();
143+
const state = createRoot(dispose => {
144+
const [value, setValue] = createSignal(NaN, { equals: false });
145+
const seen: boolean[] = [];
146+
createRenderEffect(
147+
() => isPending(value),
148+
v => void seen.push(v)
149+
);
150+
return { dispose, value, setValue, seen };
151+
});
152+
try {
153+
flush();
154+
action(function* () {
155+
state.setValue(NaN);
156+
yield gate.promise;
157+
})();
158+
flush();
159+
expect(state.seen).toEqual([false, true]);
160+
expect(isPending(state.value)).toBe(true);
161+
162+
gate.resolve();
163+
await new Promise<void>(resolve => setTimeout(resolve, 0));
164+
flush();
165+
expect(state.seen).toEqual([false, true, false]);
166+
} finally {
167+
gate.resolve();
168+
state.dispose();
169+
flush();
170+
}
171+
});

0 commit comments

Comments
 (0)