expressjs / session

Simple session middleware for Express
MIT License
6.24k stars 973 forks source link

use proxies for session data #46

Open Fishrock123 opened 10 years ago

Fishrock123 commented 10 years ago

So that we can check that the store actually exists (#4) and whatever else we want.

req.session.set('thing.property', stuff)

req,session.get('thing.property')

Would be removed of favour of Proxies, eventually. In like a decade when they actually land.

A big issue would be people being idiots and assigning stuff onto the data object or get()-ed objects.

jonathanong commented 10 years ago

Meh I would just wait for proxies. This isn't really intuitive

MichaelCereda commented 10 years ago

Citating the example in README.md, you can add user-specific properties in this way

var sess = req.session;
if (sess.views) {
    sess.views++;
} else {
  sess.views = 1;
}

you can even store nested objects

Fishrock123 commented 10 years ago

@MichaelCereda This a feature proposal, not a question. ;)

MichaelCereda commented 10 years ago

Yes, I know that, but I'm guessing why it should be necessary? you can already interact with session using getter/setter.

dougwilson commented 10 years ago

@MichaelCereda to know if the session was modified to write it back to the store.

MichaelCereda commented 10 years ago

ok so you need to fire an event when session "data" is changed...isn't it?

colelawrence commented 10 years ago

In order to implement this, we would probably need to have it on a different property of req in order to not change the way it works currently. If someone wants to do something like this they may be better off looking into another library.

BUT, if it were a configuration option like asStore: true and completely change the behaviour of the req.session object, then it would be okay and non-breaking. Then again all that extra code for event firing and storing might increase the code-base by more than twice LoC, so we might as well have a different version that behaves like that.

dougwilson commented 10 years ago

@ZombieHippie this would just be a major version bump, i.e. breaking. Also, there is no event firing here, it's just keeping a dirty flag.

colelawrence commented 10 years ago

this would just be a major version bump, i.e. breaking

@dougwilson I think you are right.

MichaelCereda commented 10 years ago

Yes, a major release will be a good thing and allows to implement other breaking feature requests.

Fishrock123 commented 10 years ago

I had already assigned this to a 2.0.0 milestone. :)

jonathanong commented 10 years ago

i don't think this is worth it unless you allow atomic updates on each .get(). here's how i think sessions are supposed to look like: https://github.com/aheckmann/koa-mongodb-session

colelawrence commented 10 years ago

Thank you for the example, it should help us understand your suggestions. On Jun 17, 2014 3:01 AM, "Jonathan Ong" notifications@github.com wrote:

i don't think this is worth it unless you allow atomic updates on each .get(). here's what i think is how sessions are supposed to look like: https://github.com/aheckmann/koa-mongodb-session

— Reply to this email directly or view it on GitHub https://github.com/expressjs/session/issues/46#issuecomment-46278035.

ilanbiala commented 9 years ago

Is this still being pursued for 2.0.0? I also don't see a 2.0.0 branch, but there is a milestone. Any timeline?

Fishrock123 commented 9 years ago

No real idea, sorry. :/

Fishrock123 commented 8 years ago

Proxies are in Node v6, so this should actually be possible now.

aks- commented 7 years ago

I'd love to play with this. Or push a branch that does something interesting and make a PR and discuss it.