Fix RSC client loader support (#13663)

Author: markdalgleish

packages/react-router/lib/dom/ssr/single-fetch.tsx

@@ -166,10 +166,13 @@ type GetRouteInfoFunction = (match: DataRouteMatch) => {
   hasShouldRevalidate: boolean;
 };
 
+type ShouldAllowOptOutFunction = (match: DataRouteMatch) => boolean;
+
 export type FetchAndDecodeFunction = (
   args: DataStrategyFunctionArgs,
   basename: string | undefined,
-  targetRoutes?: string[]
+  targetRoutes?: string[],
+  shouldAllowOptOut?: ShouldAllowOptOutFunction
 ) => Promise<{ status: number; data: DecodedSingleFetchResults }>;
 
 export function getTurboStreamSingleFetchDataStrategy(
@@ -203,7 +206,8 @@ export function getSingleFetchDataStrategyImpl(
   getRouteInfo: GetRouteInfoFunction,
   fetchAndDecode: FetchAndDecodeFunction,
   ssr: boolean,
-  basename: string | undefined
+  basename: string | undefined,
+  shouldAllowOptOut: ShouldAllowOptOutFunction = () => true
 ): DataStrategyFunction {
   return async (args) => {
     let { request, matches, fetcherKey } = args;
@@ -266,7 +270,8 @@ export function getSingleFetchDataStrategyImpl(
       getRouteInfo,
       fetchAndDecode,
       ssr,
-      basename
+      basename,
+      shouldAllowOptOut
     );
   };
 }
@@ -354,7 +359,8 @@ async function singleFetchLoaderNavigationStrategy(
   getRouteInfo: GetRouteInfoFunction,
   fetchAndDecode: FetchAndDecodeFunction,
   ssr: boolean,
-  basename: string | undefined
+  basename: string | undefined,
+  shouldAllowOptOut: (match: DataRouteMatch) => boolean = () => true
 ) {
   // Track which routes need a server load for use in a `_routes` param
   let routesParams = new Set<string>();
@@ -396,7 +402,7 @@ async function singleFetchLoaderNavigationStrategy(
 
         // When a route has a client loader, it opts out of the singular call and
         // calls it's server loader via `serverLoader()` using a `?_routes` param
-        if (hasClientLoader) {
+        if (shouldAllowOptOut(m) && hasClientLoader) {
           if (hasLoader) {
             foundOptOutRoute = true;
           }

packages/react-router/lib/rsc/browser.tsx

@@ -271,7 +271,13 @@ export function getRSCSingleFetchDataStrategy(
     // pass map into fetchAndDecode so it can add payloads
     getFetchAndDecodeViaRSC(decode),
     ssr,
-    basename
+    basename,
+    // If we don't have an element, we need to hit the server loader flow
+    // regardless of whether the client loader calls `serverLoader` or not,
+    // otherwise we'll have nothing to render.
+    // TODO: Do we need to account for API routes? Do we need a
+    // `match.hasComponent` flag?
+    (match) => match.route.element != null
   );
   return async (args) =>
     args.unstable_runClientMiddleware(async () => {
@@ -288,17 +294,27 @@ export function getRSCSingleFetchDataStrategy(
       context.set(renderedRoutesContext, []);
       let results = await dataStrategy(args);
       // patch into router from all payloads in map
-      const renderedRouteById = new Map(
-        context.get(renderedRoutesContext).map((r) => [r.id, r])
-      );
+      // TODO: Confirm that it's correct for us to have multiple rendered routes
+      // with the same ID. This is currently happening in `clientLoader` cases
+      // where we're calling `fetchAndDecode` multiple times. This may be a
+      // sign of a logical error in how we're handling client loader routes.
+      const renderedRoutesById = new Map<string, RenderedRoute[]>();
+      for (const route of context.get(renderedRoutesContext)) {
+        if (!renderedRoutesById.has(route.id)) {
+          renderedRoutesById.set(route.id, []);
+        }
+        renderedRoutesById.get(route.id)!.push(route);
+      }
       for (const match of args.matches) {
-        const rendered = renderedRouteById.get(match.route.id);
-        if (rendered) {
-          window.__router.patchRoutes(
-            rendered.parentId ?? null,
-            [createRouteFromServerManifest(rendered)],
-            true
-          );
+        const renderedRoutes = renderedRoutesById.get(match.route.id);
+        if (renderedRoutes) {
+          for (const rendered of renderedRoutes) {
+            window.__router.patchRoutes(
+              rendered.parentId ?? null,
+              [createRouteFromServerManifest(rendered)],
+              true
+            );
+          }
         }
       }
       return results;
@@ -457,7 +473,14 @@ function createRouteFromServerManifest(
   let hasInitialError = payload?.errors && match.id in payload.errors;
   let initialError = payload?.errors?.[match.id];
   let isHydrationRequest =
-    match.clientLoader?.hydrate === true || !match.hasLoader;
+    match.clientLoader?.hydrate === true ||
+    !match.hasLoader ||
+    // If we don't have an element, we need to hit the server loader flow
+    // regardless of whether the client loader calls `serverLoader` or not,
+    // otherwise we'll have nothing to render.
+    // TODO: Do we need to account for API routes? Do we need a
+    // `match.hasComponent` flag?
+    !match.element;
 
   let dataRoute: DataRouteObjectWithManifestInfo = {
     id: match.id,