Initialize context.state sooner #1732
Conversation
Codecov ReportBase: 99.61% // Head: 99.61% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## master #1732 +/- ##
=======================================
Coverage 99.61% 99.61%
=======================================
Files 5 5
Lines 521 521
Branches 145 145
=======================================
Hits 519 519
Misses 2 2
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
krisstern
force-pushed
the
initialize-context-state-sooner
branch
from
91528e4
to
5c3e296
Compare
There was a problem hiding this comment.
Change looks fine for what it does.
Consider asserting this in context state test.
Hi @antaranyan Sure, I have just modified the exisitng test for |
| @@ -208,7 +208,7 @@ module.exports = class Application extends Emitter { | |||
| request.response = response | |||
| response.request = request | |||
| context.originalUrl = request.originalUrl = req.url | |||
| context.state = {} | |||
| context.state = Object.assign({}, context.state) | |||
There was a problem hiding this comment.
The whole point of ctx.state as I see it is to ensure there's no state bleed between requests.
As far as I see it, this change defeats this original purpose.
|
what's the point of this? why not just make a middleware that does what you want? |
|
Pending outcome of discussion in #1646 |
Fixes #1646
Description
Allow context creation an overridable Koa option.
Checklist