fix(Request): URLSearchParams body w/ Headers obj #1410
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1410 +/- ##
=======================================
Coverage 94.28% 94.28%
=======================================
Files 47 47
Lines 4250 4250
=======================================
Hits 4007 4007
Misses 243 243
Continue to review full report at Codecov.
|
| this[kHeaders].append('content-type', contentType) | ||
| this[kState].headersList.append('content-type', contentType) | ||
| this[kHeaders].set('content-type', contentType) | ||
| this[kState].headersList.set('content-type', contentType) |
There was a problem hiding this comment.
I think the reason for the duplication is this[kHeaders][kHeadersList] === this[kState].headersList, making this second call redundant.
See
Line 215 in 0babdb1
There was a problem hiding this comment.
I thought so too, but removing the second call caused some tests (from the node-fetch test suite) to fail.
There was a problem hiding this comment.
Yes, I think here we should use .set. If I remember correctly, the old implementation was using .set/.push as well.
|
No problem at all, there's no way anyone would have noticed these things without tests. I also discovered a bunch of bugs in the Headers class while working on this PR so this issue will end up making Headers more spec compliant! |
Fixes #1407
Although the spec says to "append" the header, setting it should make no difference as the check beforehand ensures that no 'content-type' header exists.
p.s.: I have no idea how the header was being duplicated 馃槙