Skip to content

Commit

Permalink
Revert useResolvedPath fix in splat routes (#11078)
Browse files Browse the repository at this point in the history
* Revert "Fix other code paths for resolveTo from a splat route (#11045)"

This reverts commit 58d421f.

* Revert "Add additional test case for #10983"

This reverts commit f320378.

* Revert "Fix issues with useFormAction/useResolvedPath for dot paths in param/splat routes (#10983)"

This reverts commit fe066bd.

* Fix test

* Add changeset
  • Loading branch information
brophdawg11 authored Dec 1, 2023
1 parent 30917ae commit 9cfd99d
Show file tree
Hide file tree
Showing 12 changed files with 36 additions and 427 deletions.
7 changes: 7 additions & 0 deletions .changeset/revert-resolve-to-fix.md
Original file line number Diff line number Diff line change
@@ -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.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
75 changes: 8 additions & 67 deletions packages/react-router-dom/__tests__/data-browser-router-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<Route path="/">
<Route path="foo">
<Route path=":param" element={<NoActionComponent />} />
</Route>
</Route>
),
{
window: getWindow("/foo/bar"),
}
);
let { container } = render(<RouterProvider router={router} />);

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(
<Route path="/">
<Route path="foo/:param" element={<NoActionComponent />} />
</Route>
),
{
window: getWindow("/foo/bar"),
}
);
let { container } = render(<RouterProvider router={router} />);

expect(container.querySelector("form")?.getAttribute("action")).toBe(
"/foo/bar"
);
});
});

describe("splat routes", () => {
Expand All @@ -2999,7 +2961,7 @@ function testDomRouter(
let { container } = render(<RouterProvider router={router} />);

expect(container.querySelector("form")?.getAttribute("action")).toBe(
"/foo/bar?a=1"
"/foo?a=1"
);
});

Expand All @@ -3019,7 +2981,7 @@ function testDomRouter(
let { container } = render(<RouterProvider router={router} />);

expect(container.querySelector("form")?.getAttribute("action")).toBe(
"/foo/bar"
"/foo"
);
});

Expand All @@ -3039,25 +3001,7 @@ function testDomRouter(
let { container } = render(<RouterProvider router={router} />);

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(
<Route path="/">
<Route path="foo/*" element={<NoActionComponent />} />
</Route>
),
{
window: getWindow("/foo/bar/baz"),
}
);
let { container } = render(<RouterProvider router={router} />);

expect(container.querySelector("form")?.getAttribute("action")).toBe(
"/foo/bar/baz"
"/foo"
);
});
});
Expand Down Expand Up @@ -3179,13 +3123,10 @@ function testDomRouter(
it("navigates relative to the URL for splat routes", async () => {
let router = createTestRouter(
createRoutesFromElements(
<Route path="inbox">
<Route path="messages" />
<Route
path="messages/*"
element={<Form action=".." relative="path" />}
/>
</Route>
<Route
path="inbox/messages/*"
element={<Form action=".." relative="path" />}
/>
),
{
window: getWindow("/inbox/messages/1/2/3"),
Expand All @@ -3194,7 +3135,7 @@ function testDomRouter(
let { container } = render(<RouterProvider router={router} />);

expect(container.querySelector("form")?.getAttribute("action")).toBe(
"/inbox/messages/1/2"
"/inbox"
);
});
});
Expand Down
2 changes: 1 addition & 1 deletion packages/react-router-dom/__tests__/link-href-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ describe("<Link> href", () => {
});

expect(renderer.root.findByType("a").props.href).toEqual(
"/inbox/messages/abc"
"/inbox/messages"
);
});

Expand Down
6 changes: 4 additions & 2 deletions packages/react-router-dom/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading

0 comments on commit 9cfd99d

Please sign in to comment.