fix(signals): an Errored heals when nothing reads the failed source any more (#4644)

.changeset/errored-heal-memo-reader.md

@@ -0,0 +1,5 @@
+---
+"@solidjs/signals": patch
+---
+
+An `<Errored>` heals once no reader it caught still reads the failed source, instead of staying on its fallback after navigating to another branch (#4644). This covers an error read only by memos (a `<Show>`/`<Match>` condition), and a source something outside the boundary still reads: a hoisted memo, a derived store or projection, a reader under another `<Errored>`.

packages/signals/src/boundaries.ts

@@ -369,8 +369,23 @@ function caughtBelow(b: Boundary, r: Computed<any>, s: Computed<any>): boolean {
   return false;
 }
 
+/** `n` reads `s`, directly or through derivations wherever they live (a
+ * hoisted memo, a derived store's derive, a loading boundary's output). Each
+ * upstream node is walked once (`seen`), so a shared upstream graph costs its
+ * size, not its paths. */
+function reads(n: Computed<any>, s: Computed<any>, seen?: Set<any>): any {
+  for (let l = n._deps, h; l; l = l._nextDep)
+    if ((h = l._dep) === s || ((seen ||= new Set()).size < seen.add(h).size && reads(h, s, seen)))
+      return 1;
+}
+
 /** Drop the readers that settled or died. Settled: a loading reader no
- * longer pending; an error's source neither errored nor in flight. A reader
+ * longer pending; an error's source neither errored nor in flight, or no
+ * longer read by the reader that caught it: the boundary waits on what threw
+ * only while such a reader is alive. One that re-ran clean (the tree, a
+ * memo-only read gated away) holds nothing, and a reader outside the
+ * boundary — a hoisted memo, a derived store, a reader under another
+ * boundary — never caught its error (D-39, #4644). A reader
  * that landed but is held (its value staged in a transaction) is not: the
  * content reveals at the commit, with its run. The tree is: the output reads
  * it and enters the transaction itself (its committed value shows until the
@@ -400,7 +415,7 @@ export function prune(b: Boundary, pass?: boolean | 0): number {
       // settled before it ran, a fresh boundary revealed into a hold its
       // pass then went pending in, which waited on that flight (#4568).
       if (
-        ((r._statusFlags | s._statusFlags) & mask &&
+        ((r._statusFlags & mask || (s._statusFlags & mask && reads(r, s))) &&
           !(mask & STATUS_ERROR && caughtBelow(b, r, s))) ||
         (mask & STATUS_ERROR && s._config & CONFIG_STAGED_ERROR) ||
         (r._config & CONFIG_VERDICT && r._flags & REACTIVE_IN_HEAP)

packages/signals/tests/errored-heal-memo-reader-4644.test.ts

@@ -0,0 +1,194 @@
+import { expect, it } from "vitest";
+import {
+  createErrorBoundary,
+  createMemo,
+  createProjection,
+  createRenderEffect,
+  createRoot,
+  createSignal,
+  createStore,
+  flush
+} from "../src/index.js";
+
+// #4644 (D-39, F-3, F-12): a fallback no live reader holds heals. The shape:
+// `<Errored><Show when={page() === "list"} fallback="about">{X}</Show>
+// </Errored>`, `rows` an async memo owned outside the boundary that rejects.
+// X = "memo-only": `rows` is read only by memos (a `<Show when>` condition)
+// whose value the boundary's tree flattens, so the reader the boundary
+// caught is its own tree. X = "effect": a render effect in the branch reads
+// it. Navigating away disposes every reader of `rows`: nothing on the new
+// branch reads the error, so the boundary shows it, as a fresh render with
+// the same inputs would. With another live reader of `rows` under the same
+// boundary ("kept"), the error stays up.
+const tick = async () => {
+  for (let i = 0; i < 3; i++) {
+    await new Promise(r => setTimeout(r, 0));
+    flush();
+  }
+};
+
+for (const shape of ["memo-only", "effect", "kept"] as const)
+  it(`${shape}: navigating away from a failed memo (#4644)`, async () => {
+    let reject!: (e: Error) => void;
+    const [page, setPage] = createSignal("list");
+    const frames: unknown[] = [];
+    const dispose = createRoot(d => {
+      const rows = createMemo(
+        () =>
+          new Promise<number[]>((_, r) => {
+            reject = r;
+          })
+      );
+      const view = createErrorBoundary(
+        () => {
+          if (shape === "kept")
+            createRenderEffect(
+              () => rows().length,
+              () => {}
+            );
+          const onList = createMemo(() => page() === "list");
+          return createMemo(() => {
+            if (!onList()) return "about";
+            if (shape === "effect") {
+              createRenderEffect(
+                () => rows().length,
+                () => {}
+              );
+              return "list";
+            }
+            const cond = createMemo(() => rows().length === 0);
+            return createMemo(() => (cond() ? "none" : "some"));
+          });
+        },
+        () => "ERR"
+      );
+      createRenderEffect(
+        () => view(),
+        v => {
+          frames.push(v);
+        }
+      );
+      return d;
+    });
+    flush();
+    await tick();
+    reject(new Error("boom"));
+    await tick();
+    expect(frames[frames.length - 1]).toBe("ERR");
+    setPage("about");
+    flush();
+    await tick();
+    dispose();
+    expect(frames[frames.length - 1]).toBe(shape === "kept" ? "ERR" : "about");
+  });
+
+// The boundary waits on what threw only while a reader THAT CAUGHT ITS ERROR
+// is alive (D-39). Something else still reading `rows` is not one: a memo
+// hoisted outside the boundary, a derived store or projection built from it,
+// a reader under another error boundary. Each still keeps `rows` subscribed,
+// so "is `rows` read at all" kept this boundary stuck. While the list is
+// mounted the error stays (across a prune: another reader's settling); away,
+// the boundary heals; back, the error shows again (`sync`: a sync throw
+// re-thrown by the new reader — an async source is re-asked on the new read
+// instead, which is not this rule).
+const flat = (v: unknown) => {
+  while (typeof v === "function") v = v();
+  return v;
+};
+for (const via of [
+  "hoisted memo",
+  "derived store",
+  "projection",
+  "other boundary",
+  "sync"
+] as const)
+  it(`${via}: a reader outside the boundary does not keep its error up (#4644)`, async () => {
+    let reject!: (e: Error) => void;
+    let resolveP!: (v: number) => void;
+    const [page, setPage] = createSignal("list");
+    const [fail, setFail] = createSignal(false);
+    const frames: unknown[] = [];
+    const other: unknown[] = [];
+    const dispose = createRoot(d => {
+      const rows = createMemo(() => {
+        if (via !== "sync") return new Promise<number[]>((_, r) => (reject = r));
+        if (fail()) throw new Error("sync boom");
+        return [] as number[];
+      });
+      let read: () => number[] = rows;
+      if (via === "hoisted memo" || via === "sync") read = createMemo(() => [...rows()]);
+      if (via === "derived store") {
+        const [st] = createStore<{ list: number[] }>(
+          dr => {
+            dr.list = rows();
+          },
+          { list: [] }
+        );
+        read = () => st.list;
+      }
+      if (via === "projection") {
+        const st = createProjection<{ list: number[] }>(
+          dr => {
+            dr.list = rows();
+          },
+          { list: [] }
+        );
+        read = () => st.list;
+      }
+      if (via === "other boundary") {
+        const header = createErrorBoundary(
+          () => createMemo(() => rows().length),
+          () => "ERR2"
+        );
+        createRenderEffect(
+          () => flat(header()),
+          v => void other.push(v)
+        );
+      }
+      const p = createMemo(() => new Promise<number>(r => (resolveP = r)));
+      const view = createErrorBoundary(
+        () => {
+          createRenderEffect(
+            () => p(),
+            () => {}
+          );
+          const onList = createMemo(() => page() === "list");
+          return createMemo(() => {
+            if (!onList()) return "about";
+            const cond = createMemo(() => read().length === 0);
+            return createMemo(() => (cond() ? "none" : "some"));
+          });
+        },
+        () => "ERR"
+      );
+      createRenderEffect(
+        () => flat(view()),
+        v => void frames.push(v)
+      );
+      return d;
+    });
+    const last = () => frames[frames.length - 1];
+    flush();
+    await tick();
+    if (via === "sync") setFail(true);
+    else reject(new Error("boom"));
+    await tick();
+    const failed = last();
+    resolveP(1);
+    await tick();
+    const afterPrune = last();
+    setPage("about");
+    await tick();
+    const away = last();
+    setPage("list");
+    await tick();
+    const back = last();
+    dispose();
+    expect({ failed, afterPrune, away }).toEqual({
+      failed: "ERR",
+      afterPrune: "ERR",
+      away: "about"
+    });
+    if (via === "sync") expect(back).toBe("ERR");
+    if (via === "other boundary") expect(other[other.length - 1]).toBe("ERR2");
+  });

packages/web/test/errored-heal-memo-reader-4644.spec.tsx

@@ -0,0 +1,163 @@
+/**
+ * @jsxImportSource @solidjs/web
+ * @vitest-environment jsdom
+ */
+// #4644 (D-39): an <Errored> whose fallback no live reader holds heals. A
+// list page reads a failed async memo and the boundary shows its fallback;
+// navigating to another branch under the same boundary disposes every reader
+// of the memo, so the new branch shows. A reader that is only a memo — the
+// condition of a <Show> or <Match> — used to keep the boundary stuck.
+import { expect, test } from "vitest";
+import { render, type JSX } from "@solidjs/web";
+import {
+  createMemo,
+  createSignal,
+  createStore,
+  Errored,
+  flush,
+  For,
+  Loading,
+  Match,
+  Show,
+  Switch
+} from "solid-js";
+
+const tick = async () => {
+  for (let i = 0; i < 2; i++) {
+    await new Promise(r => setTimeout(r, 0));
+    flush();
+  }
+};
+type Row = { id: number };
+
+const readers: Record<string, (rows: () => Row[]) => JSX.Element> = {
+  "Show when={rows().length === 0}": rows => (
+    <Show when={rows().length === 0} fallback={<p>{rows().length} rows</p>}>
+      <p>no rows</p>
+    </Show>
+  ),
+  "Show when={rows()}": rows => <Show when={rows()}>{r => <p>{r().length} rows</p>}</Show>,
+  "Show keyed": rows => (
+    <Show when={rows()} keyed>
+      {r => <p>{r.length} rows</p>}
+    </Show>
+  ),
+  "Switch/Match": rows => (
+    <Switch fallback={<p>{rows().length} rows</p>}>
+      <Match when={rows().length === 0}>
+        <p>no rows</p>
+      </Match>
+    </Switch>
+  ),
+  "memo of the condition": rows => {
+    const empty = createMemo(() => rows().length === 0);
+    return <p>{empty() ? "none" : "some"}</p>;
+  },
+  text: rows => <p>{rows().length} rows</p>,
+  For: rows => (
+    <ul>
+      <For each={rows()}>{r => <li>{r.id}</li>}</For>
+    </ul>
+  )
+};
+
+for (const withLoading of [true, false])
+  for (const [name, view] of Object.entries(readers))
+    test(`${withLoading ? "Loading" : "no Loading"} | ${name}: navigating away from a failed list heals <Errored> (#4644)`, async () => {
+      let rejectList!: (e: Error) => void;
+      let setPage!: (p: string) => void;
+      const el = document.createElement("div");
+      const dispose = render(() => {
+        const [page, set] = createSignal("list");
+        setPage = set;
+        const rows = createMemo(
+          () =>
+            new Promise<Row[]>((_, reject) => {
+              rejectList = reject;
+            })
+        );
+        return (
+          <Errored fallback={<p>could not load</p>}>
+            <Show when={page() === "list"} fallback={<p>about page</p>}>
+              {withLoading ? <Loading fallback={<p>loading</p>}>{view(rows)}</Loading> : view(rows)}
+            </Show>
+          </Errored>
+        );
+      }, el);
+      await tick();
+      rejectList(new Error("boom"));
+      await tick();
+      const failed = el.textContent;
+      setPage("about");
+      flush();
+      await tick();
+      const after = el.textContent;
+      dispose();
+      expect({ failed, after }).toEqual({ failed: "could not load", after: "about page" });
+    });
+
+// Only a reader the boundary caught keeps it waiting (D-39): something
+// outside it still reading the failed memo — a memo or a derived store
+// hoisted above the <Errored>, a header count under another <Errored> — did
+// not catch this boundary's error and does not keep its fallback up.
+for (const shape of [
+  "derived memo outside",
+  "derived store outside",
+  "header reader outside"
+] as const)
+  test(`${shape}: navigating away from a failed list heals <Errored> (#4644)`, async () => {
+    let rejectList!: (e: Error) => void;
+    let setPage!: (p: string) => void;
+    const el = document.createElement("div");
+    const dispose = render(() => {
+      const [page, set] = createSignal("list");
+      setPage = set;
+      const rows = createMemo(
+        () =>
+          new Promise<number[]>((_, reject) => {
+            rejectList = reject;
+          })
+      );
+      let read: () => number[] = rows;
+      if (shape === "derived memo outside") read = createMemo(() => [...rows()].sort());
+      if (shape === "derived store outside") {
+        const [st] = createStore<{ l: number[] }>(
+          d => {
+            d.l = rows();
+          },
+          { l: [] }
+        );
+        read = () => st.l;
+      }
+      return (
+        <>
+          {shape === "header reader outside" && (
+            <Errored fallback={<i>hdr-err</i>}>
+              <i>{rows().length}</i>
+            </Errored>
+          )}
+          <Errored fallback={<p>could not load</p>}>
+            <Show when={page() === "list"} fallback={<p>about page</p>}>
+              <Show when={read().length === 0} fallback={<p>some</p>}>
+                <p>no rows</p>
+              </Show>
+            </Show>
+          </Errored>
+        </>
+      );
+    }, el);
+    await tick();
+    rejectList(new Error("boom"));
+    await tick();
+    const failed = el.textContent;
+    setPage("about");
+    flush();
+    await tick();
+    const after = el.textContent;
+    dispose();
+    const hdr = shape === "header reader outside" ? "hdr-err" : "";
+    expect({ failed, after }).toEqual({
+      failed: hdr + "could not load",
+      after: hdr + "about page"
+    });
+  });