[decorators] Add support for initializers #9008
Conversation
nicolo-ribaudo
added
the
PR: New Feature 🚀
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/9681/ |
| } | ||
| }); | ||
|
|
||
| elements.forEach(function(element /*: ElementDescriptor */) { |
There was a problem hiding this comment.
Why two loops instead of combining the two?
There was a problem hiding this comment.
Becose they differ both in the if condition and in the behavior inside the if.
There was a problem hiding this comment.
I just noticed that I misinterpreted your question 🙃
The proposal says that all methods must be defined before the fields/initializers. If I used a single loop, that order wouldn't be maintained.
There was a problem hiding this comment.
This patch looks great, both the implementation and the tests! I am glad to see that it still makes sense to follow along with the spec.
You may want to add tests that demonstrate the evaluation order of static finishers vs static initializers vs static fields, and own initializers vs own fields. But this is fine to do in a follow-on PR.
|
Thanks |
|
The new test of ordering looks good to me. I'd say, this patch is ready to land, though note that we haven't discussed the feature in TC39 yet. |
|
When is the next meeting? I can enable it using a flag in the meantime |
|
November 27-29. Enabling with a flag SGTM, assuming there are no stability properties attached to the flag. |
|
We discussed this at the meeting today and decided to wait for the TC39 meeting, since we don't want to introduce a flag which we might deprecate in two weeks. |
nicolo-ribaudo
moved this from In progress
to In Progress - Low priority
in Class features
nicolo-ribaudo
moved this from In Progress - Low priority
to In progress
in Class features
|
@littledan What is the meeting outcome? |
|
Blocked by tc39/proposal-decorators#182 |
|
@nicolo-ribaudo We seem to be in agreement about adding the feature, but a name bikeshed remains. |
nicolo-ribaudo
force-pushed
the
decorators-initializer
branch
from
68c566e
to
34d14e5
Compare
|
The feature is now enabled behind an option: |
|
tc39/proposal-decorators#199 changed the name of initializers |
|
I will open a different PR for "hooks" |
lock
bot
added
the
outdated
tc39/proposal-decorators@cc2db18