Get rid of ember-classic-decorator dependency #789
Conversation
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, ...
It seems to be something super legacy. Removing it also fixes embroider scenarios.
|
@elwayman02 please take a look |
|
Thanks for bringing up this PR! It'll be great to get another legacy dependency out of the way. I left a few small comments that should be easy to address, and I think we'll be good after that. :) |
|
@elwayman02 I've addressed most of your comments and made a few other modernizations, please take a look again. |
There was a problem hiding this comment.
These changes look great, but there appears to be a small issue with the demo app where the userActive event got duplicated for all columns instead of a list of different events:
You can see the expected behavior here: https://ember-user-activity.jhawk.co/
| @@ -25,13 +25,6 @@ module.exports = { | |||
| env: { | |||
| browser: true, | |||
| }, | |||
| rules: { | |||
| let owner = getOwner(this); | ||
| return owner.lookup('service:fastboot'); | ||
| get _isFastBoot() { | ||
| return typeof FastBoot !== 'undefined'; |
| set(this, 'EVENT_THROTTLE', 0); | ||
| } | ||
| // Do not throttle in testing mode | ||
| this.EVENT_THROTTLE = Ember.testing ? 0 : 100; |
ember-data was not used ember-decorators is no longer used
I saw this technique in ember-data repo as a modern trick to detect if we are in a FastBoot env.
user-activity and user-idle services suppose that inheriting services override `defaultEvents` and `activeEvents` through default property values which is not possible if we change init to constructor. We will need a breaking change in API in order to migrate to constructors in these services. Leave TODOs there for now.
andreyfel
force-pushed
the
get-rid-of-ember-classic-decorator
branch
from
2b15cdd to
c66811f
Compare
|
@elwayman02 oops, I messed up the glimmer component conversion, now it is fixed. |
|
Bonus celebration: These changes made the embroider-safe build finally pass! Nicely done. |
|
@elwayman02 thanks for merging! Will you be able to cut a release with these changes? |
elwayman02
added
internal
dependencies
|
Changes have been released! |
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, ...