Skip to content

fix: explode optional segments for relative paths in nested routes #9684

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Dec 7, 2022
Merged

fix: explode optional segments for relative paths in nested routes #9684

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8688e60
refactor(router): handle optional path segments in nested routes
pcattori Nov 29, 2022
28836f2
fix: handle empty segments for nested routes
pcattori Dec 7, 2022
e4cc374
refactor: replace generator with function returning a list
pcattori Dec 7, 2022
647f412
unit tests
brophdawg11 Dec 7, 2022
fa7f933
Remove dynamic hash and collapse into _explodeOptionalSegments
brophdawg11 Dec 7, 2022
6f4c824
refactor: remove nullish coalescing `??` to be browser compatible
pcattori Dec 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
334 changes: 332 additions & 2 deletions packages/react-router/__tests__/path-matching-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ function pickPathsAndParams(routes: RouteObject[], pathname: string) {
let matches = matchRoutes(routes, pathname);
return (
matches &&
matches.map((match) => ({ path: match.route.path, params: match.params }))
matches.map((match) => ({
...(match.route.index ? { index: match.route.index } : {}),
...(match.route.path ? { path: match.route.path } : {}),
params: match.params,
}))
);
}

Expand Down Expand Up @@ -277,7 +281,7 @@ describe("path matching with splats", () => {
});
});

describe("path matchine with optional segments", () => {
describe("path matching with optional segments", () => {
test("optional static segment at the start of the path", () => {
let routes = [
{
Expand Down Expand Up @@ -357,6 +361,24 @@ describe("path matchine with optional segments", () => {
},
]);
});

test("optional static segment in nested routes", () => {
let nested = [
{
path: "/en?",
children: [
{
path: "abc",
},
],
},
];

expect(pickPathsAndParams(nested, "/en/abc")).toEqual([
{ path: "/en?", params: {} },
{ path: "abc", params: {} },
]);
});
});

describe("path matching with optional dynamic segments", () => {
Expand Down Expand Up @@ -439,4 +461,312 @@ describe("path matching with optional dynamic segments", () => {
},
]);
});

test("consecutive optional dynamic segments in nested routes", () => {
let nested = [
{
path: "/one/:two?",
children: [
{
path: "three/:four?",
children: [
{
path: ":five?",
},
],
},
],
},
];
expect(pickPathsAndParams(nested, "/one/dos/three/cuatro/cinco")).toEqual([
{
path: "/one/:two?",
params: { two: "dos", four: "cuatro", five: "cinco" },
},
{
path: "three/:four?",
params: { two: "dos", four: "cuatro", five: "cinco" },
},
{ path: ":five?", params: { two: "dos", four: "cuatro", five: "cinco" } },
]);
expect(pickPathsAndParams(nested, "/one/dos/three/cuatro")).toEqual([
{
path: "/one/:two?",
params: { two: "dos", four: "cuatro" },
},
{
path: "three/:four?",
params: { two: "dos", four: "cuatro" },
},
{
path: ":five?",
params: { two: "dos", four: "cuatro" },
},
]);
expect(pickPathsAndParams(nested, "/one/dos/three")).toEqual([
{
path: "/one/:two?",
params: { two: "dos" },
},
{
path: "three/:four?",
params: { two: "dos" },
},
// Matches into 5 because it's just like if we did path=""
{
path: ":five?",
params: { two: "dos" },
},
]);
expect(pickPathsAndParams(nested, "/one/dos")).toEqual([
{
path: "/one/:two?",
params: { two: "dos" },
},
]);
expect(pickPathsAndParams(nested, "/one")).toEqual([
{
path: "/one/:two?",
params: {},
},
]);
expect(pickPathsAndParams(nested, "/one/three/cuatro/cinco")).toEqual([
{
path: "/one/:two?",
params: { four: "cuatro", five: "cinco" },
},
{
path: "three/:four?",
params: { four: "cuatro", five: "cinco" },
},
{ path: ":five?", params: { four: "cuatro", five: "cinco" } },
]);
expect(pickPathsAndParams(nested, "/one/three/cuatro")).toEqual([
{
path: "/one/:two?",
params: { four: "cuatro" },
},
{
path: "three/:four?",
params: { four: "cuatro" },
},
{
path: ":five?",
params: { four: "cuatro" },
},
]);
expect(pickPathsAndParams(nested, "/one/three")).toEqual([
{
path: "/one/:two?",
params: {},
},
{
path: "three/:four?",
params: {},
},
// Matches into 5 because it's just like if we did path=""
{
path: ":five?",
params: {},
},
]);
expect(pickPathsAndParams(nested, "/one")).toEqual([
{
path: "/one/:two?",
params: {},
},
]);
});

test("prefers optional static over optional dynamic segments", () => {
let nested = [
{
path: "/one",
children: [
{
path: ":param?",
children: [
{
path: "three",
},
],
},
{
path: "two?",
children: [
{
path: "three",
},
],
},
],
},
];

// static `two` segment should win
expect(pickPathsAndParams(nested, "/one/two/three")).toEqual([
{
params: {},
path: "/one",
},
{
params: {},
path: "two?",
},
{
params: {},
path: "three",
},
]);

// fall back to param when no static match
expect(pickPathsAndParams(nested, "/one/not-two/three")).toEqual([
{
params: {
param: "not-two",
},
path: "/one",
},
{
params: {
param: "not-two",
},
path: ":param?",
},
{
params: {
param: "not-two",
},
path: "three",
},
]);

// No optional segment provided - earlier "dup" route should win
expect(pickPathsAndParams(nested, "/one/three")).toEqual([
{
params: {},
path: "/one",
},
{
params: {},
path: ":param?",
},
{
params: {},
path: "three",
},
]);
});

test("prefers index routes over optional static segments", () => {
let nested = [
{
path: "/one",
children: [
{
path: ":param?",
children: [
{
path: "three?",
},
{
index: true,
},
],
},
],
},
];

expect(pickPathsAndParams(nested, "/one/two")).toEqual([
{
params: {
param: "two",
},
path: "/one",
},
{
params: {
param: "two",
},
path: ":param?",
},
{
index: true,
params: {
param: "two",
},
},
]);
expect(pickPathsAndParams(nested, "/one")).toEqual([
{
params: {},
path: "/one",
},
{
params: {},
path: ":param?",
},
{
index: true,
params: {},
},
]);
});

test("prefers index routes over optional dynamic segments", () => {
let nested = [
{
path: "/one",
children: [
{
path: ":param?",
children: [
{
path: ":three?",
},
{
index: true,
},
],
},
],
},
];

expect(pickPathsAndParams(nested, "/one/two")).toEqual([
{
params: {
param: "two",
},
path: "/one",
},
{
params: {
param: "two",
},
path: ":param?",
},
{
index: true,
params: {
param: "two",
},
},
]);
expect(pickPathsAndParams(nested, "/one")).toEqual([
{
params: {},
path: "/one",
},
{
params: {},
path: ":param?",
},
{
index: true,
params: {},
},
]);
});
});
Loading