GET Submissions: don't merge into searchParams

[seanpdoyle]

The background

According to the HTML Specification's § 4.10.21.3 Form submission
algorithm section, submissions transmitted as GET requests mutate
the [action] URL, overriding any search parameters already
encoded into the [action] value:

Mutate action URL

Let pairs be the result of converting to a list of
name-value pairs with entry list.

Let query be the result of running the
application/x-www-form-urlencoded serializer with pairs
and encoding.

Set parsed action's query component to query.

Plan to navigate to parsed action.

Form submissions made with POST requests, on the other hand, encode
both the [action] value's query parameters and any additionally
encoded body data:

Submit as entity body

…

Plan to navigate to a new request whose URL is parsed
action, method is method, header list is
« (Content-Type, mimeType) », and body is body.

The problem

Ever since 18585fc (and subsequently hotwired/turbo#108),
Turbo's FetchRequest has merged submission values from <form>
elements with [method="get"] and without [action] attributes into
the current URL's query parameters.

For example, consider the following forms rendered on the /articles
page:

<form>
  <label for="q">Term</label>
  <input id="q" name="q">
  <button>Search</button>
</form>

<!-- elsewhere in the page -->
<form>
  <button name="order" value="asc">Sort ascending</button>
  <button name="order" value="desc">Sort ascending</button>
</form>

Without Turbo, entering "Hotwire" as the search term into
input[name="q"] and clicking the "Search" button would navigate the
browser to /articles?q=Hotwire. Then, clicking the "Sort ascending"
button would navigate the page to /articles?order=asc.

With Turbo, entering "Hotwire" as the search term into
input[name="q"] and clicking the "Search" button navigates the
browser to /articles?q=Hotwire. Then, clicking the "Sort ascending"
button navigates the page to /articles?q=Hotwire&order=asc,
effectively merging values from the page's URL and the <form>
element's fields.

The solution

This commit modifies the way that FetchRequest constructs its url: URL and body: FormData | URLSearchParams properties.

First, it always assigns a body property, but conditionally
encodes that value into the fetchOptions: RequestInit property based
on whether or not the request is an idempotent GET.

Next, if constructed with a body: URLSearchParams that has entries,
replace the url: URL instance's search params entirely with
those values, like the HTML Specification algorithm entails.

If constructed with a body: URLSearchParams that is empty, pass the
url: URL through and assign the property without modifying it.

Additionally, this commit adds test cases to ensure that POST requests
transmit data in both the body and the URL.

While the previous multi-form merging behavior can be desirable, it is
not the behavior outlined by the HTML Specification, so Turbo should not
provide it out-of-the-box.

Having said that, there are two ways for applications to restore that
behavior:

1. declare a turbo:before-fetch-request event listener to merge values
   into the FetchRequest instance's body:

addEventListener("turbo:before-fetch-request", ({ target, detail: { url, fetchOptions: { method } } }) => {
  if (target instanceof HTMLFormElement && method == "GET") {
    for (const [ name, value ] of new URLSearchParams(window.location.search)) {
      // conditionally call `set` or `append`,
      // depending on your application's needs
      if (url.searchParams.has(name)) continue
      else url.searchParams.set(name, value)
    }
  }
})

2. declare a formdata event listener to merge values into the
   submitted form's FormData instance prior to entering the Turbo
   request pipeline:

addEventListener("submit", (event) => {
  if (event.defaultPrevented) return

  const { target, submitter } = event
  const action = submitter?.getAttribute("formaction") || target.getAttribute("action")

  if (target.method == "get" && !action) {
    target.addEventListener("formdata", ({ formData }) => {
      for (const [ name, value ] of new URLSearchParams(window.location.search)) {
        // conditionally call `set` or `append`,
        // depending on your application's needs
        if (formData.has(name)) continue
        else formData.set(name, value)
      }
    }, { once: true })
  }
})

The conditionals in both cases could be omitted if applications controlled the behavior more directly, like by declaring a Stimulus controller and action (e.g. <form data-controller="form" data-action="formdata->form#mergeSearchParams">).

Supporting changes

• Dispatch turbo:before-fetch-request with URL

Dispatch turbo:before-fetch-request with a direct reference to the
FetchRequest instance's url: URL property as part of the
event.detail.

[seanpdoyle]

I've just discovered that this is still slightly incorrect behavior.

I tested with an application without Turbo or any other JavaScript, and submitting a form with an [action] value that contains query parameters requests that [action] path without any key-value pairs (that is, with ? as its the value of URL.search):

<form action="/src/tests/fixtures/form.html?sort=asc">
  <!-- clicking the button navigates to `/src/tests/fixtures/form?` -->
  <button>Submit</button>
</form>

I'm in the process of adding test coverage and opening a separate PR.