Skip to content

Commit 7f140e0

Browse files
richardkallbrophdawg11
richardkall
and
brophdawg11
authored
Handle data requests with trailing slash consistently (#14644)
Co-authored-by: Matt Brophy <matt@brophy.org>
1 parent 1954af6 commit 7f140e0

File tree

16 files changed

+368
-38
lines changed

16 files changed

+368
-38
lines changed

‎.changeset/serious-bobcats-impress.md‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
---
2+
"@react-router/dev": patch
3+
"react-router": patch
4+
---
5+
6+
[UNSTABLE] Add a new `future.unstable_trailingSlashAwareDataRequests` flag to provide consistent behavior of `request.pathname` inside `middleware`, `loader`, and `action` functions on document and data requests when a trailing slash is present in the browser URL.
7+
8+
Currently, your HTTP and `request` pathnames would be as follows for `/a/b/c` and `/a/b/c/`
9+
10+
| URL `/a/b/c` | **HTTP pathname** | **`request` pathname`** |
11+
| ------------ | ----------------- | ----------------------- |
12+
| **Document** | `/a/b/c` | `/a/b/c` ✅ |
13+
| **Data** | `/a/b/c.data` | `/a/b/c` ✅ |
14+
15+
| URL `/a/b/c/` | **HTTP pathname** | **`request` pathname`** |
16+
| ------------- | ----------------- | ----------------------- |
17+
| **Document** | `/a/b/c/` | `/a/b/c/` ✅ |
18+
| **Data** | `/a/b/c.data` | `/a/b/c` ⚠️ |
19+
20+
With this flag enabled, these pathnames will be made consistent though a new `_.data` format for client-side `.data` requests:
21+
22+
| URL `/a/b/c` | **HTTP pathname** | **`request` pathname`** |
23+
| ------------ | ----------------- | ----------------------- |
24+
| **Document** | `/a/b/c` | `/a/b/c` ✅ |
25+
| **Data** | `/a/b/c.data` | `/a/b/c` ✅ |
26+
27+
| URL `/a/b/c/` | **HTTP pathname** | **`request` pathname`** |
28+
| ------------- | ------------------ | ----------------------- |
29+
| **Document** | `/a/b/c/` | `/a/b/c/` ✅ |
30+
| **Data** | `/a/b/c/_.data` ⬅️ | `/a/b/c/` ✅ |
31+
32+
This a bug fix but we are putting it behind an opt-in flag because it has the potential to be a "breaking bug fix" if you are relying on the URL format for any other application or caching logic.
33+
34+
Enabling this flag also changes the format of client side `.data` requests from `/_root.data` to `/_.data` when navigating to `/` to align with the new format. This does not impact the `request` pathname which is still `/` in all cases.

‎contributors.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,7 @@
340340
- renyu-io
341341
- reyronald
342342
- RFCreate
343+
- richardkall
343344
- richardscarrott
344345
- rifaidev
345346
- rimian

‎integration/single-fetch-test.ts‎

Lines changed: 193 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4476,4 +4476,197 @@ test.describe("single-fetch", () => {
44764476
await page.waitForSelector("h1");
44774477
expect(await app.getHtml("h1")).toMatch("It worked!");
44784478
});
4479+
4480+
test("always uses /{path}.data without future.unstable_trailingSlashAwareDataRequests flag", async ({
4481+
page,
4482+
}) => {
4483+
let fixture = await createFixture({
4484+
files: {
4485+
...files,
4486+
"app/routes/_index.tsx": js`
4487+
import { Link } from "react-router";
4488+
4489+
export default function Index() {
4490+
return (
4491+
<div>
4492+
<h1>Home</h1>
4493+
<Link to="/about/">Go to About (with trailing slash)</Link>
4494+
<Link to="/about">Go to About (without trailing slash)</Link>
4495+
</div>
4496+
);
4497+
}
4498+
`,
4499+
"app/routes/about.tsx": js`
4500+
import { Link, useLoaderData } from "react-router";
4501+
4502+
export function loader({ request }) {
4503+
let url = new URL(request.url);
4504+
return {
4505+
pathname: url.pathname,
4506+
hasTrailingSlash: url.pathname.endsWith("/"),
4507+
};
4508+
}
4509+
4510+
export default function About() {
4511+
let { pathname, hasTrailingSlash } = useLoaderData();
4512+
return (
4513+
<div>
4514+
<h1>About</h1>
4515+
<p id="pathname">{pathname}</p>
4516+
<p id="trailing-slash">{String(hasTrailingSlash)}</p>
4517+
<Link to="/">Go back home</Link>
4518+
</div>
4519+
);
4520+
}
4521+
`,
4522+
},
4523+
});
4524+
let appFixture = await createAppFixture(fixture);
4525+
let app = new PlaywrightFixture(appFixture, page);
4526+
4527+
let requests: string[] = [];
4528+
page.on("request", (req) => {
4529+
let url = new URL(req.url());
4530+
if (url.pathname.endsWith(".data")) {
4531+
requests.push(url.pathname + url.search);
4532+
}
4533+
});
4534+
4535+
// Document load without trailing slash
4536+
await app.goto("/about");
4537+
await page.waitForSelector("#pathname");
4538+
expect(await page.locator("#pathname").innerText()).toEqual("/about");
4539+
expect(await page.locator("#trailing-slash").innerText()).toEqual("false");
4540+
4541+
// Client-side navigation without trailing slash
4542+
await app.goto("/");
4543+
await app.clickLink("/about");
4544+
await page.waitForSelector("#pathname");
4545+
expect(await page.locator("#pathname").innerText()).toEqual("/about");
4546+
expect(await page.locator("#trailing-slash").innerText()).toEqual("false");
4547+
expect(requests).toEqual(["/about.data"]);
4548+
requests = [];
4549+
4550+
// Document load with trailing slash
4551+
await app.goto("/about/");
4552+
await page.waitForSelector("#pathname");
4553+
expect(await page.locator("#pathname").innerText()).toEqual("/about/");
4554+
expect(await page.locator("#trailing-slash").innerText()).toEqual("true");
4555+
4556+
// Client-side navigation with trailing slash
4557+
await app.goto("/");
4558+
await app.clickLink("/about/");
4559+
await page.waitForSelector("#pathname");
4560+
expect(await page.locator("#pathname").innerText()).toEqual("/about");
4561+
expect(await page.locator("#trailing-slash").innerText()).toEqual("false");
4562+
expect(requests).toEqual(["/about.data"]);
4563+
requests = [];
4564+
4565+
// Client-side navigation back to /
4566+
await app.clickLink("/");
4567+
await page.waitForSelector("h1:has-text('Home')");
4568+
expect(requests).toEqual(["/_root.data"]);
4569+
requests = [];
4570+
});
4571+
4572+
test("uses {path}.data or {path}/_.data depending on trailing slash with future.unstable_trailingSlashAwareDataRequests flag", async ({
4573+
page,
4574+
}) => {
4575+
let fixture = await createFixture({
4576+
files: {
4577+
...files,
4578+
"react-router.config.ts": js`
4579+
import type { Config } from "@react-router/dev/config";
4580+
4581+
export default {
4582+
future: {
4583+
unstable_trailingSlashAwareDataRequests: true,
4584+
}
4585+
} satisfies Config;
4586+
`,
4587+
"app/routes/_index.tsx": js`
4588+
import { Link } from "react-router";
4589+
4590+
export default function Index() {
4591+
return (
4592+
<div>
4593+
<h1>Home</h1>
4594+
<Link to="/about/">Go to About (with trailing slash)</Link>
4595+
<Link to="/about">Go to About (without trailing slash)</Link>
4596+
</div>
4597+
);
4598+
}
4599+
`,
4600+
"app/routes/about.tsx": js`
4601+
import { Link, useLoaderData } from "react-router";
4602+
4603+
export function loader({ request }) {
4604+
let url = new URL(request.url);
4605+
return {
4606+
pathname: url.pathname,
4607+
hasTrailingSlash: url.pathname.endsWith("/"),
4608+
};
4609+
}
4610+
4611+
export default function About() {
4612+
let { pathname, hasTrailingSlash } = useLoaderData();
4613+
return (
4614+
<div>
4615+
<h1>About</h1>
4616+
<p id="pathname">{pathname}</p>
4617+
<p id="trailing-slash">{String(hasTrailingSlash)}</p>
4618+
<Link to="/">Go back home</Link>
4619+
</div>
4620+
);
4621+
}
4622+
`,
4623+
},
4624+
});
4625+
let appFixture = await createAppFixture(fixture);
4626+
let app = new PlaywrightFixture(appFixture, page);
4627+
4628+
let requests: string[] = [];
4629+
page.on("request", (req) => {
4630+
let url = new URL(req.url());
4631+
if (url.pathname.endsWith(".data")) {
4632+
requests.push(url.pathname + url.search);
4633+
}
4634+
});
4635+
4636+
// Document load without trailing slash
4637+
await app.goto("/about");
4638+
await page.waitForSelector("#pathname");
4639+
expect(await page.locator("#pathname").innerText()).toEqual("/about");
4640+
expect(await page.locator("#trailing-slash").innerText()).toEqual("false");
4641+
4642+
// Client-side navigation without trailing slash
4643+
await app.goto("/");
4644+
await app.clickLink("/about");
4645+
await page.waitForSelector("#pathname");
4646+
expect(await page.locator("#pathname").innerText()).toEqual("/about");
4647+
expect(await page.locator("#trailing-slash").innerText()).toEqual("false");
4648+
expect(requests).toEqual(["/about.data"]);
4649+
requests = [];
4650+
4651+
// Document load with trailing slash
4652+
await app.goto("/about/");
4653+
await page.waitForSelector("#pathname");
4654+
expect(await page.locator("#pathname").innerText()).toEqual("/about/");
4655+
expect(await page.locator("#trailing-slash").innerText()).toEqual("true");
4656+
4657+
// Client-side navigation with trailing slash
4658+
await app.goto("/");
4659+
await app.clickLink("/about/");
4660+
await page.waitForSelector("#pathname");
4661+
expect(await page.locator("#pathname").innerText()).toEqual("/about/");
4662+
expect(await page.locator("#trailing-slash").innerText()).toEqual("true");
4663+
expect(requests).toEqual(["/about/_.data"]);
4664+
requests = [];
4665+
4666+
// Client-side navigation back to /
4667+
await app.clickLink("/");
4668+
await page.waitForSelector("h1:has-text('Home')");
4669+
expect(requests).toEqual(["/_.data"]);
4670+
requests = [];
4671+
});
44794672
});

