From 7b787bf5ea6e23923d42c39125bdcd8cfc38adef Mon Sep 17 00:00:00 2001 From: Jaff Parker Date: Wed, 8 Jun 2022 08:40:23 -0400 Subject: [PATCH 1/5] fix: relative navigation from index routes (#8697) * fix: relative navigation from index routes * fix: tests --- contributors.yml | 2 + packages/react-router-dom-v5-compat/index.ts | 4 +- .../lib/components.tsx | 2 +- .../react-router/__tests__/navigate-test.tsx | 45 +++++++++++++++++++ packages/react-router/lib/hooks.tsx | 6 ++- packages/router/utils.ts | 7 ++- 6 files changed, 59 insertions(+), 7 deletions(-) diff --git a/contributors.yml b/contributors.yml index 5bc61ba49e..3c5e0f4454 100644 --- a/contributors.yml +++ b/contributors.yml @@ -68,3 +68,5 @@ - rtmann - williamsdyyz - vikingviolinist +- KutnerUri +- JaffParker diff --git a/packages/react-router-dom-v5-compat/index.ts b/packages/react-router-dom-v5-compat/index.ts index d504b8b796..63d48118f3 100644 --- a/packages/react-router-dom-v5-compat/index.ts +++ b/packages/react-router-dom-v5-compat/index.ts @@ -69,7 +69,7 @@ export type { Pathname, Search, RoutesProps, -} from "./react-router-dom"; +} from "../react-router-dom"; export { BrowserRouter, HashRouter, @@ -108,7 +108,7 @@ export { useResolvedPath, useRoutes, useSearchParams, -} from "./react-router-dom"; +} from "../react-router-dom"; export type { StaticRouterProps } from "./lib/components"; export { CompatRouter, CompatRoute, StaticRouter } from "./lib/components"; diff --git a/packages/react-router-dom-v5-compat/lib/components.tsx b/packages/react-router-dom-v5-compat/lib/components.tsx index 59ce5c1a91..9b2461a587 100644 --- a/packages/react-router-dom-v5-compat/lib/components.tsx +++ b/packages/react-router-dom-v5-compat/lib/components.tsx @@ -9,7 +9,7 @@ import { useHistory, Route as RouteV5 } from "react-router-dom"; // We are a wrapper around react-router-dom v6, so bring it in // and bundle it because an app can't have two versions of // react-router-dom in its package.json. -import { Router, Routes, Route } from "../react-router-dom"; +import { Router, Routes, Route } from "../../react-router-dom"; // v5 isn't in TypeScript, they'll also lose the @types/react-router with this // but not worried about that for now. diff --git a/packages/react-router/__tests__/navigate-test.tsx b/packages/react-router/__tests__/navigate-test.tsx index ef6ba56bc2..006daeb464 100644 --- a/packages/react-router/__tests__/navigate-test.tsx +++ b/packages/react-router/__tests__/navigate-test.tsx @@ -1,6 +1,7 @@ import * as React from "react"; import * as TestRenderer from "react-test-renderer"; import { MemoryRouter, Navigate, Routes, Route } from "react-router"; +import { Outlet } from "../lib/components"; describe("", () => { describe("with an absolute href", () => { @@ -45,5 +46,49 @@ describe("", () => { `); }); + + it("skips a level from an index route", () => { + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create( + + + + } /> + + About} /> + + + ); + }); + + expect(renderer.toJSON()).toMatchInlineSnapshot(` +

+ About +

+ `); + }); + + it("skips a level from a layout route", () => { + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create( + + + }> + } /> + + About} /> + + + ); + }); + + expect(renderer.toJSON()).toMatchInlineSnapshot(` +

+ About +

+ `); + }); }); }); diff --git a/packages/react-router/lib/hooks.tsx b/packages/react-router/lib/hooks.tsx index 1f5adfabf4..e5e9c663d6 100644 --- a/packages/react-router/lib/hooks.tsx +++ b/packages/react-router/lib/hooks.tsx @@ -158,6 +158,7 @@ export function useNavigate(): NavigateFunction { let routePathnamesJson = JSON.stringify( matches.map((match) => match.pathnameBase) ); + let isPathlessRoute = !!matches[matches.length - 1]?.route.index; let activeRef = React.useRef(false); React.useEffect(() => { @@ -182,7 +183,8 @@ export function useNavigate(): NavigateFunction { let path = resolveTo( to, JSON.parse(routePathnamesJson), - locationPathname + locationPathname, + isPathlessRoute ); if (basename !== "/") { @@ -195,7 +197,7 @@ export function useNavigate(): NavigateFunction { options ); }, - [basename, navigator, routePathnamesJson, locationPathname] + [routePathnamesJson, locationPathname, isPathlessRoute, basename, navigator] ); return navigate; diff --git a/packages/router/utils.ts b/packages/router/utils.ts index 607ebca205..cc7b570461 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -656,7 +656,8 @@ function resolvePathname(relativePath: string, fromPathname: string): string { export function resolveTo( toArg: To, routePathnames: string[], - locationPathname: string + locationPathname: string, + isPathlessRoute = false ): Path { let to = typeof toArg === "string" ? parsePath(toArg) : { ...toArg }; let toPathname = toArg === "" || to.pathname === "" ? "/" : to.pathname; @@ -672,7 +673,9 @@ export function resolveTo( if (toPathname == null) { from = locationPathname; } else { - let routePathnameIndex = routePathnames.length - 1; + let routePathnameIndex = isPathlessRoute + ? routePathnames.length - 2 + : routePathnames.length - 1; if (toPathname.startsWith("..")) { let toSegments = toPathname.split("/"); From cb5655843e827e4de47b55ca47fbe40e543d6401 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 8 Jun 2022 08:50:51 -0400 Subject: [PATCH 2/5] Handle nested pathless layout routes --- .../react-router/__tests__/navigate-test.tsx | 32 +++++++++++++++++-- packages/react-router/lib/hooks.tsx | 13 ++++++-- packages/router/utils.ts | 6 ++-- 3 files changed, 42 insertions(+), 9 deletions(-) diff --git a/packages/react-router/__tests__/navigate-test.tsx b/packages/react-router/__tests__/navigate-test.tsx index 006daeb464..890d4b643a 100644 --- a/packages/react-router/__tests__/navigate-test.tsx +++ b/packages/react-router/__tests__/navigate-test.tsx @@ -47,7 +47,7 @@ describe("", () => { `); }); - it("skips a level from an index route", () => { + it("handles upward navigatino from an index routes", () => { let renderer: TestRenderer.ReactTestRenderer; TestRenderer.act(() => { renderer = TestRenderer.create( @@ -69,7 +69,7 @@ describe("", () => { `); }); - it("skips a level from a layout route", () => { + it("handles upward navigation from inside a pathless layout route", () => { let renderer: TestRenderer.ReactTestRenderer; TestRenderer.act(() => { renderer = TestRenderer.create( @@ -90,5 +90,33 @@ describe("", () => { `); }); + + it("handles upward navigation from inside multiple pathless layout routes", () => { + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create( + + + + }> + }> + }> + } /> + + + + + About} /> + + + ); + }); + + expect(renderer.toJSON()).toMatchInlineSnapshot(` +

+ About +

+ `); + }); }); }); diff --git a/packages/react-router/lib/hooks.tsx b/packages/react-router/lib/hooks.tsx index e5e9c663d6..d4f1b73b81 100644 --- a/packages/react-router/lib/hooks.tsx +++ b/packages/react-router/lib/hooks.tsx @@ -158,8 +158,15 @@ export function useNavigate(): NavigateFunction { let routePathnamesJson = JSON.stringify( matches.map((match) => match.pathnameBase) ); - let isPathlessRoute = !!matches[matches.length - 1]?.route.index; + // figure out how many levels of "pathless" routes we have from the current + // leaf match, so we can properly handle relative .. links + let pathlessDepth = Math.max( + [...matches] + .reverse() + .findIndex((m) => !m.route.index && m.route.path?.length), + 0 + ); let activeRef = React.useRef(false); React.useEffect(() => { activeRef.current = true; @@ -184,7 +191,7 @@ export function useNavigate(): NavigateFunction { to, JSON.parse(routePathnamesJson), locationPathname, - isPathlessRoute + pathlessDepth ); if (basename !== "/") { @@ -197,7 +204,7 @@ export function useNavigate(): NavigateFunction { options ); }, - [routePathnamesJson, locationPathname, isPathlessRoute, basename, navigator] + [routePathnamesJson, locationPathname, pathlessDepth, basename, navigator] ); return navigate; diff --git a/packages/router/utils.ts b/packages/router/utils.ts index cc7b570461..6c10101896 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -657,7 +657,7 @@ export function resolveTo( toArg: To, routePathnames: string[], locationPathname: string, - isPathlessRoute = false + pathlessDepth = 0 ): Path { let to = typeof toArg === "string" ? parsePath(toArg) : { ...toArg }; let toPathname = toArg === "" || to.pathname === "" ? "/" : to.pathname; @@ -673,9 +673,7 @@ export function resolveTo( if (toPathname == null) { from = locationPathname; } else { - let routePathnameIndex = isPathlessRoute - ? routePathnames.length - 2 - : routePathnames.length - 1; + let routePathnameIndex = routePathnames.length - 1 - pathlessDepth; if (toPathname.startsWith("..")) { let toSegments = toPathname.split("/"); From 4d37127b1395fa0cd15c8a9ac88568cf03febb2d Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 8 Jun 2022 08:52:03 -0400 Subject: [PATCH 3/5] cleanups --- contributors.yml | 3 +-- packages/react-router-dom-v5-compat/index.ts | 4 ++-- packages/react-router-dom-v5-compat/lib/components.tsx | 2 +- packages/react-router/__tests__/navigate-test.tsx | 5 ++--- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/contributors.yml b/contributors.yml index 3c5e0f4454..b340059b5a 100644 --- a/contributors.yml +++ b/contributors.yml @@ -23,6 +23,7 @@ - hyesungoh - IbraRouisDev - Isammoc +- JaffParker - JakubDrozd - janpaepke - jimniels @@ -68,5 +69,3 @@ - rtmann - williamsdyyz - vikingviolinist -- KutnerUri -- JaffParker diff --git a/packages/react-router-dom-v5-compat/index.ts b/packages/react-router-dom-v5-compat/index.ts index 63d48118f3..d504b8b796 100644 --- a/packages/react-router-dom-v5-compat/index.ts +++ b/packages/react-router-dom-v5-compat/index.ts @@ -69,7 +69,7 @@ export type { Pathname, Search, RoutesProps, -} from "../react-router-dom"; +} from "./react-router-dom"; export { BrowserRouter, HashRouter, @@ -108,7 +108,7 @@ export { useResolvedPath, useRoutes, useSearchParams, -} from "../react-router-dom"; +} from "./react-router-dom"; export type { StaticRouterProps } from "./lib/components"; export { CompatRouter, CompatRoute, StaticRouter } from "./lib/components"; diff --git a/packages/react-router-dom-v5-compat/lib/components.tsx b/packages/react-router-dom-v5-compat/lib/components.tsx index 9b2461a587..59ce5c1a91 100644 --- a/packages/react-router-dom-v5-compat/lib/components.tsx +++ b/packages/react-router-dom-v5-compat/lib/components.tsx @@ -9,7 +9,7 @@ import { useHistory, Route as RouteV5 } from "react-router-dom"; // We are a wrapper around react-router-dom v6, so bring it in // and bundle it because an app can't have two versions of // react-router-dom in its package.json. -import { Router, Routes, Route } from "../../react-router-dom"; +import { Router, Routes, Route } from "../react-router-dom"; // v5 isn't in TypeScript, they'll also lose the @types/react-router with this // but not worried about that for now. diff --git a/packages/react-router/__tests__/navigate-test.tsx b/packages/react-router/__tests__/navigate-test.tsx index 890d4b643a..0f7a5b40f2 100644 --- a/packages/react-router/__tests__/navigate-test.tsx +++ b/packages/react-router/__tests__/navigate-test.tsx @@ -1,7 +1,6 @@ import * as React from "react"; import * as TestRenderer from "react-test-renderer"; -import { MemoryRouter, Navigate, Routes, Route } from "react-router"; -import { Outlet } from "../lib/components"; +import { MemoryRouter, Navigate, Outlet, Routes, Route } from "react-router"; describe("", () => { describe("with an absolute href", () => { @@ -47,7 +46,7 @@ describe("", () => { `); }); - it("handles upward navigatino from an index routes", () => { + it("handles upward navigation from an index routes", () => { let renderer: TestRenderer.ReactTestRenderer; TestRenderer.act(() => { renderer = TestRenderer.create( From 1acd125e2a36ea568b4162b688b0901669142ad8 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 8 Jun 2022 09:59:51 -0400 Subject: [PATCH 4/5] adjust approach to filter pathless routes --- .../react-router/__tests__/navigate-test.tsx | 79 ++++++++++++++++++- packages/react-router/lib/hooks.tsx | 25 +++--- packages/router/utils.ts | 5 +- 3 files changed, 92 insertions(+), 17 deletions(-) diff --git a/packages/react-router/__tests__/navigate-test.tsx b/packages/react-router/__tests__/navigate-test.tsx index 0f7a5b40f2..4a6446608d 100644 --- a/packages/react-router/__tests__/navigate-test.tsx +++ b/packages/react-router/__tests__/navigate-test.tsx @@ -90,13 +90,13 @@ describe("", () => { `); }); - it("handles upward navigation from inside multiple pathless layout routes", () => { + it("handles upward navigation from inside multiple pathless layout routes + index route", () => { let renderer: TestRenderer.ReactTestRenderer; TestRenderer.act(() => { renderer = TestRenderer.create( - + }> }> }> @@ -117,5 +117,80 @@ describe("", () => { `); }); + + it("handles upward navigation from inside multiple pathless layout routes + path route", () => { + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create( + + + }> + }> + }> + }> + } + /> + + + + + About} /> + + + ); + }); + + expect(renderer.toJSON()).toMatchInlineSnapshot(` +

+ About +

+ `); + }); + + it("handles parent navigation from inside multiple pathless layout routes", () => { + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create( + + + +

Home

+ + + } + > + }> + }> + }> + +

Page

+ + + } + /> + + + + + About} /> + + + ); + }); + + expect(renderer.toJSON()).toMatchInlineSnapshot(` +

+ Home +

+ `); + }); }); }); diff --git a/packages/react-router/lib/hooks.tsx b/packages/react-router/lib/hooks.tsx index d4f1b73b81..3c6e56dea6 100644 --- a/packages/react-router/lib/hooks.tsx +++ b/packages/react-router/lib/hooks.tsx @@ -155,18 +155,20 @@ export function useNavigate(): NavigateFunction { let { matches } = React.useContext(RouteContext); let { pathname: locationPathname } = useLocation(); - let routePathnamesJson = JSON.stringify( - matches.map((match) => match.pathnameBase) + // Ignore pathless matches (i.e., share the same pathname as their ancestor) + let pathContributingMatches = matches.filter( + (match, index) => + index === 0 || match.pathnameBase !== matches[index - 1].pathnameBase ); - // figure out how many levels of "pathless" routes we have from the current - // leaf match, so we can properly handle relative .. links - let pathlessDepth = Math.max( - [...matches] - .reverse() - .findIndex((m) => !m.route.index && m.route.path?.length), - 0 + if (matches.length > 0 && matches[matches.length - 1].route.index) { + pathContributingMatches.pop(); + } + + let routePathnamesJson = JSON.stringify( + pathContributingMatches.map((match) => match.pathnameBase) ); + let activeRef = React.useRef(false); React.useEffect(() => { activeRef.current = true; @@ -190,8 +192,7 @@ export function useNavigate(): NavigateFunction { let path = resolveTo( to, JSON.parse(routePathnamesJson), - locationPathname, - pathlessDepth + locationPathname ); if (basename !== "/") { @@ -204,7 +205,7 @@ export function useNavigate(): NavigateFunction { options ); }, - [routePathnamesJson, locationPathname, pathlessDepth, basename, navigator] + [routePathnamesJson, locationPathname, basename, navigator] ); return navigate; diff --git a/packages/router/utils.ts b/packages/router/utils.ts index 6c10101896..607ebca205 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -656,8 +656,7 @@ function resolvePathname(relativePath: string, fromPathname: string): string { export function resolveTo( toArg: To, routePathnames: string[], - locationPathname: string, - pathlessDepth = 0 + locationPathname: string ): Path { let to = typeof toArg === "string" ? parsePath(toArg) : { ...toArg }; let toPathname = toArg === "" || to.pathname === "" ? "/" : to.pathname; @@ -673,7 +672,7 @@ export function resolveTo( if (toPathname == null) { from = locationPathname; } else { - let routePathnameIndex = routePathnames.length - 1 - pathlessDepth; + let routePathnameIndex = routePathnames.length - 1; if (toPathname.startsWith("..")) { let toSegments = toPathname.split("/"); From 6ae50bc67a6189406bc838cb52c24d76945f0f7f Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Wed, 8 Jun 2022 10:02:29 -0400 Subject: [PATCH 5/5] undo reordering of deps array --- packages/react-router/lib/hooks.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-router/lib/hooks.tsx b/packages/react-router/lib/hooks.tsx index 3c6e56dea6..11ce9accee 100644 --- a/packages/react-router/lib/hooks.tsx +++ b/packages/react-router/lib/hooks.tsx @@ -205,7 +205,7 @@ export function useNavigate(): NavigateFunction { options ); }, - [routePathnamesJson, locationPathname, basename, navigator] + [basename, navigator, routePathnamesJson, locationPathname] ); return navigate;