emberjs / ember-classic-decorator Goto Github PK

in my app, updating ember-classic-decorator from 1.0.3 -> 1.0.5 as the only change throws console errors and breaks the app when running in dev mode

vendor.js:113160 Uncaught TypeError: Cannot read property '@ember-data/model/index' of undefined
    at patchDataDecorators (vendor.js:113160)
    at vendor.js:376666
vendor.js:252 Uncaught Error: Could not find module `miragejs` imported from `ember-cli-mirage/response`
    at missingModule (vendor.js:252)
    at findModule (vendor.js:263)
    at Module.findDeps (vendor.js:173)
    at findModule (vendor.js:267)
    at Module.findDeps (vendor.js:173)
    at findModule (vendor.js:267)
    at Module.findDeps (vendor.js:173)
    at findModule (vendor.js:267)
    at requireModule (vendor.js:29)
    at r (vendor.js:181)
    at resolveInitializer (vendor.js:359737)
    at registerInitializers (vendor.js:359754)
    at loadInitializers (vendor.js:359795)
    at Module.callback (neon-tetra.js:1442)
    at Module.exports (vendor.js:111)
    at requireModule (vendor.js:32)
    at neon-tetra.js:46831

$ ember -v
ember-cli: 3.12.0
node: 10.17.0
os: darwin x64

Here is a reproduction. Ignore the repo name, initially I thought this is a Babel bug because I couldn't imagine what else could be doing that. But it's ember-classic-decorator.

Clone the repo, npm install, ember serve --environment=production. Then, in dist/assets/babel-bug-*.js you'll see undefined !== this.bar. The original is cols[0]?.bar !== this.bar. I've no idea who or why it changes this but this is a major problem.

ember new classic-decorator-bug
cd classic-decorator-bug
ember install ember-classic-decorator
ember serve

You'll see a bunch of Importing from @ember/string without having the @ember/string package in your project is deprecated. warnings in the console. They shouldn't be there as a brand-new Ember app doesn't have them. Remove ember-classic-decorator and run the app to see.

Hi!
I have a question about the classic decorator. Documentation says it needs to be used only for the classes that inherits directly from EmberObject. However when I try to extends from RESTAdapter or RESTSerializer like in the example below, it raises an error: Uncaught (in promise) Error: You defined the class ApplicationEmberObject that extends from Ember.Object using native class syntax, but you didn't mark it with the @classic decorator. All user classes that extend from this class must be marked as @classic, since they use classic features. If you want to remove the @classic decorator, see the guides on how to convert this class into the Octane version: [TODO_GUIDE_LINK].

import classic from 'ember-classic-decorator';
import RESTSerializer from 'ember-data/serializers/rest';

export default class ApplicationEmberObject extends RESTSerializer {
  serializeIntoHash(data, _type, record, options) {
    Object.assign(data, this.serialize(record, options));
  }
}

Adding @classic on top of the export default class ApplicationEmberObject extends RESTSerializer removes the error. Is this a bug?

Btw. I think it would be good to link TODO_GUIDE_LINK.

After upgrading to Ember 3.27, I get an error for each adapter and serializer, even though they extend from DS.Adapter and DS.Serializer, respectively:

Error while processing route: ... You defined the class <app@serializer:my-model::constructor> that extends from EmberObject using native class syntax, but you didn't mark it with the @classic decorator. All user classes that extend from this class must be marked as @classic, since they use classic features. If you want to remove the @classic decorator, you must remove the base class. For components, you can do this by converting to Glimmer components. For plain classes that extend from EmberObject, you can convert them into plain native classes that do not extend from EmberObject.

These modules no longer resolve:

issue #30 was fixed by #33.
Since last release was in November, I would appreciate a new release with this fix to NPM.

Thanks.

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.

This one was a bit tricky to debug, but what I've found is if you extend a service from another service, both native (not-classic), and both use a constructor, (and have ember-classic-decorator package installed), the app will crash

// app/services/abc.js
export default class Abc from Service {
  constructor(...args) {
    super(...args)
  }
}
// app/services/def.js
export default class Def from Abc {
  constructor(...args) {
    super(...args)
  }
  
  someMethod() { ... }
}

// app/routes/application.js
@service def
model() {
  this.def.someMethod() // crashes here
}

You get this fun error

The relevant logic that throws that error is here

I think the underlying cause is this part of ember.js, which (when debug mode) uses EmberObject with an init hook as an ancestor of the Service instead of the CoreObject.
https://github.com/emberjs/ember.js/blame/297ab0e771409d61dcb014a9a40144cd7fdd5ee8/packages/@ember/-internals/runtime/lib/system/object.js#L34-L51

