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

add: ctx.locals as a recommended namespace for passing information to the frontend #366

Merged
merged 1 commit into from
Nov 18, 2014

Conversation

matthewmueller
Copy link
Contributor

Just throwing this PR out there. It was discussed in: #338.

Basically this gives you a namespace to attach to that you don't have to worry about this object being undefined. Alternatively, you could always add:

app.use(function *(next) {
  this.locals = this.locals || {};
  yield next;
});

at the top of your stack. I feel like this is common enough use-case that it should be added to core, but open to your thoughts.

@matthewmueller
Copy link
Contributor Author

alternatively, we could call it ctx.state to avoid the confusion of express locals

@tj
Copy link
Member

tj commented Nov 18, 2014

sweet! I agree with the name change to differentiate, ctx.state sounds fine to me! A little section in the docs would be awesome too if you have a minute

@matthewmueller
Copy link
Contributor Author

done! let me know if there's anything else I should update.

@jonathanong
Copy link
Member

can we squash this? :)

@fengmk2
Copy link
Member

fengmk2 commented Nov 18, 2014

confusion of ctx.status and ctx.state, read-write similar.
I think ctx.state is not a good name.

@jonathanong
Copy link
Member

maybe if english is your second language :) it's not like you pronounce it state-us. both are okay with me.

@hallas
Copy link
Contributor

hallas commented Nov 18, 2014

When we add this we need to heavily stress the idiomatic use of this object to store auxiliary information.
Since we now provide an official way of doing this, we need to make sure people get it.

@fengmk2
Copy link
Member

fengmk2 commented Nov 18, 2014

@jonathanong 😢

@matthewmueller
Copy link
Contributor Author

can we squash this? :

@jonathanong sure, not too familiar with that workflow. Should I open up a new PR with the single commit?

@dead-horse
Copy link
Member

@matthewmueller git rebase master -i

@matthewmueller
Copy link
Contributor Author

@dead-horse right, but i can't push this on the same branch because the commit histories don't match up.

@hallas
Copy link
Contributor

hallas commented Nov 18, 2014

git push --force 👯

@matthewmueller
Copy link
Contributor Author

ya'll are living on the edge, huh... pushed!

tj added a commit that referenced this pull request Nov 18, 2014
add: ctx.locals as a recommended namespace for passing information to the frontend
@tj tj merged commit b854d00 into koajs:master Nov 18, 2014
@tj
Copy link
Member

tj commented Nov 18, 2014

thanks man!

@fundon
Copy link
Contributor

fundon commented Nov 25, 2014

wow, I missed something.
good news.

thanks @coderhaoxin
yes, we can deprecate it when release new version.

cesarandreu added a commit to cesarandreu/koa-jwt that referenced this pull request Dec 8, 2014
stiang added a commit to koajs/jwt that referenced this pull request Dec 8, 2014
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.

8 participants