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

[BUGFIX release] Ensure ember-testing is loaded lazily #19540

Merged
merged 1 commit into from
May 12, 2021

Conversation

pzuraq
Copy link
Contributor

@pzuraq pzuraq commented May 11, 2021

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:

import { registerWaiter } from '@ember/test';

if (someCondition) {
  registerWaiter(() => {});
}

Would become this:

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.

Fixes #19538, emberjs/ember-cli-babel#406

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.
unregisterWaiter = Test.unregisterWaiter;
} else {
let testingNotAvailableMessage = () => {
throw new Error('Attempted to use test utilities, but `ember-testing` was not included');
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

@chriskrycho
Copy link
Contributor

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Could not find module ember-testing imported from @ember/test/index on ember 3.27.0
3 participants