Why no `addInitializer` for class fields?

Question

Why no `addInitializer` for class fields?

trusktr opened this issue 2 years ago · 28 comments

And why is a class field limited to only one initializer-returning decorator?

cannot add additional initialization logic

It is going to lead to frustration when someone tries to do this and it doesn't work:

@doSomethingWithInitialValue
@doSomethingElseWithInitialValue
foo = 123

when both return an initializer.

And if both of those should work and I read the explainer wrong (and it needs an update), then I see no reason why we can't keep the API consistent and use context.addInitializer() instead of a return value.

Answer 1 · 2022-05-17T18:36:57.000Z

The spec actually has addInitializer for fields, this was a bit of a mismatch between explainer and spec. Will be updating the explainer about this in the future.

And why is a class field limited to only one initializer-returning decorator?

They are not limited, they are sequenced. @doSomethingElseWithInitialValue would receive the value returned by @doSomethingWithInitialValue.

And if both of those should work and I read the explainer wrong (and it needs an update), then I see no reason why we can't keep the API consistent and use context.addInitializer() instead of a return value.

addInitializer initializers run prior to all class field assignments. They are a separate phase during construction and for performance reasons cannot be interleaved with normal class field initialzers.

Answer 2 · 2022-05-19T16:31:11.000Z

addInitializer initializers run prior to all class field assignments. They are a separate phase during construction and for performance reasons cannot be interleaved with normal class field initialzers.

Is there an explainer to the performance difference? At first thought, there doesn't seem to be any difference.

Answer 3 · 2022-05-20T22:42:55.000Z

@pzuraq I can't imagine why initializers added with context.addInitializer would be any slower than initializers added by returning them from a decorator.

I also don't see why context.addInitializer can happen at a different stage for class field (during initialization of the class field). addInitializer for methods already happens at a different time that addInitializer for classes. addInitializer for fields would happen on field initialization, which is after method initialization.

How does that make it slower?

Answer 4 · 2022-05-21T00:14:25.000Z

@trusktr the reason has to do with optimizations that browser engines make during the initialization of a class. Specifically, browser engines are able to create a single operation (as I understand it, this is probably dramatically simplifying) for the assignment of all of the class's fields. Because class fields are knowable at parse time, this operation can be created at one time during initial compile.

If context.addInitializer initializers could add an initializer just before or just after a class field was assigned, then it would mean that checks would have to be added for each decorated field, regardless of whether or not they actually used addInitializer. This was considered too much of an impact overall.

Previously, initializers did act this way, but we also had the @init: syntax required for them. That allowed the engine to know ahead of time whether or not it would need to emit that extra check. However, @init: was considered terrible for UX by many stakeholders, so we decided to compromise by not interleaving initializers.

This is documented in the PR which made this change: #436

Answer 5 · 2022-05-27T02:17:21.000Z

@pzuraq Thanks for explaining, but I still don't understand. I read #436, but it doesn't say much about those mechanics you mentioned.

How is it that either of these,

a decorator adds an initializer by returning it, or
a decorator adds an initializer by passing it into a context.addInitializer call

has anything to do with what the engine does with the initializers after the decorator call?

Either way, the engine will have the initializers at the same time, which is right after synchronous decorator execution.

Answer 6 · 2022-05-27T02:21:31.000Z

Or in other words, these two appear to lead to exactly the same result:

function decorator() {
  return () => 123
}

// engine calls decorator()
// engine now has initializer immediately after decorator() call

function decorator(_, ctx) {
  ctx.addInitializer(() => 123)
}

// engine calls decorator()
// engine now has initializer immediately after decorator() call (same as before!)

Answer 7 · 2022-05-27T16:31:02.000Z

From my understanding there are differences between the two kinds of field initializers. For example a returned initializer function (first example) is the only one defining the value for the property with its return value. Added initializer (second example) returns are ignored, those initializers having a return type of void. They simply run code after the property has been assigned - which is another difference and I think the most important here: returned initializers get called before the property is assigned to the instance whereas added initializers are called after. So they both get called when the instance has a different shape.

