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

[2314] Add setup method in session service #2318

Merged
merged 8 commits into from
Oct 29, 2021

Conversation

BobrImperator
Copy link
Collaborator

@BobrImperator BobrImperator commented Sep 3, 2021

closes #2314
closes #1854, and closes #2279

  • Adds #setup method for session service
  • Adds useSessionSetupMethod 'run time' configuration for setup-session-restoration intiializer to return early
  • Adds deprecation when initializer is being used indicating the migration to session#setup instead.
  • Documents how to use the #setup method along with the configuration options guides/upgrate-to-v4.md.

I was able to successfully confirm this working in my private project which conveniently is written in typescript #1854, #2279.

Copy link
Member

@marcoow marcoow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great! I think we should do a few more things though:

  • in all public methods of the session service, we should assert that setup has been called so that people that forget to do it get better feedback
  • use this new option in the test app
  • strip the initializer from the app build completely if the new option is set

guides/upgrade-to-v4.md Outdated Show resolved Hide resolved
@BobrImperator BobrImperator force-pushed the 2314-session-setup branch 4 times, most recently from cddfd74 to 687a94b Compare October 7, 2021 14:02
@BobrImperator BobrImperator requested review from marcoow and removed request for marcoow October 15, 2021 10:22
Copy link
Member

@marcoow marcoow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

guides/upgrade-to-v4.md Outdated Show resolved Hide resolved
packages/ember-simple-auth/bower.json Outdated Show resolved Hide resolved
@BobrImperator BobrImperator force-pushed the 2314-session-setup branch 2 times, most recently from dbe2f7e to 416703f Compare October 29, 2021 14:37
@BobrImperator BobrImperator merged commit 70f217a into mainmatter:master Oct 29, 2021
@bertdeblock
Copy link
Contributor

This PR also introduces 3 new deprecations (aside from the intended setup method one):

`import { deprecate } from '@ember/application/deprecations';` has been deprecated, please update to `import { deprecate } from '@ember/debug';`
When calling `deprecate` you must provide `for` in options. Missing options.for in "ember-simple-auth.initializer.setup-session-restoration" deprecation
When calling `deprecate` you must provide `since` in options. Missing options.since in "ember-simple-auth.initializer.setup-session-restoration" deprecation

@steveszc
Copy link

steveszc commented Nov 3, 2021

Should there be a link to the upgrade guide in the README?
Right now the README does not mention anything about the session's setup method...

@BobrImperator
Copy link
Collaborator Author

@steveszc Thanks, will try to work out better documentation for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants