Remove global Ember import #413
Conversation
That's one of the deprecations that prevent Ember 4 compatibility
|
@josemarluedke I wouldn't have merged this as it still fails on ember-canary (and maybe on other Ember versions, too, but they were cancelled early) 😬 |
|
I believe it's failing because ember-auto-import v1 is used and ember v4 requires v2. |
|
If that was the case then #414 would pass, wouldn't it? 🤔 |
|
Yes, when rebased with the changes from this PR. |
|
Published as v4.0.0. |
| @@ -137,7 +137,7 @@ export default class ApolloService extends Service { | |||
| let config = getOwner(this).resolveRegistration('config:environment'); | |||
| if (config && config.apollo) { | |||
| return config.apollo; | |||
| } else if (Ember.testing) { | |||
| } else if (isTesting()) { | |||
There was a problem hiding this comment.
This could (should?) be combined with macroCondition(...), so the testing code is stripped out when isTesting() is false.
| @@ -137,7 +137,7 @@ export default class ApolloService extends Service { | |||
| let config = getOwner(this).resolveRegistration('config:environment'); | |||
| if (config && config.apollo) { | |||
| return config.apollo; | |||
| } else if (Ember.testing) { | |||
| } else if (isTesting()) { | |||
There was a problem hiding this comment.
This could (should?) be combined with macroCondition(...), so the testing code is stripped out when isTesting() is false.
There was a problem hiding this comment.
@bertdeblock Thanks for pointing this out! I created another PR to address this: #416 /cc @josemarluedke
|
I think the PR description is a bit confusing. |
|
@bertdeblock You're right. I was under the impression that importing the global Ember will throw an error in Ember 4 but now I re-read the entry in the deprecation guide and it turns out that it only throws if you use I still think this is very valuable to have (otherwise this add-on prevents tree-shaking parts of Ember) but the description is indeed misleading, @josemarluedke sorry about that. |
Yup, this seems better anyways 👍 |
This is one of the deprecations that prevent Ember 4 compatibility.
isTestingcan be imported from@embroider/macrosand relying on this package does not mean the add-on needs to be built with Embroider.