Skip to content

Commit d4cf1df

Browse files
younghoonkimjoseph0926
authored andcommitted
fix: generate relative route IDs when using relative() helper
1 parent bd341ff commit d4cf1df

File tree

3 files changed

+226
-5
lines changed

3 files changed

+226
-5
lines changed
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
import { relative, route, configRoutesToRouteManifest } from "../config/routes";
2+
3+
describe("relative() route ID generation - Fix #12325", () => {
4+
describe("Verify updated behavior", () => {
5+
it("When using relative(), route IDs are generated based on relative paths", () => {
6+
const { route, layout, index } = relative("app/routes");
7+
8+
const routes = [
9+
layout("_layout.tsx", [
10+
route("home", "home.tsx"),
11+
route("about", "about.tsx"),
12+
index("_index.tsx"),
13+
]),
14+
];
15+
16+
const appDirectory = process.cwd();
17+
const manifest = configRoutesToRouteManifest(appDirectory, routes);
18+
19+
const layoutRoute = Object.values(manifest).find((r) =>
20+
r.file.includes("_layout"),
21+
);
22+
const homeRoute = Object.values(manifest).find((r) =>
23+
r.file.includes("home"),
24+
);
25+
const indexRoute = Object.values(manifest).find((r) =>
26+
r.file.includes("_index"),
27+
);
28+
29+
console.log("Layout ID:", layoutRoute?.id);
30+
console.log("Home ID:", homeRoute?.id);
31+
console.log("Index ID:", indexRoute?.id);
32+
33+
expect(layoutRoute?.id).toBe("app/routes/_layout");
34+
expect(homeRoute?.id).toBe("app/routes/home");
35+
expect(indexRoute?.id).toBe("app/routes/_index");
36+
37+
expect(layoutRoute?.id).not.toContain(process.cwd());
38+
expect(homeRoute?.id).not.toContain(process.cwd());
39+
expect(indexRoute?.id).not.toContain(process.cwd());
40+
});
41+
42+
it("Even when using normal route(), relative path IDs are still generated", () => {
43+
const routes = [route("home", "app/routes/home.tsx")];
44+
45+
const appDirectory = process.cwd();
46+
const manifest = configRoutesToRouteManifest(appDirectory, routes);
47+
48+
const homeRoute = Object.values(manifest)[0];
49+
50+
console.log("Normal route ID:", homeRoute.id);
51+
52+
expect(homeRoute.id).toBe("app/routes/home");
53+
expect(homeRoute.id).not.toContain(process.cwd());
54+
});
55+
56+
it("If an ID is explicitly specified, it is preserved as is", () => {
57+
const { route } = relative("app/routes");
58+
59+
const routes = [route("home", "home.tsx", { id: "custom-home-id" })];
60+
61+
const appDirectory = process.cwd();
62+
const manifest = configRoutesToRouteManifest(appDirectory, routes);
63+
const homeRoute = Object.values(manifest)[0];
64+
65+
expect(homeRoute.id).toBe("custom-home-id");
66+
});
67+
68+
it("Nested relative paths are handled correctly", () => {
69+
const { route, layout } = relative("app/routes/admin");
70+
71+
const routes = [
72+
layout("_layout.tsx", [
73+
route("users", "users.tsx"),
74+
route("settings", "settings.tsx"),
75+
]),
76+
];
77+
78+
const appDirectory = process.cwd();
79+
const manifest = configRoutesToRouteManifest(appDirectory, routes);
80+
81+
const layoutRoute = Object.values(manifest).find((r) =>
82+
r.file.includes("_layout"),
83+
);
84+
const usersRoute = Object.values(manifest).find((r) =>
85+
r.file.includes("users"),
86+
);
87+
88+
expect(layoutRoute?.id).toBe("app/routes/admin/_layout");
89+
expect(usersRoute?.id).toBe("app/routes/admin/users");
90+
});
91+
92+
it("Route IDs returned from useMatches() should be relative paths", () => {
93+
const { route, layout, index } = relative("app/routes");
94+
const routes = [
95+
layout("root.tsx", [index("_index.tsx"), route("about", "about.tsx")]),
96+
];
97+
98+
const manifest = configRoutesToRouteManifest(process.cwd(), routes);
99+
const matchIds = Object.keys(manifest);
100+
101+
console.log("Route IDs that useMatches() will return:", matchIds);
102+
103+
expect(matchIds).toEqual([
104+
"app/routes/root",
105+
"app/routes/_index",
106+
"app/routes/about",
107+
]);
108+
});
109+
});
110+
111+
describe("Various signature tests", () => {
112+
it("All route() overloads work correctly", () => {
113+
const { route } = relative("app/routes");
114+
115+
const r1 = route("path1", "file1.tsx");
116+
expect(r1.id).toBe("app/routes/file1");
117+
118+
const r2 = route("path2", "file2.tsx", [route("child", "child.tsx")]);
119+
expect(r2.id).toBe("app/routes/file2");
120+
121+
const r3 = route("path3", "file3.tsx", { caseSensitive: true });
122+
expect(r3.id).toBe("app/routes/file3");
123+
124+
const r4 = route("path4", "file4.tsx", { index: true }, [
125+
route("child", "child.tsx"),
126+
]);
127+
expect(r4.id).toBe("app/routes/file4");
128+
});
129+
130+
it("All layout() overloads work correctly", () => {
131+
const { layout } = relative("app/routes");
132+
133+
const l1 = layout("layout1.tsx");
134+
expect(l1.id).toBe("app/routes/layout1");
135+
136+
const l2 = layout("layout2.tsx", [route("child", "child.tsx")]);
137+
expect(l2.id).toBe("app/routes/layout2");
138+
139+
const l3 = layout("layout3.tsx", { id: "custom" });
140+
expect(l3.id).toBe("custom");
141+
142+
const l4 = layout("layout4.tsx", {}, [route("child", "child.tsx")]);
143+
expect(l4.id).toBe("app/routes/layout4");
144+
});
145+
});
146+
});

