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

isTesting from @embroider/macros returns false in tests if ember-classic-decorator is a devDependency #99

Open
mrloop opened this issue Oct 5, 2022 · 2 comments · May be fixed by #105

Comments

@mrloop
Copy link

mrloop commented Oct 5, 2022

I've a minimal reproduction here https://github.com/mrloop/em-user-activity initially thought it was ember-user-activity causing the issue but it depends on ember-classic-decorator and the issue occurs with just ember-classic-decorator declared as a devDependency.

In the demonstration app the following test passes when ember-classic-decorator is not used.

import { module, test } from 'qunit';
import { setupTest } from 'my-data/tests/helpers';
import { isTesting } from '@embroider/macros';

module('Unit | Controller | isTesting', function (hooks) {
  setupTest(hooks);

  test('it exists', function (assert) {
    assert.true(isTesting(), 'isTesting should be true');
  });
});

When ember-classic-decorator is installed isTesting() returns false in the test.

@mrloop mrloop changed the title @embroider/macros isTesting returns false from tests if ember-classic-decorator is a devDependency isTesting from @embroider/macros returns false in tests if ember-classic-decorator is a devDependency Oct 6, 2022
@bartocc
Copy link

bartocc commented Nov 7, 2022

Took me quite a while to get to the same conclusion @mrloop!

To give a bit more context, this issue happens when using a combinasion of [email protected]+ AND [email protected]+.

I hit this issue while upgrading or app from [email protected] to [email protected]. To be more precise, the first ember-data release causing the issue is [email protected]. And our app also uses [email protected]

Looks like ember-classic-decorator is causing the require of module @embroider/macros/runtime

At the end of this module, we do

  let updaters = typeof window !== 'undefined' ? window._embroider_macros_runtime_config : undefined;
  if (updaters) {
    let methods = updaterMethods();
    for (let updater of updaters) {
      updater(methods);
    }
  }

But window._embroider_macros_runtime_config is not set yet
=> so we don't enter the if
=> we don't run any updater
=> we don't set isTesting to true

@bartocc
Copy link

bartocc commented Nov 7, 2022

Here's a repo with a bare ember app + [email protected] and [email protected] and a test to reproduce.

https://github.com/bartocc/ember-classic-decorator-issues-99

andreyfel added a commit to andreyfel/ember-user-activity that referenced this issue Jan 24, 2023
ember-classic-decorator was supposed to be used during the transition
period while converting from classic classes to native.

It is time to get rid of it as it creates issues like this:
emberjs/ember-classic-decorator#99
emberjs/ember-classic-decorator#74
etc.

The transition is easy, just replace this.set with set(this, ...
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 a pull request may close this issue.

2 participants