Remix Form method GET not working as native browser Form

[wahyubucil]

First of all, I'm a little bit confused about where to put this issue. At first, I thought it was an issue. But turnout I looked at the test it seems like it was built-in. I hope we can discuss it directly here.

So my problem is, that I create a simple form to search with a GET method, in the older version (1.5.1) it works as expected but not in the latest version.

Here's the demo: https://stackblitz.com/edit/node-sxbjyb

Suppose you're trying to submit. It will append every submitted value on search params instead of setting an existing one. Like this: /?index=&q=test&q=test+again&q=test+third+time

I use the nightly build. If I use the latest one which is 1.6.8. I will get: /?index=&index=&index=&q=test&q=test+again&q=test+third+time. But I'm glad that the ?index issue is already resolved even tho it's not been released yet.

I am looking at the remix source code. I think it's all because of this line. But I think it's not as simple as changing append to set because we also need to support this use case.

Something interesting happen when I decide to use the native browser Form. Native browser Form always replace all of the search params, instead of appending them. And I think that is supposed to be the expected behavior.

IMO, we need to change the useFormAction, if it's a GET and no action, then we just return the URL without any search params. If it's on the index route, we just return ?index without any other search params.

It's not the framework's duty to provide the search params to the action. In my experience, it's always the framework user that defines search params. Whether we put it directly on the form action, <input type="hidden" />, or even sync it with the input value. It's all on us, not the framework. Even if we're using SPA, we need to make it sync with our state right?

At first, I want to create a PR for this. But after seeing this test. I decide not to do it and prefer to discuss it first.

To conclude, we had 2 problems:

1. Older version that I used before which is 1.5.1 works as expected
2. Remix Form behave differently from the native browser form when it comes to the GET method

I think it's a major issue, so hopefully, we can clear it ASAP. Thanks in advance

[wahyubucil]

Also, if we decided to not behave like a browser form and still provide the search params in the action, I had several solutions for handling the search params. But honestly, I still prefer to make Remix Form works as Native Browser Form just like the earlier version of Remix.

[kiliman]

The duplicate ?index param is a bug in 1.6.8 and has been fixed for next release. Either roll back to to 1.6.7 or wait for the update.

#3998

[wahyubucil]

Nope. I already know the ?index param is solved. In my demo, I already use the nightly build. But the problem is on the other search params. You can try my demo, fill in the input text, and submit it multiple times. You will see a lot of the same params.

Thanks for the response btw @kiliman.

[kiliman]

I see. You're right. I saw the ?index in your post and assumed that was the issue you were having.

I believe the issue is that they had updated Remix to handle the correct POST URL if action was not specified. It should post to the current URL (including search params).

However, for GET forms, it should always replace the current search params with the form values.

So need to add the following:

if (method.toLowerCase() === "get") {
  // clear existing search params on form get
  url.search = "";
  for (let [name, value] of formData) {
    if (typeof value === "string") {
      url.searchParams.append(name, value);
    } else {
      throw new Error(`Cannot submit binary form data using GET`);
    }
  }
}

[brophdawg11]

Yeah it looks like the new behavior to include search params in the action if left unspecified unearthed a separate longstanding bug where params are appended instead of replaced on GET submissions. I've been looking into this this AM and will have a PR up shortly 👍

[wahyubucil]

Cool. I'm supposed to create this as an issue, haha. But I'm glad the problem is valid and will be fixed.

Thanks, @brophdawg11.

For future reference, here's the PR that fixes this issue: #4046

[jenseng]

Yeah this is buggy, I have a <Form> that stopped working on the latest remix version (i.e. a get submit appends to the search rather than clearing it) ... turning off JS (and/or switching to a native form) it works as expected

[abenhamdine]

Anyone knows if it's fixed in last version 1.7.0 ?
Looks like it's not when I see the changelog https://github.com/remix-run/remix/releases/tag/remix%401.7.0

[brophdawg11]

Not yet - the PR is here: #4046. I'll look to merge this for 1.7.1 next week 👍

[abenhamdine]

ah yes, didn't see the PR. Great news. thx ! 👍