Skip to content

Commit 05da793

Browse files
jacob-ebeyjacob-ebey
authored
fix(rsc): proxy side-effect redirects from actions for document and callServer requests (#14131)
1 parent 3abae20 commit 05da793

File tree

7 files changed

+267
-112
lines changed

7 files changed

+267
-112
lines changed

‎.changeset/wicked-ads-doubt.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"react-router": patch
3+
---
4+
5+
proxy server action side-effect redirects from actions for document and callServer requests

‎integration/rsc/rsc-nojs-test.ts

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
import { test, expect } from "@playwright/test";
2+
import getPort from "get-port";
3+
4+
import { implementations, js, setupRscTest, validateRSCHtml } from "./utils";
5+
6+
test.use({ javaScriptEnabled: false });
7+
8+
implementations.forEach((implementation) => {
9+
let stop: () => void;
10+
11+
test.afterEach(() => {
12+
stop?.();
13+
});
14+
15+
test.describe(`RSC nojs (${implementation.name})`, () => {
16+
test("Supports React Server Functions side-effect redirect headers for document requests", async ({
17+
page,
18+
}, { project }) => {
19+
test.skip(
20+
implementation.name === "parcel" || project.name !== "chromium",
21+
"TODO: figure out why parcel isn't working here",
22+
);
23+
24+
let port = await getPort();
25+
stop = await setupRscTest({
26+
implementation,
27+
port,
28+
files: {
29+
"src/routes/home.actions.ts": js`
30+
"use server";
31+
import { redirect } from "react-router";
32+
33+
export async function redirectAction() {
34+
redirect("/?redirected=true", { headers: { "x-test": "test" } });
35+
return "redirected";
36+
}
37+
`,
38+
"src/routes/home.client.tsx": js`
39+
"use client";
40+
import { useState } from "react";
41+
42+
export function Counter() {
43+
const [count, setCount] = useState(0);
44+
return <button type="button" onClick={() => setCount(c => c + 1)} data-count>Count: {count}</button>;
45+
}
46+
`,
47+
"src/routes/home.tsx": js`
48+
"use client";
49+
import {useActionState} from "react";
50+
import { redirectAction } from "./home.actions";
51+
import { Counter } from "./home.client";
52+
53+
export default function HomeRoute(props) {
54+
const [state, action] = useActionState(redirectAction, null);
55+
return (
56+
<div>
57+
<form action={action}>
58+
<button type="submit" data-submit>
59+
Redirect via Server Function
60+
</button>
61+
</form>
62+
{state && <div data-testid="state">{state}</div>}
63+
<Counter />
64+
</div>
65+
);
66+
}
67+
`,
68+
},
69+
});
70+
71+
await page.goto(`http://localhost:${port}/`);
72+
73+
const responseHeadersPromise = new Promise<Record<string, string>>(
74+
(resolve) => {
75+
page.addListener("response", (response) => {
76+
if (response.request().method() === "POST") {
77+
resolve(response.headers());
78+
}
79+
});
80+
},
81+
);
82+
83+
await page.click("[data-submit]");
84+
85+
await page.waitForURL(`http://localhost:${port}/?redirected=true`);
86+
87+
expect((await responseHeadersPromise)["x-test"]).toBe("test");
88+
89+
// Ensure this is using RSC
90+
validateRSCHtml(await page.content());
91+
});
92+
});
93+
});

‎integration/rsc/rsc-test.ts

Lines changed: 14 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -1,100 +1,7 @@
11
import { test, expect } from "@playwright/test";
2-
import { sync as spawnSync } from "cross-spawn";
32
import getPort from "get-port";
43

5-
import {
6-
type TemplateName,
7-
createDev,
8-
createProject,
9-
} from "../helpers/vite.js";
10-
11-
const js = String.raw;
12-
13-
type Implementation = {
14-
name: string;
15-
template: TemplateName;
16-
/** Build a production app */
17-
build: ({ cwd }: { cwd: string }) => ReturnType<typeof spawnSync>;
18-
/** Run a production app */
19-
run: ({ cwd, port }: { cwd: string; port: number }) => Promise<() => void>;
20-
/** Run the dev server */
21-
dev: ({ cwd, port }: { cwd: string; port: number }) => Promise<() => void>;
22-
};
23-
24-
// Run tests against vite and parcel to ensure our code is bundler agnostic
25-
const implementations: Implementation[] = [
26-
{
27-
name: "vite",
28-
template: "rsc-vite",
29-
build: ({ cwd }: { cwd: string }) => spawnSync("pnpm", ["build"], { cwd }),
30-
run: ({ cwd, port }) =>
31-
createDev(["server.js", "-p", String(port)])({
32-
cwd,
33-
port,
34-
env: {
35-
NODE_ENV: "production",
36-
},
37-
}),
38-
dev: ({ cwd, port }) =>
39-
createDev(["node_modules/vite/bin/vite.js", "--port", String(port)])({
40-
cwd,
41-
port,
42-
}),
43-
},
44-
{
45-
name: "parcel",
46-
template: "rsc-parcel",
47-
build: ({ cwd }: { cwd: string }) => spawnSync("pnpm", ["build"], { cwd }),
48-
run: ({ cwd, port }) =>
49-
createDev(["dist/server/server.js", "-p", String(port)])({
50-
cwd,
51-
port,
52-
env: {
53-
NODE_ENV: "production",
54-
},
55-
}),
56-
dev: ({ cwd, port }) =>
57-
createDev(["node_modules/parcel/lib/bin.js"])({
58-
// Since we run through parcels dev server we can't use `-p` because that
59-
// only changes the dev server and doesn't pass through to the internal
60-
// server. So we setup the internal server to choose from `RR_PORT`
61-
env: { RR_PORT: String(port) },
62-
cwd,
63-
port,
64-
}),
65-
},
66-
] as Implementation[];
67-
68-
async function setupRscTest({
69-
implementation,
70-
port,
71-
dev,
72-
files,
73-
}: {
74-
implementation: Implementation;
75-
port: number;
76-
dev?: boolean;
77-
files: Record<string, string>;
78-
}) {
79-
let cwd = await createProject(files, implementation.template);
80-
81-
let { error, status, stderr, stdout } = implementation.build({ cwd });
82-
if (status !== 0) {
83-
console.error("Error building project", {
84-
status,
85-
error,
86-
stdout: stdout?.toString(),
87-
stderr: stderr?.toString(),
88-
});
89-
throw new Error("Error building project");
90-
}
91-
return dev
92-
? implementation.dev({ cwd, port })
93-
: implementation.run({ cwd, port });
94-
}
95-
96-
const validateRSCHtml = (html: string) =>
97-
expect(html).toMatch(/\(self\.__FLIGHT_DATA\|\|=\[\]\)\.push\(/);
4+
import { implementations, js, setupRscTest, validateRSCHtml } from "./utils";
985

996
implementations.forEach((implementation) => {
1007
let stop: () => void;
@@ -1268,7 +1175,7 @@ implementations.forEach((implementation) => {
12681175
import { redirect } from "react-router";
12691176
12701177
export async function redirectAction() {
1271-
redirect("/?redirected=true");
1178+
redirect("/?redirected=true", { headers: { "x-test": "test" } });
12721179
return "redirected";
12731180
}
12741181
`,
@@ -1317,13 +1224,25 @@ implementations.forEach((implementation) => {
13171224
"Count: 1",
13181225
);
13191226

1227+
const responseHeadersPromise = new Promise<Record<string, string>>(
1228+
(resolve) => {
1229+
page.addListener("response", (response) => {
1230+
if (response.request().method() === "POST") {
1231+
resolve(response.headers());
1232+
}
1233+
});
1234+
},
1235+
);
1236+
13201237
// Submit the form to trigger server function redirect
13211238
await page.click("[data-submit]");
13221239

13231240
await expect(page.getByTestId("state")).toHaveText("redirected");
13241241

13251242
await page.waitForURL(`http://localhost:${port}/?redirected=true`);
13261243

1244+
expect((await responseHeadersPromise)["x-test"]).toBe("test");
1245+
13271246
expect(await page.locator("[data-count]").textContent()).toBe(
13281247
"Count: 1",
13291248
);

‎integration/rsc/utils.ts

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
import { expect } from "@playwright/test";
2+
import { sync as spawnSync } from "cross-spawn";
3+
4+
import {
5+
type TemplateName,
6+
createDev,
7+
createProject,
8+
} from "../helpers/vite.js";
9+
10+
export const js = String.raw;
11+
12+
export type Implementation = {
13+
name: string;
14+
template: TemplateName;
15+
/** Build a production app */
16+
build: ({ cwd }: { cwd: string }) => ReturnType<typeof spawnSync>;
17+
/** Run a production app */
18+
run: ({ cwd, port }: { cwd: string; port: number }) => Promise<() => void>;
19+
/** Run the dev server */
20+
dev: ({ cwd, port }: { cwd: string; port: number }) => Promise<() => void>;
21+
};
22+
23+
// Run tests against vite and parcel to ensure our code is bundler agnostic
24+
export const implementations: Implementation[] = [
25+
{
26+
name: "vite",
27+
template: "rsc-vite",
28+
build: ({ cwd }: { cwd: string }) => spawnSync("pnpm", ["build"], { cwd }),
29+
run: ({ cwd, port }) =>
30+
createDev(["server.js", "-p", String(port)])({
31+
cwd,
32+
port,
33+
env: {
34+
NODE_ENV: "production",
35+
},
36+
}),
37+
dev: ({ cwd, port }) =>
38+
createDev(["node_modules/vite/bin/vite.js", "--port", String(port)])({
39+
cwd,
40+
port,
41+
}),
42+
},
43+
{
44+
name: "parcel",
45+
template: "rsc-parcel",
46+
build: ({ cwd }: { cwd: string }) => spawnSync("pnpm", ["build"], { cwd }),
47+
run: ({ cwd, port }) =>
48+
createDev(["dist/server/server.js", "-p", String(port)])({
49+
cwd,
50+
port,
51+
env: {
52+
NODE_ENV: "production",
53+
},
54+
}),
55+
dev: ({ cwd, port }) =>
56+
createDev(["node_modules/parcel/lib/bin.js"])({
57+
// Since we run through parcels dev server we can't use `-p` because that
58+
// only changes the dev server and doesn't pass through to the internal
59+
// server. So we setup the internal server to choose from `RR_PORT`
60+
env: { RR_PORT: String(port) },
61+
cwd,
62+
port,
63+
}),
64+
},
65+
] as Implementation[];
66+
67+
export async function setupRscTest({
68+
implementation,
69+
port,
70+
dev,
71+
files,
72+
}: {
73+
implementation: Implementation;
74+
port: number;
75+
dev?: boolean;
76+
files: Record<string, string>;
77+
}) {
78+
let cwd = await createProject(files, implementation.template);
79+
80+
let { error, status, stderr, stdout } = implementation.build({ cwd });
81+
if (status !== 0) {
82+
console.error("Error building project", {
83+
status,
84+
error,
85+
stdout: stdout?.toString(),
86+
stderr: stderr?.toString(),
87+
});
88+
throw new Error("Error building project");
89+
}
90+
return dev
91+
? implementation.dev({ cwd, port })
92+
: implementation.run({ cwd, port });
93+
}
94+
95+
export const validateRSCHtml = (html: string) =>
96+
expect(html).toMatch(/\(self\.__FLIGHT_DATA\|\|=\[\]\)\.push\(/);

0 commit comments

Comments
 (0)