diff --git a/.changeset/revert-resolve-to-fix.md b/.changeset/revert-resolve-to-fix.md new file mode 100644 index 0000000000..9f23cf12e6 --- /dev/null +++ b/.changeset/revert-resolve-to-fix.md @@ -0,0 +1,7 @@ +--- +"react-router-dom": patch +"react-router": patch +"@remix-run/router": patch +--- + +Revert the `useResolvedPath` fix for splat routes due to a large number of applications that were relying on the buggy behavior (see https://github.com/remix-run/react-router/issues/11052#issuecomment-1836589329). We plan to re-introduce this fix behind a future flag in the next minor version. diff --git a/package.json b/package.json index 85c4525fe2..ce06a4af7e 100644 --- a/package.json +++ b/package.json @@ -110,7 +110,7 @@ }, "filesize": { "packages/router/dist/router.umd.min.js": { - "none": "49.4 kB" + "none": "49.3 kB" }, "packages/react-router/dist/react-router.production.min.js": { "none": "13.9 kB" diff --git a/packages/react-router-dom/__tests__/data-browser-router-test.tsx b/packages/react-router-dom/__tests__/data-browser-router-test.tsx index af2f85c214..8bd9e7b1b0 100644 --- a/packages/react-router-dom/__tests__/data-browser-router-test.tsx +++ b/packages/react-router-dom/__tests__/data-browser-router-test.tsx @@ -2942,44 +2942,6 @@ function testDomRouter( "/foo/bar" ); }); - - it("includes param portion of path when no action is specified (inline splat)", async () => { - let router = createTestRouter( - createRoutesFromElements( - - - } /> - - - ), - { - window: getWindow("/foo/bar"), - } - ); - let { container } = render(); - - expect(container.querySelector("form")?.getAttribute("action")).toBe( - "/foo/bar" - ); - }); - - it("includes splat portion of path when no action is specified (nested splat)", async () => { - let router = createTestRouter( - createRoutesFromElements( - - } /> - - ), - { - window: getWindow("/foo/bar"), - } - ); - let { container } = render(); - - expect(container.querySelector("form")?.getAttribute("action")).toBe( - "/foo/bar" - ); - }); }); describe("splat routes", () => { @@ -2999,7 +2961,7 @@ function testDomRouter( let { container } = render(); expect(container.querySelector("form")?.getAttribute("action")).toBe( - "/foo/bar?a=1" + "/foo?a=1" ); }); @@ -3019,7 +2981,7 @@ function testDomRouter( let { container } = render(); expect(container.querySelector("form")?.getAttribute("action")).toBe( - "/foo/bar" + "/foo" ); }); @@ -3039,25 +3001,7 @@ function testDomRouter( let { container } = render(); expect(container.querySelector("form")?.getAttribute("action")).toBe( - "/foo/bar" - ); - }); - - it("includes splat portion of path when no action is specified (inline splat)", async () => { - let router = createTestRouter( - createRoutesFromElements( - - } /> - - ), - { - window: getWindow("/foo/bar/baz"), - } - ); - let { container } = render(); - - expect(container.querySelector("form")?.getAttribute("action")).toBe( - "/foo/bar/baz" + "/foo" ); }); }); @@ -3179,13 +3123,10 @@ function testDomRouter( it("navigates relative to the URL for splat routes", async () => { let router = createTestRouter( createRoutesFromElements( - - - } - /> - + } + /> ), { window: getWindow("/inbox/messages/1/2/3"), @@ -3194,7 +3135,7 @@ function testDomRouter( let { container } = render(); expect(container.querySelector("form")?.getAttribute("action")).toBe( - "/inbox/messages/1/2" + "/inbox" ); }); }); diff --git a/packages/react-router-dom/__tests__/link-href-test.tsx b/packages/react-router-dom/__tests__/link-href-test.tsx index 51de95ac07..762f972ddc 100644 --- a/packages/react-router-dom/__tests__/link-href-test.tsx +++ b/packages/react-router-dom/__tests__/link-href-test.tsx @@ -530,7 +530,7 @@ describe(" href", () => { }); expect(renderer.root.findByType("a").props.href).toEqual( - "/inbox/messages/abc" + "/inbox/messages" ); }); diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx index 7fbaff3ae6..c3a1bd947c 100644 --- a/packages/react-router-dom/index.tsx +++ b/packages/react-router-dom/index.tsx @@ -1543,8 +1543,10 @@ export function useFormAction( // object referenced by useMemo inside useResolvedPath let path = { ...useResolvedPath(action ? action : ".", { relative }) }; - // If no action was specified, browsers will persist current search params - // when determining the path, so match that behavior + // Previously we set the default action to ".". The problem with this is that + // `useResolvedPath(".")` excludes search params of the resolved URL. This is + // the intended behavior of when "." is specifically provided as + // the form action, but inconsistent w/ browsers when the action is omitted. // https://github.com/remix-run/remix/issues/927 let location = useLocation(); if (action == null) { diff --git a/packages/react-router/__tests__/useResolvedPath-test.tsx b/packages/react-router/__tests__/useResolvedPath-test.tsx index b4f3d70ae2..d6615e865f 100644 --- a/packages/react-router/__tests__/useResolvedPath-test.tsx +++ b/packages/react-router/__tests__/useResolvedPath-test.tsx @@ -1,16 +1,7 @@ import * as React from "react"; import * as TestRenderer from "react-test-renderer"; import type { Path } from "react-router"; -import { - MemoryRouter, - Routes, - Route, - RouterProvider, - createMemoryRouter, - useResolvedPath, - Outlet, - useParams, -} from "react-router"; +import { MemoryRouter, Routes, Route, useResolvedPath } from "react-router"; function ShowResolvedPath({ path }: { path: string | Path }) { return 
{JSON.stringify(useResolvedPath(path))}
; @@ -94,7 +85,7 @@ describe("useResolvedPath", () => { }); describe("in a splat route", () => { - it("resolves . to the route path (nested splat)", () => { + it("resolves . to the route path", () => { let renderer: TestRenderer.ReactTestRenderer; TestRenderer.act(() => { renderer = TestRenderer.create( @@ -108,312 +99,11 @@ describe("useResolvedPath", () => { ); }); - expect(renderer.toJSON()).toMatchInlineSnapshot(` - 
-          {"pathname":"/users/mj","search":"","hash":""}
-
- `); - }); - - it("resolves .. to the parent route path (nested splat)", () => { - let renderer: TestRenderer.ReactTestRenderer; - TestRenderer.act(() => { - renderer = TestRenderer.create( - - - - } /> - - - - ); - }); - expect(renderer.toJSON()).toMatchInlineSnapshot(` 
           {"pathname":"/users","search":"","hash":""}
`); }); - - it("resolves . to the route path (inline splat)", () => { - let renderer: TestRenderer.ReactTestRenderer; - TestRenderer.act(() => { - renderer = TestRenderer.create( - - - - } /> - - - - ); - }); - - expect(renderer.toJSON()).toMatchInlineSnapshot(` - 
-          {"pathname":"/users/name/mj","search":"","hash":""}
-
- `); - }); - - it("resolves .. to the parent route path (inline splat)", () => { - let renderer: TestRenderer.ReactTestRenderer; - TestRenderer.act(() => { - renderer = TestRenderer.create( - - - - } /> - - - - ); - }); - - expect(renderer.toJSON()).toMatchInlineSnapshot(` - 
-          {"pathname":"/users","search":"","hash":""}
-
- `); - }); - - it("resolves . to the route path (descendant route)", () => { - let renderer: TestRenderer.ReactTestRenderer; - TestRenderer.act(() => { - renderer = TestRenderer.create( - - - - } /> - - } - /> - - - ); - }); - - expect(renderer.toJSON()).toMatchInlineSnapshot(` - 
-          {"pathname":"/users/mj","search":"","hash":""}
-
- `); - }); - - it("resolves .. to the parent route path (descendant route)", () => { - let renderer: TestRenderer.ReactTestRenderer; - TestRenderer.act(() => { - renderer = TestRenderer.create( - - - - } /> - - } - /> - - - ); - }); - - expect(renderer.toJSON()).toMatchInlineSnapshot(` - 
-          {"pathname":"/users","search":"","hash":""}
-
- `); - }); - }); - - describe("in a param route", () => { - it("resolves . to the route path (nested param)", () => { - let renderer: TestRenderer.ReactTestRenderer; - TestRenderer.act(() => { - renderer = TestRenderer.create( - - - - } /> - - - - ); - }); - - expect(renderer.toJSON()).toMatchInlineSnapshot(` - 
-          {"pathname":"/users/mj","search":"","hash":""}
-
- `); - }); - - it("resolves .. to the parent route (nested param)", () => { - let renderer: TestRenderer.ReactTestRenderer; - TestRenderer.act(() => { - renderer = TestRenderer.create( - - - - } /> - - - - ); - }); - - expect(renderer.toJSON()).toMatchInlineSnapshot(` - 
-          {"pathname":"/users","search":"","hash":""}
-
- `); - }); - - it("resolves . to the route path (inline param)", () => { - let renderer: TestRenderer.ReactTestRenderer; - TestRenderer.act(() => { - renderer = TestRenderer.create( - - - - } - /> - - - - ); - }); - - expect(renderer.toJSON()).toMatchInlineSnapshot(` - 
-          {"pathname":"/users/name/mj","search":"","hash":""}
-
- `); - }); - - it("resolves .. to the parent route (inline param)", () => { - let renderer: TestRenderer.ReactTestRenderer; - TestRenderer.act(() => { - renderer = TestRenderer.create( - - - - } - /> - - - - ); - }); - - expect(renderer.toJSON()).toMatchInlineSnapshot(` - 
-          {"pathname":"/users","search":"","hash":""}
-
- `); - }); - }); - - // Follow up test to https://github.com/remix-run/react-router/pull/10983 to - // ensure we do this consistently across route types - it("resolves relative paths consistently across route types", async () => { - let router = createMemoryRouter([ - { - path: "/", - Component: Outlet, - children: [ - { - path: "static/foo", - Component: () =>

