[BUGFIX release] Ensure ember-testing is loaded lazily #19540
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The recently modules API update means we are now loading real modules,
not polyfills based on the global. This means that the modules
themselves are _eagerly required_, rather than being references to a
value on the global. For example, previously, this:
```js
import { registerWaiter } from '@ember/test';
if (someCondition) {
registerWaiter(() => {});
}
```
Would become this:
```js
if (someCondition) {
Ember.Test.registerWaiter(() => {});
}
```
In either example, `registerWaiter` may or may not be called based on
the state of `someCondition`. However, in the second case, if
`Ember.Test` is not defined at all, it's completely ok as long as
`someCondition` is `false`. It's never called, so we never get an error
telling us `Ember.Test` is undefined.
Without the transform, the module is eagerly required, along with all of
its dependencies. If no one included `ember-testing`, then that means
it will throw an error immediately.
This PR makes the `@ember/test` module load `ember-testing` lazily, and
if it's not available (e.g. in a production environment) it replaces the
values with a function that throws a helpful error.
runspired
approved these changes
May 11, 2021
| unregisterWaiter = Test.unregisterWaiter; | ||
| } else { | ||
| let testingNotAvailableMessage = () => { | ||
| throw new Error('Attempted to use test utilities, but `ember-testing` was not included'); |
There was a problem hiding this comment.
probably this should include a note that the user may want to wrap their usage in if (DEBUG) {} or in if (Ember.testing) {}, otherwise lgtm.
|
Starting some digging: I'm seeing what appears to be a related error on 3.28.0-beta.6. It's not the same issue, but appears similar: it appeared for us on 3.27.0 and continued through 3.27.3 and up into the 3.28 beta series. I will likely open a new issue and linking it to this one tomorrow as I start chasing it down! |
22a
added a commit
to 22a/ffxiv-meter
that referenced
this pull request
Aug 7, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The recently modules API update means we are now loading real modules,
not polyfills based on the global. This means that the modules
themselves are eagerly required, rather than being references to a
value on the global. For example, previously, this:
Would become this:
In either example,
registerWaitermay or may not be called based onthe state of
someCondition. However, in the second case, ifEmber.Testis not defined at all, it's completely ok as long assomeConditionisfalse. It's never called, so we never get an errortelling us
Ember.Testis undefined.Without the transform, the module is eagerly required, along with all of
its dependencies. If no one included
ember-testing, then that meansit will throw an error immediately.
This PR makes the
@ember/testmodule loadember-testinglazily, andif it's not available (e.g. in a production environment) it replaces the
values with a function that throws a helpful error.
Fixes #19538, emberjs/ember-cli-babel#406