Skip to content

Commit e9e6a78

Browse files
fix(signals): hold optimistic store layers while their truth is in flight (#2951)
A bare (transaction-less) optimistic store write made in the same tick as a refetch trigger died at plain flush end: the transition adopting the write had no reason to stay open — transitionBlocked only recognizes signal-form optimistic nodes, where the pending async and the override share one node — so it settled in the flush that started the refetch and its settle consumed the layer. The next click's draft then composed on committed base, clobbering the still-rendered optimistic row. createOptimisticStore now wraps the engine's transitionBlocked: a transition holding an optimistic store whose firewall is still pending stays open, so the layer rides until truth lands (projection landing) or the transaction settles, and consecutive writes stack on the live optimistic view. Plain-form stores have no firewall and keep their flush-scoped flash semantics; #2899 disjoint-action independence is untouched (owner stamps still scope each settle). Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent cce7dcb commit e9e6a78

5 files changed

Lines changed: 231 additions & 6 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+
Bare optimistic store writes made while the store's own refetch is in flight now hold until truth lands and compose across consecutive writes, instead of reverting at plain flush end and clobbering each other (#2951)

packages/solid-signals/src/core/scheduler.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -875,9 +875,14 @@ function transitionComplete(transition: Transition): boolean {
875875
}
876876
}
877877
// Override blockage lives with the engine. Absent hook = "no optimistic
878-
// blockage", which is exact: only _optimisticWrite (engine) pushes to
879-
// _optimisticNodes, so without the engine the loop was vacuous anyway.
880-
if (done && transition._optimisticNodes.length && GlobalQueue._transitionBlocked!(transition))
878+
// blockage", which is exact: only the engine pushes to _optimisticNodes
879+
// (via _optimisticWrite) or _optimisticStores (via _trackOptimisticStore),
880+
// so without the engine the loop was vacuous anyway.
881+
if (
882+
done &&
883+
(transition._optimisticNodes.length || transition._optimisticStores.size) &&
884+
GlobalQueue._transitionBlocked!(transition)
885+
)
881886
done = false;
882887
done && (transition._done = true);
883888
return done;

