Skip to content

Commit

Permalink
Allow * only after / in route paths
Browse files Browse the repository at this point in the history 
Fixes #8072
Fixes REM-258
Fixes REM-375
  • Loading branch information
@mjackson
mjackson committed Oct 7, 2021
1 parent f80147b commit 8ccac66
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 59 deletions.
57 changes: 57 additions & 0 deletions packages/react-router/__tests__/matchPath-test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,63 @@ describe("matchPath", () => {
});
});

describe("matchPath *", () => {
it("matches the root URL", () => {
expect(matchPath("*", "/")).toMatchObject({
pathname: "/",
pathnameBase: "/"
});
expect(matchPath("/*", "/")).toMatchObject({
pathname: "/",
pathnameBase: "/"
});
});

it("matches a URL with a segment", () => {
expect(matchPath("*", "/users")).toMatchObject({
pathname: "/users",
pathnameBase: "/"
});
expect(matchPath("/*", "/users")).toMatchObject({
pathname: "/users",
pathnameBase: "/"
});
});

it("matches a URL with a segment and a trailing slash", () => {
expect(matchPath("*", "/users/")).toMatchObject({
pathname: "/users/",
pathnameBase: "/"
});
expect(matchPath("/*", "/users/")).toMatchObject({
pathname: "/users/",
pathnameBase: "/"
});
});

it("matches a URL with multiple segments", () => {
expect(matchPath("*", "/users/mj")).toMatchObject({
pathname: "/users/mj",
pathnameBase: "/"
});
expect(matchPath("/*", "/users/mj")).toMatchObject({
pathname: "/users/mj",
pathnameBase: "/"
});
});

it("matches a URL with multiple segments and a trailing slash", () => {
expect(matchPath("*", "/users/mj/")).toMatchObject({
pathname: "/users/mj/",
pathnameBase: "/"
});
expect(matchPath("/*", "/users/mj/")).toMatchObject({
pathname: "/users/mj/",
pathnameBase: "/"
});
});
});

