Refactor dataStrategy for easier RSC abstraction

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

@@ -18,9 +18,8 @@ import {
 import { createRequestInit } from "./data";
 import type { AssetsManifest, EntryContext } from "./entry";
 import { escapeHtml } from "./markup";
-import type { RouteModule, RouteModules } from "./routeModules";
+import type { RouteModules } from "./routeModules";
 import invariant from "./invariant";
-import type { EntryRoute } from "./routes";
 
 export const SingleFetchRedirectSymbol = Symbol("SingleFetchRedirect");
 
@@ -32,15 +31,22 @@ export type SingleFetchRedirectResult = {
   replace: boolean;
 };
 
+// Shared/serializable type used by both turbo-stream and RSC implementations
+type DecodedSingleFetchResults =
+  | { routes: { [key: string]: SingleFetchResult } }
+  | { redirect: SingleFetchRedirectResult };
+
+// This and SingleFetchResults are only used over the wire, and are converted to
+// AgnosticSingleFetchResults in `fethAndDecode`.  This way turbo-stream/RSC
+// can use the same `unwrapSingleFetchResult` implementation
 export type SingleFetchResult =
   | { data: unknown }
   | { error: unknown }
   | SingleFetchRedirectResult;
 
-export type SingleFetchResults = {
-  [key: string]: SingleFetchResult;
-  [SingleFetchRedirectSymbol]?: SingleFetchRedirectResult;
-};
+export type SingleFetchResults =
+  | { [key: string]: SingleFetchResult }
+  | { [SingleFetchRedirectSymbol]: SingleFetchRedirectResult };
 
 interface StreamTransferProps {
   context: EntryContext;
@@ -50,6 +56,12 @@ interface StreamTransferProps {
   nonce?: string;
 }
 
+// some status codes are not permitted to have bodies, so we want to just
+// treat those as "no data" instead of throwing an exception.
+// 304 is not included here because the browser should fill those responses
+// with the cached body content.
+export const NO_BODY_STATUS_CODES = new Set([100, 101, 204, 205]);
+
 // StreamTransfer recursively renders down chunks of the `serverHandoffStream`
 // into the client-side `streamController`
 export function StreamTransfer({
@@ -250,12 +262,13 @@ async function singleFetchActionStrategy(
     let result = await handler(async () => {
       let url = singleFetchUrl(request.url, basename);
       let init = await createRequestInit(request);
-      let { data, status } = await fetchAndDecode(url, init);
-      actionStatus = status;
-      return unwrapSingleFetchResult(
-        data as SingleFetchResult,
+      let { data, status } = await fetchAndDecode(
+        url,
+        init,
         actionMatch!.route.id
       );
+      actionStatus = status;
+      return unwrapSingleFetchResult(data, actionMatch!.route.id);
     });
     return result;
   });
@@ -316,23 +329,17 @@ async function singleFetchLoaderNavigationStrategy(
   matches: DataStrategyFunctionArgs["matches"],
   basename: string | undefined
 ) {
-  // Track which routes need a server load - in case we need to tack on a
-  // `_routes` param
+  // Track which routes need a server load for use in a `_routes` param
   let routesParams = new Set<string>();
 
-  // We only add `_routes` when one or more routes opts out of a load via
-  // `shouldRevalidate` or `clientLoader`
+  // Only add `_routes` when at least 1 route opts out via `shouldRevalidate`/`clientLoader`
   let foundOptOutRoute = false;
 
-  // Deferreds for each route so we can be sure they've all loaded via
-  // `match.resolve()`, and a singular promise that can tell us all routes
-  // have been resolved
+  // Deferreds per-route so we can be sure they've all loaded via `match.resolve()`
   let routeDfds = matches.map(() => createDeferred<void>());
-  let routesLoadedPromise = Promise.all(routeDfds.map((d) => d.promise));
 
-  // Deferred that we'll use for the call to the server that each match can
-  // await and parse out it's specific result
-  let singleFetchDfd = createDeferred<SingleFetchResults>();
+  // Deferred we'll use for the singleular call to the server
+  let singleFetchDfd = createDeferred<DecodedSingleFetchResults>();
 
   // Base URL and RequestInit for calls to the server
   let url = stripIndexParam(singleFetchUrl(request.url, basename));
@@ -347,10 +354,8 @@ async function singleFetchLoaderNavigationStrategy(
         routeDfds[i].resolve();
 
         let manifestRoute = manifest.routes[m.route.id];
+        invariant(manifestRoute, "No manifest route found for dataStrategy");
 
-        // Note: If this logic changes for routes that should not participate
-        // in Single Fetch, make sure you update getLowestLoadingIndex above
-        // as well
         if (!m.shouldLoad) {
           // If we're not yet initialized and this is the initial load, respect
           // `shouldLoad` because we're only dealing with `clientLoader.hydrate`
@@ -364,7 +369,6 @@ async function singleFetchLoaderNavigationStrategy(
           // via `shouldRevalidate`
           if (
             m.route.id in router.state.loaderData &&
-            manifestRoute &&
             m.route.shouldRevalidate
           ) {
             if (manifestRoute.hasLoader) {
@@ -378,7 +382,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 (manifestRoute && manifestRoute.hasClientLoader) {
+        if (manifestRoute.hasClientLoader) {
           if (manifestRoute.hasLoader) {
             foundOptOutRoute = true;
           }
@@ -405,7 +409,7 @@ async function singleFetchLoaderNavigationStrategy(
         try {
           let result = await handler(async () => {
             let data = await singleFetchDfd.promise;
-            return unwrapSingleFetchResults(data, m.route.id);
+            return unwrapSingleFetchResult(data, m.route.id);
           });
           results[m.route.id] = {
             type: "data",
@@ -422,7 +426,7 @@ async function singleFetchLoaderNavigationStrategy(
   );
 
   // Wait for all routes to resolve above before we make the HTTP call
-  await routesLoadedPromise;
+  await Promise.all(routeDfds.map((d) => d.promise));
 
   // We can skip the server call:
   // - On initial hydration - only clientLoaders can pass through via `clientLoader.hydrate`
@@ -437,24 +441,18 @@ async function singleFetchLoaderNavigationStrategy(
   ) {
     singleFetchDfd.resolve({});
   } else {
-    try {
-      // When one or more routes have opted out, we add a _routes param to
-      // limit the loaders to those that have a server loader and did not
-      // opt out
-      if (ssr && foundOptOutRoute && routesParams.size > 0) {
-        url.searchParams.set(
-          "_routes",
-          matches
-            .filter((m) => routesParams.has(m.route.id))
-            .map((m) => m.route.id)
-            .join(",")
-        );
-      }
+    // When routes have opted out, add a `_routes` param to filter server loaders
+    // Skipped in `ssr:false` because we expect to be loading static `.data` files
+    if (ssr && foundOptOutRoute && routesParams.size > 0) {
+      let routes = [...routesParams.keys()].join(",");
+      url.searchParams.set("_routes", routes);
+    }
 
+    try {
       let data = await fetchAndDecode(url, init);
-      singleFetchDfd.resolve(data.data as SingleFetchResults);
+      singleFetchDfd.resolve(data.data);
     } catch (e) {
-      singleFetchDfd.reject(e as Error);
+      singleFetchDfd.reject(e);
     }
   }
 
@@ -491,7 +489,7 @@ function fetchSingleLoader(
     let singleLoaderUrl = new URL(url);
     singleLoaderUrl.searchParams.set("_routes", routeId);
     let { data } = await fetchAndDecode(singleLoaderUrl, init);
-    return unwrapSingleFetchResults(data as SingleFetchResults, routeId);
+    return unwrapSingleFetchResult(data, routeId);
   });
 }
 
@@ -540,8 +538,9 @@ export function singleFetchUrl(
 
 async function fetchAndDecode(
   url: URL,
-  init: RequestInit
-): Promise<{ status: number; data: unknown }> {
+  init: RequestInit,
+  routeId?: string
+): Promise<{ status: number; data: DecodedSingleFetchResults }> {
   let res = await fetch(url, init);
 
   // If this 404'd without hitting the running server (most likely in a
@@ -550,27 +549,39 @@ async function fetchAndDecode(
     throw new ErrorResponseImpl(404, "Not Found", true);
   }
 
-  // some status codes are not permitted to have bodies, so we want to just
-  // treat those as "no data" instead of throwing an exception.
-  // 304 is not included here because the browser should fill those responses
-  // with the cached body content.
-  const NO_BODY_STATUS_CODES = new Set([100, 101, 204, 205]);
   if (NO_BODY_STATUS_CODES.has(res.status)) {
-    if (!init.method || init.method === "GET") {
-      // SingleFetchResults can just have no routeId keys which will result
-      // in no data for all routes
-      return { status: res.status, data: {} };
-    } else {
-      // SingleFetchResult is for a singular route and can specify no data
-      return { status: res.status, data: { data: undefined } };
+    let routes: { [key: string]: SingleFetchResult } = {};
+    if (routeId) {
+      routes[routeId] = { data: undefined };
     }
+    return {
+      status: res.status,
+      data: { routes },
+    };
   }
 
   invariant(res.body, "No response body to decode");
 
   try {
     let decoded = await decodeViaTurboStream(res.body, window);
-    return { status: res.status, data: decoded.value };
+    let data: DecodedSingleFetchResults;
+    if (!init.method || init.method === "GET") {
+      let typed = decoded.value as SingleFetchResults;
+      if (SingleFetchRedirectSymbol in typed) {
+        data = { redirect: typed[SingleFetchRedirectSymbol] };
+      } else {
+        data = { routes: typed };
+      }
+    } else {
+      let typed = decoded.value as SingleFetchResult;
+      invariant(routeId, "No routeId found for single fetch call decoding");
+      if ("redirect" in typed) {
+        data = { redirect: typed };
+      } else {
+        data = { routes: { [routeId]: typed } };
+      }
+    }
+    return { status: res.status, data };
   } catch (e) {
     // Can't clone after consuming the body via turbo-stream so we can't
     // include the body here.  In an ideal world we'd look for a turbo-stream
@@ -637,53 +648,50 @@ export function decodeViaTurboStream(
   });
 }
 
-function unwrapSingleFetchResults(
-  results: SingleFetchResults,
+function unwrapSingleFetchResult(
+  result: DecodedSingleFetchResults,
   routeId: string
 ) {
-  let redirect = results[SingleFetchRedirectSymbol];
-  if (redirect) {
-    return unwrapSingleFetchResult(redirect, routeId);
+  if ("redirect" in result) {
+    let {
+      redirect: location,
+      revalidate,
+      reload,
+      replace,
+      status,
+    } = result.redirect;
+    throw redirect(location, {
+      status,
+      headers: {
+        // Three R's of redirecting (lol Veep)
+        ...(revalidate ? { "X-Remix-Revalidate": "yes" } : null),
+        ...(reload ? { "X-Remix-Reload-Document": "yes" } : null),
+        ...(replace ? { "X-Remix-Replace": "yes" } : null),
+      },
+    });
   }
 
-  return results[routeId] !== undefined
-    ? unwrapSingleFetchResult(results[routeId], routeId)
-    : null;
-}
-
-function unwrapSingleFetchResult(result: SingleFetchResult, routeId: string) {
-  if ("error" in result) {
-    throw result.error;
-  } else if ("redirect" in result) {
-    let headers: Record<string, string> = {};
-    if (result.revalidate) {
-      headers["X-Remix-Revalidate"] = "yes";
-    }
-    if (result.reload) {
-      headers["X-Remix-Reload-Document"] = "yes";
-    }
-    if (result.replace) {
-      headers["X-Remix-Replace"] = "yes";
-    }
-    throw redirect(result.redirect, { status: result.status, headers });
-  } else if ("data" in result) {
-    return result.data;
+  let routeResult = result.routes[routeId];
+  if ("error" in routeResult) {
+    throw routeResult.error;
+  } else if ("data" in routeResult) {
+    return routeResult.data;
   } else {
     throw new Error(`No response found for routeId "${routeId}"`);
   }
 }
 
 function createDeferred<T = unknown>() {
   let resolve: (val?: any) => Promise<void>;
-  let reject: (error?: Error) => Promise<void>;
+  let reject: (error?: unknown) => Promise<void>;
   let promise = new Promise<T>((res, rej) => {
     resolve = async (val: T) => {
       res(val);
       try {
         await promise;
       } catch (e) {}
     };
-    reject = async (error?: Error) => {
+    reject = async (error?: unknown) => {
       rej(error);
       try {
         await promise;

packages/react-router/lib/server-runtime/routes.ts

@@ -12,7 +12,10 @@ import type {
   SingleFetchResult,
   SingleFetchResults,
 } from "../dom/ssr/single-fetch";
-import { decodeViaTurboStream } from "../dom/ssr/single-fetch";
+import {
+  SingleFetchRedirectSymbol,
+  decodeViaTurboStream,
+} from "../dom/ssr/single-fetch";
 import invariant from "./invariant";
 import type { ServerRouteModule } from "../dom/ssr/routeModules";
 
@@ -100,7 +103,9 @@ export function createStaticHandlerDataRoutes(
               let decoded = await decodeViaTurboStream(stream, global);
               let data = decoded.value as SingleFetchResults;
               invariant(
-                data && route.id in data,
+                data &&
+                  !(SingleFetchRedirectSymbol in data) &&
+                  route.id in data,
                 "Unable to decode prerendered data"
               );
               let result = data[route.id] as SingleFetchResult;

packages/react-router/lib/server-runtime/server.ts

@@ -24,18 +24,23 @@ import type { ServerRoute } from "./routes";
 import { createStaticHandlerDataRoutes, createRoutes } from "./routes";
 import { createServerHandoffString } from "./serverHandoff";
 import { getDevServerHooks } from "./dev";
-import type { SingleFetchResult, SingleFetchResults } from "./single-fetch";
 import {
   encodeViaTurboStream,
   getSingleFetchRedirect,
   singleFetchAction,
   singleFetchLoaders,
-  SingleFetchRedirectSymbol,
   SINGLE_FETCH_REDIRECT_STATUS,
-  NO_BODY_STATUS_CODES,
 } from "./single-fetch";
 import { getDocumentHeaders } from "./headers";
 import type { EntryRoute } from "../dom/ssr/routes";
+import type {
+  SingleFetchResult,
+  SingleFetchResults,
+} from "../dom/ssr/single-fetch";
+import {
+  NO_BODY_STATUS_CODES,
+  SingleFetchRedirectSymbol,
+} from "../dom/ssr/single-fetch";
 import type { MiddlewareEnabled } from "../types/future";
 
 export type RequestHandler = (