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