This is where I think the optimization breaks down (though I personally don't know anything about the actual optimizations). Where before you had:

🧑‍💼 User code in returned initializer
💻 Internal code define property

With the inclusion of addInitializer for fields you now have

🧑‍💼 User code in returned initializer
💻 Internal code define property
🧑‍💼 User code in added initializer

You now have two places to worry about user code running rather than one.

Because class fields are knowable at parse time, this operation can be created at one time during initial compile.

I'm not sure where this fits in because I don't think you should be able to know class fields at parse time, at least not per instance given you could have something like:

class A {
    x = A.o = this
    y = (() => { throw 0 })();
}
try { 
    new A
} catch {
    console.log('x' in A.o, 'y' in A.o) // true, false
}

The field y never makes it on to the instance of A. The interweaving is still necessary for fields because of cases like this, whereas with methods its not because they are all inherited in one fell swoop. Presumably with the extra initializer for fields any checks between field assignments that already exist would now get doubled (one before, and now one after).

Answer 8 · 2022-05-27T23:01:58.000Z

How is it that either of these,

a decorator adds an initializer by returning it, or

a decorator adds an initializer by passing it into a context.addInitializer call

has anything to do with what the engine does with the initializers after the decorator call?

I suggest reading this blog post for more details about the startup costs of class fields, and the types of optimizations that the Chrome team and others are taking to make them as performant as standard property assignment. While it doesn't touch much on this exact area, I think it demonstrates how this is a very important and area for optimizations, and that any extra operations we must perform here could have significant impacts at scale.

With the inclusion of addInitializer for fields you now have

🧑‍💼 User code in returned initializer

💻 Internal code define property

🧑‍💼 User code in added initializer

This is incorrect, the order is:

🧑‍💼 User code in added initializer
🧑‍💼 User code in returned initializer
💻 Internal code define property

. I'm not sure where this fits in because I don't think you should be able to know class fields at parse time, at least not per instance given you could have something like:

My understanding is that this is not about knowing the class fields themselves, it's about knowing when and how user code checks will have to be inserted. Again, IANA browser engine dev, but you could imagine that with this class:

class C {
  @dec
  a = 123;

  @dec
  b = 123;
}

With the current setup, the initialization logic looks like:

1. Create class object
2. Check to see if any added initializers exist. If so, run them.
3. Run `a` initializers
4. Define result of previous step on class object
5. Run `b` initializers
6. Define result of previous step on class object

Were the initializers interleaved, it would look like:

1. Create class object
2. Check to see if any added initializers exist for `a`. If so, run them.
3. Run `a` initializers
4. Define result of previous step on class object
5. Check to see if any added initializers exist for `b`. If so, run them.
6. Run `b` initializers
7. Define result of previous step on class object

This is an extra operation which would have to be added (e.g. you could not possibly know if there were added initializers until the decorator itself were evaluated, at run time).

Answer 9 · 2022-05-27T23:04:13.000Z

Or in other words, these two appear to lead to exactly the same result:

I think you may be misunderstanding how addInitializer works, as these two operations have different behavior.

For a field decorator, the returned callback is executed as a pipeline. This allows multiple decorators to affect the initialized value:

const addOne = () => value => value + 1;
const double = () => value => value * 2;

class C {
  @double
  @addOne
  x = 1;
  
  @addOne
  @double
  y = 1;
}

new C().x; // 4 
new C().y; // 2

Above, the decorators for x and y are roughly equivalent to the following:

class C {
  x = double()(addOne()(1)); // (1 + 1) * 2 = 4
  y = addOne()(double()(1)); // (1 * 2) + 1 = 2
}

However, addInitializer has nothing to do with the field's initializer, but rather general initialization of the class instance:

const init = (_, ctx) => {
  ctx.addInitializer(function () { // must be a `function` to get the correct `this`
    console.log(this.constructor.name);
  });
}

class C {
  @init x = 1;
}

new C(); // prints: 'C'

Answer 10 · 2022-05-28T05:56:42.000Z

class C {
 x = double()(addOne()(1)); // (1 + 1) * 2 = 4
 y = addOne()(double()(1)); // (1 * 2) + 1 = 2
}

Can I ask you to explain why field y has value of 2 but not of 3 since 1 * 2 + 1 = 3 or it is just a mistake?

Answer 11 · 2022-06-16T18:46:27.000Z

I think that's just a typo.

I think you may be misunderstanding how addInitializer works, as these two operations have different behavior.

They actually don't have different behavior, because in the context of the current spec, one does not exist for class fields!

If we added ctx.addInitializer for fields, it can behave in any way want. We can design it to behave just like the return values do.

Also keep in mind that ctx.addInitializer for methods are currently spec'd to run at a different time than ctx.addInitializer for classes. This clearly implies that we could add an additional ctx.addInitializer for fields that runs in any way and at any time we want...

The real question is: why is the return value, which has a DX that is inconsistent with the other type of decorators, actually better?

So far, no one has been able to actually explain this, and having developer API consistency is valuable.

Answer 12 · 2022-06-21T02:27:20.000Z

I re-opened this at #473 because there has not been any legitimate explanation as to why context.addInitializer (something that can be spec'd any way we want) is not in the spec for class fields instead of return values, for API consistency sake.

It may be that the previous version of it was bad and was removed (instead of being modified). But currently there is no version of it, which means we can make it be as good as any version we can imagine, and we can remove the function return value.

Answer 13 · 2022-06-21T04:09:16.000Z

@trusktr why is #469 (comment) illegitimate? Clearly some members of the committee believe that the extra operations that would be required for your proposed solution are a non-starter. Your proposed behavior was discussed early in the design process and discarded for this reason.

I’ve closed the other issue because the answer to it is the same as this one - we cannot have the behavior that you describe due to performance constraints. It was proposed, it was discussed, and it was turner down by the committee, for the reasons described above.

Answer 14 · 2022-06-21T05:58:37.000Z

This is an extra operation which would have to be added (e.g. you could not possibly know if there were added initializers until the decorator itself were evaluated, at run time).

Maybe I'm not understanding.

Are you saying that with a decorator that returns an initializer, the engine knows something about the to-be-returned initializer without running the decorator at runtime? But with context.addInitializer the engine needs to actually execute the decorator?

Are you saying that the engine will statically analyze a decorator, and it will see that there is a returned function in a decorator (before running it) and do something with that?

@trusktr why is #469 (comment) illegitimate?

Because, unless there's some magic I'm not aware of,

decorators need to be executed so that the engine can get initializers (initializers that may contain closures over variables inside the decorators, which seems to eliminate the static analysis I just mentioned) regardless if those initializers are added by
1. passing a function into a method call in a decorator, or
2. added by returning a function from a decorator,
both of which need to capture scope within the decorator at runtime; and
once the engine has the initializers by either of those two methods it can then run the class initialization logic in any order that the engine desires.

What I'm saying is that point 1 seems completely decoupled from point 2, but you seem to be implying that they are not (and if that's the case, then I'm misunderstanding something).

Answer 15 · 2022-06-21T06:17:13.000Z

What I'm saying is, with context.addInitializer, the following (it seems to me) is still completely possible (because the spec is a set of rules that we define):

Create class object
Check to see if any added initializers (does not matter if returned or passed to a method) exist. If so, run them.
Run a initializers
Define result of previous step on class object
Run b initializers
Define result of previous step on class object'

There is no interleaving!

(I'm not proposing to allow both returning and passing-to-method. Returning would be removed in favor of passing-to-method, for consistency)

How does the choice of returning an initializer vs passing an initializer to a method (there should only be one option, not both) have anything to do with whether or not the initializers are interleaved?

This question is what has not been explained, unless I completely misunderstand something.

Answer 16 · 2022-06-21T12:27:51.000Z

Thank you for elaborating, as the question being asked has changed several times now and it's hard to know exactly what is still confusing.

I believe what you are saying is:

It is confusing that class fields have two different types of initializers, the return value and those added by addInitializer.
Therefore, you propose removing the return value and having addInitializer instead be the way which class fields add/replace their initializers during decoration.

This would satisfy the performance constraints and is a viable alternative to the current behavior. However, I believe that it would be an equally confusing API for a few reasons:

All other decorators return a value as their main decoration. This would force users to learn a different method for one type of decorator, making it an outlier that could cause confusion.
addInitializer would have a different signature specifically for fields:
```
// methods
function addInitializer(init: () => void): void;

// fields
function addInitializer<T>(init: (value: T) => T): void;
```
This would be confusing for users in my experience, as in some cases they would have to pass a different type of function to addInitializer, and is overall less consistent.
Functions added to addInitializer would have completely different timing semantics than in all other decorators. Again, this would be less consistent overall and could cause confusion.

In addition, it is conceivable that users may want to run code during the pre-field-class-initialization step, and this change would make that impossible. While having both types of initializers may be a little confusing, it is also more powerful, and this was a requested capability by some members of the committee and the reason why addInitializer exists on fields as well and no change was made.

I believe that ultimately this is a bikesheddable API design choice - both solutions are confusing in their own ways, and consistent in their own ways. Personally I believe the current design is the more intuitive and better one, and given this proposal has moved to stage 3 already I don't think it is likely that it will change, as changes in stage 3 are meant to only be for critical issues based on implementation experience (see the TC39 process doc).

Answer 17 · 2022-06-21T21:09:34.000Z

I believe what you are saying is:

It is confusing that class fields have two different types of initializers, the return value and those added by addInitializer.

That's not what I'm saying and I'm not confused. The current spec states that "field" decorators do not have a context.addInitializer method available to them in this section:

addInitializer: Allows the user to add additional initialization logic. This is available for all decorators which operate per-class, as opposed to per-instance (in other words, decorators which do not have kind "field" - discussed in more detail below).

(This piece of text does not link to the "detail below", making it a bit difficult to follow.)

Further down below, the Class Fields section shows the type definition for field decorators to be:

type ClassFieldDecorator = (value: undefined, context: {
  kind: "field";
  name: string | symbol;
  access: { get(): unknown, set(value: unknown): void };
  static: boolean;
  private: boolean;
}) => (initialValue: unknown) => unknown | void;

(which seems to have a mistake in the return type definition)

Then it states that

Unlike methods and accessors, class fields do not have a direct input value when being decorated. Instead, users can optionally return an initializer function which runs when the field is assigned, receiving the initial value of the field and returning a new initial value. If any other type of value besides a function is returned, an error will be thrown.

So I'm not confused about class fields having

two different types of initializers, the return value and those added by addInitializer.

because the spec currently says that field decorators don't have both, that they have only returned initializers.

2. Therefore, you propose removing the return value and having addInitializer instead be the way which class fields add/replace their initializers during decoration.

What I proposed (this whole time, sorry that I didn't explain it well previously) is, why not to remove returned initializers, and replace them with context.addInitializer calls?

The type of a field decorator would be changed to the following:

type ClassFieldDecorator = (value: undefined, context: {
  kind: "field";
  name: string | symbol;
  access: { get(): unknown, set(value: unknown): void };
  static: boolean;
  private: boolean;
  addInitializer(initializer: (initialValue: unknown) => unknown): void;
}) => void;

What I'm thinking is that an initializer passed into context.addInitializer would behave exactly the same as the ones that are currently returned, with no interleaving.

The wording below the type def would be modified to:

Unlike methods and accessors, class fields do not have a direct input value when being decorated, so they do not return anything. Users can optionally add an initializer function that runs when the field is assigned during class construction, receiving the initial value of the field and returning a new initial value. If anything is returned from the decorator, an error will be thrown.

We can expand our @logged decorator to be able to handle class fields as well, logging when the field is assigned and what the value is.
function logged(value, { kind, name, addInitializer }) {
  if (kind === "field") {
    addInitializer(function (initialValue) {
      console.log(`initializing ${name} with value ${initialValue}`);
      return initialValue;
    });
    return;
  }

  // ...
}

class C {
  @logged x = 1;
}

new C();
// initializing x with value 1

etc.

This would make the decorator dev experience more consistent.

I don't yet understand why using context.addInitializer the way I've described would be worse than returned initializers.

Answer 18 · 2022-06-21T22:27:34.000Z

@trusktr the explainer document you linked to is not the spec. This is the spec: https://arai-a.github.io/ecma262-compare/?pr=2417

In the spec, addInitializer is included in fields. The mismatch between the spec and the explainer is noted in this issue here: #464

It was discussed in the same plenary when decorators advanced to stage 3, and the decision was that addInitializer should be included with class fields. So, the spec is correct, and the explainer needs to be updated. I have not had time to do that, if you would like to make a PR that would be very helpful, but I'll get to it soon if not.

I don't yet understand why using context.addInitializer the way I've described would be worse than returned initializers.

I believe I explained my reasoning in #469 (comment). As mentioned there, the proposed behavior you put forward works technically, and I have admitted that I can see why you believe it is more intuitive. I believe that the spec'd behavior as is is more intuitive however, and I don't believe we can objectively find a reason for one being better than the other, other than it is slightly more powerful to be able to add initializers in both places in the current spec.

Given the proposal has already advanced to stage 3, I don't believe any changes can be made here based on some developers finding it more or less intuitive. This goes for changes that I myself would like (I in fact made a proposal for a change at the last plenary, but it was shot down for similar reasons).

Answer 19 · 2022-06-21T23:09:12.000Z

Oops, so I mistakenly thought the explainer was up to date and somehow didn't get that above.

Then both, returned initializers, and those added with context.addInitializer, go into the same [[Initializers]] list and behave the same?

I took a at 15.7.6 ApplyDecoratorsToElementDefinition, and at steps 5.j.i and 5.j.ii the variable initializer appears without having first been defined, unless I'm not reading that correctly. What is initializer there?

Answer 20 · 2022-06-22T00:51:09.000Z

Then both, returned initializers, and those added with context.addInitializer, go into the same [[Initializers]] list and behave the same?

They do not, returned initializers are added to the class element list of initializers, context.addInitializer initializers are added to extraInitializers and run prior to class fields being assigned.

I took a at 15.7.6 ApplyDecoratorsToElementDefinition, and at steps 5.j.i and 5.j.ii the variable initializer appears without having first been defined, unless I'm not reading that correctly. What is initializer there?

Yes that appears to be a mistake, initializer should be value there.

Answer 21 · 2022-06-22T05:18:33.000Z

They do not, returned initializers are added to the class element list of initializers, context.addInitializer initializers are added to extraInitializers and run prior to class fields being assigned.

That's a little confusing. Now we have to ways to add what otherwise appear to be the same thing, yet they behave differently. (one is interleaved, the other isn't, right?)

Why don't all class field initializers (returned or passed) behave the same?

Answer 22 · 2022-06-22T12:02:34.000Z

See the detailed reasoning in #469 (comment):

All other decorators return a value as their main decoration. This would force users to learn a different method for one type of decorator, making it an outlier that could cause confusion.
addInitializer would have a different signature specifically for fields:
// methods
function addInitializer(init: () => void): void;

// fields
function addInitializer<T>(init: (value: T) => T): void;
This would be confusing for users in my experience, as in some cases they would have to pass a different type of function to addInitializer, and is overall less consistent.
Functions added to addInitializer would have completely different timing semantics than in all other decorators. Again, this would be less consistent overall and could cause confusion.
In addition, it is conceivable that users may want to run code during the pre-field-class-initialization step, and this change would make that impossible. While having both types of initializers may be a little confusing, it is also more powerful, and this was a requested capability by some members of the committee and the reason why addInitializer exists on fields as well and no change was made.

I believe that ultimately this is a bikesheddable API design choice - both solutions are confusing in their own ways, and consistent in their own ways. Personally I believe the current design is the more intuitive and better one, and given this proposal has moved to stage 3 already I don't think it is likely that it will change, as changes in stage 3 are meant to only be for critical issues based on implementation experience (see the TC39 process doc).

Answer 23 · 2022-08-16T20:36:23.000Z

About the idea of allowing field decorators to add two types of initializers, one for interleaved, one for pre-field stage, why would an overloaded addInitializer API be less ideal?

I think having initializers with or without value parameters is still nicer, and more convenient that having to deal with returning or not returning. Plus TypeScript nowadays adds great support for overloaded method types, even in plain JS with Node or DOM APIs, so I imagine that plain JS users in VS Code and other IDEs will generally be ok.

Here's a practical example of what usage of an overloaded addInitializer function could look like:

function deco() {
  if (kind === "method") {
    addInitializer(() => {...})
  } else if (kind === "field") {
    addInitializer(value => {...}, true) // pre-fields
    addInitializer(value => {...}, false) // per-field
  } else if (...) {...}

  // ...do something else regardless of decorator type...
}

where the boolean would default to one or the other. Maybe pre-fields has no value arg.

With returned initializers, code gets more complicated:

function deco() {
  if (kind === "method") {
    addInitializer(() => {...})
    doSomethingRegardless()
  } else if (kind === "field") {
    addInitializer(() => {...}) // pre-fields
    doSomethingRegardless()
    return value => {...} // per-field
  } else if (...) {
    ...
    doSomethingRegardless()
  }

  function doSomethingRegardless() {
    // ...do something else regardless of decorator type...
  }
}

or

function deco() {
  let returnInitializer = undefined

  if (kind === "method") {
    addInitializer(() => {...})
  } else if (kind === "field") {
    addInitializer(() => {...}) // pre-fields
    returnInitializer = value => {...} // per-field
  } else if (...) {...}

  // ...do something else regardless of decorator type...

  return returnInitializer // in case it was a field decorator
}

The return initializer makes the code a little less understandable and more verbose, plus dislocates ideas from each other (as in, the concept of a field initializer is now spread around the whole function, rather than in only in the conditional branch).

The first example with the overloaded addInitializer concept is definitely cleaner and simpler I'd say.

Answer 24 · 2022-08-17T15:57:19.000Z

Why could the code not be:

function deco() {
  doSomethingRegardless()

  if (kind === "method") {
    addInitializer(() => {...})
  } else if (kind === "field") {
    addInitializer(() => {...}) // pre-fields
    return value => {...} // per-field
  } else if (...) {
    ...
  }
}

addInitializer has no observable side-effects and cannot affect the outcome of the decorator. Also, the exact same issue would pop up with other types of decorated values should they choose to return a value:

function deco() {
  if (kind === "method") {
    addInitializer(() => {...})
    doSomethingRegardless()
    return function() {}
  } else if (kind === "field") {
    addInitializer(value => {...}, true) // pre-fields
    addInitializer(value => {...}, false) // per-field
    doSomethingRegardless()
  } else if (...) {
    doSomethingRegardless()
  }

  function doSomethingRegardless() {
    // ...do something else regardless of decorator type...
  }
}

The change doesn't seem to make all use cases better, just one niche use case, at the expense of making the API as a whole less consistent overall. As mentioned previously, there is detailed reasoning for the current API choices, and as this proposal is now in stage 3 changes can only be made if there are fundamental problems with the current design.