fix(signals): a leaf waiting on an unshown lane re-derives at its reveal (#4338)

.changeset/lane-unshown-leaf-reveal.md

@@ -0,0 +1,5 @@
+---
+"@solidjs/signals": patch
+---
+
+A section a change mounts now commits once its async memos land when two boundaries read `isPending()` of the same memo and that memo lands last (#4338). The nested reader's pending held the `isPending` lane, so the sibling boundary's output became a lane node before the lane had shown; its render effect, reading that node from the frame, had no committed value to show and went pending on no flight — and was registered to re-derive at the lane's reveal only when it did not throw. It held the change, the lane ends only when the change lands, and the old content stayed on screen for good. The reader is now registered for the reveal before it waits, so it re-derives there and the change commits whole.

packages/signals/src/core/lanes.ts

@@ -609,13 +609,16 @@ export function laneRead(c: Computed<any> | null, el: Signal<any> | Computed<any
       )
         return el._pendingValue;
       if (l._held || pending) {
-        if (!guess && !l._shown && status & STATUS_UNINITIALIZED) throw new NotReadyError(null);
         // Re-derived at the reveal; once per pass (the pass may also be a stale
         // reader of a transaction, REACTIVE_FRAME_READ — both reruns apply).
+        // Before the throw below: a leaf that waits on a lane that has never
+        // shown has no flight to settle it — only the reveal re-derives it,
+        // and its pending holds its frame until then (#4338).
         if (!(c._flags & REACTIVE_SCREEN_READ)) {
           c._flags |= REACTIVE_FRAME_READ | REACTIVE_SCREEN_READ;
           l._reruns.push(c);
         }
+        if (!guess && !l._shown && status & STATUS_UNINITIALIZED) throw new NotReadyError(null);
         return display(el);
       }
       // A verdict reader reads ahead of the seam's verdict: re-derived on the

packages/signals/tests/lane-unshown-leaf-4338.test.ts

@@ -0,0 +1,113 @@
+/**
+ * #4338: a render effect that reads a verdict lane's node while that lane is
+ * held and has never shown waits for the lane's reveal. The node has no
+ * committed value, so the leaf throws NotReady and its pending holds its
+ * frame (F-2); that throw names no flight, so nothing but the reveal can
+ * re-derive it. It was registered for the reveal only when it did not
+ * throw, so the leaf stayed pending: the hold waited on it, and it waited on
+ * a lane that ends only when the hold lands.
+ *
+ * Shape (the issue's `<Section>`): two `<Errored>` boundaries read
+ * `isPending(slow)`, one of them nested under `isPending(fast)`. `fast`
+ * lands first and mounts the nested reader, whose pending on `slow` holds
+ * the verdict lane; `slow` lands next and the sibling boundary's output
+ * becomes a lane node before the lane has shown.
+ */
+import { afterEach, beforeEach, expect, it, vi } from "vitest";
+import {
+  createErrorBoundary,
+  createMemo,
+  createRenderEffect,
+  createRoot,
+  flush,
+  isPending,
+  latest
+} from "../src/index.js";
+
+beforeEach(() => vi.useFakeTimers());
+afterEach(() => vi.useRealTimers());
+
+const sleep = (ms: number) => new Promise(r => setTimeout(r, ms));
+const latestOr = (read: () => string) => {
+  try {
+    return latest(read) ?? "";
+  } catch {
+    return "";
+  }
+};
+/** `<Show when>` with a static child: the child is built in the value memo. */
+function Show(when: () => unknown, children: () => unknown) {
+  const value = createMemo(when);
+  const condition = createMemo(value, { equals: (a, b) => !a === !b });
+  return createMemo(() => (condition() ? children() : undefined));
+}
+interface El {
+  tag: string;
+  text?: string;
+}
+const el = (tag: string, text?: () => string): El => {
+  const node: El = { tag };
+  if (text) createRenderEffect(text, v => void (node.text = v));
+  return node;
+};
+const unwrap = (v: unknown): unknown =>
+  typeof v === "function" ? unwrap(v()) : Array.isArray(v) ? v.map(unwrap) : v;
+const html = (v: unknown): string =>
+  Array.isArray(v)
+    ? v.map(html).join("")
+    : v == null
+      ? ""
+      : typeof v === "object"
+        ? `<${(v as El).tag}>${(v as El).text ?? ""}</${(v as El).tag}>`
+        : String(v);
+
+for (const [label, fastMs, slowMs, errored] of [
+  ["the twice-read memo lands last", 5, 30, true],
+  ["the twice-read memo lands last, no error boundaries", 5, 30, false],
+  ["the twice-read memo lands first", 30, 5, true]
+] as const)
+  it(`#4338: every reader of isPending(slow) shows when both land (${label})`, async () => {
+    const shown: { a?: unknown; b?: unknown } = {};
+    const screen = () => html(shown.a) + "|" + html(shown.b);
+    const dispose = createRoot(d => {
+      const fast = createMemo(async () => (await sleep(fastMs), "fast"));
+      const slow = createMemo(async () => (await sleep(slowMs), "slow"));
+      const boundary = (fn: () => unknown) =>
+        errored ? createErrorBoundary(fn, () => "err") : fn();
+      const first = boundary(() =>
+        Show(
+          () => !isPending(() => fast()),
+          () => [
+            el("p", () => latestOr(() => fast())),
+            Show(
+              () => !isPending(() => slow()),
+              () => el("b", () => latestOr(() => slow()))
+            )
+          ]
+        )
+      );
+      const second = boundary(() =>
+        Show(
+          () => !isPending(() => slow()),
+          () => el("span")
+        )
+      );
+      // The two holes of the section's `<div>`, one render effect each.
+      createRenderEffect(
+        () => unwrap(first),
+        a => void (shown.a = a)
+      );
+      createRenderEffect(
+        () => unwrap(second),
+        b => void (shown.b = b)
+      );
+      return d;
+    });
+    flush();
+    for (let t = 0; t < 40; t += 5) {
+      await vi.advanceTimersByTimeAsync(5);
+      flush();
+    }
+    expect(screen()).toBe("<p>fast</p><b>slow</b>|<span></span>");
+    dispose();
+  });

packages/web/test/recreated-subtree-pending-readers-4338.spec.tsx

@@ -0,0 +1,93 @@
+/**
+ * @jsxImportSource @solidjs/web
+ * @vitest-environment jsdom
+ */
+// #4338: a section the change mounts commits once both of its async memos
+// have landed (F-2: the old section stays until then, then the new one shows
+// whole) — also when two sibling <Errored> boundaries read isPending() of
+// the same memo and that memo lands after the other one.
+import { afterEach, beforeEach, expect, test, vi } from "vitest";
+import {
+  Errored,
+  For,
+  Loading,
+  Show,
+  createMemo,
+  createSignal,
+  flush,
+  isPending,
+  latest
+} from "solid-js";
+import { render } from "../src/index.js";
+
+beforeEach(() => vi.useFakeTimers());
+afterEach(() => vi.useRealTimers());
+
+const sleep = (ms: number) => new Promise(r => setTimeout(r, ms));
+const latestOr = <T,>(read: () => T, fallback: T): T => {
+  try {
+    return latest(read) ?? fallback;
+  } catch {
+    return fallback;
+  }
+};
+const section = (k: string) =>
+  `<div><p>fast-${k}</p><b>slow-${k}</b><!----><span>side</span><!----></div>`;
+
+for (const [label, fastMs, slowMs, loading] of [
+  ["twice-read memo lands last", 5, 30, true],
+  ["twice-read memo lands last, no <Loading>", 5, 30, false],
+  ["twice-read memo lands first", 30, 5, true]
+] as const)
+  test(`the re-created section commits whole (${label})`, async () => {
+    function Section(props: { k: string }) {
+      const fast = createMemo(async () => {
+        await sleep(fastMs);
+        return "fast-" + props.k;
+      });
+      const slow = createMemo(async () => {
+        await sleep(slowMs);
+        return "slow-" + props.k;
+      });
+      return (
+        <div>
+          <Errored fallback={<i>err1</i>}>
+            <Show when={!isPending(() => fast())}>
+              <p>{latestOr(() => fast(), "")}</p>
+              <Show when={!isPending(() => slow())}>
+                <b>{latestOr(() => slow(), "")}</b>
+              </Show>
+            </Show>
+          </Errored>
+          <Errored fallback={<i>err2</i>}>
+            <Show when={!isPending(() => slow())}>
+              <span>side</span>
+            </Show>
+          </Errored>
+        </div>
+      );
+    }
+    const [key, setKey] = createSignal("a");
+    const list = createMemo(() => [key()]);
+    const container = document.createElement("div");
+    const rows = () => <For each={list()}>{k => <Section k={k} />}</For>;
+    const dispose = render(
+      () => (loading ? <Loading fallback={<i>loading</i>}>{rows()}</Loading> : rows()),
+      container
+    );
+    const before = loading ? "<i>loading</i>" : "";
+    const frames = async (from: string, to: string) => {
+      const last = Math.max(fastMs, slowMs);
+      for (let t = 0; t < last; t += 5) {
+        expect(container.innerHTML).toBe(from);
+        await vi.advanceTimersByTimeAsync(5);
+        flush();
+      }
+      expect(container.innerHTML).toBe(to);
+    };
+    await frames(before, section("a"));
+    setKey("b");
+    flush();
+    await frames(section("a"), section("b"));
+    dispose();
+  });