-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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 LTS 4.4] Don't run getters while applying mixins #20405
Conversation
This change ensures that getters are never evaluated while applying mixins. It relies on the fact that all getters (including undecorated ones) get converted into classic decorators when the mixin is originally created.
5e17b73
to
281760d
Compare
Ember CLI dropped support for Node 12 in 4.6.0.
281760d
to
3746d7d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me! I backported the node version change, because outdated node is what was making the smoke test fail.
@@ -155,7 +155,7 @@ | |||
"typescript": "~4.6.0" | |||
}, | |||
"engines": { | |||
"node": ">= 12.*" | |||
"node": ">= 14.*" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we can bump this in a point release since it would be a breaking change that would be very unexpected. Bumping the version for a particular CI job should be fine though I am curious why it failed: theoretically the smoke test should work on 4.4 without changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't the rule about releases that each release supports whatever node version is still under maintenance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We discussed on discord, but writing here as well: I think a node version bump in a patch release would be very surprising to users. The policy states only when we drop support on the primary branch of ember-cli https://github.com/ember-cli/ember-cli/blob/master/docs/node-support.md
#20405 changes engines.node. We can't do that within the LTS channel without breaking peope.
Backport of #20388
This change ensures that getters are never evaluated while applying mixins.
It relies on the fact that all getters (including undecorated ones) get converted into classic decorators when the mixin is originally created.