Skip to content

Commit

Permalink
Fix issues with useFormAction/useResolvedPath for dot paths in param/…
Browse files Browse the repository at this point in the history 
…splat routes (#10983)
  • Loading branch information
@brophdawg11
brophdawg11 authored Nov 3, 2023
1 parent 4f6c454 commit fe066bd
Show file tree
Hide file tree
Showing 6 changed files with 322 additions and 10 deletions.
7 changes: 7 additions & 0 deletions .changeset/fix-resolve-path.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"react-router": patch
---

Fix bug in `useResolvedPath` that would cause `useResolvedPath(".")` in a splat route to lose the splat portion of the URL path.

- ⚠️ This fixes a quite long-standing bug specifically for `"."` paths inside a splat route which incorrectly dropped the splat portion of the URL. If you are relative routing via `"."` inside a splat route in your application you should double check that your logic is not relying on this buggy behavior and update accordingly.
102 changes: 99 additions & 3 deletions packages/react-router-dom/__tests__/data-browser-router-test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2653,6 +2653,46 @@ function testDomRouter(
"/foo/bar"
);
});

it("does not include dynamic parameters from a parent layout route", async () => {
let router = createTestRouter(
createRoutesFromElements(
<Route path="/">
<Route path="foo" element={<ActionEmptyComponent />}>
<Route path=":param" element={<h1>Param</h1>} />
</Route>
</Route>
),
{
window: getWindow("/foo/bar"),
}
);
let { container } = render(<RouterProvider router={router} />);

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

it("does not include splat parameters from a parent layout route", async () => {
let router = createTestRouter(
createRoutesFromElements(
<Route path="/">
<Route path="foo" element={<ActionEmptyComponent />}>
<Route path="*" element={<h1>Splat</h1>} />
</Route>
</Route>
),
{
window: getWindow("/foo/bar/baz/qux"),
}
);
let { container } = render(<RouterProvider router={router} />);

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

describe("index routes", () => {
Expand Down Expand Up @@ -2876,6 +2916,44 @@ 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 @@ -2895,7 +2973,7 @@ function testDomRouter(
let { container } = render(<RouterProvider router={router} />);

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

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

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

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

expect(container.querySelector("form")?.getAttribute("action")).toBe(
"/foo"
"/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"
);
});
});
Expand Down
2 changes: 1 addition & 1 deletion packages/react-router-dom/__tests__/link-href-test.tsx
View file
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"
"/inbox/messages/abc"
);
});

Expand Down
6 changes: 2 additions & 4 deletions packages/react-router-dom/index.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1478,10 +1478,8 @@ export function useFormAction(
// object referenced by useMemo inside useResolvedPath
let path = { ...useResolvedPath(action ? action : ".", { relative }) };

// 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.
// If no action was specified, browsers will persist current search params
// when determining the path, so match that behavior
// https://github.com/remix-run/remix/issues/927
let location = useLocation();
if (action == null) {
Expand Down
209 changes: 208 additions & 1 deletion packages/react-router/__tests__/useResolvedPath-test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ describe("useResolvedPath", () => {
});

describe("in a splat route", () => {
it("resolves . to the route path", () => {
it("resolves . to the route path (nested splat)", () => {
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(
Expand All @@ -99,6 +99,213 @@ describe("useResolvedPath", () => {
);
});

expect(renderer.toJSON()).toMatchInlineSnapshot(`
<pre>
{"pathname":"/users/mj","search":"","hash":""}
</pre>
`);
});

it("resolves .. to the parent route path (nested splat)", () => {
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(
<MemoryRouter initialEntries={["/users/mj"]}>
<Routes>
<Route path="/users">
<Route path="*" element={<ShowResolvedPath path=".." />} />
</Route>
</Routes>
</MemoryRouter>
);
});

expect(renderer.toJSON()).toMatchInlineSnapshot(`
<pre>
{"pathname":"/users","search":"","hash":""}
</pre>
`);
});

it("resolves . to the route path (inline splat)", () => {
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(
<MemoryRouter initialEntries={["/users/name/mj"]}>
<Routes>
<Route path="/users">
<Route path="name/*" element={<ShowResolvedPath path="." />} />
</Route>
</Routes>
</MemoryRouter>
);
});

expect(renderer.toJSON()).toMatchInlineSnapshot(`
<pre>
{"pathname":"/users/name/mj","search":"","hash":""}
</pre>
`);
});

it("resolves .. to the parent route path (inline splat)", () => {
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(
<MemoryRouter initialEntries={["/users/name/mj"]}>
<Routes>
<Route path="/users">
<Route path="name/*" element={<ShowResolvedPath path=".." />} />
</Route>
</Routes>
</MemoryRouter>
);
});

expect(renderer.toJSON()).toMatchInlineSnapshot(`
<pre>
{"pathname":"/users","search":"","hash":""}
</pre>
`);
});

it("resolves . to the route path (descendant route)", () => {
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(
<MemoryRouter initialEntries={["/users/mj"]}>
<Routes>
<Route
path="/users/*"
element={
<Routes>
<Route path="mj" element={<ShowResolvedPath path="." />} />
</Routes>
}
/>
</Routes>
</MemoryRouter>
);
});

expect(renderer.toJSON()).toMatchInlineSnapshot(`
<pre>
{"pathname":"/users/mj","search":"","hash":""}
</pre>
`);
});

it("resolves .. to the parent route path (descendant route)", () => {
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(
<MemoryRouter initialEntries={["/users/mj"]}>
<Routes>
<Route
path="/users/*"
element={
<Routes>
<Route path="mj" element={<ShowResolvedPath path=".." />} />
</Routes>
}
/>
</Routes>
</MemoryRouter>
);
});

expect(renderer.toJSON()).toMatchInlineSnapshot(`
<pre>
{"pathname":"/users","search":"","hash":""}
</pre>
`);
});
});

describe("in a param route", () => {
it("resolves . to the route path (nested param)", () => {
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(
<MemoryRouter initialEntries={["/users/mj"]}>
<Routes>
<Route path="/users">
<Route path=":name" element={<ShowResolvedPath path="." />} />
</Route>
</Routes>
</MemoryRouter>
);
});

expect(renderer.toJSON()).toMatchInlineSnapshot(`
<pre>
{"pathname":"/users/mj","search":"","hash":""}
</pre>
`);
});

it("resolves .. to the parent route (nested param)", () => {
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(
<MemoryRouter initialEntries={["/users/mj"]}>
<Routes>
<Route path="/users">
<Route path=":name" element={<ShowResolvedPath path=".." />} />
</Route>
</Routes>
</MemoryRouter>
);
});

expect(renderer.toJSON()).toMatchInlineSnapshot(`
<pre>
{"pathname":"/users","search":"","hash":""}
</pre>
`);
});

it("resolves . to the route path (inline param)", () => {
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(
<MemoryRouter initialEntries={["/users/name/mj"]}>
<Routes>
<Route path="/users">
<Route
path="name/:name"
element={<ShowResolvedPath path="." />}
/>
</Route>
</Routes>
</MemoryRouter>
);
});

expect(renderer.toJSON()).toMatchInlineSnapshot(`
<pre>
{"pathname":"/users/name/mj","search":"","hash":""}
</pre>
`);
});

it("resolves .. to the parent route (inline param)", () => {
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(
<MemoryRouter initialEntries={["/users/name/mj"]}>
<Routes>
<Route path="/users">
<Route
path="name/:name"
element={<ShowResolvedPath path=".." />}
/>
</Route>
</Routes>
</MemoryRouter>
);
});

expect(renderer.toJSON()).toMatchInlineSnapshot(`
<pre>
{"pathname":"/users","search":"","hash":""}
Expand Down
Loading

0 comments on commit fe066bd

Please sign in to comment.