From 55019b477ff005f89471b68f7963fece76093bc3 Mon Sep 17 00:00:00 2001 From: thomaflette Date: Tue, 7 Jul 2026 22:04:52 +0900 Subject: [PATCH] fix(signals): retain latest pending companions during refetch --- .changeset/ispending-companion-retention.md | 19 ++ packages/solid-signals/src/core/core.ts | 16 ++ packages/solid-signals/src/core/scheduler.ts | 16 ++ packages/solid-signals/src/core/types.ts | 1 + .../isPending-companion-retention.test.ts | 227 ++++++++++++++++++ 5 files changed, 279 insertions(+) create mode 100644 .changeset/ispending-companion-retention.md create mode 100644 packages/solid-signals/tests/isPending-companion-retention.test.ts diff --git a/.changeset/ispending-companion-retention.md b/.changeset/ispending-companion-retention.md new file mode 100644 index 000000000..221febeec --- /dev/null +++ b/.changeset/ispending-companion-retention.md @@ -0,0 +1,19 @@ +--- +"@solidjs/signals": patch +--- + +Fix `isPending(() => latest(x))` in a user effect looping the scheduler on the +first write after the async source settles ("Potential Infinite Loop Detected" +in dev, an unbounded spin in prod). The pending-flag companion signal is +written optimistically, so an ambient transition completing reverted its `true` +override while the async it reports on was still in flight. The revert +re-notified the subscribed effect, which re-armed the write forever, and +`isPending` never read `true`. + +Companion signals for `latest()` shadows now carry a revert-deferral policy: +transition completion waits while the companion's reported source +(parent/firewall) is still in-flight, then lets the normal optimistic reversion +path clear the override once that source settles or is disposed. + +The regression coverage includes async memos, store-leaf `latest()` reads, and +multiple watchers of the same source. diff --git a/packages/solid-signals/src/core/core.ts b/packages/solid-signals/src/core/core.ts index 7949fdef7..04b7449c4 100644 --- a/packages/solid-signals/src/core/core.ts +++ b/packages/solid-signals/src/core/core.ts @@ -1137,6 +1137,14 @@ function getPendingSignal(el: Signal | Computed): Signal { // Propagate parent-child lane relationship for isPending(() => latest(x)) if (el._parentSource) { el._pendingSignal._parentSource = el; + el._pendingSignal._deferRevert = () => { + const source = getCompanionPendingSource(el) as Computed; + return ( + !(source._flags & REACTIVE_DISPOSED) && + !!(source._statusFlags & STATUS_PENDING) && + !(source._statusFlags & STATUS_UNINITIALIZED) + ); + }; } if (computePendingState(el)) setSignal(el._pendingSignal, true); if (__DEV__) devTrackCompanionOwner(el); @@ -1198,6 +1206,14 @@ function computePendingState(el: Signal | Computed): boolean { return !!(comp._statusFlags & STATUS_PENDING && !(comp._statusFlags & STATUS_UNINITIALIZED)); } +function getCompanionPendingSource(el: Signal | Computed): Signal | Computed { + if (el._parentSource) { + const parentNode = el._parentSource as FirewallSignal; + return parentNode._firewall || parentNode; + } + return (el as FirewallSignal)._firewall || el; +} + /** * Keep the lazily-created isPending()/latest() companion nodes in sync with a * new value. Every path that produces a value for `el` — direct set, async diff --git a/packages/solid-signals/src/core/scheduler.ts b/packages/solid-signals/src/core/scheduler.ts index 6b662c9f7..349cd3d2c 100644 --- a/packages/solid-signals/src/core/scheduler.ts +++ b/packages/solid-signals/src/core/scheduler.ts @@ -383,6 +383,14 @@ export class GlobalQueue extends Queue { try { if (__DEV__) devCheckFlushStart(); runHeap(dirtyQueue, GlobalQueue._update); + if (!activeTransition && transitions.size) { + for (const transition of transitions) { + if (transitionComplete(transition)) { + activeTransition = transition; + break; + } + } + } if (activeTransition) { const isComplete = transitionComplete(activeTransition); if (!isComplete) { @@ -801,6 +809,10 @@ function transitionComplete(transition: Transition): boolean { if (transition._actions.length) return false; let done = true; for (const [source, reporters] of transition._asyncReporters) { + if (source._flags & REACTIVE_DISPOSED) { + transition._asyncReporters.delete(source); + continue; + } let hasLive = false; for (const reporter of reporters) { if (reporterBlocksSource(reporter, source)) { @@ -821,6 +833,10 @@ function transitionComplete(transition: Transition): boolean { if (done) { for (let i = 0; i < transition._optimisticNodes.length; i++) { const node = transition._optimisticNodes[i]; + if (hasActiveOverride(node) && node._deferRevert?.()) { + done = false; + break; + } if ( hasActiveOverride(node) && "_statusFlags" in node && diff --git a/packages/solid-signals/src/core/types.ts b/packages/solid-signals/src/core/types.ts index 3564384df..82daa54f2 100644 --- a/packages/solid-signals/src/core/types.ts +++ b/packages/solid-signals/src/core/types.ts @@ -46,6 +46,7 @@ export interface RawSignal { _overrideValue?: T | typeof NOT_PENDING; _optimisticLane?: OptimisticLane; _pendingSignal?: Signal; // Lazy signal for isPending() + _deferRevert?: () => boolean; // Active override blocks transition completion while true _latestValueComputed?: Computed; // Lazy computed for latest() _parentSource?: Signal | Computed; // Back-reference for parent-child lane relationship } diff --git a/packages/solid-signals/tests/isPending-companion-retention.test.ts b/packages/solid-signals/tests/isPending-companion-retention.test.ts new file mode 100644 index 000000000..69109b303 --- /dev/null +++ b/packages/solid-signals/tests/isPending-companion-retention.test.ts @@ -0,0 +1,227 @@ +import { + createEffect, + createMemo, + createRoot, + createSignal, + createStore, + flush, + isPending, + latest +} from "../src/index.js"; + +const wait = (ms: number) => new Promise(r => setTimeout(r, ms)); + +/** + * isPending()'s companion signal is written optimistically, so an ambient + * transition completing used to revert its `true` override while the async it + * reports on was still in flight. The revert re-notified the subscribed + * effect, which re-armed the write: an infinite flush loop ("Potential + * Infinite Loop Detected" in dev, an unbounded spin in prod), during which + * isPending never read `true`. resolveOptimisticNodes now defers reverting a + * companion while its resolved source (parent/firewall for latest() shadows) + * is still in-flight. + */ +describe("isPending companion retention across transitions", () => { + function watchPending(read: () => boolean) { + const seen: boolean[] = []; + createRoot(() => { + createEffect(read, v => { + seen.push(v); + }); + }); + return seen; + } + + function trackUncaught() { + const errors: string[] = []; + const handler = (e: Error) => { + errors.push(String(e)); + }; + process.on("uncaughtException", handler); + return { + errors, + done: () => { + try { + flush(); + } catch (e) { + errors.push(String(e)); + } finally { + process.off("uncaughtException", handler); + } + } + }; + } + + it("isPending(() => latest(x)) in a user effect survives a post-settle refetch without looping", async () => { + const guard = trackUncaught(); + const [version, refetch] = createSignal(0); + const data = createMemo(async () => { + const v = version(); + await wait(20); + return `v${v}`; + }); + const seen = watchPending(() => isPending(() => latest(data))); + + await wait(60); // initial load settles + refetch(1); // the write that used to spin the scheduler + await wait(10); // mid-flight + expect(seen.at(-1)).toBe(true); // pending is observable + await wait(60); // refetch settles + guard.done(); + + expect(seen.at(-1)).toBe(false); // and clears + expect(guard.errors).toEqual([]); // no "Potential Infinite Loop Detected." + }); + + it("control: isPending(x) directly never looped and still works", async () => { + const guard = trackUncaught(); + const [version, refetch] = createSignal(0); + const data = createMemo(async () => { + const v = version(); + await wait(20); + return `v${v}`; + }); + const seen = watchPending(() => isPending(data)); + + await wait(60); + refetch(1); + await wait(60); + guard.done(); + + expect(seen.at(-1)).toBe(false); + expect(guard.errors).toEqual([]); + }); + + it("shared update: pending remains observable until the transition commits", async () => { + // UI timing is pinned in @solidjs/web's latest-async test. + const guard = trackUncaught(); + const [version, refetch] = createSignal(0); + const fast = createMemo(async () => { + const v = version(); + await wait(15); + return `fast v${v}`; + }); + const slow = createMemo(async () => { + const v = version(); + await wait(120); + return `slow v${v}`; + }); + const seenFast = watchPending(() => isPending(() => latest(fast))); + const seenSlow = watchPending(() => isPending(() => latest(slow))); + + await wait(200); // both settle + refetch(1); + await wait(300); // transition fully commits + guard.done(); + + expect(seenFast).toContain(true); // pending was observable + expect(seenSlow).toContain(true); + expect(seenFast.at(-1)).toBe(false); // and settles false + expect(seenSlow.at(-1)).toBe(false); + expect(guard.errors).toEqual([]); // no "Potential Infinite Loop Detected." + }); + + it("companion survives an unrelated signal write flushing mid-flight", async () => { + const guard = trackUncaught(); + const [version, refetch] = createSignal(0); + const [unrelated, setUnrelated] = createSignal(0); + const data = createMemo(async () => { + const v = version(); + await wait(50); + return `v${v}`; + }); + const seen = watchPending(() => isPending(() => latest(data))); + createRoot(() => { + createEffect(unrelated, () => {}); + }); + + await wait(100); // settle + refetch(1); // go pending + await wait(10); + setUnrelated(1); // unrelated flush completes while data is in flight + await wait(10); + expect(seen.at(-1)).toBe(true); // companion kept its override + await wait(100); + guard.done(); + + expect(seen.at(-1)).toBe(false); + expect(guard.errors).toEqual([]); + }); + + it("store-leaf shape: isPending(() => latest(() => store.value)) survives a refetch", async () => { + const guard = trackUncaught(); + const [version, refetch] = createSignal(0); + const [store] = createStore( + async () => { + const v = version(); + await wait(20); + return { value: `v${v}` }; + }, + {} as { value?: string } + ); + const seen = watchPending(() => isPending(() => latest(() => store.value))); + + await wait(60); + refetch(1); + await wait(60); + guard.done(); + + expect(seen.at(-1)).toBe(false); + expect(guard.errors).toEqual([]); // was: "Potential Infinite Loop Detected." + }); + + it("two watchers of the same source survive a refetch", async () => { + const guard = trackUncaught(); + const [version, refetch] = createSignal(0); + const data = createMemo(async () => { + const v = version(); + await wait(20); + return `v${v}`; + }); + const a = watchPending(() => isPending(() => latest(data))); + const b = watchPending(() => isPending(() => latest(data))); + + await wait(60); + refetch(1); + await wait(60); + guard.done(); + + expect(a.at(-1)).toBe(false); + expect(b.at(-1)).toBe(false); + expect(guard.errors).toEqual([]); // was: "Potential Infinite Loop Detected." + }); + + it("safety net: a source disposed mid-flight reverts the companion to false", async () => { + const guard = trackUncaught(); + const [version, refetch] = createSignal(0); + let dispose!: () => void; + let data!: () => string; + createRoot(d => { + dispose = d; + data = createMemo(async () => { + const v = version(); + await wait(50); + return `v${v}`; + }); + }); + // the watcher outlives the source's root + const seen = watchPending(() => isPending(() => latest(data))); + const [unrelated, setUnrelated] = createSignal(0); + createRoot(() => { + createEffect(unrelated, () => {}); + }); + + await wait(100); // settle + refetch(1); // go pending + await wait(10); + expect(seen.at(-1)).toBe(true); + dispose(); // source disposed while its async is in flight + setUnrelated(1); // provoke a flush so the parked companion is re-examined + await wait(100); + guard.done(); + + // the companion must not stay latched true after the source is gone + expect(seen.at(-1)).toBe(false); + expect(guard.errors).toEqual([]); + }); +});