describe("matchPath warnings", () => {
let consoleWarn: jest.SpyInstance<void, any>;
beforeEach(() => {
Expand Down
33 changes: 33 additions & 0 deletions packages/react-router/__tests__/params-decode-test.tsx
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import * as React from "react";
import { create as createTestRenderer } from "react-test-renderer";
import { MemoryRouter as Router, Routes, Route, useParams } from "react-router";

describe("Decoding params", () => {
it("works", () => {
function Content() {
return <p>The params are {JSON.stringify(useParams())}</p>;
}

let renderer = createTestRenderer(
<Router initialEntries={["/content/%2F"]}>
<Routes>
<Route
path="content/*"
element={
<Routes>
<Route path=":id" element={<Content />} />
</Routes>
}
/>
</Routes>
</Router>
);

expect(renderer.toJSON()).toMatchInlineSnapshot(`
<p>
The params are
{"*":"/","id":"/"}
</p>
`);
});
});
40 changes: 19 additions & 21 deletions packages/react-router/__tests__/path-matching-test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,9 @@ describe("path matching with a basename", () => {
expect(matches).toHaveLength(1);
expect(matches).toMatchObject([
{
params: { userId: "michael" },
pathname: "/users/michael",
params: { userId: "michael" }
pathnameBase: "/users/michael"
}
]);
});
Expand All @@ -159,16 +160,19 @@ describe("path matching with a basename", () => {
expect(matches).toHaveLength(3);
expect(matches).toMatchObject([
{
params: { userId: "michael", courseId: "react" },
pathname: "/users/michael",
params: { userId: "michael", courseId: "react" }
pathnameBase: "/users/michael"
},
{
params: { userId: "michael", courseId: "react" },
pathname: "/users/michael/subjects",
params: { userId: "michael", courseId: "react" }
pathnameBase: "/users/michael/subjects"
},
{
params: { userId: "michael", courseId: "react" },
pathname: "/users/michael/subjects/react",
params: { userId: "michael", courseId: "react" }
pathnameBase: "/users/michael/subjects/react"
}
]);
});
Expand All @@ -184,7 +188,8 @@ describe("path matching with splats", () => {
expect(match).not.toBeNull();
expect(match[0]).toMatchObject({
params: { id: "mj", "*": "secrets.txt" },
pathname: "/users/mj/files/secrets.txt"
pathname: "/users/mj/files/secrets.txt",
pathnameBase: "/users/mj/files"
});
});

Expand All @@ -196,18 +201,7 @@ describe("path matching with splats", () => {
});
});

test("splat after something other than /", () => {
let routes = [{ path: "users/:id/files-*" }];
let match = matchRoutes(routes, "/users/mj/files-secrets.txt")!;

expect(match).not.toBeNull();
expect(match[0]).toMatchObject({
params: { id: "mj", "*": "secrets.txt" },
pathname: "/users/mj/files-secrets.txt"
});
});

test("parent route with splat after /", () => {
test("parent route with splat", () => {
let routes = [
{ path: "users/:id/files/*", children: [{ path: "secrets.txt" }] }
];
Expand All @@ -216,7 +210,8 @@ describe("path matching with splats", () => {
expect(match).not.toBeNull();
expect(match[0]).toMatchObject({
params: { id: "mj", "*": "secrets.txt" },
pathname: "/users/mj/files/secrets.txt"
pathname: "/users/mj/files/secrets.txt",
pathnameBase: "/users/mj/files"
});
expect(match[1]).toMatchObject({
params: { id: "mj", "*": "secrets.txt" },
Expand All @@ -233,15 +228,18 @@ describe("path matching with splats", () => {
expect(match).not.toBeNull();
expect(match[0]).toMatchObject({
params: { "*": "one/two/three" },
pathname: "/one/two/three"
pathname: "/one/two/three",
pathnameBase: "/"
});
expect(match[1]).toMatchObject({
params: { "*": "one/two/three" },
pathname: "/one/two/three"
pathname: "/one/two/three",
pathnameBase: "/"
});
expect(match[2]).toMatchObject({
params: { "*": "one/two/three" },
pathname: "/one/two/three"
pathname: "/one/two/three",
pathnameBase: "/"
});
});
});
82 changes: 44 additions & 38 deletions packages/react-router/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,15 @@ interface RouteContextObject<ParamKey extends string = string> {
outlet: React.ReactElement | null;
params: Readonly<Params<ParamKey>>;
pathname: string;
pathnameBase: string;
route: RouteObject | null;
}

const RouteContext = React.createContext<RouteContextObject>({
outlet: null,
params: {},
pathname: "/",
pathnameBase: "/",
route: null
});

Expand Down Expand Up @@ -589,6 +591,7 @@ export function useRoutes(
let {
params: parentParams,
pathname: parentPathname,
pathnameBase: parentPathnameBase,
route: parentRoute
} = React.useContext(RouteContext);

Expand Down Expand Up @@ -628,18 +631,31 @@ export function useRoutes(
}

let locationFromContext = useLocation();
let location = locationArg
? typeof locationArg === "string"
? parsePath(locationArg)
: locationArg
: locationFromContext;
let pathname = location.pathname || "/";

let parentPathnameStart = getPathnameStart(parentPathname, parentParams);
let location;
if (locationArg) {
let parsedLocationArg =
typeof locationArg === "string" ? parsePath(locationArg) : locationArg;

invariant(
parentPathnameBase === "/" ||
parsedLocationArg.pathname?.startsWith(parentPathnameBase),
`When overriding the location using \`<Routes location>\` or \`useRoutes(routes, location)\`, ` +
`the location pathname must begin with the portion of the URL pathname that was ` +
`matched by all parent routes. The current pathname base is "${parentPathnameBase}" ` +
`but pathname "${parsedLocationArg.pathname}" was given in the \`location\` prop.`
);

location = parsedLocationArg;
} else {
location = locationFromContext;
}

let pathname = location.pathname || "/";
let remainingPathname =
parentPathnameStart === "/"
parentPathnameBase === "/"
? pathname
: pathname.slice(parentPathnameStart.length);
: pathname.slice(parentPathnameBase.length) || "/";
let matches = matchRoutes(routes, { pathname: remainingPathname });

if (__DEV__) {
Expand All @@ -654,7 +670,8 @@ export function useRoutes(
matches.map(match =>
Object.assign({}, match, {
params: Object.assign({}, parentParams, match.params),
pathname: joinPaths([parentPathnameStart, match.pathname])
pathname: joinPaths([parentPathnameBase, match.pathname]),
pathnameBase: joinPaths([parentPathnameBase, match.pathnameBase])
})
)
);
Expand Down Expand Up @@ -756,6 +773,10 @@ export interface RouteMatch<ParamKey extends string = string> {
* The portion of the URL pathname that was matched.
*/
pathname: string;
/**
* The portion of the URL pathname that was matched before child routes.
*/
pathnameBase: string;
/**
* The route object that was used to match.
*/
Expand Down Expand Up @@ -912,6 +933,7 @@ function compareIndexes(a: number[], b: number[]): number {

function matchRouteBranch<ParamKey extends string = string>(
branch: RouteBranch,
// TODO: attach original route object inside routesMeta so we don't need this arg
routesArg: RouteObject[],
pathname: string
): RouteMatch<ParamKey>[] | null {
Expand All @@ -924,13 +946,13 @@ function matchRouteBranch<ParamKey extends string = string>(
for (let i = 0; i < routesMeta.length; ++i) {
let meta = routesMeta[i];
let end = i === routesMeta.length - 1;
let trailingPathname =
let remainingPathname =
matchedPathname === "/"
? pathname
: pathname.slice(matchedPathname.length) || "/";
let match = matchPath(
{ path: meta.relativePath, caseSensitive: meta.caseSensitive, end },
trailingPathname
remainingPathname
);

if (!match) return null;
Expand All @@ -941,20 +963,12 @@ function matchRouteBranch<ParamKey extends string = string>(

matches.push({
params: matchedParams,
pathname:
match.pathname === "/"
? matchedPathname
: joinPaths([matchedPathname, match.pathname]),
pathname: joinPaths([matchedPathname, match.pathname]),
pathnameBase: joinPaths([matchedPathname, match.pathnameBase]),
route
});

let pathnameStart = getPathnameStart(match.pathname, match.params);
if (pathnameStart !== "/") {
// Add only the portion of the match.pathname that comes before the * to
// the matchedPathname. This allows child routes to match against the
// portion of the pathname that was matched by the *.
matchedPathname = joinPaths([matchedPathname, pathnameStart]);
}
matchedPathname = joinPaths([matchedPathname, match.pathnameBase]);

routes = route.children!;
}
Expand All @@ -978,6 +992,7 @@ export function renderMatches(
outlet,
params: match.params,
pathname: match.pathname,
pathnameBase: match.pathnameBase,
route: match.route
}}
/>
Expand Down Expand Up @@ -1092,10 +1107,11 @@ function compilePath(
end = true
): [RegExp, string[]] {
warning(
!path.endsWith("*") || path.endsWith("/*"),
`Route path "${path}" ends with \`*\` but it will be treated as ` +
`if it ended with \`/*\`. To get rid of this warning, please ` +
`adjust the route path to end with \`/*\`.`
path === "*" || !path.endsWith("*") || path.endsWith("/*"),
`Route path "${path}" will be treated as if it were ` +
`"${path.replace(/\*$/, "/*")}" because the \`*\` character must ` +
`always follow a \`/\` in the pattern. To get rid of this warning, ` +
`please change the route path to "${path.replace(/\*$/, "/*")}".`
);

let paramNames: string[] = [];
Expand All @@ -1113,7 +1129,7 @@ function compilePath(
if (path.endsWith("*")) {
paramNames.push("*");
regexpSource +=
path === "/*"
path === "*" || path === "/*"
? "(.*)$" // Already matched the initial /, just match the rest
: "(?:\\/(.+)|\\/*)$"; // Don't include the / in params["*"]
} else {
Expand Down Expand Up @@ -1215,16 +1231,6 @@ function getToPathname(to: To): string | undefined {
: to.pathname;
}

function getPathnameStart(pathname: string, params: Params): string {
let splat = params["*"];
if (!splat) return pathname;
let pathnameStart = pathname.slice(0, -splat.length);
if (splat.startsWith("/")) return pathnameStart;
let index = pathnameStart.lastIndexOf("/");
if (index > 0) return pathnameStart.slice(0, index);
return "/";
}

function stripBasename(pathname: string, basename: string): string | null {
if (basename === "/") return pathname;

Expand Down

0 comments on commit 8ccac66

Please sign in to comment.