Static:{useResolvedPath("foo").pathname}

, - }, - { - path: "static/foo/foo", - Component: () => ( -

Static:{useResolvedPath("foo/foo").pathname}

- ), - }, - { - path: "dynamic/:param", - Component: () =>

Dynamic:{useResolvedPath("foo").pathname}

, - }, - { - path: "dynamic/:param1/:param2", - Component: () => ( -

Dynamic:{useResolvedPath("foo/foo").pathname}

- ), - }, - { - path: "splat/*", - Component: () => ( -

Splat:{useResolvedPath(useParams()["*"]).pathname}

- ), - }, - ], - }, - ]); - - let renderer: TestRenderer.ReactTestRenderer; - TestRenderer.act(() => { - renderer = TestRenderer.create(); - }); - // @ts-expect-error - if (!renderer) throw new Error("renderer not defined"); - - await TestRenderer.act(() => router.navigate("/static/foo")); - expect(renderer.toJSON()).toMatchInlineSnapshot(` -

- Static: - /static/foo/foo -

- `); - - await TestRenderer.act(() => router.navigate("/dynamic/foo")); - expect(renderer.toJSON()).toMatchInlineSnapshot(` -

- Dynamic: - /dynamic/foo/foo -

- `); - - await TestRenderer.act(() => router.navigate("/splat/foo")); - expect(renderer.toJSON()).toMatchInlineSnapshot(` -

