diff --git a/contributors.yml b/contributors.yml index de3007d16..9823b82e3 100644 --- a/contributors.yml +++ b/contributors.yml @@ -55,6 +55,7 @@ - rtmann - ryanflorence - sanketshah19 +- senseibarni - sergiodxa - shamsup - shihanng diff --git a/packages/react-router-dom/__tests__/trailing-slashes-test.tsx b/packages/react-router-dom/__tests__/trailing-slashes-test.tsx new file mode 100644 index 000000000..168116614 --- /dev/null +++ b/packages/react-router-dom/__tests__/trailing-slashes-test.tsx @@ -0,0 +1,454 @@ +import { JSDOM } from "jsdom"; +import * as React from "react"; +import * as ReactDOM from "react-dom"; +import { act } from "react-dom/test-utils"; + +import { + BrowserRouter, + Routes, + Route, + Navigate, + Link, + Outlet, + useSearchParams, + useLocation, +} from "react-router-dom"; + +describe("trailing slashes", () => { + let node: HTMLDivElement; + beforeEach(() => { + node = document.createElement("div"); + document.body.appendChild(node); + }); + + afterEach(() => { + document.body.removeChild(node); + node = null!; + }); + + describe(" element", () => { + describe("with a basename that does not contain a trailing slash", () => { + test("lets root links control trailing slashes (index route)", () => { + let window = getWindowImpl("/app"); + act(() => { + ReactDOM.render( + + + + + + + } + /> + + , + node + ); + }); + + expect(node.querySelectorAll("a")).toMatchInlineSnapshot(` + NodeList [ + , + , + ] + `); + }); + + test('lets root links control trailing slashes (path="")', () => { + let window = getWindowImpl("/app"); + act(() => { + ReactDOM.render( + + + + + + + } + /> + + , + node + ); + }); + + expect(node.querySelectorAll("a")).toMatchInlineSnapshot(` + NodeList [ + , + , + ] + `); + }); + + test("lets nested link control trailing slashes", () => { + let window = getWindowImpl("/app/parent/child"); + act(() => { + ReactDOM.render( + + + + + + + + } + > + + + + + + + + } + > + + + + + + + } + /> + + + + , + node + ); + }); + + expect(node.querySelectorAll("a")).toMatchInlineSnapshot(` + NodeList [ + , + , + , + , + , + , + , + , + , + , + ] + `); + }); + }); + + describe("with a basename that contains a trailing slash", () => { + test("always contains trailing slashes on root links (index route)", () => { + let window = getWindowImpl("/app/"); + act(() => { + ReactDOM.render( + + + + + + + } + /> + + , + node + ); + }); + + expect(node.querySelectorAll("a")).toMatchInlineSnapshot(` + NodeList [ + , + , + ] + `); + }); + + test('always contains trailing slashes on root links (path="" route)', () => { + let window = getWindowImpl("/app/"); + act(() => { + ReactDOM.render( + + + + + + + } + /> + + , + node + ); + }); + + expect(node.querySelectorAll("a")).toMatchInlineSnapshot(` + NodeList [ + , + , + ] + `); + }); + + test("always contains root trailing slashes in nested routes", () => { + let window = getWindowImpl("/app/parent/child"); + act(() => { + ReactDOM.render( + + + + + + + + } + > + + + + + + + + } + > + + + + + + + } + /> + + + + , + node + ); + }); + + expect(node.querySelectorAll("a")).toMatchInlineSnapshot(` + NodeList [ + , + , + , + , + , + , + , + , + , + , + ] + `); + }); + }); + }); + + describe("in a element", () => { + it("should not include trailing slash on empty string path", () => { + let window = getWindowImpl("/foo/bar"); + spyOn(window.history, "pushState").and.callThrough(); + + expect(window.location.href).toBe("https://remix.run/foo/bar"); + + act(() => { + ReactDOM.render( + + + 👋} /> + } /> + + , + node + ); + }); + + expect(window.location.href).toBe("https://remix.run/foo"); + }); + + it("should not include trailing slash via useSearchParams", () => { + let window = getWindowImpl("/foo"); + spyOn(window.history, "pushState").and.callThrough(); + + expect(window.location.href).toBe("https://remix.run/foo"); + + function SetSearchParams() { + let [, setSearchParams] = useSearchParams(); + React.useEffect( + () => setSearchParams({ key: "value" }), + [setSearchParams] + ); + return

👋

; + } + + act(() => { + ReactDOM.render( + + + } /> + + , + node + ); + }); + + expect(window.location.href).toBe("https://remix.run/foo?key=value"); + }); + + it("should include trailing slash from the Navigate", () => { + let window = getWindowImpl("/foo/bar"); + spyOn(window.history, "pushState").and.callThrough(); + + expect(window.location.href).toBe("https://remix.run/foo/bar"); + + act(() => { + ReactDOM.render( + + + 👋} /> + } /> + + , + node + ); + }); + + expect(window.location.href).toBe("https://remix.run/foo/"); + }); + + it("should include trailing slash from the basename", () => { + let window = getWindowImpl("/foo/bar"); + spyOn(window.history, "pushState").and.callThrough(); + + expect(window.location.href).toBe("https://remix.run/foo/bar"); + + act(() => { + ReactDOM.render( + + + 👋} /> + } /> + + , + node + ); + }); + + expect(window.location.href).toBe("https://remix.run/foo/"); + }); + + it("should include trailing slash from both", () => { + let window = getWindowImpl("/foo/bar"); + spyOn(window.history, "pushState").and.callThrough(); + + expect(window.location.href).toBe("https://remix.run/foo/bar"); + + act(() => { + ReactDOM.render( + + + 👋} /> + } /> + + , + node + ); + }); + + expect(window.location.href).toBe("https://remix.run/foo/"); + }); + }); +}); + +function getWindowImpl(initialUrl: string, isHash = false): Window { + // Need to use our own custom DOM in order to get a working history + const dom = new JSDOM(``, { url: "https://remix.run/" }); + dom.window.history.replaceState(null, "", (isHash ? "#" : "") + initialUrl); + return dom.window as unknown as Window; +} diff --git a/packages/react-router/__tests__/useNavigate-test.tsx b/packages/react-router/__tests__/useNavigate-test.tsx index fccf15c4d..ac5bd16cc 100644 --- a/packages/react-router/__tests__/useNavigate-test.tsx +++ b/packages/react-router/__tests__/useNavigate-test.tsx @@ -68,7 +68,7 @@ describe("useNavigate", () => { } function ShowLocationState() { - return

location.state: {JSON.stringify(useLocation().state)}

; + return

location.state:{JSON.stringify(useLocation().state)}

; } let renderer: TestRenderer.ReactTestRenderer; @@ -91,7 +91,7 @@ describe("useNavigate", () => { expect(renderer.toJSON()).toMatchInlineSnapshot(`

- location.state: + location.state: {"from":"home"}

`); diff --git a/packages/react-router/lib/components.tsx b/packages/react-router/lib/components.tsx index 02fc92a20..953776481 100644 --- a/packages/react-router/lib/components.tsx +++ b/packages/react-router/lib/components.tsx @@ -323,7 +323,9 @@ export function Router({ ` You should never have more than one in your app.` ); - let basename = normalizePathname(basenameProp); + // Preserve trailing slashes on basename, so we can let the user control + // the enforcement of trailing slashes throughout the app + let basename = basenameProp.replace(/^\/*/, "/"); let navigationContext = React.useMemo( () => ({ basename, navigator, static: staticProp }), [basename, navigator, staticProp] diff --git a/packages/react-router/lib/hooks.tsx b/packages/react-router/lib/hooks.tsx index e11e23051..5e4cb7fc2 100644 --- a/packages/react-router/lib/hooks.tsx +++ b/packages/react-router/lib/hooks.tsx @@ -2,6 +2,7 @@ import * as React from "react"; import { isRouteErrorResponse, Location, + normalizePathname, ParamParseKey, Params, Path, @@ -54,11 +55,24 @@ export function useHref(to: To): string { let joinedPathname = pathname; if (basename !== "/") { let toPathname = getToPathname(to); - let endsWithSlash = toPathname != null && toPathname.endsWith("/"); - joinedPathname = - pathname === "/" - ? basename + (endsWithSlash ? "/" : "") - : joinPaths([basename, pathname]); + + // Only append a trailing slash to the raw basename if the basename doesn't + // already have one and this wasn't specifically a route to "". This + // allows folks to control the trailing slash behavior when using a basename + let appendSlash = + !basename.endsWith("/") && + to !== "" && + (to as Path)?.pathname !== "" && + toPathname != null && + toPathname.endsWith("/"); + + if (pathname !== "/") { + joinedPathname = joinPaths([basename, pathname]); + } else if (appendSlash) { + joinedPathname = basename + "/"; + } else { + joinedPathname = basename; + } } return navigator.createHref({ pathname: joinedPathname, search, hash }); @@ -194,7 +208,13 @@ export function useNavigate(): NavigateFunction { ); if (basename !== "/") { - path.pathname = joinPaths([basename, path.pathname]); + // If this was a blank path, just use the basename directly, this gives + // the user control over trailing slash behavior + let toPath = typeof to === "string" ? parsePath(to) : to; + let isBlankPath = toPath.pathname == null || toPath.pathname === ""; + path.pathname = isBlankPath + ? basename + : joinPaths([basename, path.pathname]); } (!!options.replace ? navigator.replace : navigator.push)( diff --git a/packages/router/utils.ts b/packages/router/utils.ts index dec414313..77ad209f5 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -578,13 +578,18 @@ export function stripBasename( return null; } - let nextChar = pathname.charAt(basename.length); + // We want to leave trailing slash behavior in the user's control, so if they + // specify a basename with a trailing slash, we should support it + let startIndex = basename.endsWith("/") + ? basename.length - 1 + : basename.length; + let nextChar = pathname.charAt(startIndex); if (nextChar && nextChar !== "/") { // pathname does not start with basename/ return null; } - return pathname.slice(basename.length) || "/"; + return pathname.slice(startIndex) || "/"; } /**