Skip to content

Commit b9cefee

Browse files
refactor(signals): remove the Loading notify-through remap
After #2856 narrowed the remap to fire only while the ERROR dimension was still live in the notification mask, the rule collapsed into the generic consume-and-forward path: each boundary consumes only its own status dimension from the mask and forwards the remainder, so an error inside a Loading reaches its Errored natively. The only residual behavior — suppressing pending collection on a node that is simultaneously pending and errored — is unreachable: status propagation (notifyStatus) assigns a single dimension per node, and the effect-phase error path notifies with literal ERROR-only flags. Pins the two paths the remap used to intercept (sync error in the mounting flush and reactive error after a healthy commit, both Errored > Loading > content). Full signals, solid, and solid-web (client/SSR/hydration) suites pass. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 08b88fb commit b9cefee

3 files changed

Lines changed: 78 additions & 18 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@solidjs/signals": patch
3+
---
4+
5+
Removed the `Loading` queue's notify-through remap. After #2856 narrowed it to fire only while the ERROR dimension was still live in the notification mask, the rule collapsed into the generic consume-and-forward path: each boundary consumes only its own status dimension and forwards the remainder, so an error inside a `Loading` reaches its `Errored` natively. The only residual behavior — suppressing pending collection on a node that is simultaneously pending and errored — is unreachable, as status propagation never sets both dimensions on one node. Added pins for the two paths the remap used to intercept (sync error in the mounting flush and reactive error after commit, both `Errored > Loading > content`).

packages/solid-signals/src/boundaries.ts

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -301,24 +301,16 @@ export class CollectionQueue extends Queue {
301301
}
302302
}
303303

304-
// Notify-through: a real error inside a `Loading` is not this boundary's to
305-
// handle — remap it to ERROR and forward toward the `Errored` that catches it.
306-
// This must run before the initialized passthrough below so the error still
307-
// reaches its catcher after this boundary has committed. Guarded on the ERROR
308-
// dimension still being live in `type`: an `Errored` nested below this
309-
// `Loading` consumes ERROR from the mask when it catches, and remapping off
310-
// the node's raw `flags` alone would resurrect that consumed error and route
311-
// it past the boundary that already handled it (the Loading > Errored >
312-
// content composition escape).
313-
// The mirror case (pending inside an `Errored`) needs no explicit rule: the
314-
// PENDING dimension survives `type &= ~collectionType` below and reaches the
315-
// outer `Loading` natively, while a pending already caught by an inner
316-
// `Loading` arrives here as a bare ERROR-dimension remainder and is correctly
317-
// swallowed.
318-
if (this._collectionType & STATUS_PENDING && type & STATUS_ERROR && flags & STATUS_ERROR) {
319-
return super.notify(node, STATUS_ERROR, flags, error);
320-
}
321-
304+
// Routing is dimension-independent: each boundary consumes only its own
305+
// status dimension from the mask (`type &= ~collectionType` below) and
306+
// forwards the remainder up the queue chain. An error inside a `Loading`
307+
// needs no special rule — the ERROR dimension survives consumption here and
308+
// reaches the `Errored` that catches it natively, and `flags & collectionType`
309+
// keeps this boundary from collecting a node that isn't actually pending.
310+
// Symmetrically, a pending inside an `Errored` forwards on the PENDING
311+
// dimension, while a status already caught by an inner boundary arrives with
312+
// its dimension consumed from the mask and is correctly not re-routed
313+
// (the Loading > Errored > content composition escape, #2856).
322314
if (this._collectionType & STATUS_PENDING && this._initialized)
323315
return super.notify(node, type, flags, error);
324316

packages/solid-signals/tests/createErrorBoundary.test.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -923,6 +923,69 @@ it("catches an async rejection at the inner Errored (Errored > Loading > Errored
923923
dispose();
924924
});
925925

926+
/**
927+
* Dimension-independent routing pins (remap removal follow-up to #2856): a sync
928+
* error inside a `Loading` with NO inner `Errored` must forward on the ERROR
929+
* dimension past the `Loading` to the outer `Errored` natively — the queue
930+
* chain consumes only each boundary's own dimension, so no explicit
931+
* notify-through rule is needed. Pinned for both the uninitialized boundary
932+
* (throw in the mounting flush) and the committed one (reactive throw), the two
933+
* paths the old remap intercepted.
934+
*/
935+
it("routes a sync mount error past a Loading to the outer Errored (Errored > Loading > content)", () => {
936+
let rendered: unknown;
937+
const dispose = createRoot(dispose => {
938+
const outer = createErrorBoundary(
939+
() =>
940+
createLoadingBoundary(
941+
() => {
942+
throw new Error("boom on mount");
943+
},
944+
() => "loading"
945+
)(),
946+
() => "outer caught"
947+
);
948+
createRenderEffect(
949+
() => (rendered = outer()),
950+
() => {}
951+
);
952+
return dispose;
953+
});
954+
flush();
955+
expect(rendered).toBe("outer caught");
956+
dispose();
957+
});
958+
959+
it("routes a reactive sync error past a committed Loading to the outer Errored (Errored > Loading > content)", () => {
960+
const [$boom, setBoom] = createSignal(false);
961+
let rendered: unknown;
962+
const dispose = createRoot(dispose => {
963+
const outer = createErrorBoundary(
964+
() =>
965+
createLoadingBoundary(
966+
() => {
967+
if ($boom()) throw new Error("boom");
968+
return "ok";
969+
},
970+
() => "loading"
971+
)(),
972+
() => "outer caught"
973+
);
974+
createRenderEffect(
975+
() => (rendered = outer()),
976+
() => {}
977+
);
978+
return dispose;
979+
});
980+
flush();
981+
expect(rendered).toBe("ok");
982+
983+
setBoom(true);
984+
flush();
985+
expect(rendered).toBe("outer caught");
986+
dispose();
987+
});
988+
926989
/**
927990
* #2809 hydration interaction: with snapshot capture active (as during
928991
* `hydrate()`), boundary computeds must not become snapshot sources. The tree no

0 commit comments

Comments
 (0)