‎integration/vite-presets-test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,7 @@ test.describe("Vite / presets", async () => {
245245
expect(buildEndArgsMeta.futureFlags).toEqual({
246246
unstable_optimizeDeps: true,
247247
unstable_subResourceIntegrity: false,
248+
unstable_trailingSlashAwareDataRequests: false,
248249
v8_middleware: true,
249250
v8_splitRouteModules: false,
250251
v8_viteEnvironmentApi: false,

‎packages/react-router-dev/config/config.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ type ValidateConfigFunction = (config: ReactRouterConfig) => string | void;
8787
interface FutureConfig {
8888
unstable_optimizeDeps: boolean;
8989
unstable_subResourceIntegrity: boolean;
90+
unstable_trailingSlashAwareDataRequests: boolean;
9091
/**
9192
* Enable route middleware
9293
*/
@@ -634,6 +635,9 @@ async function resolveConfig({
634635
userAndPresetConfigs.future?.unstable_optimizeDeps ?? false,
635636
unstable_subResourceIntegrity:
636637
userAndPresetConfigs.future?.unstable_subResourceIntegrity ?? false,
638+
unstable_trailingSlashAwareDataRequests:
639+
userAndPresetConfigs.future?.unstable_trailingSlashAwareDataRequests ??
640+
false,
637641
v8_middleware: userAndPresetConfigs.future?.v8_middleware ?? false,
638642
v8_splitRouteModules:
639643
userAndPresetConfigs.future?.v8_splitRouteModules ?? false,

‎packages/react-router-dev/vite/plugin.ts‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2888,11 +2888,22 @@ async function prerenderData(
28882888
viteConfig: Vite.ResolvedConfig,
28892889
requestInit?: RequestInit,
28902890
) {
2891-
let normalizedPath = `${reactRouterConfig.basename}${
2892-
prerenderPath === "/"
2893-
? "/_root.data"
2894-
: `${prerenderPath.replace(/\/$/, "")}.data`
2895-
}`.replace(/\/\/+/g, "/");
2891+
let dataRequestPath: string;
2892+
if (reactRouterConfig.future.unstable_trailingSlashAwareDataRequests) {
2893+
if (prerenderPath.endsWith("/")) {
2894+
dataRequestPath = `${prerenderPath}_.data`;
2895+
} else {
2896+
dataRequestPath = `${prerenderPath}.data`;
2897+
}
2898+
} else {
2899+
if (prerenderPath === "/") {
2900+
dataRequestPath = "/_root.data";
2901+
} else {
2902+
dataRequestPath = `${prerenderPath.replace(/\/$/, "")}.data`;
2903+
}
2904+
}
2905+
let normalizedPath =
2906+
`${reactRouterConfig.basename}${dataRequestPath}`.replace(/\/\/+/g, "/");
28962907
let url = new URL(`http://localhost${normalizedPath}`);
28972908
if (onlyRoutes?.length) {
28982909
url.searchParams.set("_routes", onlyRoutes.join(","));

‎packages/react-router/lib/dom-export/hydrated-router.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ function createHydratedRouter({
186186
ssrInfo.routeModules,
187187
ssrInfo.context.ssr,
188188
ssrInfo.context.basename,
189+
ssrInfo.context.future.unstable_trailingSlashAwareDataRequests,
189190
),
190191
patchRoutesOnNavigation: getPatchRoutesOnNavigationFunction(
191192
ssrInfo.manifest,

‎packages/react-router/lib/dom/ssr/components.tsx‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ function PrefetchPageLinksImpl({
371371
matches: AgnosticDataRouteMatch[];
372372
}) {
373373
let location = useLocation();
374-
let { manifest, routeModules } = useFrameworkContext();
374+
let { future, manifest, routeModules } = useFrameworkContext();
375375
let { basename } = useDataRouterContext();
376376
let { loaderData, matches } = useDataRouterStateContext();
377377

@@ -435,7 +435,12 @@ function PrefetchPageLinksImpl({
435435
return [];
436436
}
437437

438-
let url = singleFetchUrl(page, basename, "data");
438+
let url = singleFetchUrl(
439+
page,
440+
basename,
441+
future.unstable_trailingSlashAwareDataRequests,
442+
"data",
443+
);
439444
// When one or more routes have opted out, we add a _routes param to
440445
// limit the loaders to those that have a server loader and did not
441446
// opt out
@@ -452,6 +457,7 @@ function PrefetchPageLinksImpl({
452457
return [url.pathname + url.search];
453458
}, [
454459
basename,
460+
future.unstable_trailingSlashAwareDataRequests,
455461
loaderData,
456462
location,
457463
manifest,

‎packages/react-router/lib/dom/ssr/entry.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ export interface EntryContext extends FrameworkContextObject {
4545

4646
export interface FutureConfig {
4747
unstable_subResourceIntegrity: boolean;
48+
unstable_trailingSlashAwareDataRequests: boolean;
4849
v8_middleware: boolean;
4950
}
5051

‎packages/react-router/lib/dom/ssr/routes-test-stub.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,8 @@ export function createRoutesStub(
135135
unstable_subResourceIntegrity:
136136
future?.unstable_subResourceIntegrity === true,
137137
v8_middleware: future?.v8_middleware === true,
138+
unstable_trailingSlashAwareDataRequests:
139+
future?.unstable_trailingSlashAwareDataRequests === true,
138140
},
139141
manifest: {
140142
routes: {},

0 commit comments

Comments
 (0)