search

mansona / ember-cli-notifications

⚛ Atom inspired notification messages for ember-cli
https://ember-cli-notifications.netlify.app/
MIT License
177 stars 94 forks source link

Stop using `ember-get-config` #364

Closed nlepage closed 9 months ago

nlepage commented 1 year ago

Problem

When running tests from the /tests URL after starting a development server with ember serve, the configuration returned by ember-get-config is stuck to development environment, whereas the configuration returned by the service's owner is from test environment.

This is specific to ember serve, it is not the case when using ember test --server, however using the /tests URL on the development server seems to be a fairly common usage, at least amongst the developers at https://github.com/1024pix.

ember-get-config isn't necessary, configuration can be retrieved via the notification service's owner.

Also quoted from ember-get-config's documentation :

Gaining access to an app's config file from an addon can be challenging. If possible, you should always get it through the container.

Solution

Stop using ember-get-config.

Thanks for your work :slightly_smiling_face:

netlify[bot] commented 1 year ago

Deploy Preview for ember-cli-notifications ready!

Name Link
Latest commit 257c68102fc1887065b676a7672ba0d90be6a14b
Latest deploy log https://app.netlify.com/sites/ember-cli-notifications/deploys/64c10f99c9f600000834526e
Deploy Preview https://deploy-preview-364--ember-cli-notifications.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

nlepage commented 1 year ago

I found out about #360

We are running on @embroider/compat v3.2.0, and still having the same problem, so it seems that embroider-build/embroider#1412 doesn't fix it...