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 #setup method to session service #2314

Closed
4 tasks
BobrImperator opened this issue Aug 13, 2021 · 3 comments · Fixed by #2318
Closed
4 tasks

Add #setup method to session service #2314

BobrImperator opened this issue Aug 13, 2021 · 3 comments · Fixed by #2318

Comments

@BobrImperator
Copy link
Collaborator

BobrImperator commented Aug 13, 2021

Related to #2307

During our internal talks We've decided that the way forward is to have a some kind of setup method that users will need to call to spin up the session service. Currently it's done 'automatically' via initializers, but this isn't the best solution since we're extending user code and it also leads to build problems described in #2307.

TODO

  • implement #setup method on session service
  • copy setup code from /initializers/setup-session-restoration
  • Add build configuration to conditionally funnel out the /application route
  • Based on the configuration make it so initializer's code doesn't fire /initializers/setup-session-restoration

The goal API for users to use would be like this:

@service session;

async beforeModel() {
  await this.session.setup();
}

Users would explicitly call setup method without relying on initializers.

@k3n
Copy link

k3n commented Sep 23, 2021

Is this what's referred to here?

http://ember-simple-auth.com/api/classes/ApplicationRouteMixin.html

Deprecated: Call the session service's setup method in the application route's constructor instead

@BobrImperator
Copy link
Collaborator Author

BobrImperator commented Sep 23, 2021

No and unfortunately I can't really answer you why does it say to call setup method when it doesn't exist in ESA until #2318 lands.

What it means though is that it should be safe for you to remove ApplicationRouteMixin completely, so your code is like this:

import Route from '@ember/routing/route';
import type IntlService from 'ember-intl/services/intl';
import { inject as service } from '@ember/service';

export default class Application extends Route {
  @service intl: IntlService;

  beforeModel(transition) {
    this.intl.setLocale('pl-PL');
    return super.beforeModel(transition);
  }
}

Later on though after #2318 is merged, you'll need to call the setup method though.

import Route from '@ember/routing/route';
import type IntlService from 'ember-intl/services/intl';
import { inject as service } from '@ember/service';

export default class Application extends Route {
  @service intl: IntlService;
  @service session;

  async beforeModel(transition) {
    await this.session.setup();
    this.intl.setLocale('pl-PL');
    return super.beforeModel(transition);
  }
}

Feel free to let me know if you still have any doubts @k3n

@k3n
Copy link

k3n commented Sep 23, 2021

I'm a tad confused by your response, but you answered my question indirectly by pointing out #2318 will fulfill the deprecation message mentioned in the docs, so I'm not worried about it. Thank you!

kevinansfield added a commit to TryGhost/Admin that referenced this issue Jan 22, 2022
no issue

- many "The automatic session initialization is deprecated" were shown in test output due to an old method of initializing the session service
- switched to explicit session setup in the application route's `beforeModel` hook
- mainmatter/ember-simple-auth#2314
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants