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: implement FormData Iterator #1473

Merged
merged 3 commits into from Jun 12, 2022
Merged

Conversation

KhafraDev
Copy link
Member

@KhafraDev KhafraDev commented May 31, 2022

Continuation of #1431 and #1423

Also adds in FormData.forEach which was added in the types but for some reason wasn't implemented.

@codecov-commenter
Copy link

codecov-commenter commented May 31, 2022

Codecov Report

Merging #1473 (b74198f) into main (4711982) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1473   +/-   ##
=======================================
  Coverage   94.70%   94.70%           
=======================================
  Files          49       49           
  Lines        4246     4249    +3     
=======================================
+ Hits         4021     4024    +3     
  Misses        225      225           
Impacted Files Coverage Δ
lib/fetch/formdata.js 96.77% <100.00%> (+0.07%) ⬆️
lib/fetch/headers.js 94.44% <100.00%> (-0.26%) ⬇️
lib/fetch/util.js 81.11% <100.00%> (+1.59%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4711982...b74198f. Read the comment docs.

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

@kibertoad
Copy link
Contributor

What's missing before this can be merged?

@KhafraDev
Copy link
Member Author

KhafraDev commented Jun 5, 2022

A review from ronag I presume, who is currently on vacation 😄

@mcollina
Copy link
Member

mcollina commented Jun 5, 2022

A review from ronag I presume, who is currently on vacation 😄

pretty much

@ronag ronag merged commit 3fa762e into nodejs:main Jun 12, 2022
@KhafraDev KhafraDev deleted the formdata-iterator branch June 12, 2022 18:46
@jimmywarting
Copy link
Contributor

Hmm, tried doing export const FormData = globalThis.FormData || polyfill but notices that hour tests failed failed cuz this forEach was missing...

was using NodeJS 18.3.0

have a resent nodejs release been released with this fix?

@KhafraDev
Copy link
Member Author

@jimmywarting v18.5.0 has FormData.prototype.forEach.

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.

None yet

6 participants