Fix redirects returned from loader using data

Author: brophdawg11

.changeset/purple-poems-laugh.md

@@ -0,0 +1,6 @@
+---
+"react-router": patch
+---
+
+- Fix redirects returned from loaders using `data()`
+- Allow returning `undefined` from `loader` and `action` functions

integration/defer-loader-test.ts

@@ -40,7 +40,9 @@ test.describe("deferred loaders", () => {
               }
             );
           }
-          export default function Redirect() {return null;}
+          export default function Redirect() {
+            return null;
+          }
         `,
 
         "app/routes/direct-promise-access.tsx": js`
@@ -82,17 +84,15 @@ test.describe("deferred loaders", () => {
 
   test.afterAll(async () => appFixture.close());
 
-  test.skip("deferred response can redirect on document request", async ({
+  test("deferred response can redirect on document request", async ({
     page,
   }) => {
     let app = new PlaywrightFixture(appFixture, page);
     await app.goto("/redirect");
     await page.waitForURL(/\?redirected/);
   });
 
-  test.skip("deferred response can redirect on transition", async ({
-    page,
-  }) => {
+  test("deferred response can redirect on transition", async ({ page }) => {
     let app = new PlaywrightFixture(appFixture, page);
     await app.goto("/");
     await app.clickLink("/redirect");

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

@@ -1,3 +1,5 @@
+import { isDataWithResponseInit } from "../router/router";
+import { isRedirectStatusCode } from "./responses";
 import type {
   ActionFunction,
   ActionFunctionArgs,
@@ -20,60 +22,37 @@ export interface AppLoadContext {
  */
 export type AppData = unknown;
 
-export async function callRouteAction({
-  loadContext,
-  action,
-  params,
-  request,
-  routeId,
-}: {
-  request: Request;
-  action: ActionFunction;
-  params: ActionFunctionArgs["params"];
-  loadContext: AppLoadContext;
-  routeId: string;
-}) {
-  let result = await action({
-    request: stripRoutesParam(stripIndexParam(request)),
-    context: loadContext,
-    params,
-  });
-
-  if (result === undefined) {
-    throw new Error(
-      `You defined an action for route "${routeId}" but didn't return ` +
-        `anything from your \`action\` function. Please return a value or \`null\`.`
+function checkRedirect(result: ReturnType<LoaderFunction | ActionFunction>) {
+  if (
+    isDataWithResponseInit(result) &&
+    result.init &&
+    isRedirectStatusCode(result.init.status || 200)
+  ) {
+    throw new Response(
+      new Headers(result.init.headers).get("Location")!,
+      result.init
     );
   }
-
-  return result;
 }
 
-export async function callRouteLoader({
+export async function callRouteHandler({
   loadContext,
-  loader,
+  handler,
   params,
   request,
-  routeId,
 }: {
   request: Request;
-  loader: LoaderFunction;
-  params: LoaderFunctionArgs["params"];
+  handler: LoaderFunction | ActionFunction;
+  params: LoaderFunctionArgs["params"] | ActionFunctionArgs["params"];
   loadContext: AppLoadContext;
-  routeId: string;
 }) {
-  let result = await loader({
+  let result = await handler({
     request: stripRoutesParam(stripIndexParam(request)),
     context: loadContext,
     params,
   });
 
-  if (result === undefined) {
-    throw new Error(
-      `You defined a loader for route "${routeId}" but didn't return ` +
-        `anything from your \`loader\` function. Please return a value or \`null\`.`
-    );
-  }
+  checkRedirect(result);
 
   return result;
 }

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

@@ -3,7 +3,7 @@ import type {
   LoaderFunctionArgs as RRLoaderFunctionArgs,
   ActionFunctionArgs as RRActionFunctionArgs,
 } from "../router/utils";
-import { callRouteAction, callRouteLoader } from "./data";
+import { callRouteHandler } from "./data";
 import type { FutureConfig } from "../dom/ssr/entry";
 import type { ServerRouteModule } from "./routeModules";
 
@@ -92,22 +92,20 @@ export function createStaticHandlerDataRoutes(
         ? // Need to use RR's version here to permit the optional context even
           // though we know it'll always be provided in remix
           (args: RRLoaderFunctionArgs, dataStrategyCtx?: unknown) =>
-            callRouteLoader({
+            callRouteHandler({
               request: args.request,
               params: args.params,
               loadContext: args.context,
-              loader: route.module.loader!,
-              routeId: route.id,
+              handler: route.module.loader!,
             })
         : undefined,
       action: route.module.action
         ? (args: RRActionFunctionArgs, dataStrategyCtx?: unknown) =>
-            callRouteAction({
+            callRouteHandler({
               request: args.request,
               params: args.params,
               loadContext: args.context,
-              action: route.module.action!,
-              routeId: route.id,
+              handler: route.module.action!,
             })
         : undefined,
       handle: route.module.handle,

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

@@ -331,6 +331,8 @@ async function handleDocumentRequest(
   handleError: (err: unknown) => void,
   criticalCss?: string
 ) {
+  debugger;
+
   let context;
   try {
     context = await staticHandler.query(request, {