fix(signals): a pass joined only through born-held nodes is not re-derived by a lane entry (#3969)

.changeset/born-held-lane-loop.md

@@ -0,0 +1,5 @@
+---
+"@solidjs/signals": patch
+---
+
+A render effect that reads a memo born into a held change and, in the same pass, a fresh `isPending` no longer re-runs forever while the change is held (#3969). Under the dev server every component runs inside such a memo (hot reload), so a refetch that swapped a `<Show>` to a fallback containing `<Spinner on={isPending(x)} />` tripped the "Potential Infinite Loop Detected" guard. A read of a node born held no longer counts as a staged read: there is no committed value to re-derive on.

packages/signals/src/core/core.ts

@@ -1890,15 +1890,14 @@ export function serve(el: Signal<any> | Computed<any>, c: Computed<any> | null):
  * write never shows through the lane. A verdict lane's work likewise: the
  * lane holds verdicts, not the frame's other stagings (#3851) — except a
  * verdict reader, which answered for itself (the lane seam, lanes.ts). Not
- * a read of a node born staged: it has no committed value to re-derive on,
- * and a re-run would read the same staging and re-queue every round. */
+ * a read of a node born staged at all: it has no committed value to
+ * re-derive on, and a re-run would read the same staging and re-queue every
+ * round (#3969: nor does it make a later lane read a re-derive, `enterLane`). */
 export function stagedRead(c: Computed<any>, el?: Signal<any> | Computed<any>): void {
-  c._flags |= REACTIVE_STAGED_READ;
-  if (
-    passLane !== null &&
-    !((el as Computed<any> | undefined)?._statusFlags! & STATUS_UNINITIALIZED)
-  )
-    stagedReaders.push(c);
+  if (!((el as Computed<any> | undefined)?._statusFlags! & STATUS_UNINITIALIZED)) {
+    c._flags |= REACTIVE_STAGED_READ;
+    if (passLane !== null) stagedReaders.push(c);
+  }
 }
 
 /** A10 for a staged node: a verdict reader (the pass entered a window) that

packages/signals/src/core/lanes.ts

@@ -52,6 +52,7 @@ import {
   REACTIVE_PROBE_UNANSWERED,
   REACTIVE_RECOMPUTING_DEPS,
   REACTIVE_SCREEN_READ,
+  REACTIVE_STAGED_READ,
   STATUS_PENDING,
   STATUS_UNINITIALIZED
 } from "./constants.js";
@@ -232,8 +233,10 @@ export function enterLane(l: Transaction, c: Computed<any>): void {
     // The pass read a held write before its first lane read (it joined the
     // frame's future as mainline would): lane work sees the screen — the
     // seam re-derives it on the committed world next round (`stagedReaders`,
-    // the park's repair), as it does a lane pass that read a staging.
-    if (c._flags & REACTIVE_JOINED) stagedRead(c);
+    // the park's repair), as it does a lane pass that read a staging. Not a
+    // pass whose held reads were all of nodes born held (no staged read,
+    // `stagedRead`): it has no committed world to re-derive on (#3969).
+    if (c._flags & REACTIVE_JOINED && c._flags & REACTIVE_STAGED_READ) stagedRead(c);
     return;
   }
   if (sameLane(p, l)) return setPassLane(l);

packages/web/test/hmr-spinner-loop-3969.spec.tsx

@@ -0,0 +1,110 @@
+/**
+ * @jsxImportSource @solidjs/web
+ * @vitest-environment jsdom
+ */
+// #3969: a refetch plus a write that swaps a `<Show>` to a fallback holding
+// `<Spinner on={isPending(project)} />` tripped the flush loop guard under the
+// dev server: solid-refresh runs each component inside a transparent memo
+// (`createProxy` in solid-js/refresh). Mounted by the held swap, that memo is
+// born held; the insert effect reading it also reads the verdict lane of
+// `isPending`. Its pass joined the hold only through the born-held memo, yet
+// the lane entry treated it as lane work that read a held write and re-derived
+// it on the committed world — where it read the same staging. The lane held,
+// the effect re-ran on the screen, the lane showed and re-ran it, every round.
+import { afterEach, beforeEach, expect, test, vi } from "vitest";
+import {
+  Loading,
+  Show,
+  createMemo,
+  createSignal,
+  flush,
+  isPending,
+  untrack
+} from "solid-js";
+import { render, type JSX } from "../src/index.js";
+
+beforeEach(() => vi.useFakeTimers());
+afterEach(() => vi.useRealTimers());
+
+// What solid-refresh's proxy does around a component body.
+const hmr =
+  <P,>(c: (p: P) => JSX.Element) =>
+  (props: P) =>
+    createMemo(() => untrack(() => c(props)), { transparent: true }) as unknown as JSX.Element;
+
+for (const wrapped of ["none", "spinner", "app", "both"] as const)
+  test(`Save settles without the loop guard (HMR-wrapped: ${wrapped})`, async () => {
+    const loops: string[] = [];
+    const E = globalThis.Error;
+    const W = function (this: unknown, m?: string, o?: ErrorOptions) {
+      if (String(m).includes("Infinite Loop")) loops.push(String(m));
+      return Reflect.construct(E, [m, o], new.target || W);
+    } as unknown as ErrorConstructor;
+    Object.defineProperty(W, "prototype", { value: E.prototype });
+    Object.setPrototypeOf(W, E);
+    globalThis.Error = W;
+    try {
+      const wait = (ms: number) => new Promise(r => setTimeout(r, ms));
+      const [refresh, setRefresh] = createSignal(0);
+      const fetchAfter = <T,>(value: T) => (refresh(), wait(20).then(() => value));
+      const id = <P,>(c: (p: P) => JSX.Element) => c;
+      const Spinner = (wrapped === "spinner" || wrapped === "both" ? hmr : id)(
+        (props: { on: boolean }) => (
+          <Show when={props.on}>
+            <span>updating</span>
+          </Show>
+        )
+      );
+      let select!: () => void;
+      let save!: () => void;
+      const App = (wrapped === "app" || wrapped === "both" ? hmr : id)(() => {
+        const project = createMemo(() => fetchAfter({ id: "a" }));
+        const rows = createMemo(() => fetchAfter([{ id: project().id }]));
+        const tags = createMemo(() => fetchAfter(["A", "B"]));
+        const [selected, setSelected] = createSignal(false);
+        select = () => setSelected(true);
+        save = () => {
+          setRefresh(n => n + 1);
+          setSelected(false);
+        };
+        return (
+          <Loading fallback={<p>loading</p>}>
+            <div>
+              <Show
+                when={tags().length > 0 && selected()}
+                fallback={<Spinner on={isPending(project)} />}
+              >
+                <span>selected</span>
+              </Show>
+              <p>{rows()[0]!.id}</p>
+            </div>
+          </Loading>
+        );
+      });
+      const container = document.createElement("div");
+      const dispose = render(() => <App />, container);
+      const settle = async () => {
+        for (let i = 0; i < 5; i++) {
+          await vi.advanceTimersByTimeAsync(25);
+          flush();
+        }
+      };
+      await settle();
+      expect(container.textContent).toBe("a");
+      select();
+      flush();
+      await settle();
+      expect(container.textContent).toBe("selecteda");
+      save();
+      flush();
+      // The swap to the fallback reads the refetching `tags`: it holds with
+      // the refetch, so the screen keeps what it showed until it lands.
+      expect(container.textContent).toBe("selecteda");
+      await settle();
+      expect(loops).toEqual([]);
+      expect(container.textContent).toBe("a");
+      dispose();
+    } finally {
+      globalThis.Error = E;
+    }
+  });