packages/solid-signals/src/store/optimistic.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { STATUS_PENDING } from "../core/constants.js";
12
import {
23
computed,
34
CONFIG_AUTO_DISPOSE,
@@ -104,7 +105,28 @@ export function createOptimisticStore<T extends object = {}>(
104105
// STORE_OPTIMISTIC take the engine's write path, so install it before any
105106
// node can be created.
106107
installOptimisticEngine();
107-
GlobalQueue._clearOptimisticStores ||= clearOptimisticStores;
108+
if (!GlobalQueue._clearOptimisticStores) {
109+
GlobalQueue._clearOptimisticStores = clearOptimisticStores;
110+
// Store half of the engine's override blockage (#2951): signal-form
111+
// createOptimistic carries the pending async and the override on ONE node,
112+
// so transitionBlocked sees both; a derived optimistic STORE splits them —
113+
// the layer sits on store targets while the in-flight truth lives on the
114+
// firewall computed. Without this, the transition adopting a bare store
115+
// write settled in the same flush that started the refetch and its settle
116+
// consumed the layer mid-flight (follow-up writes then drafted from base,
117+
// clobbering instead of composing). Optimistic state clears when truth
118+
// lands or its transaction ends — never mid-refetch. Wrapped here (engine
119+
// is already installed above) so store-free apps never carry the check.
120+
const engineBlocked = GlobalQueue._transitionBlocked!;
121+
GlobalQueue._transitionBlocked = transition => {
122+
if (engineBlocked(transition)) return true;
123+
for (const store of transition._optimisticStores) {
124+
const firewall = (store[$TARGET] as StoreNode | undefined)?.[STORE_FIREWALL];
125+
if (firewall && firewall._statusFlags & STATUS_PENDING) return true;
126+
}
127+
return false;
128+
};
129+
}
108130
const derived = typeof first === "function";
109131
// Plain form: the second slot carries options.
110132
if (!derived && options === undefined) options = second as ProjectionOptions | undefined;

packages/solid-signals/src/store/store.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -918,8 +918,11 @@ function armOptimisticStoreWrite(target: StoreNode, store: any): void {
918918
* concurrent actions writing disjoint keys must revert independently, exactly
919919
* like optimistic signal nodes do via the transition's _optimisticNodes.
920920
* `activeTransition` is the write's transaction (action() opens it before the
921-
* body runs); null marks an ambient write that clears at plain flush end.
922-
* Same-key writes across actions keep last-write-wins layer semantics.
921+
* body runs); null marks an ambient write, which clears at plain flush end —
922+
* unless its flush's transition is blocked on the store's own in-flight truth
923+
* (pending firewall, #2951), in which case it rides that transaction to
924+
* settle. Same-key writes across actions keep last-write-wins layer
925+
* semantics.
923926
*/
924927
function stampOptimisticOwner(target: StoreNode, overrideKey: string, property: PropertyKey): void {
925928
if (overrideKey === STORE_OPTIMISTIC_OVERRIDE)
Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
/**
2+
* #2951 — bare (transaction-less) optimistic store writes made while the
3+
* store's own truth is in flight must ride that refetch, not revert at plain
4+
* flush end. Optimistic state clears when truth lands or its transaction
5+
* settles — never on a timer.
6+
*
7+
* The broken shape: a click handler writes a plain dep (triggering a refetch
8+
* of the derived optimistic store) and pushes an optimistic row in the same
9+
* tick. The flush's transition adopted the write but nothing marked it
10+
* blocked — signal-form createOptimistic carries the pending async and the
11+
* override on ONE node (so transitionBlocked sees both), while the store form
12+
* splits them between the firewall computed and the store targets' layer. The
13+
* transition settled in the same flush that started the refetch, its settle
14+
* consumed the layer, and the next click's draft composed on committed base —
15+
* clobbering the still-rendered optimistic row instead of stacking on it.
16+
*/
17+
import { expect, test } from "vitest";
18+
import {
19+
createEffect,
20+
createOptimisticStore,
21+
createRoot,
22+
createSignal,
23+
flush
24+
} from "../src/index.js";
25+
26+
const tick = () => new Promise(r => setTimeout(r, 0));
27+
28+
function setup() {
29+
let setCount!: (v: number) => void;
30+
let setS!: (fn: (s: { items: string[] }) => void) => void;
31+
let dispose!: () => void;
32+
const resolvers: ((items: string[]) => void)[] = [];
33+
const views: string[][] = [];
34+
35+
createRoot(d => {
36+
dispose = d;
37+
const [count, _setCount] = createSignal(0);
38+
setCount = _setCount;
39+
const [store, _setS] = createOptimisticStore<{ items: string[] }>(
40+
async () => {
41+
count();
42+
const items = await new Promise<string[]>(r => resolvers.push(r));
43+
return { items };
44+
},
45+
{ items: [] }
46+
);
47+
setS = _setS;
48+
createEffect(
49+
() => store.items.slice(),
50+
v => {
51+
views.push(v);
52+
}
53+
);
54+
});
55+
return { setCount, setS, resolvers, views, dispose };
56+
}
57+
58+
test("#2951: consecutive bare writes stack on the same optimistic view until truth lands", async () => {
59+
const { setCount, setS, resolvers, views, dispose } = setup();
60+
flush();
61+
await tick();
62+
resolvers[0](["A"]);
63+
await tick();
64+
flush();
65+
expect(views.at(-1)).toEqual(["A"]);
66+
67+
// Click 1: plain dep write starts a (slow) refetch; the bare optimistic
68+
// push in the same tick must survive the flush that starts it.
69+
setCount(1);
70+
setS(d => {
71+
d.items.push("U2*");
72+
});
73+
flush();
74+
await tick();
75+
expect(views.at(-1)).toEqual(["A", "U2*"]);
76+
77+
// Clicks 2 and 3 land while truth is still in flight: each draft composes
78+
// ON TOP of the live optimistic view (same primitive -> same transaction).
79+
setCount(2);
80+
setS(d => {
81+
expect(d.items.slice()).toEqual(["A", "U2*"]);
82+
d.items.push("U3*");
83+
});
84+
flush();
85+
await tick();
86+
expect(views.at(-1)).toEqual(["A", "U2*", "U3*"]);
87+
88+
setCount(3);
89+
setS(d => {
90+
d.items.push("U4*");
91+
});
92+
flush();
93+
await tick();
94+
expect(views.at(-1)).toEqual(["A", "U2*", "U3*", "U4*"]);
95+
96+
// Truth supersedes every tentative layer: the projection landing consumes
97+
// the overrides and the settled transaction sweeps the rest.
98+
const all = ["A", "U2", "U3", "U4"];
99+
for (let i = 1; i < resolvers.length; i++) resolvers[i](all);
100+
await tick();
101+
flush();
102+
await tick();
103+
flush();
104+
expect(views.at(-1)).toEqual(all);
105+
dispose();
106+
});
107+
108+
test("#2951: write order within the tick does not matter (optimistic write first)", async () => {
109+
const { setCount, setS, resolvers, views, dispose } = setup();
110+
flush();
111+
await tick();
112+
resolvers[0](["A"]);
113+
await tick();
114+
flush();
115+
116+
setS(d => {
117+
d.items.push("U2*");
118+
});
119+
setCount(1);
120+
flush();
121+
await tick();
122+
expect(views.at(-1)).toEqual(["A", "U2*"]);
123+
124+
setS(d => {
125+
d.items.push("U3*");
126+
});
127+
setCount(2);
128+
flush();
129+
await tick();
130+
expect(views.at(-1)).toEqual(["A", "U2*", "U3*"]);
131+
132+
const all = ["A", "U2", "U3"];
133+
for (let i = 1; i < resolvers.length; i++) resolvers[i](all);
134+
await tick();
135+
flush();
136+
await tick();
137+
flush();
138+
expect(views.at(-1)).toEqual(all);
139+
dispose();
140+
});
141+
142+
test("#2951: a bare write during an already in-flight refetch (later tick) also holds", async () => {
143+
const { setCount, setS, resolvers, views, dispose } = setup();
144+
flush();
145+
await tick();
146+
resolvers[0](["A"]);
147+
await tick();
148+
flush();
149+
150+
// Start the refetch alone.
151+
setCount(1);
152+
flush();
153+
await tick();
154+
expect(views.at(-1)).toEqual(["A"]);
155+
156+
// A later tick's bare optimistic write must still ride the in-flight truth.
157+
setS(d => {
158+
d.items.push("U2*");
159+
});
160+
flush();
161+
await tick();
162+
expect(views.at(-1)).toEqual(["A", "U2*"]);
163+
164+
resolvers[1](["A", "U2"]);
165+
await tick();
166+
flush();
167+
await tick();
168+
flush();
169+
expect(views.at(-1)).toEqual(["A", "U2"]);
170+
dispose();
171+
});
172+
173+
test("#2951: bare write on a derived store with settled truth keeps flash semantics", async () => {
174+
const { setS, resolvers, views, dispose } = setup();
175+
flush();
176+
await tick();
177+
resolvers[0](["A"]);
178+
await tick();
179+
flush();
180+
expect(views.at(-1)).toEqual(["A"]);
181+
182+
// No refetch in flight: an ambient optimistic write with no transaction and
183+
// no truth on the way reverts at plain flush end, as before.
184+
setS(d => {
185+
d.items.push("X*");
186+
});
187+
flush();
188+
expect(views.at(-1)).toEqual(["A"]);
189+
dispose();
190+
});

0 commit comments

Comments
 (0)