That's about as far as my understanding goes. It only happens in debug, with ember-classic-decorator installed, so it's not a huge deal, but it's a head-scratcher & time-waster so I wanted to post it in case it's an easy fix or so that others can google their way here and not waste as much time as I did 😅

Workaround, which may not work for some people, is to refactor out the constructor in the higher level ("Def") service

I am trying to have an Ember app embedded into a 3rd party page, and to coexist peacefully with its JS assets, including those based an AMD modules, that use incompatible implementations of define() and require() than those that Ember/loader.js defines (globally!).

Using either the approach to call loader.noConflict() (see ember-cli/loader.js#204) at the end of app.js or using transforms like https://github.com/richardaiko/ember-derequire fail due to this addon patching (and later reverting) the global require() here. In the case of using noConflict(), what happens is that noConflict() reverts correctly, but this addon reverts it overwritten require() afterwards, but again with Ember's version. So the globally defined one remain that of Ember/loader.js, which breaks other 3rd party scripts relying on the original AMD version.

Not sure how best to fix this universally. FWIW, I ended up removing @classic due to this...

All these usages cause Ember 3.27 to trigger deprecation warnings. It would be nice to have those fixed before Ember 4 as otherwise ember-classic-decorator won't be usable with it.

Somewhat related to #63.

Would it be possible to not use the side-effect of babel transpilation to figure out which modules belong to the app vs addons?

Under embroider, there is a single babel and a single babel config, instead of one per addon. So this kind of side effect is not reliable. As a result, ember-classic-decorator will incorrectly decide that addon modules are in __CLASSIC_OWN_CLASSES__ and emit errors for them.

I've found that extending models and routes (haven't tested with anything else yet) with a mixin mixed in doesn't throw an error and seems to work just fine.

export default class XModel extends Model.extend(XMixin) {}

In docs it states

In order to remove the classic decorator from a class, you must:

• Remove usage of mixins from the class

So now I'm confused whether I should introduce @classic on these classes or not.

Bonus question: About utility classes docs state:

Certain classes must always be marked as classic:

• Utility classes that extend directly from EmberObject

To remove the @classic decorator from them, you can:

• Rewrite utility classes so they do not extend from EmberObject at all, and only use native class syntax.

So does that mean that a class needs @classic only if it extends EmberObject directly or if any of its ancestors extends it?

Hi there.

It seems inheriting models is not allowed.

Example:

In foo.js:

import Model from '@ember-data/model';

export default class Foo extends Model {}

In foo-bar.js

import Foo from './foo';

export default class FooBar extends Foo {}

Then, when creating a FooBar: 💥

Using 3.0.0 but tried out 2.0.1 with the same result, with Ember 3.28

Cheers!
Anders

It's legal for the user to shadow window with a local variable:

let window = 'whatever';
class Foo {}

Which will result in:

let window = 'whatever';
class Foo {}
window.__CLASSIC_OWN_CLASSES__.set(Foo, true);

which throws an exception.

I think the output should change to:

__CLASSIC_OWN_CLASSES__.set(Foo, true);

so it's always accessing the global scope, regardless of what happens to window.

If the default resolver is not included, the try/catch in the resolve patch will throw before patching Ember Data and never resolve correctly. We should replace the try/catch entirely with a checking the module registry directly to see if the packages exist, before patching them.

Hi, I am trying to convert an addon within a yarn workspace. The application package.json contains the ember-classic-decorator package and changes there work. However when i convert a class in the addon I'm getting 'Could not find module ember-classic-decorator imported from @plhw-lab/integration-clouddrive/services/cloud-drive-integration.

so I've tried to install the 'ember-classic-decorator' directly in the addon's package. And I've moved the 'ember-classic-decorator' requirement from devDepenndencies to dependencies inside the application package. However I'm getting the same error.

Should this addon work with addons?

After installing 1.0.4 our build failed because the vendor.js file went up by almost 200 kb. I guess that's because of the ember-data dependency. We don't use ember-data. Why is it included in the bundle?

After applying the @classic decorator, I'm seeing this error with ember-concurrency tasks.

"eslint-plugin-ember": "^10.5.1",
"babel-eslint": "^10.1.0",

We are looking to incrementally move to native classes, e.g start with services. However, if I install this addon, I get runtime errors for all existing components, telling me I need to decorate it with @classic. Is there a way to turn this off?

After upgrade to v0.1.3 I got following issue:

