diff --git a/.changeset/purple-poems-laugh.md b/.changeset/purple-poems-laugh.md new file mode 100644 index 0000000000..4ad55ef1e3 --- /dev/null +++ b/.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 diff --git a/integration/defer-loader-test.ts b/integration/defer-loader-test.ts index 534716b389..9fc9beb6f6 100644 --- a/integration/defer-loader-test.ts +++ b/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,7 +84,7 @@ 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); @@ -90,9 +92,7 @@ test.describe("deferred loaders", () => { 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"); diff --git a/packages/react-router/lib/server-runtime/data.ts b/packages/react-router/lib/server-runtime/data.ts index 035c477cf8..97e49f91f9 100644 --- a/packages/react-router/lib/server-runtime/data.ts +++ b/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) { + 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; } diff --git a/packages/react-router/lib/server-runtime/routes.ts b/packages/react-router/lib/server-runtime/routes.ts index 994e3fde04..9ef5fe165a 100644 --- a/packages/react-router/lib/server-runtime/routes.ts +++ b/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, diff --git a/packages/react-router/lib/server-runtime/server.ts b/packages/react-router/lib/server-runtime/server.ts index f9d4ef49cc..b2a9d92d18 100644 --- a/packages/react-router/lib/server-runtime/server.ts +++ b/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, {