koa
koa copied to clipboard
don't coerce header value to string on `set`
Official Koa documentation about response.set
says This delegates to setHeader which sets or updates headers, but setHeader
doesn't convert header value types until sending them to the network. This PR fixes behavior of response.set
to be according to the documentation.
Also removes recursion on .set
calling .setHeader
directly, for performance reason.
Codecov Report
Merging #1394 into master will decrease coverage by
0.00%
. The diff coverage is100.00%
.
@@ Coverage Diff @@
## master #1394 +/- ##
==========================================
- Coverage 99.38% 99.38% -0.01%
==========================================
Files 4 4
Lines 485 484 -1
Branches 132 130 -2
==========================================
- Hits 482 481 -1
Partials 3 3
Impacted Files | Coverage Δ | |
---|---|---|
lib/response.js | 99.36% <100.00%> (-0.01%) |
:arrow_down: |
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 9ee6584...6983519. Read the comment docs.
This change is reasonable but may contains breaking change, I'd like to merge it when we start next major version's development.
@dead-horse remember that in current implementation you have two more documented ways to set a response header:
ctx.res.setHeader('Content-Length', 256);
ctx.response.header['content-length'] = 256;
I've linked above that official documentation clearly states that ctx.response.set
delegates to ctx.res.setHeader
while in fact, it's not exactly the same. So, I don't understand why do you think it's a breaking change while, according to official documentation of Koa, it should be a bug fix.
The current major has been out and in use long enough that at this point the bug is in the documentation for it. Seems reasonable to adjust it in next major as @dead-horse said.
good idea. @dead-horse can we make koa v3 branch to develop? @tinovyatkin can we together do it?