From cc9a3a6eb478fe2ac980952f51cb507f4d300b87 Mon Sep 17 00:00:00 2001 From: Birk Skyum Date: Tue, 28 Jul 2026 20:23:27 +0200 Subject: [PATCH] fix: handle rejected module promises in lazy() On the server `lazy()` attached only a success handler to the module promise. When the promise rejected, the Suspense boundary's `_loading` flag was never cleared, so `notifySuspense` never fired, the registered fragment never resolved and `renderToStream` never called `end()`: the response hung open and the error never reached an enclosing `ErrorBoundary`. Without a host `unhandledRejection` handler the un-caught `.then` in `load` crashed the process instead. The rejection is now recorded next to the resolved module, the boundary is notified so the fragment resolves, and the error is thrown where the component renders so `ErrorBoundary` can catch it. The client's hydration branch had the same success-only handler, leaving `sharedConfig.count` pinned and swallowing the error. It now decrements the count and surfaces the rejection to the nearest `ErrorBoundary`. SSR had no test harness, so this adds a second vitest project that resolves `solid-js` to the server entries. Closes #2945 --- .changeset/fix-lazy-rejected-module.md | 5 ++ packages/solid/src/render/component.ts | 25 +++++++--- packages/solid/src/server/rendering.ts | 25 +++++++--- packages/solid/test/server/lazy.spec.ts | 63 +++++++++++++++++++++++++ packages/solid/tsconfig.test.json | 6 ++- packages/solid/vite.config.mjs | 2 + packages/solid/vitest.workspace.mjs | 25 ++++++++++ packages/solid/web/test/lazy.spec.tsx | 29 +++++++++++- 8 files changed, 163 insertions(+), 17 deletions(-) create mode 100644 .changeset/fix-lazy-rejected-module.md create mode 100644 packages/solid/test/server/lazy.spec.ts create mode 100644 packages/solid/vitest.workspace.mjs diff --git a/.changeset/fix-lazy-rejected-module.md b/.changeset/fix-lazy-rejected-module.md new file mode 100644 index 000000000..5941a3de4 --- /dev/null +++ b/.changeset/fix-lazy-rejected-module.md @@ -0,0 +1,5 @@ +--- +"solid-js": patch +--- + +Handle rejected module promises in `lazy()`. On the server the promise only had a success handler, so a chunk that failed to load left the Suspense boundary's `_loading` flag set forever: `renderToStream` never called `end()`, `renderToStringAsync` never resolved, and the error never reached an enclosing `ErrorBoundary`. The rejection is now tracked alongside the resolved module, the boundary is notified, and the error is thrown where the component renders so `ErrorBoundary` can catch it. The client's hydration branch had the same success-only handler, leaving `sharedConfig.count` pinned and swallowing the error; it now decrements the count and surfaces the rejection to the nearest `ErrorBoundary`. diff --git a/packages/solid/src/render/component.ts b/packages/solid/src/render/component.ts index 5775162f8..c55264993 100644 --- a/packages/solid/src/render/component.ts +++ b/packages/solid/src/render/component.ts @@ -365,12 +365,25 @@ export function lazy>( const [s, set] = createSignal(); sharedConfig.count || (sharedConfig.count = 0); sharedConfig.count++; - (p || (p = fn())).then(mod => { - !sharedConfig.done && setHydrateContext(ctx); - sharedConfig.count!--; - set(() => mod.default); - setHydrateContext(); - }); + (p || (p = fn())).then( + mod => { + !sharedConfig.done && setHydrateContext(ctx); + sharedConfig.count!--; + set(() => mod.default); + setHydrateContext(); + }, + err => { + !sharedConfig.done && setHydrateContext(ctx); + sharedConfig.count!--; + set( + () => + (() => { + throw err; + }) as unknown as T + ); + setHydrateContext(); + } + ); comp = s; } else if (!comp) { const [s] = createResource(() => (p || (p = fn())).then(mod => mod.default)); diff --git a/packages/solid/src/server/rendering.ts b/packages/solid/src/server/rendering.ts index eeffc939e..03a63e44b 100644 --- a/packages/solid/src/server/rendering.ts +++ b/packages/solid/src/server/rendering.ts @@ -556,11 +556,14 @@ export function createResource( export function lazy>( fn: () => Promise<{ default: T }> ): T & { preload: () => Promise<{ default: T }> } { - let p: Promise<{ default: T }> & { resolved?: T }; + let p: Promise<{ default: T }> & { resolved?: T; error?: any }; let load = (id?: string) => { if (!p) { p = fn(); - p.then(mod => (p.resolved = mod.default)); + p.then( + mod => (p.resolved = mod.default), + err => (p.error = castError(err)) + ); if (id) sharedConfig.context!.lazy[id] = p; } return p; @@ -574,18 +577,26 @@ export function lazy>( if (ref) p = ref; else load(id); if (p.resolved) return p.resolved(props); + if (p.error) throw p.error; const ctx = useContext(SuspenseContext); - const track = { _loading: true, error: undefined }; + const track = { _loading: true, error: undefined as any }; if (ctx) { ctx.resources.set(id, track); contexts.add(ctx); } if (sharedConfig.context!.async) { sharedConfig.context!.block( - p.then(() => { - track._loading = false; - notifySuspense(contexts); - }) + p.then( + () => { + track._loading = false; + notifySuspense(contexts); + }, + err => { + track._loading = false; + track.error = castError(err); + notifySuspense(contexts); + } + ) ); } return ""; diff --git a/packages/solid/test/server/lazy.spec.ts b/packages/solid/test/server/lazy.spec.ts new file mode 100644 index 000000000..9e585f05d --- /dev/null +++ b/packages/solid/test/server/lazy.spec.ts @@ -0,0 +1,63 @@ +import { describe, expect, test } from "vitest"; +import { createComponent, ErrorBoundary, lazy, Suspense } from "../../src/server/index.js"; +import { renderToStream, renderToStringAsync } from "../../web/server/index.js"; + +type Text = () => string; + +function renderBroken(render: (code: () => any) => any) { + const Broken = lazy(() => Promise.reject(new Error("boom"))); + return render(() => + createComponent(ErrorBoundary, { + fallback: (err: Error) => `error boundary: ${err.message}`, + get children() { + return createComponent(Suspense, { + fallback: "loading", + get children() { + return createComponent(Broken, {}); + } + }); + } + }) + ); +} + +function collect(stream: ReturnType) { + return new Promise((resolve, reject) => { + let html = ""; + const timeout = setTimeout(() => reject(new Error("stream never ended")), 2000); + stream.pipe({ + write: (chunk: string) => (html += chunk), + end: () => { + clearTimeout(timeout); + resolve(html); + } + }); + }); +} + +describe("lazy() with a rejected module promise on the server", () => { + test("renderToStream ends and the ErrorBoundary renders", async () => { + const html = await collect(renderBroken(renderToStream)); + expect(html).toContain("error boundary: boom"); + }); + + test("renderToStringAsync resolves and the ErrorBoundary renders", async () => { + const html = await renderBroken(renderToStringAsync); + expect(html).toContain("error boundary: boom"); + }); + + test("a resolved module still renders", async () => { + const Lazy = lazy(() => Promise.resolve({ default: () => "content" })); + const html = await collect( + renderToStream(() => + createComponent(Suspense, { + fallback: "loading", + get children() { + return createComponent(Lazy, {}); + } + }) + ) + ); + expect(html).toContain("content"); + }); +}); diff --git a/packages/solid/tsconfig.test.json b/packages/solid/tsconfig.test.json index 24147743d..32c0e6e91 100644 --- a/packages/solid/tsconfig.test.json +++ b/packages/solid/tsconfig.test.json @@ -4,8 +4,10 @@ "baseUrl": "", "paths": { "solid-js/jsx-runtime": ["./src/jsx"], - "solid-js": ["./src"], + "solid-js": ["./src"] } }, - "include": ["./test", "./web/test", "./store/test"] + "include": ["./test", "./web/test", "./store/test"], + // test/server imports the server entries, which don't match the client paths above + "exclude": ["./test/server"] } diff --git a/packages/solid/vite.config.mjs b/packages/solid/vite.config.mjs index 6416fd254..afea1e718 100644 --- a/packages/solid/vite.config.mjs +++ b/packages/solid/vite.config.mjs @@ -12,6 +12,8 @@ export default defineConfig({ port: 3000 }, test: { + name: "client", + exclude: ["**/node_modules/**", "**/dist/**", "test/server/**"], coverage: { provider: "v8", reporter: ["text", "lcov"], diff --git a/packages/solid/vitest.workspace.mjs b/packages/solid/vitest.workspace.mjs new file mode 100644 index 000000000..80f2b95fc --- /dev/null +++ b/packages/solid/vitest.workspace.mjs @@ -0,0 +1,25 @@ +import { defineWorkspace } from "vitest/config"; +import { resolve } from "path"; + +export default defineWorkspace([ + "./vite.config.mjs", + // SSR tests: the default config resolves `solid-js` to the client sources, + // so the server entries get their own project. + { + test: { + name: "server", + environment: "node", + include: ["test/server/**/*.spec.ts"], + globals: true, + server: { deps: { inline: [/dom-expressions/] } } + }, + resolve: { + conditions: ["node"], + alias: { + rxcore: resolve(__dirname, "web/src/core.ts"), + "solid-js/web": resolve(__dirname, "web/server/index.ts"), + "solid-js": resolve(__dirname, "src/server/index.ts") + } + } + } +]); diff --git a/packages/solid/web/test/lazy.spec.tsx b/packages/solid/web/test/lazy.spec.tsx index a706839fe..002555c37 100644 --- a/packages/solid/web/test/lazy.spec.tsx +++ b/packages/solid/web/test/lazy.spec.tsx @@ -3,8 +3,8 @@ * @vitest-environment jsdom */ import { describe, expect, test } from "vitest"; -import { lazy, Component } from "../../src/index.js"; -import { render, Suspense } from "../src/index.js"; +import { lazy, Component, ErrorBoundary, sharedConfig } from "../../src/index.js"; +import { hydrate, render, Suspense } from "../src/index.js"; describe("lazy() disposal", () => { test("nested lazy boundaries remount after navigation-style dispose", async () => { @@ -59,3 +59,28 @@ describe("lazy() disposal", () => { second.dispose(); }); }); + +describe("lazy() while hydrating", () => { + test("a rejected module reaches the ErrorBoundary and releases the hydration count", async () => { + const Broken = lazy(() => Promise.reject(new Error("boom"))); + const div = document.createElement("div"); + const previousHY = (globalThis as any)._$HY; + (globalThis as any)._$HY = { events: [], completed: new WeakSet(), r: {} }; + try { + hydrate( + () => ( + `error boundary: ${err.message}`}> + + + ), + div + ); + await Promise.resolve(); + await Promise.resolve(); + expect(div.textContent).toBe("error boundary: boom"); + expect(sharedConfig.count).toBe(0); + } finally { + (globalThis as any)._$HY = previousHY; + } + }); +});