From 10452bd1fa93b06d2d26d0612d1ce4bae518f7c5 Mon Sep 17 00:00:00 2001 From: Brenley Dueck Date: Tue, 3 Nov 2026 13:24:56 -0600 Subject: [PATCH] fix: reset preload flags when a route preload throws --- .changeset/preload-throw-intent.md | 5 ++ src/preloadRoute.ts | 85 ++++++++++-------- src/routing.ts | 13 ++- test/preload-throw.spec.tsx | 137 +++++++++++++++++++++++++++++ 4 files changed, 199 insertions(+), 41 deletions(-) create mode 100644 .changeset/preload-throw-intent.md create mode 100644 test/preload-throw.spec.tsx diff --git a/.changeset/preload-throw-intent.md b/.changeset/preload-throw-intent.md new file mode 100644 index 00000000..71641804 --- /dev/null +++ b/.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. diff --git a/src/preloadRoute.ts b/src/preloadRoute.ts index 81883a6e..631a805a 100644 --- a/src/preloadRoute.ts +++ b/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); } /** diff --git a/src/routing.ts b/src/routing.ts index 46ed03aa..802b0327 100644 --- a/src/routing.ts +++ b/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, diff --git a/test/preload-throw.spec.tsx b/test/preload-throw.spec.tsx new file mode 100644 index 00000000..a92a3791 --- /dev/null +++ b/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; + let navigate!: ReturnType; + const Home = () => { + preload = usePreloadRoute(); + navigate = useNavigate(); + return home; + }; + const FilePage = () => { + const files = createMemo(() => getFiles()); + return ( + files-pending}> + files:{String(files())} + + ); + }; + + const caught: unknown[] = []; + const Router = createRouter({ + routes: [ + { path: "/", component: Home }, + { + path: "/broken", + preload: () => { + throw new Error("preload bug"); + }, + component: () => broken + }, + { path: "/files", component: FilePage }, + { path: "/login", component: () => login-page } + ] as const, + history: memoryHistory("/") + }); + + const root = document.createElement("div"); + const dispose = render( + () => ( + + {(props: ParentProps) => ( + { + caught.push(e()); + return

caught

; + }} + > + {props.children} + + )} + + ), + 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; + const Router = createRouter({ + routes: [ + { + path: "/", + component: () => ((navigate = useNavigate()), (home)) + }, + { + path: "/broken", + preload: () => { + throw new Error("preload bug"); + }, + component: () => broken + } + ] as const, + history: memoryHistory("/") + }); + + const root = document.createElement("div"); + const dispose = render( + () => ( + + {(props: ParentProps) => caught

}>{props.children}} + + ), + root + ); + try { + await wait(30); + navigate("/broken"); + await wait(50); + expect(root.textContent).toBe("caught"); + expect(getInPreloadFn()).toBe(false); + } finally { + dispose(); + } + }); +});