tc39/proposal-decorators

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.