Lift unstable_shouldRevalidateArgs to DataStrategyMatch

brophdawg11 · brophdawg11 · commit b84f4f2a2e48 · 2025-04-08T14:41:04.000-04:00
diff --git a/packages/react-router/lib/dom/ssr/single-fetch.tsx b/packages/react-router/lib/dom/ssr/single-fetch.tsx
@@ -302,11 +302,7 @@ async function nonSsrStrategy(
 // Loaders are trickier since we only want to hit the server once, so we
 // create a singular promise for all server-loader routes to latch onto.
 async function singleFetchLoaderNavigationStrategy(
-  {
-    request,
-    matches,
-    unstable_shouldRevalidateArgs: shouldRevalidateArgs,
-  }: DataStrategyFunctionArgs,
+  { request, matches }: DataStrategyFunctionArgs,
   manifest: AssetsManifest,
   ssr: boolean,
   router: DataRouter,
@@ -345,19 +341,18 @@ async function singleFetchLoaderNavigationStrategy(
         let manifestRoute = manifest.routes[m.route.id];
 
         let defaultShouldRevalidate =
-          !shouldRevalidateArgs ||
-          shouldRevalidateArgs.actionStatus == null ||
-          shouldRevalidateArgs.actionStatus < 400;
+          !m.unstable_shouldRevalidateArgs ||
+          m.unstable_shouldRevalidateArgs.actionStatus == null ||
+          m.unstable_shouldRevalidateArgs.actionStatus < 400;
         let shouldCall = m.unstable_shouldCallHandler(defaultShouldRevalidate);
 
         if (!shouldCall) {
-          // Mark an opt out route if we currently have data and a `shouldRevalidate`
-          // function.   This implies that the user opted out via `shouldRevalidate`
-          // so we don't want to include it in the single fetch .data request
+          // If this route opted out of revalidation, we don't want to include
+          // it in the single fetch .data request
           foundOptOutRoute ||=
-            manifestRoute?.hasLoader === true &&
-            m.route.id in router.state.loaderData &&
-            m.route.shouldRevalidate != null;
+            m.unstable_shouldRevalidateArgs != null && // This is a revalidation,
+            manifestRoute?.hasLoader === true && // for a route with a server loader,
+            m.route.shouldRevalidate != null; // and a shouldRevalidate function
           return;
         }
 
diff --git a/packages/react-router/lib/router/router.ts b/packages/react-router/lib/router/router.ts
@@ -1799,7 +1799,6 @@ export function createRouter(init: RouterInit): Router {
       let results = await callDataStrategy(
         request,
         dsMatches,
-        null,
         scopedContext,
         null
       );
@@ -1959,11 +1958,8 @@ export function createRouter(init: RouterInit): Router {
     }
 
     let routesToUse = inFlightDataRoutes || dataRoutes;
-    let {
-      navigationMatches: dsMatches,
-      revalidatingFetchers,
-      shouldRevalidateArgs,
-    } = getMatchesToLoad(
+    let { navigationMatches: dsMatches, revalidatingFetchers } =
+      getMatchesToLoad(
       request,
       scopedContext,
       mapRouteProperties,
@@ -2053,7 +2049,6 @@ export function createRouter(init: RouterInit): Router {
       await callLoadersAndMaybeResolveData(
         dsMatches,
         revalidatingFetchers,
-        shouldRevalidateArgs,
         request,
         scopedContext
       );
@@ -2339,7 +2334,6 @@ export function createRouter(init: RouterInit): Router {
     let actionResults = await callDataStrategy(
       fetchRequest,
       fetchMatches,
-      null,
       scopedContext,
       key
     );
@@ -2411,11 +2405,8 @@ export function createRouter(init: RouterInit): Router {
     let loadFetcher = getLoadingFetcher(submission, actionResult.data);
     state.fetchers.set(key, loadFetcher);
 
-    let {
-      navigationMatches: dsMatches,
-      revalidatingFetchers,
-      shouldRevalidateArgs,
-    } = getMatchesToLoad(
+    let { navigationMatches: dsMatches, revalidatingFetchers } =
+      getMatchesToLoad(
       revalidationRequest,
       scopedContext,
       mapRouteProperties,
@@ -2470,7 +2461,6 @@ export function createRouter(init: RouterInit): Router {
       await callLoadersAndMaybeResolveData(
         dsMatches,
         revalidatingFetchers,
-        shouldRevalidateArgs,
         revalidationRequest,
         scopedContext
       );
@@ -2638,7 +2628,6 @@ export function createRouter(init: RouterInit): Router {
     let results = await callDataStrategy(
       fetchRequest,
       dsMatches,
-      null,
       scopedContext,
       key
     );
@@ -2829,7 +2818,6 @@ export function createRouter(init: RouterInit): Router {
   async function callDataStrategy(
     request: Request,
     matches: DataStrategyMatch[],
-    shouldRevalidateArgs: DataStrategyFunctionArgs["unstable_shouldRevalidateArgs"],
     scopedContext: unstable_RouterContextProvider,
     fetcherKey: string | null
   ): Promise<Record<string, DataResult>> {
@@ -2840,7 +2828,6 @@ export function createRouter(init: RouterInit): Router {
         dataStrategyImpl as DataStrategyFunction<unknown>,
         request,
         matches,
-        shouldRevalidateArgs,
         fetcherKey,
         scopedContext
       );
@@ -2884,15 +2871,13 @@ export function createRouter(init: RouterInit): Router {
   async function callLoadersAndMaybeResolveData(
     matches: DataStrategyMatch[],
     fetchersToLoad: RevalidatingFetcher[],
-    shouldRevalidateArgs: DataStrategyFunctionArgs["unstable_shouldRevalidateArgs"],
     request: Request,
     scopedContext: unstable_RouterContextProvider
   ) {
     // Kick off loaders and fetchers in parallel
     let loaderResultsPromise = callDataStrategy(
       request,
       matches,
-      shouldRevalidateArgs,
       scopedContext,
       null
     );
@@ -2903,7 +2888,6 @@ export function createRouter(init: RouterInit): Router {
           let results = await callDataStrategy(
             f.request,
             f.matches,
-            shouldRevalidateArgs,
             scopedContext,
             f.key
           );
@@ -4250,7 +4234,6 @@ export function createStaticHandler(
       request,
       matches,
       null,
-      null,
       requestContext
     );
 
@@ -4587,7 +4570,6 @@ function getMatchesToLoad(
 ): {
   navigationMatches: DataStrategyMatch[];
   revalidatingFetchers: RevalidatingFetcher[];
-  shouldRevalidateArgs: DataStrategyFunctionArgs["unstable_shouldRevalidateArgs"];
 } {
   let actionResult = pendingActionResult
     ? isErrorResult(pendingActionResult[1])
@@ -4622,7 +4604,7 @@ function getMatchesToLoad(
     : undefined;
   let shouldSkipRevalidation = actionStatus && actionStatus >= 400;
 
-  let shouldRevalidateArgs: Omit<
+  let baseShouldRevalidateArgs: Omit<
     ShouldRevalidateFunctionArgs,
     "defaultShouldRevalidate"
   > | null = initialHydration
@@ -4709,13 +4691,25 @@ function getMatchesToLoad(
           isNewRouteInstance(state.matches[index], match);
       // Already checked `initialHydration` above so this should always be defined
       invariant(
-        shouldRevalidateArgs,
-        "Expected shouldRevalidateArgs to be defined for fetcher"
+        baseShouldRevalidateArgs,
+        "Expected shouldRevalidateArgs to be defined for route match"
       );
-      let shouldLoad = shouldRevalidateLoader(match, {
-        ...shouldRevalidateArgs,
+      let shouldRevalidateArgs: ShouldRevalidateFunctionArgs = {
+        ...baseShouldRevalidateArgs,
         defaultShouldRevalidate,
-      });
+      };
+      let shouldLoad = shouldRevalidateLoader(match, shouldRevalidateArgs);
+      let shouldCallHandler: DataStrategyMatch["unstable_shouldCallHandler"] = (
+        defaultOverride
+      ) =>
+          shouldRevalidateLoader(match, {
+            // We're not in initialHydration here so this will be defined
+            ...shouldRevalidateArgs!,
+            defaultShouldRevalidate:
+              typeof defaultOverride === "boolean"
+                ? defaultOverride
+                : defaultShouldRevalidate,
+        });
 
       return getDataStrategyMatch(
         mapRouteProperties,
@@ -4725,15 +4719,8 @@ function getMatchesToLoad(
         lazyRoutePropertiesToSkip,
         scopedContext,
         shouldLoad,
-        (defaultOverride) =>
-          shouldRevalidateLoader(match, {
-            // We're not in initialHydration here so this will be defined
-            ...shouldRevalidateArgs!,
-            defaultShouldRevalidate:
-              typeof defaultOverride === "boolean"
-                ? defaultOverride
-                : defaultShouldRevalidate,
-          })
+        shouldRevalidateArgs,
+        shouldCallHandler
       );
     }
   });
@@ -4846,13 +4833,17 @@ function getMatchesToLoad(
         : isRevalidationRequired;
       // Shouldn't run during initialHydration so this should always be defined
       invariant(
-        shouldRevalidateArgs,
+        baseShouldRevalidateArgs,
         "Expected shouldRevalidateArgs to be defined for fetcher"
       );
-      let shouldLoad = shouldRevalidateLoader(fetcherMatch, {
-        ...shouldRevalidateArgs,
+      let shouldRevalidateArgs: ShouldRevalidateFunctionArgs = {
+        ...baseShouldRevalidateArgs,
         defaultShouldRevalidate,
-      });
+      };
+      let shouldLoad = shouldRevalidateLoader(
+        fetcherMatch,
+        shouldRevalidateArgs
+      );
 
       if (shouldLoad) {
         revalidatingFetchers.push({
@@ -4866,7 +4857,8 @@ function getMatchesToLoad(
             fetcherMatches,
             fetcherMatch,
             lazyRoutePropertiesToSkip,
-            scopedContext
+            scopedContext,
+            shouldRevalidateArgs
           ),
           match: fetcherMatch,
           request: fetchRequest,
@@ -4876,11 +4868,7 @@ function getMatchesToLoad(
     }
   });
 
-  return {
-    navigationMatches,
-    revalidatingFetchers,
-    shouldRevalidateArgs,
-  };
+  return { navigationMatches, revalidatingFetchers };
 }
 
 function shouldLoadRouteOnHydration(
@@ -5532,6 +5520,7 @@ function getDataStrategyMatch(
   lazyRoutePropertiesToSkip: string[],
   scopedContext: unknown,
   shouldLoad: boolean,
+  unstable_shouldRevalidateArgs: DataStrategyMatch["unstable_shouldRevalidateArgs"] = null,
   unstable_shouldCallHandler: DataStrategyMatch["unstable_shouldCallHandler"] = () =>
     shouldLoad
 ): DataStrategyMatch {
@@ -5551,6 +5540,7 @@ function getDataStrategyMatch(
   return {
     ...match,
     shouldLoad,
+    unstable_shouldRevalidateArgs,
     unstable_shouldCallHandler(defaultShouldRevalidate) {
       isUsingNewApi = true;
       return unstable_shouldCallHandler(defaultShouldRevalidate);
@@ -5585,7 +5575,8 @@ function getTargetedDataStrategyMatches(
   matches: AgnosticDataRouteMatch[],
   targetMatch: AgnosticDataRouteMatch,
   lazyRoutePropertiesToSkip: string[],
-  scopedContext: unknown
+  scopedContext: unknown,
+  unstable_shouldRevalidateArgs: DataStrategyMatch["unstable_shouldRevalidateArgs"] = null
 ): DataStrategyMatch[] {
   return matches.map((match) => {
     if (match.route.id !== targetMatch.route.id) {
@@ -5594,6 +5585,7 @@ function getTargetedDataStrategyMatches(
       return {
         ...match,
         shouldLoad: false,
+        unstable_shouldRevalidateArgs,
         unstable_shouldCallHandler: () => false,
         _lazyPromises: getDataStrategyMatchLazyPromises(
           mapRouteProperties,
@@ -5613,7 +5605,8 @@ function getTargetedDataStrategyMatches(
       match,
       lazyRoutePropertiesToSkip,
       scopedContext,
-      true
+      true,
+      unstable_shouldRevalidateArgs
     );
   });
 }
@@ -5622,7 +5615,6 @@ async function callDataStrategyImpl(
   dataStrategyImpl: DataStrategyFunction<unknown>,
   request: Request,
   matches: DataStrategyMatch[],
-  shouldRevalidateArgs: DataStrategyFunctionArgs["unstable_shouldRevalidateArgs"],
   fetcherKey: string | null,
   scopedContext: unknown
 ): Promise<Record<string, DataStrategyResult>> {
@@ -5636,7 +5628,6 @@ async function callDataStrategyImpl(
   // back out below.
   let results = await dataStrategyImpl({
     matches,
-    unstable_shouldRevalidateArgs: shouldRevalidateArgs,
     request,
     params: matches[0].params,
     fetcherKey,
diff --git a/packages/react-router/lib/router/utils.ts b/packages/react-router/lib/router/utils.ts
@@ -332,6 +332,8 @@ export interface ShouldRevalidateFunction {
 export interface DataStrategyMatch
   extends AgnosticRouteMatch<string, AgnosticDataRouteObject> {
   shouldLoad: boolean;
+  // This can be null for actions calls and for initial hydration calls
+  unstable_shouldRevalidateArgs: ShouldRevalidateFunctionArgs | null;
   // TODO: Figure out a good name for this or use `shouldLoad` and add a future flag
   // This function will use a scoped version of `shouldRevalidateArgs` because
   // they are read-only but let the user provide an optional override value for
@@ -355,16 +357,6 @@ export interface DataStrategyMatch
 export interface DataStrategyFunctionArgs<Context = any>
   extends DataFunctionArgs<Context> {
   matches: DataStrategyMatch[];
-  // This can be null for actions calls and for initial hydration calls
-  // We omit the `defaultShouldRevalidate` because it's a per-route choice
-  // TODO: Should this be embedded on the match?  It only applies to revalidating
-  // matches, so going from  `/a/b -> /a/c` it doesn't really apply to `c` but
-  // could be potentially misconstrued?  If we provide it on the matches then
-  // it can have the proper `defaultShouldRevalidate` value on it
-  unstable_shouldRevalidateArgs: Omit<
-    ShouldRevalidateFunctionArgs,
-    "defaultShouldRevalidate"
-  > | null;
   // TODO: Implement
   // runMiddleware: () => unknown,
   fetcherKey: string | null;
