`Form` post submission does not retain search parameters #3133

TheRealFlyingCoder · 2022-05-08T23:27:31Z

What version of Remix are you using?

1.4.3

Steps to Reproduce

Impement a route that includes search params in the url.

/my-route?param=1234

On the page include a post form

<Form method="post">
        <button type="submit>Submit</button>
</Form>

Native browser behaviour can be seen here thanks to @kiliman

https://codesandbox.io/s/form-action-params-8r86qw?file=/app/routes/test.tsx:732-987

Expected Behavior

Following the native browser behaviour, this should post back to /my-route?param=1234

Note that a method="get" post would natively replace the search params by the nature of the submission method

Actual Behavior

The form will post to the base path /my-route and trim off the search params

The text was updated successfully, but these errors were encountered:

manV · 2022-05-10T09:28:23Z

consider this scenario

I have a site with login functionality. There are routes "/" and "/settings" and to access these routes, the user should be authenticated. The site is hosted on xyz.com

the user goes to xyz.com/settings
the user is not logged in, so gets redirected to xyz.com/login?redirectTo=%2Fsettings
the user enters incorrect inputs on login form
form submitted and backend returned validation error
?redirectTo=%2Fsettings query parameter is lost
user enters correct credentials, is authenticated and gets redirected to "/" which is not correct.

kiliman · 2022-05-10T14:13:09Z

@manV Yes, fixing this issue will solve your scenario.

You can "fix" it now, by manually specifying your action.

const location = useLocation()
return <Form action={`${location.pathname}${location.search}`}/>

The actual fix is to essentially do this automatically if action is not specified, as that is standard browser behavior.

nivekithan · 2022-05-14T14:15:20Z

I think problem lies in useFormAction,

https://github.com/remix-run/react-router/blob/931cf23a54373485607b3727dbd5f9575fe58ebe/packages/react-router/lib/hooks.tsx#L244-L256

As of now code looks like this

export function useFormAction(
  action = ".",
  // TODO: Remove method param in v2 as it's no longer needed and is a breaking change
  method: FormMethod = "get"
): string {
  let { id } = useRemixRouteContext();
  let path = useResolvedPath(action);
  let search = path.search;
  let isIndexRoute = id.endsWith("/index");

  if (action === "." && isIndexRoute) {
    search = search ? search.replace(/^\?/, "?index&") : "?index";
  }

  return path.pathname + search;
}

According to docs for useResolvedPath from https://reactrouter.com/docs/en/v6/hooks/use-resolved-path

This hook resolves the pathname of the location in the given to value against the pathname of the current location.

I think what that means is that only resolves only pathName and ignores the search key. Therefore returned value from useResolvedPath looks like this

{hash: ""
pathname: "/login"
search: ""}

even when the url is /login?redirectTo=%2Fsettings. I am not sure whether it is intended behaviour or not.

At commit a3ae536

the code was changed from

export function useFormAction(action = "."): string {
  let path = useResolvedPath(action);
  let location = useLocation();
  return path.pathname + location.search;
}

to this

export function useFormAction(action = "."): string {
  let { id } = useRemixRouteContext();
  let path = useResolvedPath(action);
  let search = path.search;

  let isIndexRoute = id.endsWith("/index");

  if (isIndexRoute && action === ".") {
    search = "?index";
  }

  return path.pathname + search;
}

As you can see previously we have used only useLocation to get search not useResolvedPath, so I thought changing

let search = path.search;

to this will work

let search = useLocation().search;

it did, but now I am getting whole new error

Warning: Prop `action` did not match. Server: "/login" Client: "/login?redirectTo=%2Fsettings"
    at form
    at http://localhost:3000/build/_shared/chunk-D4PYBZR3.js:4698:3
    at Form
    at label
    at div
    at LoginPage (http://localhost:3000/build/routes/login-QKI5NRX4.js:15:16)
    at RemixRoute (http://localhost:3000/build/_shared/chunk-D4PYBZR3.js:4352:3)
    at Outlet (http://localhost:3000/build/_shared/chunk-D4PYBZR3.js:2559:26)
    at body
    at html
    at App
    at RemixRoute (http://localhost:3000/build/_shared/chunk-D4PYBZR3.js:4352:3)
    at Routes2 (http://localhost:3000/build/_shared/chunk-D4PYBZR3.js:4335:7)
    at Router (http://localhost:3000/build/_shared/chunk-D4PYBZR3.js:2563:15)
    at RemixCatchBoundary (http://localhost:3000/build/_shared/chunk-D4PYBZR3.js:2848:10)
    at RemixErrorBoundary (http://localhost:3000/build/_shared/chunk-D4PYBZR3.js:2773:5)
    at RemixEntry (http://localhost:3000/build/_shared/chunk-D4PYBZR3.js:4232:12)
    at RemixBrowser (http://localhost:3000/build/_shared/chunk-D4PYBZR3.js:4921:27)

Now I have no idea on what this error is or where it is originating from and therefore I have no idea on how to solve it.

ionut-botizan · 2022-05-17T11:01:02Z

@nivekithan

Now I have no idea on what this error is or where it is originating from and therefore I have no idea on how to solve it.

Did you, by any chance, modified the transpiled Remix code in node_modules directly? 😁
If you did, then there are 2 places where you need to change it: both in @remix-run/react/components.js and @remix-run/react/esm/components.js.

Now, from what I can tell, the issue with the search part is that you can't just replace useResolvedPath with useLocation, because useResolvedPath parses the action prop the Form component has received, defaulting to . if no explicit action prop has been set. So, if you were to replace the useResolvedPath().search with useLocation().search, then you would overwrite any search part that could be explicitly set using the action prop.

The solution would look like this:

 export function useFormAction(
   action = ".",
   // TODO: Remove method param in v2 as it's no longer needed and is a breaking change
   method: FormMethod = "get"
 ): string {
   let { id } = useRemixRouteContext();
   let path = useResolvedPath(action);
   let search = path.search;
   let isIndexRoute = id.endsWith("/index");
+  let location = useLocation();
+
+  if (action === ".") {
+    search = location.search
+  }

   if (action === "." && isIndexRoute) {
     search = search ? search.replace(/^\?/, "?index&") : "?index";
   }

   return path.pathname + search;
 }

E.g.

1️⃣
Location: /login?redirectTo=/profile
Form action: undefined

useResolvedPath
    --> 💔 <Form method="post" action="/login" />

useLocation
    --> 💚 <Form method="post" action="/login?redirectTo=/profile" />

useResolvedPath + useLocation
    --> 💚 <Form method="post" action="/login?redirectTo=/profile" />

2️⃣
Location: /login?redirectTo=/profile
Form action: /custom-path?foo=bar

useResolvedPath
    --> 💚 <Form method="post" action="/custom-path?foo=bar" />

useLocation
    --> 💔 <Form method="post" action="/custom-path?redirectTo=/profile" />

useResolvedPath + useLocation
    --> 💚 <Form method="post" action="/custom-path?foo=bar" />

nivekithan · 2022-05-17T11:55:23Z

@ionut-botizan yeah you are correct, I cloned the remix repo locally and changed the code in packages/remix-react/components.tsx , the error is nowhere to be seen.

On your code, if we explicit set the action to ./ or to ././ or to ./././ then also we will lose the search part, where as setting the action to . will not lose the search part.

if we assume the behaviour of action to be same as to in <Link to={...} > </Link>, then when we explicitly set action to ., we should lose search part. We only should consider search part when action is undefined

So code should look like this

/**
 * Resolves a `<form action>` path relative to the current route.
 *
 * @see https://remix.run/api/remix#useformaction
 */
export function useFormAction(
  action?: string,
  // TODO: Remove method param in v2 as it's no longer needed and is a breaking change
  method: FormMethod = "get"
): string {
  let { id } = useRemixRouteContext();
  let path = useResolvedPath(action ? action : ".");
  let search = path.search;
  let isIndexRoute = id.endsWith("/index");
  let location = useLocation();

  if (action === undefined) {
    search = location.search;
  }

  if ((action === undefined || action === ".") && isIndexRoute) {
    search = search ? search.replace(/^\?/, "?index&") : "?index";
  }

  return path.pathname + search;
}

And we also have to change FormImpl to this

let FormImpl = React.forwardRef<HTMLFormElement, FormImplProps>(
  (
    {
      reloadDocument = false,
      replace = false,
      method = "get",
      action,
      encType = "application/x-www-form-urlencoded",
      fetchKey,
      onSubmit,
      ...props
    },
    forwardedRef
  ) {...}

jacargentina · 2022-05-19T00:51:04Z

#931

That was from long ago 😒

jaschaio · 2022-06-23T12:08:48Z

Is there any workaround? I tried the one from @kiliman but then I get a 405 Method now allowed error

kiliman · 2022-06-23T20:52:21Z

If you're posting to an index route, then make sure you append ?index to the URL.

jaschaio · 2022-06-24T18:31:39Z

@kiliman this still doesn't work when using a remix Form potentially because of #3564

ScottAgirs · 2022-07-04T17:31:44Z

?index

This was good to me, haha.

Any chance Remix could make this more ergonomic? : ) 🤞🏼

lili21 · 2022-08-26T05:35:58Z

the latest version(1.6.8) already fixed the issue.

TheRealFlyingCoder · 2022-12-29T23:52:18Z

I'm just closing this as stale, I'm pretty sure it was fixed but haven't had any confirmation from the team.

If it's not we can re-open

uhrohraggy · 2023-01-16T14:51:58Z

@TheRealFlyingCoder we're on 1.9.0 using a GET form submission, and the search/query params are still lost. Perhaps this is in conjunction with useSubmit.

import { Form, useSubmit } from "@remix-run/react";
...
const submit = useSubmit();
function handleChange(event) {
    submit(event.currentTarget, { replace: true });
  }

return (
    <Form
      method="get"
      onChange={handleChange}
       ...
)

package.json

"@remix-run/node": "^1.9.0",
    "@remix-run/react": "^1.9.0",
    "@remix-run/serve": "^1.9.0",
    "@remix-run/server-runtime": "^1.9.0",

machour · 2023-01-16T17:25:17Z

@uhrohraggy a submission on a form using the get method will replace the search query. Remix behaves like the browser here, so not a bug.

It seems that in Remix v2, the `<Form>` component is able to POST to the correct _implicit_ route on its own. So we can remove our usages of `useLocation` now. We can stop watching remix-run/remix#3133 for now.

Abdulbariii · 2024-06-03T21:37:13Z

It's 2024, and this issue still exists. There should be an option within the Form to control whether existing search queries are removed or not.

TheRealFlyingCoder added the bug:unverified label May 8, 2022

machour added the feat:forms label May 9, 2022

ionut-botizan mentioned this issue May 17, 2022

tests(remix-react): add failing test for default Form action #3211

Closed

TheRealFlyingCoder closed this as completed Dec 29, 2022

remix-run locked as resolved and limited conversation to collaborators Jun 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`Form` post submission does not retain search parameters #3133

`Form` post submission does not retain search parameters #3133

TheRealFlyingCoder commented May 8, 2022

manV commented May 10, 2022

kiliman commented May 10, 2022

nivekithan commented May 14, 2022

ionut-botizan commented May 17, 2022

nivekithan commented May 17, 2022

jacargentina commented May 19, 2022

jaschaio commented Jun 23, 2022

kiliman commented Jun 23, 2022

jaschaio commented Jun 24, 2022

ScottAgirs commented Jul 4, 2022

lili21 commented Aug 26, 2022

TheRealFlyingCoder commented Dec 29, 2022

uhrohraggy commented Jan 16, 2023

machour commented Jan 16, 2023

Abdulbariii commented Jun 3, 2024

Form post submission does not retain search parameters #3133

Form post submission does not retain search parameters #3133

Comments

TheRealFlyingCoder commented May 8, 2022

What version of Remix are you using?

Steps to Reproduce

Expected Behavior

Actual Behavior

manV commented May 10, 2022

kiliman commented May 10, 2022

nivekithan commented May 14, 2022

ionut-botizan commented May 17, 2022

nivekithan commented May 17, 2022

jacargentina commented May 19, 2022

jaschaio commented Jun 23, 2022

kiliman commented Jun 23, 2022

jaschaio commented Jun 24, 2022

ScottAgirs commented Jul 4, 2022

lili21 commented Aug 26, 2022

TheRealFlyingCoder commented Dec 29, 2022

uhrohraggy commented Jan 16, 2023

machour commented Jan 16, 2023

Abdulbariii commented Jun 3, 2024

`Form` post submission does not retain search parameters #3133

`Form` post submission does not retain search parameters #3133