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

Drop support for Form-Data? #1167

Closed
jimmywarting opened this issue May 22, 2021 · 6 comments · Fixed by #1212
Closed

Drop support for Form-Data? #1167

jimmywarting opened this issue May 22, 2021 · 6 comments · Fixed by #1212
Assignees
Milestone

Comments

@jimmywarting
Copy link
Collaborator

jimmywarting commented May 22, 2021

form-data isn't really spec compatible and it dose some abnormal stuff. I know many other libraries support it. But it increases the complexity to anyone who is building a http library if they decide to support it. They (we) need to learn how it works internally of how it can be read & streamed

I would like to emphasize that it's better to follow the standards instead. Any spec'ed FormData impl shouldn't need any documentation.

There are two other spec compatible FormData packages on npm that works grate on NodeJS

  1. There is my own formdata-polyfill
  2. And also formdata-node

Both of wish can support appending Blob/Files backed up by the FileSystem (and also blob-like items). Unlike form-data that can only append a one-time readable streams. A FormData should be re-useable if you want to clone the body for whatever reason. (our tests cases could use the same FormData instances - but this dose not work with form-data)

This means that all other http libraries out there needs to serialize the body themself, But fear not, there are packages out there also that can convert them

  • My formdata-polyfill have a utility function to convert any spec compatible FormData into a Blob so that can be posted instead formDataToBlob(formdata)
  • octet-stream have built a FormData Encoder
  • And we also have a encoder built in now into node-fetch itself to convert any spec compatible FormData. after the introduction of response.formData() support we now include formdata-polyfill and using formDataToBlob(fd).stream()

form-data have had a long time to address the read/modify parts but never got around to implement any those methods. That's b/c it's based on appending all entries directly into a readable stream, so they can't iterated over or modified them.

So my proposal really is to:

  1. Stop supporting form-data
  2. Throw a error or warning message that form-data is no longer supported and also mention alternative packages
  3. Remove all hacks where we utilize form.getHeaders(), getBoundary(), getLength(), hasKnownLength() - A FormData size should always be known/calculated.

So what do you think? I would rather want this to be moved into a discussion but this repo haven't enabled it (and i can't do that)

@octet-stream

This comment has been minimized.

@jimmywarting

This comment has been minimized.

@jespertheend
Copy link

I was just about to use this when I stumbled across this issue. What is the recommended alternative for sending POST requests with form data in the body?

@jespertheend
Copy link

Ah nevermind, just found out about https://github.com/node-fetch/node-fetch#post-with-form-parameters

@octet-stream
Copy link
Contributor

Alternatives mentioned above. Both of them should work just fine with node-fetch v3

@github-actions
Copy link

🎉 This issue has been resolved in version 4.0.0-beta.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants