diff --git a/.changeset/errored-heal-memo-reader.md b/.changeset/errored-heal-memo-reader.md new file mode 100644 index 000000000..7a740dde8 --- /dev/null +++ b/.changeset/errored-heal-memo-reader.md @@ -0,0 +1,5 @@ +--- +"@solidjs/signals": patch +--- + +An `` 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 ``/`` condition), and a source something outside the boundary still reads: a hoisted memo, a derived store or projection, a reader under another ``. diff --git a/packages/signals/src/boundaries.ts b/packages/signals/src/boundaries.ts index 37e837c66..5ab25bad9 100644 --- a/packages/signals/src/boundaries.ts +++ b/packages/signals/src/boundaries.ts @@ -369,8 +369,23 @@ function caughtBelow(b: Boundary, r: Computed, s: Computed): 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, s: Computed, seen?: Set): 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) diff --git a/packages/signals/tests/errored-heal-memo-reader-4644.test.ts b/packages/signals/tests/errored-heal-memo-reader-4644.test.ts new file mode 100644 index 000000000..3fe999f59 --- /dev/null +++ b/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: +// `{X} +// `, `rows` an async memo owned outside the boundary that rejects. +// X = "memo-only": `rows` is read only by memos (a `` 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((_, 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((_, 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(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"); + }); diff --git a/packages/web/test/errored-heal-memo-reader-4644.spec.tsx b/packages/web/test/errored-heal-memo-reader-4644.spec.tsx new file mode 100644 index 000000000..abb4bfb31 --- /dev/null +++ b/packages/web/test/errored-heal-memo-reader-4644.spec.tsx @@ -0,0 +1,163 @@ +/** + * @jsxImportSource @solidjs/web + * @vitest-environment jsdom + */ +// #4644 (D-39): an 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 or — 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 Row[]) => JSX.Element> = { + "Show when={rows().length === 0}": rows => ( + {rows().length} rows

}> +

no rows

+ + ), + "Show when={rows()}": rows => {r =>

{r().length} rows

}, + "Show keyed": rows => ( + + {r =>

{r.length} rows

} + + ), + "Switch/Match": rows => ( + {rows().length} rows

}> + +

no rows

+ + + ), + "memo of the condition": rows => { + const empty = createMemo(() => rows().length === 0); + return

{empty() ? "none" : "some"}

; + }, + text: rows =>

{rows().length} rows

, + For: rows => ( +
    + {r =>
  • {r.id}
    • }     +
+ ) +}; + +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 (#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((_, reject) => { + rejectList = reject; + }) + ); + return ( + could not load

}> + about page

}> + {withLoading ? loading

}>{view(rows)} : view(rows)} + + + ); + }, 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 , a header count under another — 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 (#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((_, 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" && ( + hdr-err}> + {rows().length} + + )} + could not load

}> + about page

}> + some

}> +

no rows

+ + + + + ); + }, 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" + }); + });