fix: preserve search params on unspecified <Form> action #9060
Conversation
🦋 Changeset detectedLatest commit: 4e5925f The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| } | ||
|
|
||
| expect(container.querySelector("form")?.getAttribute("action")).toBe( | ||
| "/?a=1#hash" |
There was a problem hiding this comment.
Preserves everything if no action is specified
| ); | ||
| } | ||
|
|
||
| expect(container.querySelector("form")?.getAttribute("action")).toBe("/"); |
There was a problem hiding this comment.
But only keeps pathname for action="."
| @@ -3320,7 +3320,7 @@ describe("a router", () => { | |||
| expect(t.router.state.navigation.formData).toBeUndefined(); | |||
| }); | |||
|
|
|||
| it("merges URLSearchParams for formMethod=get", async () => { | |||
| it("does not preserve existing 'action' URLSearchParams for formMethod='get'", async () => { | |||
There was a problem hiding this comment.
This was a previous misunderstanding on my part - browser do indeed blow away query params on GET submissions. I thought it would just append them
packages/react-router-dom/index.tsx
Outdated
| let [match] = routeContext.matches.slice(-1); | ||
| let { pathname, search } = useResolvedPath(action); | ||
| let path = useResolvedPath(action || location); |
There was a problem hiding this comment.
I think this should still resolve the path based on "." if the action is undefined, no? Does that make a difference here? My reading of resolveTo makes me believe that it would if we were in a nested route situation.
There was a problem hiding this comment.
I think that puts us back in the same scenario though where we lose search params? Down inside useResolvedPath -> resolveTo -> parsePath we end up with a Path of { pathname: '.' } without any search or hash. By passing the current location when no action is specified we preserve those throughout?
Just to be safe, I added a few tests for the nested route scenario to confirm they're handled - and they break if we change to let path = useResolvedPath(action || '.')
React-router implementation of remix-run/remix#3697
End state is the behavior specified in @ryanflorence's comment:
<Form method="get" action="."><Form method="get"><form method="post" action="."><form method="post">