window.__CLASSIC_HAS_CONSTRUCTOR__.add is not a function

 at Module.callback (http://localhost:7357/assets/vendor.js:109084:38)
    at Module.exports (http://localhost:7357/assets/vendor.js:111:32)
    at Module._reify (http://localhost:7357/assets/vendor.js:148:59)
    at Module.reify (http://localhost:7357/assets/vendor.js:135:27)
    at Module.exports (http://localhost:7357/assets/vendor.js:109:10)
    at Module._reify (http://localhost:7357/assets/vendor.js:148:59)
    at Module.reify (http://localhost:7357/assets/vendor.js:135:27)
    at Module.exports (http://localhost:7357/assets/vendor.js:109:10)
    at Module._reify (http://localhost:7357/assets/vendor.js:148:59)
    at Module.reify (http://localhost:7357/assets/vendor.js:135:27)

Even with ie11 in targets.js, ES6+ code is being generated - the let keyword is being used and catch without parentheses for example.

We have a custom location class that extends HistoryLocation from Ember. Looks like it throws this error, even though the extended class does not add its own constructor or init methods.

Error: You defined the class location with a 'constructor' function, but one of its ancestors, DebugFrameworkObject, uses the 'init' method. Due to the timing of the constructor and init methods, its not generally safe to use 'constructor' for class setup if the parent class is using 'init'. You can mark location with the @classic decorator and safely use 'init', or you can convert the parent class to use native classes, and switch from 'init' to 'constructor' there first.

Hi
After upgrading an addon to Ember 3.14 Im getting an error:
__EMBER_CLASSIC_DECORATOR is not defined

Addon v1.0.5

Any clue? :)

Check this comment.

Otherwise, just add ember-classic-decorator to a brand new Ember 5 app and watch it break.

I updated my TypeScript version to 4.8.2 (from 4.7.4) and started getting a bunch of bogus warnings from both ember/classic-decorator-hooks and ember/classic-decorator-no-classic-methods. That is, I started getting those on pretty much every class.

P.S. That could be related to TypeScript 4.8's change to decorator handling?

ember-classic-decorator replaces the require function here and doesn't set other required things on it like has. This breaks at least ember-cli-deprecation-workflow v2 which uses it. A discussion around that can be found after this comment.

When using the 3.0 classic decorator on a 4.1 app we get this error on our application serializer. Oddly enough on our 3.28 version with the 2.0 decorator we didn't see this error. I've added the @classic decorator to allow the app to load, but seems like this is a mistake.

Error: You defined the class <my-app@serializer:application::constructor> that extends from EmberObject using native class syntax, but you didn't mark it with the @classic decorator. All user classes that extend from this class must be marked as @classic, since they use classic features. If you want to remove the @classic decorator, you must remove the base class. For components, you can do this by converting to Glimmer components. For plain classes that extend from EmberObject, you can convert them into plain native classes that do not extend from EmberObject.

If this class is a base class provided by the framework that you should be allowed to remove @classic from, please open an issue on the classic decorators repo: https://github.com/emberjs/ember-classic-decorator

Using the latest versions of ember-exam (6.0.0) and ember-classic-decorator (2.0.0) causes ember-cli-dependency-lint to fail with:

$ ember dependency-lint
@embroider/macros
Allowed: (any single version)
Found: 0.24.1, 0.27.0
frontend
├─┬ ember-classic-decorator
│ └── @embroider/[email protected]
└─┬ ember-exam
  └── @embroider/[email protected]

I would guess it's not a problem but updating the version of @embroider/macros used by ember-classic-decorator to match the one in ember-exam would be nice so that the error goes away.

P.S. Actually it's not only ember-cli-dependency-lint that complains. I cannot even build the project any more:

ATTENTION: ember-cli-addon-guard has prevented your application from building!

Please correct the following errors:

Your application is dependent on multiple versions of the following run-time addon:

@embroider/macros
----------------------------------------
frontend
├─┬ ember-classic-decorator
│ └── @embroider/[email protected] (cacheKey: 3b57fcc2d1c3e3d29327e06a4ab3bc7f, runtime: true)
└─┬ ember-exam
  └── @embroider/[email protected] (cacheKey: cd94ba305c334ca9be294e943e57d7ff, runtime: true)

I'm using Ember 3.11.1 and latest version of ember-classic-decorator (0.1.4) and I'm getting error during boot:

TypeError: window.__CLASSIC_HAS_CONSTRUCTOR__.add is not a function

From what I see window.__CLASSIC_HAS_CONSTRUCTOR__ is a WeakMap which does not implement add function. Any idea what can be wrong?

Why does an observer get converted from this:

doSomething: observer('a', function() {
  // ...		
})

into this:

import { observes } from '@ember-decorators/object';

@observes('a')
doSomething() {
  // ...
}

instead of this?

import { observer } from '@ember/object';

@observer('a')
doSomething() {
  // ...
}

This doesn't match with how CPs are converted.