addInitializer callback arguments
Closed this issue · 6 comments
IIUC, addInitializer
callbacks are invoked with this
argument set but not a target
in the argument list. I found that this is not somewhat aligned with API patterns in the other part of the language, like Proxy
handlers (function(target, ...args)
), not even the decorator function themselves (function(targetValue, context)
).
Is there any specific reason to prefer the this
argument in addInitializer
callbacks?
My thinking when doing it this way was that initializer code is analogous to running code in the constructor, similar to class field initializers or calling a method from the constructor. So it seemed more consistent in that sense.
I generally agree though, APIs which utilize this
binding are a bit odd usually and generally require more thought and care when being interacted with, and I'm not sure there's really a benefit here, vs decorating a method where you have to take that into consideration.
@pzuraq My thinking when doing it this way was that initializer code is analogous to running code in the constructor, similar to class field initializers or calling a method from the constructor.
But those constructor, class field initializers are written in the context of the class blocks. The initializers to be used in addInitializer
callbacks are probably not. They typically are provided by libraries, like the example (https://github.com/tc39/proposal-decorators#example-initcustomelement). So in this pattern, the initializers have to be written as function () {}
s but not arrow functions () => {}
. That can be a bit awkward constraint.
I’m not sure why - arrow functions are a strange choice for working with instances, since they’re receiver-sensitive. Arrow functions are mostly meant for inline callbacks.
An arrow function expression is a compact alternative to a classic function expression but is limited and can't be used in all situations. It does not have its own bindings to this
.
For example, this is a clear mistake:
function deco(value, context) {
return () => {
this.something(); // The arrow function don't have own this
}
}
Other mistake, perhaps less clear is:
function deco(value, context) {
context.addInitializer(() => {
this.something(); // The arrow function don't have own this
});
}
As addInitializer()
receives a function and the people usually pass arrow function as a parameter, this may be a common mistake. I am not sure if the solution to avoid the confusion is to improve documentation and training or to pass this
as a parameter.
@ljharb the context in which these initializer are written is an inline callback, to be fair, passed to addInitializer
. I think this is why @legendecas and others have had the intuition that an arrow function should be ok to use, even though it is not currently.
For consistency, we're keeping addInitializer
callbacks as is, plain functions called with the this
context as the instance. This is more consistent with the way decorator functions are called in general, and decorator authors are already used to having to do this, so it should be less confusing to do it this way.