From 9c2194a136b759a7008693d9e08b9dcbfb17cd8e Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Fri, 30 May 2025 12:06:29 +1000 Subject: [PATCH 1/4] Fix RSC `fetcher.load` --- packages/react-router/lib/router/router.ts | 139 +++++++----------- playground/rsc-parcel/src/routes.ts | 11 ++ .../src/routes/fetcher/fetcher.client.tsx | 17 +++ .../rsc-parcel/src/routes/fetcher/fetcher.ts | 1 + .../src/routes/resource/resource.ts | 6 + .../src/routes/root/root.client.tsx | 3 + 6 files changed, 88 insertions(+), 89 deletions(-) create mode 100644 playground/rsc-parcel/src/routes/fetcher/fetcher.client.tsx create mode 100644 playground/rsc-parcel/src/routes/fetcher/fetcher.ts create mode 100644 playground/rsc-parcel/src/routes/resource/resource.ts diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index 9b06511b2c..b0cc25fa25 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -2237,22 +2237,50 @@ export function createRouter(init: RouterInit): Router { matches = fogOfWar.matches; } - if (!matches) { - setFetcherError( - key, - routeId, - getInternalRouterError(404, { pathname: normalizedPath }), - { flushSync } - ); - return; - } - let { path, submission, error } = normalizeNavigateOptions( true, normalizedPath, opts ); + let abortController = new AbortController(); + + if (!matches) { + if (!fogOfWar.active) { + setFetcherError( + key, + routeId, + getInternalRouterError(404, { pathname: normalizedPath }), + { flushSync } + ); + return; + } else { + let discoverResult = await discoverRoutes( + matches ?? [], + path, + abortController.signal, + key + ); + + if (discoverResult.type === "aborted") { + return; + } else if (discoverResult.type === "error") { + setFetcherError(key, routeId, discoverResult.error, { flushSync }); + return; + } else if (!discoverResult.matches) { + setFetcherError( + key, + routeId, + getInternalRouterError(404, { pathname: path }), + { flushSync } + ); + return; + } else { + matches = discoverResult.matches; + } + } + } + if (error) { setFetcherError(key, routeId, error, { flushSync }); return; @@ -2273,7 +2301,7 @@ export function createRouter(init: RouterInit): Router { match, matches, scopedContext, - fogOfWar.active, + abortController, flushSync, preventScrollReset, submission @@ -2291,7 +2319,7 @@ export function createRouter(init: RouterInit): Router { match, matches, scopedContext, - fogOfWar.active, + abortController, flushSync, preventScrollReset, submission @@ -2307,7 +2335,7 @@ export function createRouter(init: RouterInit): Router { match: AgnosticDataRouteMatch, requestMatches: AgnosticDataRouteMatch[], scopedContext: unstable_RouterContextProvider, - isFogOfWar: boolean, + abortController: AbortController, flushSync: boolean, preventScrollReset: boolean, submission: Submission @@ -2315,20 +2343,13 @@ export function createRouter(init: RouterInit): Router { interruptActiveLoads(); fetchLoadMatches.delete(key); - function detectAndHandle405Error(m: AgnosticDataRouteMatch) { - if (!m.route.action && !m.route.lazy) { - let error = getInternalRouterError(405, { - method: submission.formMethod, - pathname: path, - routeId: routeId, - }); - setFetcherError(key, routeId, error, { flushSync }); - return true; - } - return false; - } - - if (!isFogOfWar && detectAndHandle405Error(match)) { + if (!match.route.action && !match.route.lazy) { + let error = getInternalRouterError(405, { + method: submission.formMethod, + pathname: path, + routeId: routeId, + }); + setFetcherError(key, routeId, error, { flushSync }); return; } @@ -2338,7 +2359,6 @@ export function createRouter(init: RouterInit): Router { flushSync, }); - let abortController = new AbortController(); let fetchRequest = createClientSideRequest( init.history, path, @@ -2346,37 +2366,6 @@ export function createRouter(init: RouterInit): Router { submission ); - if (isFogOfWar) { - let discoverResult = await discoverRoutes( - requestMatches, - path, - fetchRequest.signal, - key - ); - - if (discoverResult.type === "aborted") { - return; - } else if (discoverResult.type === "error") { - setFetcherError(key, routeId, discoverResult.error, { flushSync }); - return; - } else if (!discoverResult.matches) { - setFetcherError( - key, - routeId, - getInternalRouterError(404, { pathname: path }), - { flushSync } - ); - return; - } else { - requestMatches = discoverResult.matches; - match = getTargetMatch(requestMatches, path); - - if (detectAndHandle405Error(match)) { - return; - } - } - } - // Call the action for the fetcher fetchControllers.set(key, abortController); @@ -2622,7 +2611,7 @@ export function createRouter(init: RouterInit): Router { match: AgnosticDataRouteMatch, matches: AgnosticDataRouteMatch[], scopedContext: unstable_RouterContextProvider, - isFogOfWar: boolean, + abortController: AbortController, flushSync: boolean, preventScrollReset: boolean, submission?: Submission @@ -2637,40 +2626,12 @@ export function createRouter(init: RouterInit): Router { { flushSync } ); - let abortController = new AbortController(); let fetchRequest = createClientSideRequest( init.history, path, abortController.signal ); - if (isFogOfWar) { - let discoverResult = await discoverRoutes( - matches, - path, - fetchRequest.signal, - key - ); - - if (discoverResult.type === "aborted") { - return; - } else if (discoverResult.type === "error") { - setFetcherError(key, routeId, discoverResult.error, { flushSync }); - return; - } else if (!discoverResult.matches) { - setFetcherError( - key, - routeId, - getInternalRouterError(404, { pathname: path }), - { flushSync } - ); - return; - } else { - matches = discoverResult.matches; - match = getTargetMatch(matches, path); - } - } - // Call the loader for this fetcher route match fetchControllers.set(key, abortController); @@ -3275,7 +3236,7 @@ export function createRouter(init: RouterInit): Router { true ); - return { active: true, matches: fogMatches || [] }; + return { active: true, matches: fogMatches }; } else { if (Object.keys(matches[0].params).length > 0) { // If we matched a dynamic param or a splat, it might only be because diff --git a/playground/rsc-parcel/src/routes.ts b/playground/rsc-parcel/src/routes.ts index 4c033dda4d..99ce6a62b0 100644 --- a/playground/rsc-parcel/src/routes.ts +++ b/playground/rsc-parcel/src/routes.ts @@ -17,6 +17,17 @@ export const routes = [ // @ts-expect-error lazy: () => import("./routes/about/about"), }, + { + id: "fetcher", + path: "fetcher", + // @ts-expect-error + lazy: () => import("./routes/fetcher/fetcher"), + }, ], }, + { + id: "resource", + path: "resource", + lazy: () => import("./routes/resource/resource"), + }, ] satisfies ServerRouteObject[]; diff --git a/playground/rsc-parcel/src/routes/fetcher/fetcher.client.tsx b/playground/rsc-parcel/src/routes/fetcher/fetcher.client.tsx new file mode 100644 index 0000000000..db250e1347 --- /dev/null +++ b/playground/rsc-parcel/src/routes/fetcher/fetcher.client.tsx @@ -0,0 +1,17 @@ +"use client"; + +import { useFetcher } from "react-router"; +import type { loader } from "../resource/resource"; + +export default function Fetcher() { + const fetcher = useFetcher(); + + return ( +
+ + 
{JSON.stringify(fetcher.data, null, 2)}
+
+ ); +} diff --git a/playground/rsc-parcel/src/routes/fetcher/fetcher.ts b/playground/rsc-parcel/src/routes/fetcher/fetcher.ts new file mode 100644 index 0000000000..e7dadeb8d8 --- /dev/null +++ b/playground/rsc-parcel/src/routes/fetcher/fetcher.ts @@ -0,0 +1 @@ +export { default } from "./fetcher.client"; diff --git a/playground/rsc-parcel/src/routes/resource/resource.ts b/playground/rsc-parcel/src/routes/resource/resource.ts new file mode 100644 index 0000000000..188f306ca6 --- /dev/null +++ b/playground/rsc-parcel/src/routes/resource/resource.ts @@ -0,0 +1,6 @@ +export function loader() { + return { + timestamp: Date.now(), + message: "Hello from resource route!", + }; +} diff --git a/playground/rsc-parcel/src/routes/root/root.client.tsx b/playground/rsc-parcel/src/routes/root/root.client.tsx index d5e190d168..5c472d7a91 100644 --- a/playground/rsc-parcel/src/routes/root/root.client.tsx +++ b/playground/rsc-parcel/src/routes/root/root.client.tsx @@ -39,6 +39,9 @@ export default function Root() {
  • About
    • +
  • + Fetcher +
    • {counter} From 5c7091df6296a0ae17f045154f2074f792462861 Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Fri, 30 May 2025 14:03:48 +1000 Subject: [PATCH 2/4] Fix type error --- playground/rsc-parcel/src/routes.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/playground/rsc-parcel/src/routes.ts b/playground/rsc-parcel/src/routes.ts index 99ce6a62b0..37206d8466 100644 --- a/playground/rsc-parcel/src/routes.ts +++ b/playground/rsc-parcel/src/routes.ts @@ -20,7 +20,6 @@ export const routes = [ { id: "fetcher", path: "fetcher", - // @ts-expect-error lazy: () => import("./routes/fetcher/fetcher"), }, ], From d5ec9223cbfd32afc7771784637b0df28e4c708e Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Fri, 30 May 2025 14:53:19 +1000 Subject: [PATCH 3/4] Move route discovery logic back to fetcher handlers --- packages/react-router/lib/router/router.ts | 144 +++++++++++++-------- 1 file changed, 88 insertions(+), 56 deletions(-) diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index b0cc25fa25..d47f3d2c6c 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -2237,56 +2237,27 @@ export function createRouter(init: RouterInit): Router { matches = fogOfWar.matches; } + if (!matches) { + setFetcherError( + key, + routeId, + getInternalRouterError(404, { pathname: normalizedPath }), + { flushSync } + ); + return; + } + let { path, submission, error } = normalizeNavigateOptions( true, normalizedPath, opts ); - let abortController = new AbortController(); - - if (!matches) { - if (!fogOfWar.active) { - setFetcherError( - key, - routeId, - getInternalRouterError(404, { pathname: normalizedPath }), - { flushSync } - ); - return; - } else { - let discoverResult = await discoverRoutes( - matches ?? [], - path, - abortController.signal, - key - ); - - if (discoverResult.type === "aborted") { - return; - } else if (discoverResult.type === "error") { - setFetcherError(key, routeId, discoverResult.error, { flushSync }); - return; - } else if (!discoverResult.matches) { - setFetcherError( - key, - routeId, - getInternalRouterError(404, { pathname: path }), - { flushSync } - ); - return; - } else { - matches = discoverResult.matches; - } - } - } - if (error) { setFetcherError(key, routeId, error, { flushSync }); return; } - let match = getTargetMatch(matches, path); // Create a new context per fetch let scopedContext = new unstable_RouterContextProvider( init.unstable_getContext ? await init.unstable_getContext() : undefined @@ -2298,10 +2269,9 @@ export function createRouter(init: RouterInit): Router { key, routeId, path, - match, matches, scopedContext, - abortController, + fogOfWar.active, flushSync, preventScrollReset, submission @@ -2316,10 +2286,9 @@ export function createRouter(init: RouterInit): Router { key, routeId, path, - match, matches, scopedContext, - abortController, + fogOfWar.active, flushSync, preventScrollReset, submission @@ -2332,10 +2301,9 @@ export function createRouter(init: RouterInit): Router { key: string, routeId: string, path: string, - match: AgnosticDataRouteMatch, requestMatches: AgnosticDataRouteMatch[], scopedContext: unstable_RouterContextProvider, - abortController: AbortController, + isFogOfWar: boolean, flushSync: boolean, preventScrollReset: boolean, submission: Submission @@ -2343,14 +2311,17 @@ export function createRouter(init: RouterInit): Router { interruptActiveLoads(); fetchLoadMatches.delete(key); - if (!match.route.action && !match.route.lazy) { - let error = getInternalRouterError(405, { - method: submission.formMethod, - pathname: path, - routeId: routeId, - }); - setFetcherError(key, routeId, error, { flushSync }); - return; + function detectAndHandle405Error(m: AgnosticDataRouteMatch) { + if (!m.route.action && !m.route.lazy) { + let error = getInternalRouterError(405, { + method: submission.formMethod, + pathname: path, + routeId: routeId, + }); + setFetcherError(key, routeId, error, { flushSync }); + return true; + } + return false; } // Put this fetcher into it's submitting state @@ -2359,6 +2330,7 @@ export function createRouter(init: RouterInit): Router { flushSync, }); + let abortController = new AbortController(); let fetchRequest = createClientSideRequest( init.history, path, @@ -2366,6 +2338,38 @@ export function createRouter(init: RouterInit): Router { submission ); + if (isFogOfWar) { + let discoverResult = await discoverRoutes( + requestMatches, + path, + fetchRequest.signal, + key + ); + + if (discoverResult.type === "aborted") { + return; + } else if (discoverResult.type === "error") { + setFetcherError(key, routeId, discoverResult.error, { flushSync }); + return; + } else if (!discoverResult.matches) { + setFetcherError( + key, + routeId, + getInternalRouterError(404, { pathname: path }), + { flushSync } + ); + return; + } else { + requestMatches = discoverResult.matches; + } + } + + let match = getTargetMatch(requestMatches, path); + + if (detectAndHandle405Error(match)) { + return; + } + // Call the action for the fetcher fetchControllers.set(key, abortController); @@ -2608,10 +2612,9 @@ export function createRouter(init: RouterInit): Router { key: string, routeId: string, path: string, - match: AgnosticDataRouteMatch, matches: AgnosticDataRouteMatch[], scopedContext: unstable_RouterContextProvider, - abortController: AbortController, + isFogOfWar: boolean, flushSync: boolean, preventScrollReset: boolean, submission?: Submission @@ -2626,12 +2629,41 @@ export function createRouter(init: RouterInit): Router { { flushSync } ); + let abortController = new AbortController(); let fetchRequest = createClientSideRequest( init.history, path, abortController.signal ); + if (isFogOfWar) { + let discoverResult = await discoverRoutes( + matches, + path, + fetchRequest.signal, + key + ); + + if (discoverResult.type === "aborted") { + return; + } else if (discoverResult.type === "error") { + setFetcherError(key, routeId, discoverResult.error, { flushSync }); + return; + } else if (!discoverResult.matches) { + setFetcherError( + key, + routeId, + getInternalRouterError(404, { pathname: path }), + { flushSync } + ); + return; + } else { + matches = discoverResult.matches; + } + } + + let match = getTargetMatch(matches, path); + // Call the loader for this fetcher route match fetchControllers.set(key, abortController); @@ -3236,7 +3268,7 @@ export function createRouter(init: RouterInit): Router { true ); - return { active: true, matches: fogMatches }; + return { active: true, matches: fogMatches || [] }; } else { if (Object.keys(matches[0].params).length > 0) { // If we matched a dynamic param or a splat, it might only be because From 6076a1ec348a8c6d656a2b5814d7a9459ffb7c6f Mon Sep 17 00:00:00 2001 From: Mark Dalgleish Date: Fri, 30 May 2025 15:16:53 +1000 Subject: [PATCH 4/4] Inline `detectAndHandle405Error` --- packages/react-router/lib/router/router.ts | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts index d47f3d2c6c..93904af28a 100644 --- a/packages/react-router/lib/router/router.ts +++ b/packages/react-router/lib/router/router.ts @@ -2311,19 +2311,6 @@ export function createRouter(init: RouterInit): Router { interruptActiveLoads(); fetchLoadMatches.delete(key); - function detectAndHandle405Error(m: AgnosticDataRouteMatch) { - if (!m.route.action && !m.route.lazy) { - let error = getInternalRouterError(405, { - method: submission.formMethod, - pathname: path, - routeId: routeId, - }); - setFetcherError(key, routeId, error, { flushSync }); - return true; - } - return false; - } - // Put this fetcher into it's submitting state let existingFetcher = state.fetchers.get(key); updateFetcherState(key, getSubmittingFetcher(submission, existingFetcher), { @@ -2366,7 +2353,13 @@ export function createRouter(init: RouterInit): Router { let match = getTargetMatch(requestMatches, path); - if (detectAndHandle405Error(match)) { + if (!match.route.action && !match.route.lazy) { + let error = getInternalRouterError(405, { + method: submission.formMethod, + pathname: path, + routeId: routeId, + }); + setFetcherError(key, routeId, error, { flushSync }); return; }