remix icon indicating copy to clipboard operation
remix copied to clipboard

<Form> submitter is serialized out of tree order

Open jenseng opened this issue 3 years ago • 6 comments
trafficstars

What version of Remix are you using?

1.7.2

Steps to Reproduce

  1. Make a <Form> like so:
<Form>
  <button type="submit" name="foo" value="override">Override</button>
  <input type="hidden" name="foo" value="default" />
</Form>
  1. Click the "Override" button

Expected Behavior

The form fields should be serialized in tree order, i.e. foo=override&foo=default (as is the case with a vanilla <form>).

See Constructing the form data set in the HTML spec. Submitted fields should be sent in the order they appear in the DOM, including the submitter (i.e. the submit button that was clicked).

Actual Behavior

The form is serialized as foo=default&foo=override.

This is problematic on the server-side if you are relying on the order to indicate precedence. formData.get("foo") will yield different values, depending on whether the form was submitted by Remix or native browser behavior.

See also https://github.com/remix-run/remix/pull/3611; prior to that fix it was broken in a different way (it would only serialize foo=default)

jenseng avatar Oct 07 '22 17:10 jenseng

On a related note, <Form> serializes image submit buttons in the wrong order and with the wrong key/values.

For example, given: <input type="image" name="image" src="..." />. If you click it, it gets serialized as image= at the very end, rather than the expected image.x=X&image.y=Y in tree order. See Step 5.2.

Probably worth lumping in to this bug report, as the root cause is the same, and the fix should be too 🤞

jenseng avatar Oct 07 '22 19:10 jenseng

I found some more differences between how <Form> and <form> behave:

  1. In <Form> submissions, Safari includes the submit button value twice (once in tree order, once at the end) due to this bug
  2. If you have file inputs and the encType is application/x-www-form-urlencoded (explicit or implicit):
    • <form> will send each file entry's name as the value, as per the spec
    • <Form> will throw an error File inputs are not supported with encType "application/x-www-form-urlencoded" ...; if this invariant check is removed, no value gets submitted.

jenseng avatar Oct 10 '22 22:10 jenseng

There's a proposal to let you pass a submitter to FormData; once implemented and widely available it should generally make this an easy fix.

In the meantime we might consider:

  1. Fully building the FormData ourselves according to the spec. I have this implemented locally in a WIP, though we might need to consider any perf implications around this.
  2. Partially fixing this (e.g. could probably handle the file input and non-image submit button issues in a relatively surgical manner)
  3. Doing nothing, but documenting these differences clearly. These things are all a bit edge-case-y, but it would be good to be clear where we deviate from the spec, so that people aren't surprised like I was :)

jenseng avatar Oct 11 '22 16:10 jenseng

Great stuff @jenseng, thanks!

machour avatar Oct 14 '22 00:10 machour

As a workaround for non-image submitters, you can use a component like so that does some hidden input hijinks:

function Button(props: DetailedHTMLProps<ButtonHTMLAttributes<HTMLButtonElement>, HTMLButtonElement>) {
  const hiddenRef = useRef<HTMLInputElement>(null);
  const buttonRef = useRef<HTMLButtonElement>(null);
  if ((props.type ?? "submit") === "submit" && props.name) {
    return (
      <>
        <input ref={hiddenRef} type="hidden" name={props.name} value={props.value} disabled />
        <button
          ref={buttonRef}
          {...props}
          onClick={(e) => {
            hiddenRef.current!.disabled = false;
            buttonRef.current!.name = "";
            setTimeout(() => {
              hiddenRef.current!.disabled = true;
              buttonRef.current!.name = props.name!;
            });
            props.onClick?.(e);
          }}
        />
      </>
    );
  } else {
    return <button {...props} />;
  }
}

Since the hijinks are done in the onClick handler, the button still has the right attributes to work before/without JavaScript

jenseng avatar Oct 17 '22 19:10 jenseng

Thank you for the detailed use cases and spec references @jenseng - these are incredibly helpful 🙌

brophdawg11 avatar Oct 31 '22 19:10 brophdawg11

I think this should be resolved when we release Remix 1.18.0 now that the relevant PRs are merged in RR. The latest FormData submitter PR should be released in RR 6.14.0

brophdawg11 avatar Jun 14 '23 21:06 brophdawg11