diff --git a/.changeset/loading-on-errorboundary-through-4639.md b/.changeset/loading-on-errorboundary-through-4639.md new file mode 100644 index 000000000..bf884a0b5 --- /dev/null +++ b/.changeset/loading-on-errorboundary-through-4639.md @@ -0,0 +1,5 @@ +--- +"@solidjs/signals": patch +--- + +An error boundary output read only behind fallbacks now holds nothing when it reaches them through a memo or another error boundary built outside the ``, or is not read at all; `LOADING_ON_OUTSIDE_HOLD` no longer fires while the re-armed fallback is on screen. diff --git a/packages/signals/docs/RULES-INDEX.md b/packages/signals/docs/RULES-INDEX.md index d88f49340..3a3012524 100644 --- a/packages/signals/docs/RULES-INDEX.md +++ b/packages/signals/docs/RULES-INDEX.md @@ -50,7 +50,7 @@ Status legend: **live** stated and standing · **ruled** carries an explicit rul | A3 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:337` | async.ts×1 core.ts×2 | derived-write-then-derivation-3733.test.ts×5 equals-comparator-errors.test.ts×1 | [ruled] Comparator throws are compute-phase errors — Errors thrown by a user `equals` comparator behave exactly like compute-phase errors (boundary-containable; loud halt without a boundary). | | A4 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:345` | — | derived-write-then-derivation-3733.test.ts×1 equals-comparator-errors.test.ts×1 | [ruled] A custom `equals` never sees `undefined` prev on first commit — A custom `equals` is never invoked with `undefined` previous value on a node's first commit. | | A5 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:353` | — | derived-write-then-derivation-3733.test.ts×1 errorHalt.test.ts×1 | [ruled] An error escaping every boundary halts the system — An error escaping every boundary permanently halts the system with `REACTIVITY_HALTED`; later writes log "Update ignored". | -| A6 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:361` | boundaries.ts×4 | derived-write-then-derivation-3733.test.ts×1 enforceLoadingBoundary.test.ts×1 | [ruled] `ASYNC_OUTSIDE_LOADING_BOUNDARY` is warn-only — `ASYNC_OUTSIDE_LOADING_BOUNDARY` is a warn-only diagnostic; an `Errored` above must not swallow it and must not show its fallback for a pending. | +| A6 | ruled | `docs/SPEC-ASYNC-SEMANTICS.md:361` | boundaries.ts×4 | derived-write-then-derivation-3733.test.ts×1 enforceLoadingBoundary.test.ts×1 loading-on-errorboundary-through-4639.test.ts×1 | [ruled] `ASYNC_OUTSIDE_LOADING_BOUNDARY` is warn-only — `ASYNC_OUTSIDE_LOADING_BOUNDARY` is a warn-only diagnostic; an `Errored` above must not swallow it and must not show its fallback for a pending. | | A7 | amended | `docs/SPEC-ASYNC-SEMANTICS.md:167` | seat.ts×1 truth.ts×1 verdict.ts×1 | fuzz-findings-l2.test.ts×1 latest-uninitialized-over-latest-4362.test.ts×2 spec-async-semantics.test.ts×2 visibility-oracle-store.states.ts×1 visibility-oracle.states.ts×1 visibility-oracle.test.ts×1 | [ruled, amended in place] Resolved async never reads `[false, undefined]` — After an async memo resolves, `[isPending(x), latest(x)]` is `[false, resolvedValue]` — never `[false, undefined]`. **Amende… | | A8 | amended | `docs/SPEC-ASYNC-SEMANTICS.md:175` | — | create-deferred-3710.test.ts×1 createMemo.test.ts×1 visibility-oracle-store.states.ts×1 visibility-oracle.states.ts×2 | [ruled, amended in place 2026-07-07] `isPending(() => latest(x))` follows `x`'s own async only — verdicts are per-channel — (**re-ruled 2026-07-07c** — was "tracks the transition the same as `isPendin… | | A9 | amended | `docs/SPEC-ASYNC-SEMANTICS.md:183` | async.ts×1 projection.ts×1 target.ts×1 | spec-async-semantics.test.ts×3 visibility-oracle-store.states.ts×4 visibility-oracle-store.test.ts×1 | [ruled, amended in place 2026-07-07] Store leaves behind a firewall report the firewall's new-question refetch — `isPending` on a store leaf behind a firewall reports the firewall's refetch like any a… | diff --git a/packages/signals/src/boundaries.ts b/packages/signals/src/boundaries.ts index 5ab25bad9..8836e8f2e 100644 --- a/packages/signals/src/boundaries.ts +++ b/packages/signals/src/boundaries.ts @@ -307,17 +307,29 @@ function armedOver(node: Computed): boolean | undefined { /** The nearest boundary on `r`'s chain showing its fallback, if any * (`hiddenHook`: `r` is behind a fallback). */ -function hiddenBy(r: Computed): Boundary | undefined { +function hiddenBy(r: Computed, seen?: Set>): Boundary | undefined { for (let b: Boundary | null | undefined = boundaryOf(r); b; b = b._parent) if (b._fallback) return b; // A boundary output passing a pending by (CONFIG_REDERIVE: an error - // boundary, A6) is transparent: on screen only through its readers. Read - // only behind fallbacks — an error boundary built outside the `` - // whose content reads it — it holds nothing (F-4, D-14, #4638). - let h: Boundary | undefined, + // boundary, A6) is transparent: on screen only through the render effects + // it reaches — through a memo or another error boundary, wherever they + // were created (Reader, F-3). Reaching none, or only effects behind + // fallbacks — an error boundary built outside the `` whose + // content reads it — it holds nothing (F-4, D-14, #4638). Reaching none, + // `h` is `1`: hidden with no boundary to name, only ever for a non-effect + // (`heldRun` asks about effects). Inside the walk a user effect reached + // through the output reads nothing on screen (Reader: never a reader), and + // a node already walked is not walked again (`seen`: had it reached a + // shown effect the walk would have stopped), so a memo lattice below the + // output costs its size, not its paths. + let h: any, s = r._subs; - if (r._config & CONFIG_REDERIVE && !(r as any)._type) - while (s !== null && (h = hiddenBy(s._sub))) s = s._nextSub; + if (seen || r._config & CONFIG_REDERIVE) + for ( + seen ||= new Set(), h = (r as any)._type != EFFECT_RENDER; + s && (h = seen.size == seen.add(s._sub).size || hiddenBy(s._sub, seen)); + ) + s = s._nextSub; return h; } @@ -543,7 +555,12 @@ function reportOutsideHold(b: Boundary): void { if (outside(s._sub, false)) return true; return false; } - if (hiddenBy(reader)) return false; + // Behind a fallback, or under a boundary whose output only such content + // reads — an error boundary built outside `b`: the fallback is seen + // (D-16, #4639). + const rb = boundaryOf(reader); + if (hiddenBy(reader) || (rb?._tree === reader && rb._output && hiddenBy(rb._output))) + return false; for (let o: Boundary | null | undefined = boundaryOf(reader); o; o = o._parent) if (o === b) return false; return true; diff --git a/packages/signals/tests/loading-on-errorboundary-through-4639.test.ts b/packages/signals/tests/loading-on-errorboundary-through-4639.test.ts new file mode 100644 index 000000000..803c69735 --- /dev/null +++ b/packages/signals/tests/loading-on-errorboundary-through-4639.test.ts @@ -0,0 +1,203 @@ +/** + * #4639 follow-up to #4638 — an error boundary built in the component body + * (outside ``) passes its source's pending by (A6). Its + * output is on screen only through the render effects it reaches (Vocabulary + * "Reader": a memo counts only through the render effects that read it, + * wherever it was created, and a pending value no render effect reaches holds + * nothing; F-3: "where the memo was created does not matter"). Reached only + * by content behind the re-armed fallback, it holds nothing outside it (F-4, + * D-14): the fallback shows with the change's frame, beside the sibling's. + * + * - memo: a memo built outside the Loading between the output and the content; + * - nested: an error boundary over an error boundary, both built outside; + * - unread: an error boundary nobody reads, beside the content's plain read; + * - header: the output also rendered outside — a reader that is the display + * (D-16): the frame waits for `detail`, and DEV reports + * LOADING_ON_OUTSIDE_HOLD. Only then: with the fallback on screen the + * report would be false (D-16, "the fallback can never be seen"). + * - ueff: the output also read by a user effect outside — never a reader + * (Vocabulary "Reader"), so it holds nothing, as a plain read of `detail`; + * - lattice: a diamond memo lattice 2 wide × 30 deep below the output, read + * by nothing on screen — each node is walked once, not once per path; + * - header-tree: a header render effect inside an error boundary built + * outside, the boundary's output mounted untracked — the header is the + * display (D-16): the frame waits and DEV reports, as `header`. + */ +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; +import { + createEffect, + createErrorBoundary, + createLoadingBoundary, + createMemo, + createRenderEffect, + createRoot, + createSignal, + flush, + untrack, + OBSERVE +} from "../src/index.js"; + +beforeEach(() => vi.useFakeTimers()); +afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); +}); + +type Mode = "direct" | "memo" | "nested" | "unread" | "ueff" | "lattice" | "header" | "header-tree"; + +function part(read: () => string) { + const el = { text: "-" }; + createRenderEffect(read, v => { + el.text = v; + }); + return el; +} + +function page(mode: Mode) { + const asks: { key: string; res: (v: any) => void; rej: (e: any) => void }[] = []; + const ask = (key: string) => new Promise((res, rej) => asks.push({ key, res, rej })); + const view: { + header: string; + headerEl?: { text: string }; + detail: { text: string } | null; + comments: { text: string } | null; + } = { header: "", detail: null, comments: null }; + const [sel, setSel] = createSignal(3); + let dispose!: () => void; + createRoot(d => { + dispose = d; + const detail = createMemo(() => ask(`detail:${sel()}`), { name: "detail" }); + const eb = (src: () => any): (() => any) => + untrack(() => + createErrorBoundary( + () => src(), + () => ({ title: "ERR" }) + ) + ); + let shown: () => any = mode === "unread" ? detail : eb(detail); + if (mode === "unread") eb(detail); + if (mode === "nested") shown = eb(shown); + let title = () => shown()?.title as string; + if (mode === "memo") title = createMemo(title, { name: "title" }); + if (mode === "ueff") + createEffect( + () => shown()?.title, + () => {} + ); + if (mode === "lattice") { + let layer = [shown, shown]; + for (let i = 0; i < 30; i++) { + const [l, r] = layer; + layer = [createMemo(() => [l(), r()]), createMemo(() => [r(), l()])]; + } + } + if (mode === "header-tree") { + const out = untrack(() => + createErrorBoundary( + () => part(() => detail().title), + () => ({ text: "ERR" }) + ) + ); + const el = untrack(out) as { text: string }; + view.headerEl = el; + } + if (mode === "header") + createRenderEffect( + () => shown()?.title, + v => { + view.header = `${v} / `; + } + ); + const comments = createMemo(() => ask(`comments:${sel()}`), { name: "comments" }); + const a = untrack(() => + createLoadingBoundary( + () => part(title), + () => part(() => `loading #${sel()}`), + { on: () => sel() } + ) + ); + createRenderEffect(a, v => { + view.detail = v; + }); + const b = untrack(() => + createLoadingBoundary( + () => part(() => `comments ${(comments() as any[]).length}`), + () => part(() => "loading comments"), + { on: () => sel() } + ) + ); + createRenderEffect(b, v => { + view.comments = v; + }); + }); + const settle = async (key: string, v?: any, reject?: boolean) => { + const i = asks.findIndex(a => a.key === key); + if (i >= 0) { + const x = asks.splice(i, 1)[0]; + reject ? x.rej(new Error("boom")) : x.res(v); + } + for (let k = 0; k < 4; k++) { + await vi.advanceTimersByTimeAsync(0); + flush(); + } + }; + const t = (el: { text: string } | null) => (el ? el.text : "-"); + const screen = () => + `${view.headerEl ? `${view.headerEl.text} / ` : view.header}${t(view.detail)} | ${t(view.comments)}`; + return { setSel, settle, screen, dispose }; +} + +function captureWarnings() { + vi.spyOn(console, "warn").mockImplementation(() => {}); + const capture = OBSERVE!.diagnostics.capture(); + return { + codes: () => capture.events.map(e => e.code).filter(c => c === "LOADING_ON_OUTSIDE_HOLD"), + stop: () => capture.stop() + }; +} + +describe("#4639 an error boundary output reached only behind fallbacks holds nothing", () => { + for (const mode of ["direct", "memo", "nested", "unread", "ueff", "lattice"] as Mode[]) + for (const reject of mode === "unread" || mode === "lattice" ? [false] : [false, true]) + test(`${mode}${reject ? ", detail rejects" : ""}: both fallbacks show now, no LOADING_ON_OUTSIDE_HOLD`, async () => { + const d = captureWarnings(); + const p = page(mode); + flush(); + await p.settle("detail:3", { title: "Crash" }); + await p.settle("comments:3", [1]); + expect(p.screen()).toBe("Crash | comments 1"); + p.setSel(1); + flush(); + await p.settle(""); + expect(p.screen()).toBe("loading #1 | loading comments"); + await p.settle("detail:1", { title: "One" }, reject); + const shown = reject ? "ERR" : "One"; + expect(p.screen()).toBe(`${shown} | loading comments`); + await p.settle("comments:1", [1, 2]); + expect(p.screen()).toBe(`${shown} | comments 2`); + expect(d.codes()).toEqual([]); + d.stop(); + p.dispose(); + }); + + for (const mode of ["header", "header-tree"] as Mode[]) + test(`${mode} (D-16): the output rendered outside holds the frame; LOADING_ON_OUTSIDE_HOLD once`, async () => { + const d = captureWarnings(); + const p = page(mode); + flush(); + await p.settle("detail:3", { title: "Crash" }); + await p.settle("comments:3", [1]); + expect(p.screen()).toBe("Crash / Crash | comments 1"); + p.setSel(1); + flush(); + await p.settle(""); + expect(p.screen()).toBe("Crash / Crash | comments 1"); + expect(d.codes()).toEqual(["LOADING_ON_OUTSIDE_HOLD"]); + await p.settle("detail:1", { title: "One" }); + expect(p.screen()).toBe("One / One | loading comments"); + await p.settle("comments:1", [1, 2]); + expect(p.screen()).toBe("One / One | comments 2"); + d.stop(); + p.dispose(); + }); +}); diff --git a/packages/web/test/loading-on-errorboundary-through-4639.spec.tsx b/packages/web/test/loading-on-errorboundary-through-4639.spec.tsx new file mode 100644 index 000000000..aee5198a6 --- /dev/null +++ b/packages/web/test/loading-on-errorboundary-through-4639.spec.tsx @@ -0,0 +1,83 @@ +/** + * @jsxImportSource @solidjs/web + * @vitest-environment jsdom + * + * #4639 follow-up to #4638 (Vocabulary "Reader", F-3, F-4, D-14): an error + * boundary built in the component body is on screen only through the render + * effects it reaches, wherever the memo or boundary between was created. Read + * through a memo or another error boundary built outside the + * ``, or not read at all, it holds nothing: a `sel` write + * shows both fallbacks with the change's frame. + * The signals-only twin: `packages/signals/tests/loading-on-errorboundary-through-4639.test.ts`. + */ +import { expect, test } from "vitest"; +import { Loading, createMemo, createSignal, flush, untrack } from "solid-js"; +import { createErrorBoundary } from "solid-js/internal"; +import { render } from "../src/index.js"; + +const tick = async () => { + for (let i = 0; i < 4; i++) { + await new Promise(r => setTimeout(r, 0)); + flush(); + } +}; + +type Mode = "memo" | "nested" | "unread"; + +for (const mode of ["memo", "nested", "unread"] as Mode[]) + test(`switch key (error boundary, ${mode}): both fallbacks show now`, async () => { + const asks: { key: string; res: (v: any) => void }[] = []; + const ask = (key: string) => new Promise(res => asks.push({ key, res })); + const settle = async (key: string, v: any) => { + asks + .splice( + asks.findIndex(a => a.key === key), + 1 + )[0] + .res(v); + await tick(); + }; + const c = document.body.appendChild(document.createElement("div")); + let select!: (n: number) => void; + const dispose = render(() => { + const [sel, setSel] = createSignal(3); + select = setSel; + const detail = createMemo(() => ask(`detail:${sel()}`)); + const eb = (src: () => any): (() => any) => + untrack(() => + createErrorBoundary( + () => src(), + () => null + ) + ); + let shown: () => any = mode === "unread" ? detail : eb(detail); + if (mode === "unread") eb(detail); + if (mode === "nested") shown = eb(shown); + let title = (): string => shown()?.title; + if (mode === "memo") title = createMemo(title); + const comments = createMemo(() => ask(`comments:${sel()}`)); + return ( + <> + loading #{sel()}}> + {title()} + + loading comments}> + comments {(comments() as any[]).length} + + + ); + }, c); + await tick(); + await settle("detail:3", { title: "Crash" }); + await settle("comments:3", [1]); + expect(c.textContent).toBe("Crash comments 1"); + select(1); + await tick(); + expect(c.textContent).toBe("loading #1 loading comments"); + await settle("detail:1", { title: "One" }); + expect(c.textContent).toBe("One loading comments"); + await settle("comments:1", [1, 2]); + expect(c.textContent).toBe("One comments 2"); + dispose(); + c.remove(); + });