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(Body): Discourage form-data and buffer() #1212

Merged
merged 4 commits into from Sep 4, 2021
Merged

Conversation

jimmywarting
Copy link
Collaborator

@jimmywarting jimmywarting commented Jul 18, 2021

Purpose

  • We do serialize spec compatible FormData ourself now.
  • We like to reduce the complexity of the core by removing support for form-data.
  • form-data have had a long time to become spec compliant but haven't done that
  • using res.buffer is no good as it's a nodejs only feature, if ppl depend on this it only makes it more node-specific
    we now strive to more spec compatible ways and more cross platform solutions.

Changes

This will output a one time warning when using form-data
image


  • Readme recommends 2 other spec compatible alternatives already
  • The onetime warning references the issue at hand to resolve this.

@jimmywarting jimmywarting requested review from gr2m, LinusU and xxczaki Jul 18, 2021
@jimmywarting
Copy link
Collaborator Author

@jimmywarting jimmywarting commented Jul 18, 2021

added this also:

image

NodeJS plans to impl fetch into core. Guess they will be more spec compatible and not have res.buffer(). this will just be a preparations for it + it makes it more cross compatible

The alternative is that we remove it right now...

src/body.js Outdated Show resolved Hide resolved
@jimmywarting jimmywarting changed the title Discurage form data Discurage form-data and buffer() Jul 18, 2021
@Richienb
Copy link
Member

@Richienb Richienb commented Aug 5, 2021

Since node-fetch v3 bumps a major version, it would make sense to just remove things before a stable version ships if need be.

@jimmywarting
Copy link
Collaborator Author

@jimmywarting jimmywarting commented Aug 5, 2021

I'm fine with removing form-data and buffer right now before shipping v3 if anyone else agree

@octet-stream
Copy link
Contributor

@octet-stream octet-stream commented Aug 5, 2021

I think that removing of the form-data package right away will have a huge impact and broke code for many developers. Perhaps it would be preferable to have some transition period before its support will be gone for good.

@jimmywarting
Copy link
Collaborator Author

@jimmywarting jimmywarting commented Aug 5, 2021

I think that removing of the form-data package right away will have a huge impact and broke code for many developers. Perhaps it would be preferable to have some transition period before its support will be gone for good.

Where thinking quite the same thing when i made this PR.

plans for v4 could be:

  • impl whatwg streams body
  • remove form-data
  • remove buffer()

It would give them time to prepare

@Richienb
Copy link
Member

@Richienb Richienb commented Aug 5, 2021

In which case, we could implement new features and deprecate old ones in v3 and remove them in v4.

LinusU
LinusU approved these changes Aug 6, 2021
Copy link
Member

@LinusU LinusU left a comment

👍

@jimmywarting jimmywarting changed the title Discurage form-data and buffer() chore(Body): Discurage form-data and buffer() Aug 21, 2021
@jimmywarting jimmywarting added this to the Version 3.x.x milestone Aug 31, 2021
@jimmywarting jimmywarting self-assigned this Sep 4, 2021
@jimmywarting jimmywarting changed the title chore(Body): Discurage form-data and buffer() fix(Body): Discurage form-data and buffer() Sep 4, 2021
@jimmywarting jimmywarting requested a review from Richienb Sep 4, 2021
@jimmywarting
Copy link
Collaborator Author

@jimmywarting jimmywarting commented Sep 4, 2021

@gr2m can you review this also?

gr2m
gr2m approved these changes Sep 4, 2021
@jimmywarting jimmywarting merged commit 471f08c into main Sep 4, 2021
8 checks passed
@jimmywarting jimmywarting deleted the discurage-form-data branch Sep 4, 2021
@jimmywarting jimmywarting changed the title fix(Body): Discurage form-data and buffer() fix(Body): Discourage form-data and buffer() Sep 13, 2021
@kandaris
Copy link

@kandaris kandaris commented Dec 31, 2021

So what would an alternative look like for the code below since form-data has been Deprecated?
code

Considering using busboy, any recommendations?

@jimmywarting
Copy link
Collaborator Author

@jimmywarting jimmywarting commented Dec 31, 2021

@kandaris this only shows how you are handling the response, now how you use the form-data package.
also busboy is for decoding bodies only. not constructing them...
(fyi fetch can both encode and decoding bodies)

node-fetch have opted to use formdata-polyfill to deserializer bodies to a formdata using request.formData() or response.formData()

dos i would recommend using that one as well to avoid any other similar pacakges npm i formdata-polyfill fetch-blob

import { fileFromSync } from 'fetch-blob/from.js'
import { FormData } from 'formdata-polyfill/esm.min.js'

const fd = new FormData()
const file = fileFromSync('./package.json')

fd.append('upload', file)

fetch(url, {method: 'post', body: fd})

There are a way to get it without installing it if you do:

const FormData = (await new Response(new URLSearchParams).formData()).constructor

this way you will always depend on same version as we use.

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

Successfully merging this pull request may close these issues.

6 participants