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

feat: support FormData #286

Merged
merged 8 commits into from
Apr 10, 2024
Merged

feat: support FormData #286

merged 8 commits into from
Apr 10, 2024

Conversation

climba03003
Copy link
Member

@climba03003 climba03003 commented Apr 5, 2024

Support FormData and provide the Content-Type header automatically.

Closes fastify/fastify-multipart#516
Closes fastify/help#1017

Checklist

@climba03003

This comment was marked as outdated.

Signed-off-by: KaKa <23028015+climba03003@users.noreply.github.com>
@climba03003 climba03003 requested a review from a team April 7, 2024 17:01
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work! Can you add a test for Node v14 and v16?

test/index.test.js Outdated Show resolved Hide resolved
test/index.test.js Outdated Show resolved Hide resolved
Comment on lines +2105 to +2110
// supports >= node@18
runFormDataUnitTest('native', { FormData: globalThis.FormData, Blob: globalThis.Blob })
// supports >= node@16
runFormDataUnitTest('undici', { FormData: require('undici').FormData, Blob: require('node:buffer').Blob })
// supports >= node@14
runFormDataUnitTest('formdata-node', { FormData: require('formdata-node').FormData, Blob: require('formdata-node').Blob })
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverage should be fine for all supported node version.
undici only supports FormData higher than node@16.
So, we need formdata-node for node@14.

It also show that we are not limited to undici implementation only, but support all spec compliance FormData

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina requested review from Uzlopak and zekth April 8, 2024 09:48
@climba03003 climba03003 requested a review from a team April 10, 2024 16:43
@mcollina mcollina merged commit 974a1c9 into master Apr 10, 2024
21 checks passed
@gurgunday gurgunday deleted the native-formdata branch April 10, 2024 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants