Skip to content

Commit

Permalink
Fix redirects returned from loader using data
Browse files Browse the repository at this point in the history
  • Loading branch information
brophdawg11 committed Sep 20, 2024
1 parent d1bb894 commit fbe3516
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 50 deletions.
6 changes: 6 additions & 0 deletions .changeset/purple-poems-laugh.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"react-router": patch
---

- Fix redirects returned from loaders using `data()`
- Allow returning `undefined` from `loader` and `action` functions
10 changes: 5 additions & 5 deletions integration/defer-loader-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -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");
Expand Down
55 changes: 17 additions & 38 deletions packages/react-router/lib/server-runtime/data.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { isDataWithResponseInit } from "../router/router";
import { isRedirectStatusCode } from "./responses";
import type {
ActionFunction,
ActionFunctionArgs,
Expand All @@ -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;
}
Expand Down
12 changes: 5 additions & 7 deletions packages/react-router/lib/server-runtime/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions packages/react-router/lib/server-runtime/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,8 @@ async function handleDocumentRequest(
handleError: (err: unknown) => void,
criticalCss?: string
) {
debugger;

let context;
try {
context = await staticHandler.query(request, {
Expand Down

0 comments on commit fbe3516

Please sign in to comment.