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
[Bug]: Decorator execution order does not match evaluation order @babel/plugin-proposal-decorators
#15608
Comments
Hey @pgoforth! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly. If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite. |
Good question. The behaviour is expected. The related evaluation order is specified in 15.7.6 ApplyDecoratorsToElementDefinition step m:
The Now in your case, the decorators are applied in this order:
When decorator2 is applied, the initializer which prints When decorator1 is applied, the initializer which prints At this point, the function set(val) {
// defined by `decorator1`
log.push('first: value')
return (function set(val) {
// defined by `decorator2`
log.push('second: value')
return set.call(this, val)
}).call(this, val)
} When It seems to me the |
So if I'm reading what you are saying correctly @JLHwung, the spec is being adhered to correctly, but the result is unexpected and perhaps the spec needs adjustment? |
Personally I don't think the current spec behaviour is unexpected: For every class element, it has a list of initializers, but it has at most one setter, so the modified setter must overwrite what was defined before. If you believe such behaviour is unwanted, please open a new issue in https://github.com/tc39/proposal-decorators. Babel will follow if the spec behaviour is changed. |
I do not believe any behavior related to multiple initializers and one setter is inappropriate, my objection is to the order of execution of the |
Maybe you can comment on 7.3.34 InitializeFieldOrAccessor and 7.3.35 InitializeInstanceElements. It is straightforward for me that Anyway I will close this issue as there is no action item for Babel at this moment. |
@pgoforth the ordering is consistent in that it is always applied from innermost decorator to outermost decorator. In the case of methods, getters, and setters, this feels a bit different because of the nature of “outermost”. When a method is wrapped, the outer method can order itself before or after the inner method. For instance, you could update your example like to so to have consistent ordering: function logAccessorDecoratorRun(log, name) {
log.push(`${name}: generator`);
return function (target, { addInitializer }) {
log.push(`${name}: wrapper`);
addInitializer(function () {
log.push(`${name}: addInitializer`);
});
const { init, set } = target;
return {
init(initialValue) {
log.push(`${name}: init: ${initialValue} - ${init}`);
return init ? init.call(this, initialValue) : initialValue;
},
set(val) {
let ret = set ? set.call(this, val) : val;
log.push(`${name}: value: ${val}`);
return ret;
},
};
};
} |
@JLHwung Wanted to bring this up for reference. tc39/proposal-decorators#508 |
@pgoforth Thanks for the heads-up. As I mentioned in another thread (#15570 (comment)), this issue will be addressed in #15570. |
💻
How are you using Babel?
babel-loader (webpack)
Input code
Configuration file name
babel.config.js
Configuration
Current and expected behavior
When using
@babel/plugin-proposal-decorators
at version2022-03
Behavior
Using the following decorator:
Current
The order of the
init
method does not match the sequential order of theset
method when more than one decorator is applied to the same initialized accessor.Expected
The order of the
init
method should match the sequential order of theset
method when more than one decorator is applied to the same initialized accessor.Environment
System:
Binaries:
npmPackages:
Possible solution
I can see two possible solutions:
Currently they are evaluated in order, but they initialization happend inside-out, and then execution of get/set happens in order. If the initialization is changed to evaluate in spec order then the issue would be resolved.
I believe that the first option more closely matches the intention of the spec, but the issue would be resolved through the second option as well. I am presenting both because I do not know which would be easier.
Additional context
The spec only provides for the order of initial evaluation of the decorators, so I can see where there might be confusion. I believe the intention of the spec is that the decorators are not only evaluated in the specified order, but also have their calling methods maintain that same order.
The text was updated successfully, but these errors were encountered: