Skip to content

Commit b19a80b

Browse files
fix: detect and error on invalid pathnames (#9375)
* fix: detect and error on invalid pathnames * Enhance error message * add changeset * Extract function * Update warning message Co-authored-by: Michael Jackson <[email protected]> * fix test error message Co-authored-by: Michael Jackson <[email protected]>
1 parent 540f94b commit b19a80b

File tree

3 files changed

+118
-1
lines changed

3 files changed

+118
-1
lines changed

.changeset/dirty-yaks-repair.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"react-router": patch
3+
"@remix-run/router": patch
4+
---
5+
6+
fix: throw error when receiving invalid path object (#9375)

packages/react-router/__tests__/useNavigate-test.tsx

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,82 @@ describe("useNavigate", () => {
164164
`);
165165
});
166166

167+
it("throws on invalid destination path objects", () => {
168+
function Home() {
169+
let navigate = useNavigate();
170+
171+
return (
172+
<div>
173+
<h1>Home</h1>
174+
<button onClick={() => navigate({ pathname: "/about/thing?search" })}>
175+
click 1
176+
</button>
177+
<button onClick={() => navigate({ pathname: "/about/thing#hash" })}>
178+
click 2
179+
</button>
180+
<button
181+
onClick={() => navigate({ pathname: "/about/thing?search#hash" })}
182+
>
183+
click 3
184+
</button>
185+
<button
186+
onClick={() =>
187+
navigate({
188+
pathname: "/about/thing",
189+
search: "?search#hash",
190+
})
191+
}
192+
>
193+
click 4
194+
</button>
195+
</div>
196+
);
197+
}
198+
199+
let renderer: TestRenderer.ReactTestRenderer;
200+
TestRenderer.act(() => {
201+
renderer = TestRenderer.create(
202+
<MemoryRouter initialEntries={["/home"]}>
203+
<Routes>
204+
<Route path="home" element={<Home />} />
205+
</Routes>
206+
</MemoryRouter>
207+
);
208+
});
209+
210+
expect(() =>
211+
TestRenderer.act(() => {
212+
renderer.root.findAllByType("button")[0].props.onClick();
213+
})
214+
).toThrowErrorMatchingInlineSnapshot(
215+
`"Cannot include a '?' character in a manually specified \`to.pathname\` field [{\\"pathname\\":\\"/about/thing?search\\"}]. Please separate it out to the \`to.search\` field. Alternatively you may provide the full path as a string in <Link to=\\"...\\"> and the router will parse it for you."`
216+
);
217+
218+
expect(() =>
219+
TestRenderer.act(() => {
220+
renderer.root.findAllByType("button")[1].props.onClick();
221+
})
222+
).toThrowErrorMatchingInlineSnapshot(
223+
`"Cannot include a '#' character in a manually specified \`to.pathname\` field [{\\"pathname\\":\\"/about/thing#hash\\"}]. Please separate it out to the \`to.hash\` field. Alternatively you may provide the full path as a string in <Link to=\\"...\\"> and the router will parse it for you."`
224+
);
225+
226+
expect(() =>
227+
TestRenderer.act(() => {
228+
renderer.root.findAllByType("button")[2].props.onClick();
229+
})
230+
).toThrowErrorMatchingInlineSnapshot(
231+
`"Cannot include a '?' character in a manually specified \`to.pathname\` field [{\\"pathname\\":\\"/about/thing?search#hash\\"}]. Please separate it out to the \`to.search\` field. Alternatively you may provide the full path as a string in <Link to=\\"...\\"> and the router will parse it for you."`
232+
);
233+
234+
expect(() =>
235+
TestRenderer.act(() => {
236+
renderer.root.findAllByType("button")[3].props.onClick();
237+
})
238+
).toThrowErrorMatchingInlineSnapshot(
239+
`"Cannot include a '#' character in a manually specified \`to.search\` field [{\\"pathname\\":\\"/about/thing\\",\\"search\\":\\"?search#hash\\"}]. Please separate it out to the \`to.hash\` field. Alternatively you may provide the full path as a string in <Link to=\\"...\\"> and the router will parse it for you."`
240+
);
241+
});
242+
167243
describe("with state", () => {
168244
it("adds the state to location.state", () => {
169245
function Home() {

packages/router/utils.ts

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -819,6 +819,22 @@ function resolvePathname(relativePath: string, fromPathname: string): string {
819819
return segments.length > 1 ? segments.join("/") : "/";
820820
}
821821

822+
function getInvalidPathError(
823+
char: string,
824+
field: string,
825+
dest: string,
826+
path: Partial<Path>
827+
) {
828+
return (
829+
`Cannot include a '${char}' character in a manually specified ` +
830+
`\`to.${field}\` field [${JSON.stringify(
831+
path
832+
)}]. Please separate it out to the ` +
833+
`\`to.${dest}\` field. Alternatively you may provide the full path as ` +
834+
`a string in <Link to="..."> and the router will parse it for you.`
835+
);
836+
}
837+
822838
/**
823839
* @private
824840
*/
@@ -828,7 +844,26 @@ export function resolveTo(
828844
locationPathname: string,
829845
isPathRelative = false
830846
): Path {
831-
let to = typeof toArg === "string" ? parsePath(toArg) : { ...toArg };
847+
let to: Partial<Path>;
848+
if (typeof toArg === "string") {
849+
to = parsePath(toArg);
850+
} else {
851+
to = { ...toArg };
852+
853+
invariant(
854+
!to.pathname || !to.pathname.includes("?"),
855+
getInvalidPathError("?", "pathname", "search", to)
856+
);
857+
invariant(
858+
!to.pathname || !to.pathname.includes("#"),
859+
getInvalidPathError("#", "pathname", "hash", to)
860+
);
861+
invariant(
862+
!to.search || !to.search.includes("#"),
863+
getInvalidPathError("#", "search", "hash", to)
864+
);
865+
}
866+
832867
let isEmptyPath = toArg === "" || to.pathname === "";
833868
let toPathname = isEmptyPath ? "/" : to.pathname;
834869

0 commit comments

Comments
 (0)