packages/react-router-dev/__tests__/route-config-test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,7 @@ describe("route config", () => {
508508
},
509509
],
510510
"file": "{{CWD}}/path/to/dirname/nested/parent.tsx",
511+
"id": "path/to/dirname/nested/parent",
511512
"path": "parent",
512513
}
513514
`);
@@ -524,6 +525,7 @@ describe("route config", () => {
524525
[
525526
{
526527
"file": "{{CWD}}/path/to/dirname/nested/without-options.tsx",
528+
"id": "path/to/dirname/nested/without-options",
527529
"index": true,
528530
},
529531
{
@@ -552,6 +554,7 @@ describe("route config", () => {
552554
},
553555
],
554556
"file": "{{CWD}}/path/to/dirname/nested/parent.tsx",
557+
"id": "path/to/dirname/nested/parent",
555558
}
556559
`);
557560
});

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

Lines changed: 77 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,14 @@ export { route, index, layout, prefix };
318318
* splitting route config into multiple files within different directories.
319319
*/
320320
export function relative(directory: string): typeof helpers {
321+
const generateRelativeId = (file: string): string => {
322+
const joined = Path.join(directory, file);
323+
const relativePath = Path.isAbsolute(joined)
324+
? Path.relative(process.cwd(), joined)
325+
: joined;
326+
return Path.normalize(stripFileExtension(relativePath));
327+
};
328+
321329
return {
322330
/**
323331
* Helper function for creating a route config entry, for use within
@@ -326,29 +334,93 @@ export function relative(directory: string): typeof helpers {
326334
* `relative` call that created this helper.
327335
*/
328336
route: (path, file, ...rest) => {
329-
return route(path, Path.resolve(directory, file), ...(rest as any));
337+
const absolutePath = Path.resolve(directory, file);
338+
const relativeId = generateRelativeId(file);
339+
340+
if (!rest || (rest as any).length === 0) {
341+
return route(path, absolutePath, { id: relativeId });
342+
}
343+
344+
const [first, second] = rest;
345+
346+
if (Array.isArray(first)) {
347+
return route(path, absolutePath, { id: relativeId }, first);
348+
}
349+
350+
const options = first as CreateRouteOptions;
351+
const modifiedOptions = options.id
352+
? options
353+
: { ...options, id: relativeId };
354+
355+
if (second !== undefined) {
356+
return route(
357+
path,
358+
absolutePath,
359+
modifiedOptions,
360+
second as RouteConfigEntry[],
361+
);
362+
}
363+
364+
return route(path, absolutePath, modifiedOptions);
330365
},
366+
331367
/**
332368
* Helper function for creating a route config entry for an index route, for
333369
* use within `routes.ts`. Note that this helper has been scoped, meaning
334370
* that file path will be resolved relative to the directory provided to the
335371
* `relative` call that created this helper.
336372
*/
337373
index: (file, ...rest) => {
338-
return index(Path.resolve(directory, file), ...(rest as any));
374+
const absolutePath = Path.resolve(directory, file);
375+
const relativeId = generateRelativeId(file);
376+
377+
if (!rest || rest.length === 0) {
378+
return index(absolutePath, { id: relativeId });
379+
}
380+
381+
const options = rest[0] as CreateIndexOptions;
382+
const modifiedOptions = options.id
383+
? options
384+
: { ...options, id: relativeId };
385+
return index(absolutePath, modifiedOptions);
339386
},
387+
340388
/**
341389
* Helper function for creating a route config entry for a layout route, for
342390
* use within `routes.ts`. Note that this helper has been scoped, meaning
343391
* that file path will be resolved relative to the directory provided to the
344392
* `relative` call that created this helper.
345393
*/
346394
layout: (file, ...rest) => {
347-
return layout(Path.resolve(directory, file), ...(rest as any));
395+
const absolutePath = Path.resolve(directory, file);
396+
const relativeId = generateRelativeId(file);
397+
398+
if (!rest || (rest as any).length === 0) {
399+
return layout(absolutePath, { id: relativeId });
400+
}
401+
402+
const [first, second] = rest;
403+
404+
if (Array.isArray(first)) {
405+
return layout(absolutePath, { id: relativeId }, first);
406+
}
407+
408+
const options = first as CreateLayoutOptions;
409+
const modifiedOptions = options.id
410+
? options
411+
: { ...options, id: relativeId };
412+
413+
if (second !== undefined) {
414+
return layout(
415+
absolutePath,
416+
modifiedOptions,
417+
second as RouteConfigEntry[],
418+
);
419+
}
420+
421+
return layout(absolutePath, modifiedOptions);
348422
},
349423

350-
// Passthrough of helper functions that don't need relative scoping so that
351-
// a complete API is still provided.
352424
prefix,
353425
};
354426
}

0 commit comments

Comments
 (0)