fix: reset preload flags when a route preload throws

.changeset/preload-throw-intent.md

@@ -0,0 +1,5 @@
+---
+"@solidjs/router": patch
+---
+
+A route `preload` that throws no longer leaves the router in preload mode. `usePreloadRoute`, link preloading and route mounting now reset their preload flags even when the preload throws. Before, one throwing preload made every later query read count as a preload, so query redirects and Responses were ignored for the rest of the session.

src/preloadRoute.ts

@@ -1,5 +1,11 @@
 import { runWithOwner, untrack } from "solid-js";
-import { resolveLazySubtree, setInPreloadFn, setPreloadIntent, useRouter } from "./routing.js";
+import {
+  getInPreloadFn,
+  resolveLazySubtree,
+  setInPreloadFn,
+  setPreloadIntent,
+  useRouter
+} from "./routing.js";
 import { serverRouteArgs, serverRouteArgsEqual, serverRouteOf } from "./serverRouteShared.js";
 import type {
   MaybePreloadableComponent,
@@ -51,43 +57,48 @@ export function preloadRoute(router: RouterContext, url: URL, preloadData?: bool
     return a;
   };
   const prevIntent = setPreloadIntent("preload");
-  for (let match in next) {
-    const { route, params } = next[match];
-    const { preload, component } = route;
-    (component as MaybePreloadableComponent | undefined)?.preload?.();
-    const now = current && current[match];
-    const unchanged =
-      now &&
-      now.route.key === route.key &&
-      serverRouteArgsEqual(
-        inputs(params, query, url.search, route),
-        inputs(now.params, location.query, location.search, route)
-      );
-    setInPreloadFn(true);
-    preloadData &&
-      !unchanged &&
-      runWithOwner(router._routeOwner(), () => {
-        // A server component route's data IS its call: warm the same
-        // query entry the render will read, under the same derived args.
-        const server = serverRouteOf(component);
-        server && server.call(serverRouteArgs(route, params, query));
-        preload &&
-          preload({
-            params,
-            location: {
-              pathname: url.pathname,
-              search: url.search,
-              hash: url.hash,
-              query,
-              state: null,
-              key: ""
-            },
-            intent: "preload"
-          });
-      });
-    setInPreloadFn(false);
+  const prevInPreloadFn = getInPreloadFn();
+  try {
+    for (let match in next) {
+      const { route, params } = next[match];
+      const { preload, component } = route;
+      (component as MaybePreloadableComponent | undefined)?.preload?.();
+      const now = current && current[match];
+      const unchanged =
+        now &&
+        now.route.key === route.key &&
+        serverRouteArgsEqual(
+          inputs(params, query, url.search, route),
+          inputs(now.params, location.query, location.search, route)
+        );
+      setInPreloadFn(true);
+      preloadData &&
+        !unchanged &&
+        runWithOwner(router._routeOwner(), () => {
+          // A server component route's data IS its call: warm the same
+          // query entry the render will read, under the same derived args.
+          const server = serverRouteOf(component);
+          server && server.call(serverRouteArgs(route, params, query));
+          preload &&
+            preload({
+              params,
+              location: {
+                pathname: url.pathname,
+                search: url.search,
+                hash: url.hash,
+                query,
+                state: null,
+                key: ""
+              },
+              intent: "preload"
+            });
+        });
+      setInPreloadFn(prevInPreloadFn);
+    }
+  } finally {
+    setInPreloadFn(prevInPreloadFn);
+    setPreloadIntent(prevIntent);
   }
-  setPreloadIntent(prevIntent);
 }
 
 /**

src/routing.ts

@@ -1184,11 +1184,16 @@ export function createRouteContext(
   const params = wrapParams(() => mergeParams(matches()));
 
   (component as MaybePreloadableComponent | undefined)?.preload?.();
+  const prevInPreloadFn = inPreloadFn;
   inPreloadFn = true;
-  const data = preload
-    ? preload({ params, location, intent: router.intent?.() || "initial" })
-    : undefined;
-  inPreloadFn = false;
+  let data;
+  try {
+    data = preload
+      ? preload({ params, location, intent: router.intent?.() || "initial" })
+      : undefined;
+  } finally {
+    inPreloadFn = prevInPreloadFn;
+  }
 
   const route: RouteContext = {
     parent,

test/preload-throw.spec.tsx

@@ -0,0 +1,137 @@
+import { Errored, createMemo, Loading, type ParentProps } from "solid-js";
+import { render } from "@solidjs/web";
+import { vi } from "vitest";
+import { createRouter, memoryHistory, query, useNavigate, usePreloadRoute } from "../src/index.js";
+import { getInPreloadFn, setInPreloadFn } from "../src/routing.js";
+
+const wait = (ms: number) => new Promise(r => setTimeout(r, ms));
+
+describe("a route preload that throws", () => {
+  const originalScrollTo = window.scrollTo;
+  beforeEach(() => {
+    window.scrollTo = vi.fn() as any;
+  });
+  afterAll(() => {
+    window.scrollTo = originalScrollTo;
+  });
+
+  test("in usePreloadRoute does not leave later query reads in preload mode", async () => {
+    const getFiles = query(async () => {
+      throw new Response(null, { status: 302, headers: { Location: "/login" } });
+    }, "preload-throw-files");
+
+    let preload!: ReturnType<typeof usePreloadRoute>;
+    let navigate!: ReturnType<typeof useNavigate>;
+    const Home = () => {
+      preload = usePreloadRoute();
+      navigate = useNavigate();
+      return <span>home</span>;
+    };
+    const FilePage = () => {
+      const files = createMemo(() => getFiles());
+      return (
+        <Loading fallback={<span>files-pending</span>}>
+          <span>files:{String(files())}</span>
+        </Loading>
+      );
+    };
+
+    const caught: unknown[] = [];
+    const Router = createRouter({
+      routes: [
+        { path: "/", component: Home },
+        {
+          path: "/broken",
+          preload: () => {
+            throw new Error("preload bug");
+          },
+          component: () => <span>broken</span>
+        },
+        { path: "/files", component: FilePage },
+        { path: "/login", component: () => <span>login-page</span> }
+      ] as const,
+      history: memoryHistory("/")
+    });
+
+    const root = document.createElement("div");
+    const dispose = render(
+      () => (
+        <Router>
+          {(props: ParentProps) => (
+            <Errored
+              fallback={e => {
+                caught.push(e());
+                return <p>caught</p>;
+              }}
+            >
+              {props.children}
+            </Errored>
+          )}
+        </Router>
+      ),
+      root
+    );
+    try {
+      await wait(30);
+      expect(root.textContent).toBe("home");
+
+      expect(() => preload("/broken", { preloadData: true })).toThrow("preload bug");
+      expect(getInPreloadFn()).toBe(false);
+
+      // a preload started from inside another preload hands the flag back
+      setInPreloadFn(true);
+      try {
+        expect(() => preload("/broken", { preloadData: true })).toThrow("preload bug");
+        expect(getInPreloadFn()).toBe(true);
+      } finally {
+        setInPreloadFn(false);
+      }
+
+      navigate("/files");
+      await wait(150);
+      expect(caught).toEqual([]);
+      expect(root.textContent).toContain("login-page");
+    } finally {
+      dispose();
+    }
+  });
+
+  test("during navigation resets the in-preload flag", async () => {
+    let navigate!: ReturnType<typeof useNavigate>;
+    const Router = createRouter({
+      routes: [
+        {
+          path: "/",
+          component: () => ((navigate = useNavigate()), (<span>home</span>))
+        },
+        {
+          path: "/broken",
+          preload: () => {
+            throw new Error("preload bug");
+          },
+          component: () => <span>broken</span>
+        }
+      ] as const,
+      history: memoryHistory("/")
+    });
+
+    const root = document.createElement("div");
+    const dispose = render(
+      () => (
+        <Router>
+          {(props: ParentProps) => <Errored fallback={<p>caught</p>}>{props.children}</Errored>}
+        </Router>
+      ),
+      root
+    );
+    try {
+      await wait(30);
+      navigate("/broken");
+      await wait(50);
+      expect(root.textContent).toBe("caught");
+      expect(getInPreloadFn()).toBe(false);
+    } finally {
+      dispose();
+    }
+  });
+});