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

Decorate request with session key #88

Closed
hueniverse opened this issue Jan 2, 2016 · 7 comments
Closed

Decorate request with session key #88

hueniverse opened this issue Jan 2, 2016 · 7 comments
Assignees
Labels
Milestone

Comments

@hueniverse
Copy link
Member

@hueniverse hueniverse commented Jan 2, 2016

hapi v12 removes the request.session placeholder. It will not break this module but you should officially decorate the request with the key to make sure other plugins can't if both are loaded. It might be best to call it something more unique like request.yar so that other plugins like (hapi-auth-cookie) won't have a conflict (as they should stop using the request.auth.session key).

@mark-bradshaw

This comment has been minimized.

Copy link
Contributor

@mark-bradshaw mark-bradshaw commented Jan 3, 2016

Thanks for the heads up.

@mark-bradshaw

This comment has been minimized.

Copy link
Contributor

@mark-bradshaw mark-bradshaw commented Jan 5, 2016

This has been taken care of with the v6 release.

@PavelPolyakov

This comment has been minimized.

Copy link

@PavelPolyakov PavelPolyakov commented Jan 8, 2016

Hi @mark-bradshaw,

Could you, please, explain, why we've moved to request.yar from request.session?
As yar is most used session plugin for hapi, I think it can continue to use request.session, but decorate the server, instead of the direct assignment.

I saw that hapi-auth-cookie moved from auth.session already.

If yar would use request.session and the other plugin would do the same, the startup error would make it obvious for the developer, that he should choose the only plugin or resolve the conflict.

The problem is, that if everybody would fulfil the new recommendations, then the request.session would not be used at all, despite the fact that it's most logical name to store the session content.

Am I miss something?

Regards,

@mark-bradshaw

This comment has been minimized.

Copy link
Contributor

@mark-bradshaw mark-bradshaw commented Jan 9, 2016

@PavelPolyakov I'm following Eran's request on this. He asked that we move our data to something more unique, like request.yar. My read of Eran's request was that he is asking for the change to make the various session type plugins able to coexist. I might be wrong in that.

I understand that we could still try to stay on request.session and just let developers pick one plugin or the other, but it does seem that this gives maximum flexibility in plugin choice without causing conflicts. I know hapi-auth-cookie has done the same and moved data to request.cookieAuth.

But I do see the point of what you're saying, and I have to agree that request.session is a very logical place for all this. Perhaps we can find a way to allow people to choose to have yar at that location instead as a config option.

@hueniverse

This comment has been minimized.

Copy link
Member Author

@hueniverse hueniverse commented Jan 9, 2016

I think it's better to use a unique name as many people use both session modules at the same time.

@devinivy

This comment has been minimized.

Copy link
Member

@devinivy devinivy commented Jan 9, 2016

I generally agree with that sentiment. This falls in line with the idea behind hapi's decorators, which enforce that there be no naming conflicts. Naming conflicts cause the worst kind of confusion. To start, I imagine the case of registering a plugin that registers yar (or some other session-related plugin).

If someone wanted to, wouldn't it be just a small amount of code to place request.yar on request.session using the { apply: true } option of server.decorate()?

@mark-bradshaw

This comment has been minimized.

Copy link
Contributor

@mark-bradshaw mark-bradshaw commented Jan 9, 2016

I guess it would depend on the order of the decorations. If yar went first
that could work.

On Sat, Jan 9, 2016 at 10:52 AM devin ivy notifications@github.com wrote:

I generally agree with that sentiment. This falls in line with the idea
behind hapi's decorators, which enforce that there be no naming conflicts.
Naming conflicts cause the worst kind of confusion. To start, I imagine the
case of registering a plugin that registers yar (or some other
session-related plugin).

If someone wanted to, wouldn't it be just a small amount of code to place
request.yar on request.session using the { apply: true } option of
server.decorate()?


Reply to this email directly or view it on GitHub
#88 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.