koa
koa copied to clipboard
initialize `context.state` sooner
Expected Behavior
If there's information I want to be available in every request, I'd like to be able to specify that information when I initialize the app, using app.context.state, like so:
const app = new Koa()
app.context.state.prop = true // TypeError: Cannot set properties of undefined (setting 'prop')
app.use(ctx => console.log(ctx.state.prop))
Best practice for Koa is to use context.state to pass information through middleware, and to use app.context add properties used throughout the app, so this seems like the right approach. It's also parallel to the way that I can set app.locals in an Express app to make that information available to res.locals.
Actual Behavior
The above code snippet throws a TypeError because context.state isn't initialized until the app receives a request, here:
https://github.com/koajs/koa/blob/aa816ca523e0f7f3ca7623163762a2e63a7b0ee3/lib/application.js#L168-L181
The following also doesn't work, because the state property gets overridden by the code linked above:
const app = new Koa()
app.context.state = { prop: true }
app.use(ctx => console.log(ctx.state.prop)) // returns undefined
Suggested Solution
Add the state property to the context prototype instead of initializing it when a request comes in. Then, in app.createContext(), create a new state object that only lives for the lifetime of the request, using app.context.state as its prototype.
Replace:
https://github.com/koajs/koa/blob/aa816ca523e0f7f3ca7623163762a2e63a7b0ee3/lib/application.js#L179
with:
// context.state = Object.assign({}, context.state) // <- old suggestion
context.state = Object.create(context.state) // <- this is probably better
Workaround
The workaround is to add the information directly to app.context rather than app.context.state. However, this is undesirable because it goes against the best practice of using context.state to encapsulate state.
const app = new Koa()
app.context.prop = true
app.use(ctx => console.log(ctx.prop)) // returns true
I'd be happy to open a PR for this if it's a change you'd like implemented.
I like to see Set or Map here. It should behave better. @miwnwski ?!
If you want some info on every request, you should just modify context as you described.
State should be reserved for per request specific data/state, or you can just add a middleware to set it for each request.
In other words, there nothing wrong with doing e.g. app.context.db = new Knex(knexConfig)
Changing app.context.state type and/or its behaviour would be a pretty significant semver breakage though.
Edit:
I see, I read this a bit too fast before responding. I guess what would be more appropriate would be to expose the state object initialisation through Application instead of changing its type.
Though I'm not as involved anymore so I have no strong opinions about this.
So after having slept on this I think a PR that makes context creation an overridable Koa option is the way to go. If anyone wants to take a stab at this it would be welcome.
Hi, I would like to work on this issue.
I am wondering if there is a development guide for contributors somewhere in the repo?
this has not being solved yet, i ran into this today
Reading my initial issue again, I'd actually recommend using Object.create() instead of Object.assign() for the change. I've edited the original comment to reflect this.
@dwhieb The change does not work
app.use((ctx, next)=> {ctx.state.sth = 'something'; await next();}
Put this before all MWs and you are good to go. A context has no meaning without a request. So the context can be valid in MWs chain only.