Allow plain thunks to execute as a decorator element

[littledan]

Proposal: Allow undefined as a key for field decorators. The semantics would be to just execute the initializers for its side effect. (Another superficial alternative would be to make this a new element type, which doesn't have a key field.)

A couple reasons this comes up:

• For static: If lexical declarations are added to classes, I'd imagine they would execute code in a manner which is interspersed with static field declarations. A class decorator can see and manipulate static field declarations. Therefore, we need some reification of the execution of lexical declarations, if only to preserve the way that they are interspersed. In this proposal, each lexical declaration's initialization would be thunked and put into the class element list. (Note: this means that we'd want to make arguments an earlier error in lexical declarations.)
• For instances: Often a decorator will want to run code as part of instance construction, towards the beginning of construction. This would provide that. Counter-point: Some of the time, the code that you want to run is actually at the end of construction, and an "instance finisher" would be more appropriate. (I've come across cases where the "beginning" semantics seemed more appropriate, but I can't remember them at the moment.)

[ljharb]

i would not expect a class decorator to be able to have any ability to see or manipulate lexical class declarations; any more then i would expect a method decorator to have any ability to see or manipulate lexical declarations inside the method.

[littledan]

The manipulation would not be of the lexical bindings, just the code to initialize them.

The problem I'm trying to solve here is, class decorators have the power to reorder static public field declarations, but then it is unclear what order the lexical declarations (which cannot be decorated) should be executed in.

I see a few possibilities here:

• Execute initialization of lexical bindings in a pass before or after static public fields
• Don't let class decorators see static public fields (you would need to decorate them directly)
• Do something like the above plan.

Do you have a preference among these there? Or do you see other possibilities?

[ljharb]

When you say "reorder", can you clarify what you mean exactly? Do they get a list of initializers, and can replace it with an alternate list?

[littledan]

Yes, class decorators can already do this with static field declarations.

[ljharb]

Is there an example of where a lexical declaration would depend on the ordering of a specific static field? (it's easy to conceive of the reverse). I'm wondering if "when a class is decorated, all lexical decls are evaluated first, and all static fields after that" would be reasonable.

[littledan]

It's possible I'm overthinking this, but my intuition is that more passes will be more counter-intuitive. I don't have a practical example on hand, but here's a case where the issue is observable.

let arr = [];
function decorator(desc) { return desc; }
@decorator
class C {
  <placeholder> let x = arr.push(1);
  static y = arr.push(2);
  <placeholder> const z = arr.push(3);
  static w = arr.push(4);
};

My intuition would be that arr ends up with [1, 2, 3, 4], not [1, 3, 2, 4] or [2, 4, 1, 3], if @decorator doesn't change anything about the fields.

But, what if we have the following decorator?

function decorator(desc) {
  desc.elements.reverse();
  return desc;
}

This decorator will make the w declaration come before the y declaration. What should happen to the x and z declarations relative to those? If we do it as a second pass, there's an easy answer (nothing) but you lose the interleaving.

[ljharb]

I think losing the interleaving is fine - the interleaving is a property of the original lexical code, and the decorator can’t rewrite the original code - it can only replace the result of it.

[littledan]

Could you explain a little more why losing the interleaving is fine for you? I agree that I don't have a great use case for decorators changing things here, though.

[ljharb]

It's more that, I don't see any value in preserving it, and I think that if these are lexical class declarations, then exposing any information about them to decorators is breaking the way lexical declarations work elsewhere. It wouldn't make sense to decorate a function and get access to the variables it declared lexically inside it, why would it make sense for a class decorator to do so?

[littledan]

OK, I think we don't have to add this feature in the first round for decorators. @rbuckton's alternative for tc39/proposal-static-class-features#23 doesn't require it. We could always add this as a follow-on feature, but let's keep it in our back pocket in case any killer use cases come up in continued discussions with programmers.

[mweststrate]

This proposal seems to exactly address #153 / mobxjs/mobx#1732, no place to run initialization code as soon as a decorator produces a "method" kind currently requires a hack-around by introducing a meaningless attribute (which can't be easily removed and ends up in the ownProperties list).

The proposal itself, key: undefined does feels quite counter-intuitive, a dedicated kind would feel neater (and not like abusing a field declaration):

{
   kind: "initializer",
   placement: "prototype" | "own" | "static" // or just limit to "own"?
   initializer(target) {
      // target is instance, prototype or constructor based on placement?
   }
   // no: descriptor, 
   // no: key
   finisher // as-is
   extras // as-is
}

For reference, the real life use case: https://github.com/mobxjs/mobx/blob/6092288297c0cc596064c555ec4203c5c57f5a94/src/api/observabledecorator.ts#L57-L95

[littledan]

@mweststrate It looks like ObservableObjectAdministration uses a Map to hold the underlying observed properties, right? I was hoping that, with decorators and private fields, you could hold the underlying backing store as a synthetic private field, whose initializer could do the setup action (e.g., evaluating the initializer expression); I was also hoping that prototype (not own) getter/setters could be usable. Now I can see that would be a rather big code change for MobX, but I am still wondering if it might be a possible way out without this feature.

[mweststrate]

@littledan that would probably work indeed, but here are my concerns in order of least to most importance:

1. I would expect that maps for the backing data in the end will outperform additional properties introduced on the targeted object (or there prototype). Not that it matters much probably
2. The reason of the ObservableObjectAdministration is that MobX objects can be used in many different ways, and one common abstraction keeps the code base manageable: MobX has 4 decorator implementations (TS, babel 6, babel 7, and built-in decorate utility) that can be used against 3 different targets (plain objects, proxies, class instances)
3. Having standardized how things are stored has a few advantages: it doesn't pollute the target object. Since those are usually class instances, many other things might happening in there as well. Having all mobx related stuff grouped is both easier for people debugging, and developer tools. Also there is quite some meta and tracking information stored, that goes beyond the backing of an individual property (ObservableObjectAdministration has 9 general purpose fields, most of them would also end up in the target object directly)
4. Backing observable props using direct private properties has always a downside: Either because they are not really private (like using a non-enumerable, symbol property as is done currently) or because they are actually really private, making it nearly impossible to do the reflection that is needed to build good devtool / analysis tools. For that reason it is neat that MobX with it's administration object now pollutes one, clearly recognizable property, the $mobx symbol based property, which is both clear for devs debugging, and tools that are using reflection.

The biggest concern however is: using decorators should not force where properties descriptors are stored, or how they are backed! Not being able to control the initialization of an object from properties properties was a big concern in the earlier proposal, and it is not entirely solved yet. (I thought it was from review, but giving the work arounds in the MR shows some stuff is still missing): namely extending the 'constructor' by running instance initialization code.

Yes, a significant rewrite of MobX, using private properties will probably work within the current proposal (I expect), but needing such a rewrite, that not just changes the decorator implementation (which is fine), but also requires changing the entire internal administration should raise some alarms in the first place.

[Lodin]

@littledan, @mweststrate, why not just allow an initializer property for method kind? It could return nothing but allow to make some preparations during constructor phase. And I believe it would resolve this issue I'm suffering from too.

UPD: Oh I see disadvantages of my approach. Only-initializer field is more universal.

[littledan]

Thanks for the detailed explanation, @mweststrate . See this patch out for review, based on @mweststrate 's original API proposal, which other reviewers also preferred: #165

[mweststrate]

@littledan thanks! this seems to match my use cases nicely.