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

Default-Method isn't temporary, but persistent #14

Closed
DanielDornhardt opened this issue Feb 19, 2015 · 7 comments
Closed

Default-Method isn't temporary, but persistent #14

DanielDornhardt opened this issue Feb 19, 2015 · 7 comments

Comments

@DanielDornhardt
Copy link

Hi, this just bit me - maybe I just misunderstood the docs, which say:


To define the default type for session variables, set persistent_session.default_method to your preferred type in your config/settings.json file:

{
  "public": {
    "persistent_session": {
      "default_method": 'your-preferred-type'
    }
  }
}

persistent_session.default_method can take one of the following values:

persistent
authenticated
In any other case the default_method will fall back to temporary


While in the code it is:

https://github.com/okgrow/meteor-persistent-session/blob/master/lib/persistent_session.js

// initialize default method setting
var default_method = 'persistent'; // valid options: 'temporary', 'persistent', 'authenticated'
if (Meteor.settings &&
    Meteor.settings.public &&
    Meteor.settings.public.persistent_session) {
  default_method = Meteor.settings.public.persistent_session.default_method;
}
  • Did I misread the docs? I expected the default Session.set() and Session.get() - commands to stay temporary if Meteor.settings.public.persistent_session wan't set, but they were indeed persistent, which would confuse my code (and actually third party - code which relied on sessions being non-persistent).
@DanielDornhardt
Copy link
Author

Also, it might be nice to document that the "persistent" - option also leads to session variables being shared across browser windows/tabs. That is implied by using localstorage, but that makes the behaviour differ from the default session in two major ways (persistent across reloads + shared between windows), not only one.

@nickw
Copy link

nickw commented Mar 3, 2015

Agreed, it appears the docs are incorrect. I was just bit by this as well.

@pauldowman
Copy link
Member

Thanks @DanielDornhardt and @nickw, and sorry for the slow reply. We'll fix it!

@pauldowman
Copy link
Member

I tend to think it should be as the docs say, which will actually be a breaking change, what do you think @rgould? Definitely should bump the major version # then.

@nickw
Copy link

nickw commented Mar 3, 2015

@pauldowman yeah, that was my expectation as well.

@rgould
Copy link
Contributor

rgould commented Mar 16, 2015

This should be fixed by df73869, which has been released now. Reopen this if the issue persists!

@rgould rgould closed this as completed Mar 16, 2015
@DanielDornhardt
Copy link
Author

Thank you!

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

No branches or pull requests

4 participants