- Splat: - /splat/foo/foo -

- `); - - await TestRenderer.act(() => router.navigate("/static/foo/foo")); - expect(renderer.toJSON()).toMatchInlineSnapshot(` -

- Static: - /static/foo/foo/foo/foo -

- `); - - await TestRenderer.act(() => router.navigate("/dynamic/foo/foo")); - expect(renderer.toJSON()).toMatchInlineSnapshot(` -

- Dynamic: - /dynamic/foo/foo/foo/foo -

- `); - - await TestRenderer.act(() => router.navigate("/splat/foo/foo")); - expect(renderer.toJSON()).toMatchInlineSnapshot(` -

- Splat: - /splat/foo/foo/foo/foo -

- `); }); }); diff --git a/packages/react-router/lib/components.tsx b/packages/react-router/lib/components.tsx index 31ec6cdf9f..3c4a95e2f2 100644 --- a/packages/react-router/lib/components.tsx +++ b/packages/react-router/lib/components.tsx @@ -14,7 +14,7 @@ import { AbortedDeferredError, Action as NavigationType, createMemoryHistory, - UNSAFE_getResolveToMatches as getResolveToMatches, + UNSAFE_getPathContributingMatches as getPathContributingMatches, UNSAFE_invariant as invariant, parsePath, resolveTo, @@ -281,7 +281,7 @@ export function Navigate({ // StrictMode they navigate to the same place let path = resolveTo( to, - getResolveToMatches(matches), + getPathContributingMatches(matches).map((match) => match.pathnameBase), locationPathname, relative === "path" ); diff --git a/packages/react-router/lib/hooks.tsx b/packages/react-router/lib/hooks.tsx index 094368d994..390553abee 100644 --- a/packages/react-router/lib/hooks.tsx +++ b/packages/react-router/lib/hooks.tsx @@ -18,7 +18,7 @@ import { IDLE_BLOCKER, Action as NavigationType, UNSAFE_convertRouteMatchToUiMatch as convertRouteMatchToUiMatch, - UNSAFE_getResolveToMatches as getResolveToMatches, + UNSAFE_getPathContributingMatches as getPathContributingMatches, UNSAFE_invariant as invariant, isRouteErrorResponse, joinPaths, @@ -197,7 +197,9 @@ function useNavigateUnstable(): NavigateFunction { let { matches } = React.useContext(RouteContext); let { pathname: locationPathname } = useLocation(); - let routePathnamesJson = JSON.stringify(getResolveToMatches(matches)); + let routePathnamesJson = JSON.stringify( + getPathContributingMatches(matches).map((match) => match.pathnameBase) + ); let activeRef = React.useRef(false); useIsomorphicLayoutEffect(() => { @@ -309,7 +311,10 @@ export function useResolvedPath( ): Path { let { matches } = React.useContext(RouteContext); let { pathname: locationPathname } = useLocation(); - let routePathnamesJson = JSON.stringify(getResolveToMatches(matches)); + + let routePathnamesJson = JSON.stringify( + getPathContributingMatches(matches).map((match) => match.pathnameBase) + ); return React.useMemo( () => diff --git a/packages/router/__tests__/path-resolution-test.ts b/packages/router/__tests__/path-resolution-test.ts index 0f3501fb8c..6025fd2ea9 100644 --- a/packages/router/__tests__/path-resolution-test.ts +++ b/packages/router/__tests__/path-resolution-test.ts @@ -138,7 +138,7 @@ describe("path resolution", () => { path: "*", }, ], - "/foo/bar", + "/foo", false ); }); @@ -391,21 +391,6 @@ describe("path resolution", () => { ]); }); - it("from an index route", () => { - assertRoutingToChild([ - { - path: "bar", - children: [ - { - id: "activeRoute", - index: true, - }, - { path: "baz" }, - ], - }, - ]); - }); - it("from a dynamic param route", () => { assertRoutingToChild([ { @@ -415,15 +400,6 @@ describe("path resolution", () => { }, ]); }); - - it("from a splat route", () => { - assertRoutingToChild([ - { - id: "activeRoute", - path: "*", - }, - ]); - }); /* eslint-enable jest/expect-expect */ }); diff --git a/packages/router/index.ts b/packages/router/index.ts index 060360d34a..cbacea8bf8 100644 --- a/packages/router/index.ts +++ b/packages/router/index.ts @@ -87,7 +87,7 @@ export { ErrorResponseImpl as UNSAFE_ErrorResponseImpl, convertRoutesToDataRoutes as UNSAFE_convertRoutesToDataRoutes, convertRouteMatchToUiMatch as UNSAFE_convertRouteMatchToUiMatch, - getResolveToMatches as UNSAFE_getResolveToMatches, + getPathContributingMatches as UNSAFE_getPathContributingMatches, } from "./utils"; export { diff --git a/packages/router/router.ts b/packages/router/router.ts index 58aa6335bf..21453256af 100644 --- a/packages/router/router.ts +++ b/packages/router/router.ts @@ -40,7 +40,6 @@ import { convertRouteMatchToUiMatch, convertRoutesToDataRoutes, getPathContributingMatches, - getResolveToMatches, immutableRouteKeys, isRouteErrorResponse, joinPaths, @@ -3341,7 +3340,7 @@ function normalizeTo( // Resolve the relative path let path = resolveTo( to ? to : ".", - getResolveToMatches(contextualMatches), + getPathContributingMatches(contextualMatches).map((m) => m.pathnameBase), stripBasename(location.pathname, basename) || location.pathname, relative === "path" ); diff --git a/packages/router/utils.ts b/packages/router/utils.ts index 06e9a1db7f..ccbe5537b7 100644 --- a/packages/router/utils.ts +++ b/packages/router/utils.ts @@ -1145,17 +1145,6 @@ export function getPathContributingMatches< ); } -// Return the array of pathnames for the current route matches - used to -// generate the routePathnames input for resolveTo() -export function getResolveToMatches< - T extends AgnosticRouteMatch = AgnosticRouteMatch ->(matches: T[]) { - // Use the full pathname for the leaf match so we include splat values for "." links - return getPathContributingMatches(matches).map((match, idx) => - idx === matches.length - 1 ? match.pathname : match.pathnameBase - ); -} - /** * @private */