From ab68f0882186405007e453ee235d30956e58f3a8 Mon Sep 17 00:00:00 2001 From: Chance Strickland Date: Tue, 7 Sep 2021 15:42:37 -0700 Subject: [PATCH] Fix relative navigation issues (#8004) --- .../react-router/__tests__/navigate-test.tsx | 451 ++++++++++++------ packages/react-router/index.tsx | 31 +- 2 files changed, 340 insertions(+), 142 deletions(-) diff --git a/packages/react-router/__tests__/navigate-test.tsx b/packages/react-router/__tests__/navigate-test.tsx index 1a21f77d1d..a41f1cd248 100644 --- a/packages/react-router/__tests__/navigate-test.tsx +++ b/packages/react-router/__tests__/navigate-test.tsx @@ -22,7 +22,7 @@ describe("navigate", () => { node = null!; }); - describe("with an absolute href", () => { + describe("with an absolute `to` value", () => { it("navigates to the correct URL", () => { function Home() { let navigate = useNavigate(); @@ -70,205 +70,380 @@ describe("navigate", () => { }); }); - describe("with a relative href", () => { - it("navigates to the correct URL", () => { - function Home() { + describe("with a relative `to` value", () => { + describe("with a search value", () => { + it("navigates to the correct URL with params", () => { + function Home() { + let navigate = useNavigate(); + + function handleClick() { + navigate({ + pathname: "../about", + search: new URLSearchParams({ user: "mj" }).toString() + }); + } + + return ( +
+

Home

+ +
+ ); + } + + function About() { + let user = new URLSearchParams(useLocation().search).get("user"); + return

About {user}

; + } + + act(() => { + ReactDOM.render( + + + } /> + } /> + + , + node + ); + }); + + let button = node.querySelector("button"); + + act(() => { + button?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + + expect(node.innerHTML).toMatchInlineSnapshot(`"

About mj

"`); + }); + }); + + describe("with a search value and no pathname", () => { + function Bakery() { let navigate = useNavigate(); + let user = new URLSearchParams(useLocation().search).get("user"); function handleClick() { - navigate("../about"); + navigate({ + search: user ? "" : new URLSearchParams({ user: "mj" }).toString() + }); } return (
-

Home

- +

Bakery

+ {user &&

Welcome {user}

} + +
); } + function Muffins() { + return

Yay, muffins!

; + } + function About() { - return

About

; + let user = new URLSearchParams(useLocation().search).get("user"); + return

About {user}

; } - act(() => { - ReactDOM.render( - - - } /> - } /> - - , - node - ); - }); + it("navigates relative to the current location's pathname", () => { + act(() => { + ReactDOM.render( + + + }> + } /> + + } /> + + , + node + ); + }); - expect(node.innerHTML).toMatchInlineSnapshot( - `"

Home

"` - ); + let button = node.querySelector("button"); - let button = node.querySelector("button"); - expect(button).not.toBeNull(); + act(() => { + button?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); - act(() => { - button?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + expect(node.innerHTML).toMatchInlineSnapshot( + `"

Bakery

Welcome mj

Yay, muffins!

"` + ); }); - - expect(node.innerHTML).toMatchInlineSnapshot(`"

About

"`); }); - }); - describe("with a search param", () => { - it("navigates to the correct URL with params", () => { - function Home() { + describe("with a hash value and no pathname", () => { + function Bakery() { let navigate = useNavigate(); - function handleClick() { navigate({ - pathname: "../about", - search: new URLSearchParams({ user: "mj" }).toString() + hash: "#about" }); } return (
-

Home

- +

Bakery

+ + +

About us

+

We bake delicious cakes!

); } - function About() { - let user = new URLSearchParams(useLocation().search).get("user"); - return

About {user}

; + function Muffins() { + return

Yay, muffins!

; } - act(() => { - ReactDOM.render( - - - } /> - } /> - - , - node + it("navigates relative to the current location's pathname", () => { + act(() => { + ReactDOM.render( + + + }> + } /> + + + , + node + ); + }); + + let button = node.querySelector("button"); + + act(() => { + button?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + + expect(node.innerHTML).toMatchInlineSnapshot( + `"

Bakery

Yay, muffins!

About us

We bake delicious cakes!

"` ); }); + }); - let button = node.querySelector("button"); + describe("with a pathname", () => { + it("navigates relative to the route's pathname", () => { + function Home() { + let navigate = useNavigate(); + + function handleClick() { + navigate("../about"); + } + + return ( +
+

Home

+ +
+ ); + } - act(() => { - button?.dispatchEvent(new MouseEvent("click", { bubbles: true })); - }); + function About() { + return

About

; + } + + act(() => { + ReactDOM.render( + + + } /> + } /> + + , + node + ); + }); + + expect(node.innerHTML).toMatchInlineSnapshot( + `"

Home

"` + ); + + let button = node.querySelector("button"); + expect(button).not.toBeNull(); - expect(node.innerHTML).toMatchInlineSnapshot(`"

About mj

"`); + act(() => { + button?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + + expect(node.innerHTML).toMatchInlineSnapshot(`"

About

"`); + }); }); - }); - describe("with a search param and no pathname", () => { - function Bakery() { - let navigate = useNavigate(); + describe("with a pathname, called from a parent route", () => { + it("navigates relative to the route's pathname", () => { + function Layout() { + let navigate = useNavigate(); + function handleClick() { + navigate("about"); + } + return ( + <> +

Title

+ + + + ); + } + + function Home() { + return

Home

; + } + + function About() { + return

About

; + } - let user = new URLSearchParams(useLocation().search).get("user"); - function handleClick() { - navigate({ - search: user ? "" : new URLSearchParams({ user: "mj" }).toString() + act(() => { + ReactDOM.render( + + + }> + } /> + } /> + + + , + node + ); }); - } - return ( -
-

Bakery

- {user &&

Welcome {user}

} - - -
- ); - } + expect(node.innerHTML).toMatchInlineSnapshot( + `"

Title

Home

"` + ); - function Muffins() { - return

Yay, muffins!

; - } + let button = node.querySelector("button"); + expect(button).not.toBeNull(); - function About() { - let user = new URLSearchParams(useLocation().search).get("user"); - return

About {user}

; - } + act(() => { + button?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); - it("resolves using the current location", () => { - act(() => { - ReactDOM.render( - - - }> - } /> - - } /> - - , - node + expect(node.innerHTML).toMatchInlineSnapshot( + `"

Title

About

"` ); }); + }); - let button = node.querySelector("button"); + describe("with a pathname, called from a nested route", () => { + it("navigates relative to the route's pathname", () => { + function Layout() { + return ( + <> +

Title

+ + + ); + } - act(() => { - button?.dispatchEvent(new MouseEvent("click", { bubbles: true })); - }); + function Home() { + let navigate = useNavigate(); + function handleClick() { + navigate("../about"); + } + return ( + <> +

Home

+ + + ); + } - expect(node.innerHTML).toMatchInlineSnapshot( - `"

Bakery

Welcome mj

Yay, muffins!

"` - ); - }); - }); + function About() { + return

About

; + } - describe("with a hash param and no pathname", () => { - function Bakery() { - let navigate = useNavigate(); - function handleClick() { - navigate({ - hash: "#about" + act(() => { + ReactDOM.render( + + + }> + } /> + } /> + + + , + node + ); }); - } - return ( -
-

Bakery

- - -

About us

-

We bake delicious cakes!

-
- ); - } + expect(node.innerHTML).toMatchInlineSnapshot( + `"

Title

Home

"` + ); - function Muffins() { - return

Yay, muffins!

; - } + let button = node.querySelector("button"); + expect(button).not.toBeNull(); - it("resolves using the current location", () => { - act(() => { - ReactDOM.render( - - - }> - } /> - - - , - node + act(() => { + button?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + + expect(node.innerHTML).toMatchInlineSnapshot( + `"

Title

About

"` ); }); + }); - let button = node.querySelector("button"); + describe("when the pathname is '.'", () => { + it("navigates relative to the route's pathname", () => { + function Layout() { + return ( + <> +

Title

+ + + ); + } - act(() => { - button?.dispatchEvent(new MouseEvent("click", { bubbles: true })); - }); + function Home() { + let navigate = useNavigate(); + function handleClick() { + navigate("./#about"); + } + return ( +
+ +

About

+
+ ); + } - expect(node.innerHTML).toMatchInlineSnapshot( - `"

Bakery

Yay, muffins!

About us

We bake delicious cakes!

"` - ); + function About() { + return

About

; + } + + act(() => { + ReactDOM.render( + + + }> + } /> + } /> + + + , + node + ); + }); + + expect(node.innerHTML).toMatchInlineSnapshot( + `"

Title

About

"` + ); + + let button = node.querySelector("button"); + expect(button).not.toBeNull(); + + act(() => { + button?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + + expect(node.innerHTML).toMatchInlineSnapshot( + `"

Title

About

"` + ); + }); }); }); }); diff --git a/packages/react-router/index.tsx b/packages/react-router/index.tsx index acb11199a4..7cd09fdd91 100644 --- a/packages/react-router/index.tsx +++ b/packages/react-router/index.tsx @@ -418,8 +418,9 @@ export function useNavigate(): NavigateFunction { ); let navigator = React.useContext(NavigatorContext); - let { basename } = React.useContext(RouteContext); - let { pathname } = useLocation(); + let { basename, pathname: parentRoutePathname } = + React.useContext(RouteContext); + let { pathname: currentLocationPathname } = useLocation(); let activeRef = React.useRef(false); React.useEffect(() => { @@ -432,7 +433,29 @@ export function useNavigate(): NavigateFunction { if (typeof to === "number") { navigator.go(to); } else { - let path = resolvePath(to, pathname, basename); + let toPathname = + // Empty strings should be treated the same as / paths + to === "" || (to as Path).pathname === "" + ? "/" + : typeof to === "string" + ? parsePath(to).pathname + : to.pathname; + + let path = resolvePath( + to, + // If a pathname is explicitly provided in `to`, it should be + // relative to the parent route context. This is explained in `Note + // on `` values` in our migration guide from v5 as a means + // of disambiguation between `to` values that begin with `/` and + // those that do not. However, this is problematic for `to` values + // that do not provide a pathname. `to` can simply be a search or + // hash string, in which case we should assume that the navigation + // is relative to the current location's pathname and *not* the + // pathname from the parent route. + toPathname ? parentRoutePathname : currentLocationPathname, + basename + ); + (!!options.replace ? navigator.replace : navigator.push)( path, options.state @@ -446,7 +469,7 @@ export function useNavigate(): NavigateFunction { ); } }, - [basename, navigator, pathname] + [basename, navigator, parentRoutePathname, currentLocationPathname] ); return navigate;