fix: handle rejected module promises in lazy()

.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`.

packages/solid/src/render/component.ts

@@ -365,12 +365,25 @@ export function lazy<T extends Component<any>>(
       const [s, set] = createSignal<T>();
       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<T>(() => (p || (p = fn())).then(mod => mod.default));

packages/solid/src/server/rendering.ts

@@ -556,11 +556,14 @@ export function createResource<T, S>(
 export function lazy<T extends Component<any>>(
   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<T extends Component<any>>(
     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 "";

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<Text>(() => 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<typeof renderToStream>) {
+  return new Promise<string>((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<Text>(() => Promise.resolve({ default: () => "content" }));
+    const html = await collect(
+      renderToStream(() =>
+        createComponent(Suspense, {
+          fallback: "loading",
+          get children() {
+            return createComponent(Lazy, {});
+          }
+        })
+      )
+    );
+    expect(html).toContain("content");
+  });
+});

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"]
 }

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"],

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

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<Component>(() => 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(
+        () => (
+          <ErrorBoundary fallback={(err: Error) => `error boundary: ${err.message}`}>
+            <Broken />
+          </ErrorBoundary>
+        ),
+        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;
+    }
+  });
+});