Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(remix-react): fix submitter serialization (alt approach) #4475

Closed
wants to merge 1 commit into from

Conversation

jenseng
Copy link
Contributor

@jenseng jenseng commented Nov 1, 2022

Note: I have two proposals on how we can fix the submitter serialization bugs. The other approach is here.

Bring <Form> submissions in line with the spec with respect to how and where form submitters are serialized within the data set.

Alt solution: We accomplish this by constructing the form data set ourselves according to the spec.

  • Tests

Problems fixed:

  1. Serialize submitters in tree order (i.e. where they appear in the DOM)
  2. Serialize Image Button submitter correctly (i.e. separate x and y coordinate entries, rather than a single empty entry)
  3. Stop sending multiple submitter entries in older WebKit

References: #4342
Spec: https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#constructing-form-data-set

@changeset-bot
Copy link

changeset-bot bot commented Nov 1, 2022

🦋 Changeset detected

Latest commit: 4dbdc64

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages
Name Type
@remix-run/react Patch
create-remix Patch
remix Patch
@remix-run/architect Patch
@remix-run/cloudflare Patch
@remix-run/cloudflare-pages Patch
@remix-run/cloudflare-workers Patch
@remix-run/deno Patch
@remix-run/dev Patch
@remix-run/eslint-config Patch
@remix-run/express Patch
@remix-run/netlify Patch
@remix-run/node Patch
@remix-run/serve Patch
@remix-run/server-runtime Patch
@remix-run/vercel Patch

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

@jenseng
Copy link
Contributor Author

jenseng commented Nov 1, 2022

This feels a tad less hacky than the other approach, but it's a lot more code to maintain, and as the spec evolves we'll have to keep tweaking the implementation, increasing the likelihood of bugs/etc.

Bring `<Form>` submissions in line with the spec with respect to how and where
form submitters are serialized within the data set. We accomplish this by
constructing the form data set ourselves according to the spec.

Problems fixed:
1. Serialize submitters in tree order (i.e. where they appear in the DOM)
2. Serialize Image Button submitter correctly (i.e. separate x and y coordinate
   entries, rather than a single empty entry)
3. Stop sending multiple entries in older WebKit

References: remix-run#4342
Spec: https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#constructing-form-data-set
@jenseng
Copy link
Contributor Author

jenseng commented Jan 10, 2023

Closing in favor of remix-run/react-router#9866, since the code has moved there

@jenseng jenseng closed this Jan 10, 2023
jenseng added a commit to jenseng/react-router that referenced this pull request Jan 10, 2023
This is the RR port of remix-run/remix#4475

Bring `<Form>` submissions in line with the spec with respect to how and where
form submitters are serialized within the data set. We accomplish this by
constructing the form data set ourselves according to the spec.

Problems fixed:
1. Serialize submitters in tree order (i.e. where they appear in the DOM)
2. Serialize Image Button submitter correctly (i.e. separate x and y coordinate
   entries, rather than a single empty entry)
3. Stop sending multiple entries in older WebKit

References: remix-run/remix#4342
Spec: https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#constructing-form-data-set
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants