Field declarations overwrite properties on the prototype

[mweststrate]

I think this issue has been raised a few times already in several of the open issues, like #100, but I thought it would probably be good to report a very specific, real life use case.

The following snippet is the official way to enhance a class with observable capabalities in MobX, when decorator syntax is not used. (See decorate docs)

class Counter {
  counter = 0
}

decorate(Counter, { counter: observable })

Which is currently semantically equivalent to:

class Counter {
  @observable counter = 0
}

This first example works fine with TypeScript and babel-plugin-proposal-class-properties, but only if loose mode is used (that translates the initializes to an assignment in the constructor). In standards mode, this mechanism no longer works.

The simple reason is, the decorate call introduces the count property on the Counter property, and gives it a getter and setter. If just an assignment is made in the constructor, this works fine. However, if a new property is introduces, the property on the prototype is completely ignored and a new one is introduced on the instance.

In pseudo code decorate does something along these lines:

class Counter {
    counter = 3
}

Object.defineProperty(Counter.prototype, "counter", {
  set(counter) {
    this._counter = counter * 2 
  },
  get() {
    return this._counter * 2 
  }
})

const counter = new Counter()

console.log(counter.counter)

Printing the counter will yield 12 in loose mode, but 3 in standards mode, as in standards mode the field would be re-introduced in a non-interceptable way.

---

I search this repo for it, but couldn't really find it, what was the motivation to change the spec in this regards? (or if it never changed; what is the motivation to deviate from what existing transpilers are doing?).

And more specific: how could I express "decorating" my class fields in the future with the above API? After all, changing a prototype after the initial class declaration is not that uncommon in JavaScript.

---

A potentially future risk of this approach is that it will render MobX completely incompatible with create-react-app, if this proposal is finalized before the decorators standard, as the decorate utility is currently the only way in which CRA users can obtain decorator-like behavior without the syntax. For some background: https://mobx.js.org/best/decorators.html

N.B. note that this problem also happens when the field is declared, but never initialized.

---

Edit: for clarification to other readers of this issue, the difference in compilation between loose and standards mode is:

// Loose:
var Counter = function Counter() {
   this.x = 3 // assignment being picked up by property on the prototype
}

// Standard:
var Counter = function Counter() {
   Object.defineProperty(obj, "counter", { // Property definition hides prototype property
      value: 3,
      enumerable: true,
      configurable: true,
      writable: true
    });
};

[bakkot]

search this repo for it, but couldn't really find it, what was the motivation to change the spec in this regards?

Just answered this here. It's also in the notes at least here and I think in other places, though I can't dig them up just now.

A potentially future risk of this approach is that it will render MobX completely incompatible with create-react-app

Surely people who want to trigger setters and not use decorators can still do so in the constructor?

[littledan]

I hope we can enable this kind of decorator by adding a feature to decorators, as we discussed.

You can also see this summary of Set vs Define by @bakkot .

[mweststrate]

Thanks for the pointers! Had a hard time finding them myself, as there are so many issues already around this area.

Surely people who want to trigger setters and not use decorators can still do so in the constructor?

I don't think people write code in terms of "I want to trigger a setter". That is exactly the kind of low level detail I want to abstract away as a library author. But yes, using the constructor would be a valid work around, but the biggest problem it poses: how are people supposed to migrate over (after a few years of Babel and TS).

The current proposal, although I do see the consistency of it, makes it quite impossible to detect mistakes ("plz use constructor instead of initializer"), so I see potentially a lot of issues filed with this problem, without being able to assist users as early as possible.

Which is basically the same issue as the "deprecation setter" as mentioned in #42. This patterns are extremely valuable for libraries to hint users in the right direction with fast feedback, and I the pattern is heavily used in libraries such as React as well. For example when state is assigned directly in a Component class it will print: Line 34: Do not mutate state directly. Use setState() react/no-direct-mutation-state Edit: Doh, that was a linter rule, nonetheless, I'm pretty sure I've seen such protection mechanisms in libraries

For me the clearest way forward would be to treat assignments as what they always have been; imperative statements rather than declarations, which would make the class body consist of two types of things. This would be consistent with the other class members that I see, as there is a clear syntactic difference with a declaration x() { } and assignment x => () { }. This would also be consistent with the proposed double declarations examples in #42 that do not trigger a setter; those are not assignments after all. (I do find those examples as argument little contrived though, I think declaring a member twice in the same definition should be flagged by any transpiler, linter etc, and can't remember every seeing a real life example of such a thing where it wasn't a bug / accidental).

Counter self argument: class { x } would become kind of pointless (not being clearly an assignment or declaration), not entirely sure whether that is bad. For example class A extends B { x: Type } could be a great way to refine the type of the the x field in e.g. Typescript without any runtime semantic changes; in the current proposal that could actually "break" the class, when someone just wanted to narrow the type of the field in an inheritance chain.

Summary:

Thing	Placement	Notes
x() {}	prototype	defineProperty, non-enumerable
x = y	instance	assignment, enumerable
x	-	no effect at all
x = undefined	instance	assignment, enumerable
static x() {}	constructor	defineProperty, non-enumerable
static x = y	constructor	assignment, non-enumerable (so could be intercepted theoretically by declaring a static set x() { } first(?))

[rdking]

@bakkot The problem that now yet another developer is trying to get you to see is that the approach you're taking for implementing public fields goes against the general expectation that developers have about what is supposed to happen.

By avoiding the prototype in parsing the definition, you avoid the foot-gun of accidental static-like data in what's expected to be an instance property. That's reasonable. By using a property definition in the constructor instead of a simple set operation, you completely ignore the natural side effects of having a prototype. That's called blind-siding the developer for no gain. This is what I mean when I mention breaking existing functionality just to avoid something you think of as a foot-gun.

[mbrowne]

What is the most significant foot-gun the current proposal is trying to prevent? I'm still not clear on that. @bakkot gave several reasons for not using [[Set]] and I'd like to make sure that first of all we're actually talking about the same things. And it had better outweigh major foot-guns like the one @hax raised. Let me outline why the issue he cited is so problematic...

Suppose you start off with this:

class ExistingGoodClass {
  x
}
class Subclass {
  x = 1  // in the subclass we want a different default value
}

IMO allowing redeclaration of the same property like this in a subclass isn't a good idea in the first place (you can already assign default values in the constructor, so restricting this wouldn't result in any loss of functionality, and would avoid potential confusion). But if we're going to allow it as the current proposal does, then we have to consider what happens if the author of ExistingGoodClass decides to refactor to a getter/setter pair, reasonably assuming that this shouldn't break any code outside the class itself and that subclasses will inherit the getter/setter:

class ExistingGoodClass {
  get x() {...}
  set x(v) {...}
}

But actually, the subclass won't inherit the getter/setter, and I'm guessing any decorators on the parent class wouldn't apply either, since the instance property defined by Subclass would take precedence.

But it occurred to me that there's actually a bigger problem, which wouldn't be helped by using [[Set]] instead of [[CreateDataPropertyOrThrow]]. Suppose we started with just this:

class ExistingGoodClass {
}

Now suppose someone adds a subclass and the author of ExistingGoodClass isn't aware of it:

class Subclass extends ExistingGoodClass {
  get x() {...}
  set x(v) {...}
}

Later on, the author of ExistingGoodClass decides to add a field x:

class ExistingGoodClass {
  x
}

Now the instance-level x takes precedence and completely breaks Subclass. If there are any classes that extend Subclass and rely on x then those would break too.

Perhaps this issue is unavoidable given how the language already works, at least if we want some equivalence with traditional OOP in JS and developer expectations of default placement (instance vs. prototype). I'll have to think about it more...

But the fewer such issues the better, IMO...we can at least prevent the first issue by using [[Set]]. I think to do otherwise would be violating the prototypal inheritance model of JS and how most developers would expect it to work. And just make it harder to understand what is going on.

[mbrowne]

I thought about it some more and realized that using [[Set]] instead of [[CreateDataPropertyOrThrow]] actually would help with the second example as well. With [[Set]] semantics, this:

class ExistingGoodClass {
  x
}

Would be functionally equivalent to this:

class ExistingGoodClass {
  constructor() {
    // DOES respect the getter/setter defined by subclass if there is one
    this.x = undefined
  }
}

[mweststrate]

Clear overview @mbrowne. I think the most important observation here is: the first pattern is regularly applied in practice (field -> getter), and the second case is unavoidable (after all, this is what happens with normal method overrides as well). So for that I would simply argue the model: define properties by default, make exception for field declarations and assignments, as that is how the language is used extensively "out there", both Babel and TS are compiling code like that for years.

One could make an argument that it is not the most consistent proposal possible (I think I would agree to that), but IMHO that can't outweigh the argument that the current proposal doesn't make it possible to migrate, or even detect code that assumes the old way of working. So unless there is a clear migration path (for example, alternative syntax), this proposal simply doesn't look backward compatible to me.

Or, put differently: https://twitter.com/SeaRyanC/status/1052621631594553344

Edit: well, and what you set in your last comment :-)

[rdking]

@mbrowne I don't know what @bakkot's answer will be, but I can show a few quick problems with public properties on a class in ES. We have to remember not to think about how proposal-class-fields intends to implement this. Keep in mind that everything inside a class definition is currently part of the prototype.

var counter = 0;
function tool() {
  var a = ('a').charCodeAt(0);
  var letter = String.fromCharCode(a+(counter%26));
  return { [`${letter}${counter++}`]: counter };
}
class A {
  x = tool();
}

If everything inside the class definition is part of the prototype, then what exactly does x=tool() mean? It could be any of the following:
1. add an x data property to the prototype and assign value received from evaluating tool() at the time of definition
2. add an x data property to the prototype and evaluate this.x = tool() in the constructor
3. add a get x()/set x() accessor pair to the prototype performing simple access to a new private property with name Symbol('x') value and assign that private property the value received from evaluating tool() at the time of definition
4. add a get x()/set x() accessor pair to the prototype performing simple access to a new private Symbol('x') value and evaluate this.x = tool() in the constructor
5. skip the prototype and evaluate this.x = tool() in the constructor
6. skip the prototype and define an own property x in the constructor with value received from evaluating tool()

Now for the problems:

• 1 suffers from the ambiguity of intent when assigning an object to a prototype property. Every instance has a copy of the object. The expectation may or may not be exactly that. Consider in Java, the ability to define nested classes. In that case, it is definitely the desire that every instance be able to see the exact same class definition. On the other hand, consider assigning an empty array. The likely intention is for each instance to have its own copy of that array for storing instance-specific values.
• 2 suffers from late binding issues. Suppose A had base classes, and the constructor of one of those bases depended on a derived class to provide that x value. Since A would not be able to set x until all base class's constructors returned, there would likely be an error due to x=undefined. The whole purpose of adding things to the class definition is to ensure that those values are set even before a single line of constructor code is run.
• 3 suffers the same problem as 1 as well as from immediate binding to a non-target since the value to assign would be calculated even before the property to store it in exists.
  4 suffers the same problems as both 2 and 3 but not 1
  5 suffers the same problem as 2
  6 suffers the same problems as both 2 and 3 but not 1 as well as ignoring the prototype. Of the prototype of A contained a set x(v){} accessor, it will never be called on the assignment of x due to the direct masking of those accessors by a data property directly on the instance.

Seen from this perspective, the whole issue of defining public properties in a class definition leads to problems.

Solutions:

• The problem for 2 has no real solution. This eliminates approaches 2, 4, 5, & 6 if this problem is a deal-breaker.
• The problem for 1 is easily remedied by having the developer learn that only values which are not instance unique should be assigned in the class definition.
• The problem for 3 can only be eliminated if it is feasible to allow a prototype to contain private fields. In this way, the behavior of a private field on the prototype would parallel the behavior of a public field on the prototype (save for the prototype tree traversal).

What this means is that, (imo) only approaches 1 and 3 are even remotely viable. Since approach 3 can be performed programmatically within the class definition, there's no real cause for a short-hand version. That only leaves approach 1 as the most reasonable meaning for such a definition.

[ljharb]

Everything inside the class definition is certainly not part of the prototype - all static things are attached to the constructor, and the constructor is the constructor.

[rdking]

@ljharb Good catch. However, static is presently the only exception. In general, it can be said that the contents of the class definition are always immediately defined onto an object that is a product of the class keyword. Introducing this "public field" notation breaks from that in a peculiar way with consequences described by my previous post.

[mbrowne]

@rdking, continuing from #150 (comment):

Since properties are either data properties or accessor properties, it might be better if redeclaration of a data property that doesn't changing the property type from data to accessor is illegal. Likewise, changing from accessor to data should also be illegal. Redefining accessor properties as new accessor properties should be allowed as this might represent some useful change to the accessors.

I think I agree with you, although it looks like maybe there's a typo in there (was the "is illegal" in the first sentence what you intended to write?) Yeah, I can see overriding getters and setters as being useful. I don't see the usefulness of redeclaring a data property, no matter whether the subclass is keeping it a data property or changing it to accessors.

@RyanCavanaugh

[rdking]

@mbrowne

I think I agree with you, although it looks like maybe there's a typo in there (was the "is illegal" in the first sentence what you intended to write?)

Here it is in a different way:

• <not defined> -> data = ok
• <not defined> -> accessor = ok
• data -> data = illegal
• data -> accessor = ok
• accessor-> data = illegal
• accessor->accessor = ok

[yyx990803]

Here I would raise an opposite case: in Vue (v2 with vue-class-component, or v3 which is currently in development using native classes), we have the following syntax for defining components:

class Parent extends Component {
  // this is a reactive "data" property
  foo = 1

  // this is a computed property with no setter
  get bar() {
    return this.foo
  }
}

Now when a child class extends the parent like this:

class Child extends Parent {
  bar = 2

  someMethod() {
    console.log(this.bar) // should this be 1 or 2?
  }
}

• With [[Set]] semantics: logs 1. Confusing isn't it?
• With [[Define]] semantics: logs 2, makes more sense to me.

Notice that even with the [[Set]] semantics, a field with the same name on the child class shadows a property on the parent class' prototype, but not the getter. It would make logical sense for a child field with initializers to either always shadow or not - it seems very inconsistent for the initializer behavior to differ based on whether a property on the parent is defined as a plain property or a getter.

Using the [[Set]] semantics, I would always need to be aware of the implementation details of the Parent class when I access a field in the Child class - is the field declaration defining a property or triggering a getter? Am I accessing an own property or triggering a parent setter? IMO this leads to more confusing/mentally taxing code than the benefit it brings. This is what we have to deal with when manually setting properties in the constructor and I think class fields should avoid that mistake.

With fields using [[Define]] semantics, if I want to explicit trigger a parent setter, I would do a manual set instead:

class Child extends Parent {
  constructor() {
    this.bar = 2
  }
}

This requires the developer to be aware of the underlying semantic differences, but I would argue the developer would need to be aware of such differences regardless. With [[Set]], I would be forced to use the following if I want to explicitly shadow a parent class property:

class Child extends Parent {
  constructor() {
    Object.defineProperty(this, 'bar', { value: 2 })
  }
}

So:

• There are valid use cases for both semantics
• There are workarounds for doing the opposite of either semantics
• Developers need to understand the semantic difference of [[Set]] vs. [[Define]] regardless of which one fields end up using

The thing that makes [[Define]] more sensible to me is the consistency shown in the example: when I see a field declaration I know it's always a plain property and initialized to the value I set it to.

[nicolo-ribaudo]

If we use [[Set]], how would decorators which change the property descriptor work?

class Base {
  get x() {}
  set x(v) {}
}

class SubClass extends Base {
  @writable(false) x = 2;
}

Should we just ignore the decorator?

[hax]

If we really want definition semantic, we have several options:

1. add keyword: public x = 1 then it's clear there is a definition.
2. use different notation: x := 1 also clear it's not assignment.
3. drop initialization

Choose what you like, but please do not just leave the footgun...

[rdking]

@yyx990803

• With [[Set]] semantics: logs 1. Confusing isn't it?
• With [[Define]] semantics: logs 2, makes more sense to me.

Not confusing at all. Incorrect though. Remember, everything inside a class definition is implicitly strict. So, with [[Set]] semantics, when the constructor tries to run this.bar = 2, you get a nice little throw that tells you bar can't be set. If you want to make bar settable, you'd have to do something like this:

class Child extends Parent {
  _bar = 2
  get bar() { return this._bar; }
  set bar(v) { this._bar = v }
  someMethod() {
    console.log(this.bar) // should this be 1 or 2?
  }
}

There really is no need to have public properties go and redefine things on the instance. If you want to re-define a public property, do it explicitly either in the class definition as above or in the constructor with Object.defineProperty.

Notice that even with the [[Set]] semantics, a field with the same name on the child class shadows a field on the parent class that uses initializers.

You might want to check that again. With [[Set]] semantics, given your Child class, this.bar = 2 would be executed in the constructor, causing it to throw.

Using the [[Set]] semantics, I would always need to be aware of the implementation details of the Parent class when I access a field in the Child class - is the field declaration defining a property or triggering a getter? Am I accessing an own property or triggering a parent setter? IMO this leads to more confusing/mentally taxing code than the benefit it brings. This is what we have to deal with when manually setting properties in the constructor and I think class fields should avoid that mistake.

This is the very nature of object oriented inheritance. Part of the behavior of a child is dictated by the parent. If that were not the case, inheritance would be a completely useless concept. Those who already think it is simply write code in a way that eschew vertical inheritance in favor of horizontal inheritance (inherit by encapsulation).

With fields using [[Define]] semantics, if I want to explicit trigger a parent setter, I would do a manual set instead

That would be reversing the default behavior of ES. Not a good move unless you're trying to confuse almost every developer out there. Essentially, = means [[Set]]. Introducing a condition where = means [[Define]] will undoubtedly put a undue strain on the existing mental model.

[rdking]

@nicolo-ribaudo

If we use [[Set]], how would decorators which change the property descriptor work? Should we just ignore the decorator?

That depends. Just follow existing behavior. If I have this already:

a = {
  __proto__: {
    _x: 0,
    get x() { return this._x; },
    set x(v) { this._x = v; }
  }
};

//What would this code do?
Object.defineProperty(a, "x", () => {
  var retval = Object.getOwnPropertyDescriptor(a, 'x');
  retval.writable = true;
  return retval;
});

[yyx990803]

@rdking

Notice that even with the [[Set]] semantics, a field with the same name on the child class shadows a field on the parent class that uses initializers.

You might want to check that again. With [[Set]] semantics, given your Child class, this.bar = 2 would be executed in the constructor, causing it to throw.

My wording was off, I meant parent prototype properties vs. getter/setters. (edited, although you read it wrong too), what I meant is this:

class A {
  get bar() {}
  set bar() {}
}
A.prototype.foo = 1

class B extends A {
  constructor() {
    super()
    this.foo = 2 // shadows
    this.bar = 2 // does not shadow
  }
}

This is the very nature of object oriented inheritance. Part of the behavior of a child is dictated by the parent. If that were not the case, inheritance would be a completely useless concept.

I'm only debating the behavior for the class field declarations, not the behavior of set operation itself. My point is that it's fine for field declarations to have a different semantics from set, when manual set is always available inside the constructor. Using define for class fields does not take away anything.

With fields using [[Define]] semantics, if I want to explicit trigger a parent setter, I would do a manual set instead

That would be reversing the default behavior of ES. Not a good move unless you're trying to confuse almost every developer out there.

I have no idea what you are talking about. It's not reversing any behavior at all.

[RyanCavanaugh]

My point is that it's fine for field declarations to have a different semantics from set, when manual set is always available inside the constructor. Using define for class fields does not take away anything.

It takes away from predictability by changing what = means in JavaScript. You could call defineProperty from the constructor instead too if [[Set]] were used. The entire feature here is syntactic sugar for an existing operation, the question is just which existing operation.

This is like if someone said const [a] = arr; should do something subtly different from const a = arr[0]; because if you wanted to initialize with the 0th element you could always write the latter form instead. The entire point of sugar is to make common operations palatable, and the common operation is a [[Set]] from the constructor, not a [[Define]]

[yyx990803]

@RyanCavanaugh

I'd say it more along the lines of declaring a variable vs. assigning a variable:

// because of the `let` we know this is going to shadow `foo` in an outer scope,
// if there is one
let foo
// or
let foo = 1

// this we know is going to rely on knowledge of outer scope
foo = 1

// because this is a class field *declaration*, we know it's going to shadow `foo`
// of parent class, if there is one
class Foo extends Base {
  foo;
  // or
  foo = 1
}

class Foo extends Base {
  constructor() {
    // because this is an explicit set *operation*, we know it's going to rely on
    // whether parent class has a foo getter, so when we read this piece of code
    // we'd know to check the Base implementation to make sure we understand
    // what's going on
    this.foo = 1
  }
}

BTW, I'm aware that the define semantics breaks TS, especially when declaring properties without initializers. I think it could be worked around with Yehuda's declare prop: type suggestion though.

On the other hand - if there's a way to tweak the proposal so that

1. Use [[Set]] semantics for field initializers

2. Provide an alternative way to explicitly opt into the[[Define]] behavior, e.g. declare foo = 1 or something like foo := 1 like suggested by @hax, I would be ok with that too.

[nicolo-ribaudo]

I don't think that we need two different syntaxes, since we can quite easily change the the declaration behavior to assignment (or assignment to declaration) using a decorator:

class Base {
  set x(v) { alert("Set x to " + v); }
  get x() { return 0; }
}

class SubClass extends Base {
  x = 1;
}

var s = new SubClass();
alert("x with declare is " + s.x);

class SubClass2 extends Base {
  @assign x = 1;
}

s = new SubClass2();
alert("x with assign is " + s.x);

function assign(desc) {
  return {
    kind: "field",
    placement: "own",
    key: somehowGetPrivateName(),
    descriptor: desc.descriptor,
    initializer() {
      this[desc.key] = desc.initializer.call(this)
    }
  };
}

function somehowGetPrivateName() {
  return "___private___" + Math.random();
}

try it out

[mbrowne]

@nicolo-ribaudo

If we use [[Set]], how would decorators which change the property descriptor work?

This question only comes up if the language allows changing an inherited property from a data property (i.e. a property with a value in its descriptor) to an accessor or vice versa. I can think of one such use case that should be supported—the reverse of your example:

class Base {
  x
}

class SubClass extends Base {
  @someDecorator
  get x() {}
  @someDecorator
  set x(v) {}
}

For instances of SubClass, the x property will be completely overridden by SubClass's accessors (assuming [[Set]]), including any associated decorators, so it should just work. (Actually, no own property would even be created in the first place for instances of SubClass.)

As for other use cases, I agree with @rdking's proposed rules for changing inherited properties, in which case I don't think we'd have any issues with decorators:

• <not defined> -> data = ok
• <not defined> -> accessor = ok
• data -> data = illegal
• data -> accessor = ok
• accessor-> data = illegal
• accessor->accessor = ok

[hax]

@nicolo-ribaudo

Decorator usage @assign x = 1 may work (I'm not sure), but the problem of x = 1 still here. When you review code, how do you know the author really mean "I want a definition here", not "I want a assignment" but I forgot add @assign accidently? Or more possible, the author just don't aware the subtle semantic difference we discussed, and never think about it?

So what a code reviewer could do? Check the whole class hierarchy to make sure no x exists in all base classes then add @assign to the line? Or maybe we just reject the whole PR and educate the author: "you should learn more JavaScript before you commit code 😝"?

And, we should remember, there is already tons of Babel/TS code using assignment semantic, strictly speaking, the authors of these code never think about the semantic difference we discussed today, the code just work! But if we change the semantic, they are all forced to check every x = 1 like code to make sure what it really mean in the time they wrote it, before they can upgraded their babel/TS. But we know though the footgun shoot you, it only kill you when there is a getter/setter in base classes. This is a edge case. So such checking code is just a BIG burden, but a LITTLE benefit. So I just think they will eventually decide let it go and let's pray I myself not be shot...

Well, to be honest, this is one of the most tricky footgun I see in my 20 years engineering...

[rdking]

@nicolo-ribaudo I've pointed this out before somewhere else, but I'll do it again here for your sake. When creating an object literal, each key is applied using [[Define]]. This makes perfect sense given that these objects are new and don't even have a prototype. When using the class keyword, it's exactly the same, a new object gets created: a prototype. So it's perfectly ok to use [[Define]] on that object and any other objects created along with it (which is a good thing since static members go on the constructor).

What this proposal intends to do is weird. Adding items to the class definition that doesn't affect the prototype or its properties goes against the very design of the class keyword. Even decorators only affect the prototype and its properties (by manipulating their definitions). What's more is that = is being used in a way that doesn't mean [[Set]]. Can you point to anything else in the language where this operator doesn't mean [[Set]]? Good luck finding it, because it doesn't exist.

If one of the goals of any proposal is to make sure that the new feature resembles ES, this part of the proposal is an utter failure.

[mweststrate]

But if we change the semantic, they are all forced to check every x = 1 like code to make sure what it really mean in the time they wrote it, before they can upgraded their babel/TS

Yes, that is my whole point. If this is a blank slate problem, I think [[set]] is still the more natural meaning, for the same reasons as @RyanCavanaugh stated. But even if [[define]] would have the better arguments, the whole discussion is still moot imho: because [[set]] is the common pattern already and used extensively. This proposal wants to change the current semantics, without a migration path, or even a way to detect cases where people accidentally rely on the old behavior.

In that sense it is totally different for changes that have been made in for example decorators stage 2; the syntactic changes can statically be found and fixed, and the semantic changes can be detected at runtime (decorators are called with different arguments at runtime, so libs can detect that and implement a both the old and new behavior). However, with the [[set]] to [[define]] changes, things simply stop working, and it is even impossible to detect that it is happening! The only thing we can do is to wait and pray that unit or E2E tests pick suddenly introduced bugs up before shipping.

---

And honestly, I didn't really find much real life use cases where I wanted to re-define a property in a subclass (stubbing in unit tests is the only one I could come up with).
Has anybody a practical use case of that? And if someone wanted to do so, defineProp would work perfectly in a constructor.

The case the other way is pretty common though, a subclass wants to change the initial default value of a field declared in a parent, but keep behavior of that assignment as defined by the parent (just being a plain field, or performing invariant / type checking, printing deprecation messages etc).

So, imho, these assignments should all do the same, rather than deviating for one of the 3.

class A { 
  x = 1
  constructor() {
     this.x = 1
  }
}

new A().x = 1

---

Actually, I would predict that if x = 1 would change from assignment to declaration; that the first thing the community does is to introduce a lint rule: no-field-declarations, just like no-variable-shadowing is now a very commonly used rule.

With x = 1 people won't realize they are shadowing a field. Where for variables it is kind of fine to shadow, because it is at least lexically detectable (for both tools and human readers). With classes a completely different story; it is very hard to determine statistically if shadowing is happening, as the shadowing would be across files and even libraries.

Actually, the problem here is even worse then normal variable shadowing, because with normal variable shadowing earlier closures are bounded to earlier variable definitions, but with classes earlier definitions (the base) get actually 'bound' to the newer field definitions in the subclass! So this actually reduces the control and containment of base classes over their own fields, rather then increasing it.

So how are we supposed to help users? "you are shadowing a base field. Either pick a new field name or move the initial value assignment to the constructor".

So I think we would end with a very consistent, but completely useless feature if [[define]] semantic are chosen. At least the feature shouldn't be used in combination with extends. And for a non extended class the net result of both approaches is the same anyway.

If being able to define is that valuable, just add separate syntax for that class X { let a = 1 }. Then at least there is a conversation starter with users where we could explain the difference in meaning, and the intended semantics could be expressed clearly.

• In summary, without base classes [[define]] is conceptually more consistent, but effectively not better than [[set]]
• With base classes, [[define]] rather than [[set]] is way worse, as it gives a false sense of isolation, it shadows fields in a horrible way, and there seem not that many actually use cases where the behavior is particularly meaningful (such existing, real world uses cases could be found by finding classes that use Object.defineProperty in their constructor in current code bases)

[mbrowne]

@yyx990803 I don't think your example is a realistic use case, or if it is then it's a very confusing design. Ordinarily when subclassing and using an existing property name (in ES6), one expects the property to use any getters and setters defined in the base class (as long as they haven't been overridden). Using the same property name in a different way as if it's a totally separate class is a recipe for confusion.

BTW, does Vue even recommend using inheritance with components as a way to add additional reactive data properties? Based on my experience with React components, which I think is similar enough to apply here, you'd be better off using composition...React users have found that using inheritance in this way causes a lot of problems. When it comes to components, inheritance is generally reserved for inheriting helper methods, not creating a specialized version of an existing UI component.

[littledan]

Since class declarations are so important to frameworks, and we're seeing a discussion between two framework maintainers, I asked some more framework authors what they thought about this issue.

• Leaders from Vue (@yyx990803), Angular (@mhevery) and RxJS (@benlesh) expressed support for Define
• Leaders from Polymer (@justinfagnani), MobX (@mweststrate) and Ionic (@manucorporat) expressed support for Set

Historically, leaders from Ember (@wycats) and React (@sebmarkbage) were deeply involved in TC39 at the time that the committee came to the decision of Define. @sebmarkbage's coworker at Facebook @jeffmo championed the Define semantics, and my understanding is that React didn't and doesn't have a particular stance on this issue. @wycats argued for Set, raising most of the issues discussed on this thread (including Babel/TS compatibility and working well with metaprogramming constructs), but agreed to move forward with Define. TypeScript was also represented in TC39 at the time and, like React, my understanding was that they were OK with either outcome.

I think the opinions expressed here were based on a pretty full understanding of the problem space and the tradeoffs, and that we've had a pretty thorough discussion of the issue.

Given the stable differences of opinion for this tradeoff, I'd propose that we stick with the existing TC39 consensus of Define semantics.

As a partial mitigation for the issues @mweststrate raises, we could permit decorators to transform a field declaration into Set semantics, even if the default is Define. That possibility is discussed in tc39/proposal-decorators#44 . I would be interested in your feedback on that proposal.

[mbrowne]

@yyx990803 I'm guessing that the use case you presented is indeed a realistic one. Sorry, on the surface it looked like a contrived example to me. I didn't realize until now that you are the creator of Vue. I always try to treat everyone with respect and I stand by my opinion, so in that sense my response would have been similar regardless of whom I was replying to, but maybe I misunderstood something... Just to confirm, does your example reflect a real use case for subclassing Vue components, or is it more theoretical?

[rdking]

@littledan I get why the library authors would choose define semantics. The problem is that there's nothing in the language that makes it non-cumbersome to replace a definition in a base class. With decorators, however, this seems to be a moot point. a decorator @override can be constructed that would be perfectly easy to understand an describes exactly what's going on. The current proposal is counter-intuitive in this respect. I would like to know what the surveyed library developers think about that idea.

[trusktr]

different upstream/downstream codes may have requirements in conflict.

Yeah, that's tricky. However, I think sticking to the standard that everyone already knows ([[Set]]) is the best way, so that the requirements in upstream or downstream code are based on existing conventions already used in millions of existing applications and libraries.

This keeps any problems that already exist (in the programmer ecosystem) right in the same place, without creating new problems. Everything works exactly as it currently does, as everyone already knows and expects.

a problem that never needed to exist in the first place.

Exactly.

[trusktr]

Honestly, if [[Define]] was the pre-existing standard instead of [[Set]] in all current code, then I'd be fine keeping [[Define]] in place. It is consistency that is most important here.

Had [[Define]] been the de facto standard up until now, then all existing libraries and applications would be written with that consideration in mind, and accessors would unfortunately not be as useful.

Honestly the actual utility of one over the other isn't entirely a huge difference, people would only treat inheritance differently (f.e. preference given to methods instead of accessors, if [[Define]] was the existing standard, engineers would coming up with best practices around [[Define]]), and it would all be perfectly fine.

Switching from the de facto [[Set]], to [[Define]], is what the major problem is.

(I still prefer [[Set]], I find it more useful because it make accessors have a higher position of importance.)

[ljharb]

[[Define]] is the standard when declaring and initiaizing object properties; [[Set]] is the standard when assigning to them. The argument generally stood on the idea that anything declaratively placed in the class body is declaring and initializing things, not assigning to them - hence, class fields should be consistent with initializing properties in object literals, not consistent with assigning properties to objects.

[trusktr]

No, [[Set]] is the standard, because most people don't use Object.defineProperty on a regular basis any time they "define" properties or "set" them. Most people use the form this.foo = 5 to both "define" and "set" a property, and they may even be completely unaware of the terminology. Please prove me wrong.

[ljharb]

@trusktr i'm afraid you're incorrect:

var parent = { set foo(v) { throw new Error('oops'); } };
var child = {
  __proto__: parent,
  foo: 2,
};
child.foo === 2 // no exception

[jhpratt]

@ljharb That's a highly misleading example. Let's take a look at the standard way to initialize properties in a class, which is what this proposal is (not objects, as you've shown).

class A {
  set foo(v) { throw new Error(); }
}

class B extends A {
  constructor() {
    super();
    this.foo = 2;
  }
}

new B();

When is the last time you used Object.defineProperty in the circumstances of a class? I don't recall ever using it.

[trusktr]

@ljharb You've found an edge case that doesn't disprove what I said because most people don't write code like that, and Mozilla DN (the de facto source of web/JS documentation) even tells people not to use __proto__ in big bold red.

[ljharb]

@trusktr even without __proto__, it uses [[Define]] because it doesn't trigger setters on Object.prototype, so it's not an edge case, it's the primary case.

@jhpratt the comparison isn't to "how you have to do it now in class", it's "when this is a part of the language, what should it mean" - and there were many delegates that felt very strongly that things that appear directly inside a class body should be declarative, and thus, use [[Define]] semantics.

[jhpratt]

@ljharb My concern was with your example. It is misleading because you demonstrated the behavior of inheritance within a simple object, not how code is typically used today.

I understand what you're trying to show, but you failed to demonstrate it. Sure, I'm in favor of [[Set]], but I believe that examples should be representative of the issue at hand. Currently, initializing fields are almost always done using [[Set]], as it's normally done in the constructor. It's possible to use Object.defineProperty, but I don't recall ever using it. Have you?

[ljharb]

@jhpratt certainly i have not, and I advocated for [[Set]] for a long time. I'm explaining the belief held by many other delegates: that nothing declaratively and directly inside the class body should trigger getters/setters.

[mbrowne]

In case anyone missed it, a section was added to the readme about this issue:
https://github.com/tc39/proposal-class-fields/blob/master/README.md#public-fields-created-with-objectdefineproperty

It didn't convince me that Define is the better default, but at least it hints at the reasoning behind the decision. (I still don't understand why the downsides of Set were considered more important than the big practical issues with Define, but it seems the committee was fully aware of both sides of the argument when they made their decision.)

[rdking]

Here's the fun part, both of you are right. It's a nuanced issue.

When declaring an object literal, all properties are indeed defined onto the new object. Reason? Everything being created is a direct product of the syntax being evaluated. Define is therefore the correct approach.

On the other side.....

When declaring an object factory (like a class), everything is declared onto either the prototype or the constructor for the same reasons as above. Both are products of evaluating the class definition. However, running the constructor function is an initialization step, which comes after attaching the prototype containing all the pre-defined elements. It is then up to the developer to choose whether or not to use [[Set]] or [[Define]] semantics. Most of the time, [[Set]] is used.

This is where the board is ignoring over a decade of developer usage patterns. A class instance is not something created by the class keyword. Therefore, things defined in the class definition should not be defined onto a class instance. It's all well and good that TC39 thinks it's better to be able to reason about the result of an assignment. However, this causes a new issues that did not previously exist, while the issue of being able to reason about what happens during an assignment is something ES developers are already accustom to doing. Why is TC39 trying to fix a problem that isn't problematic for us?

[justinbmeyer]

The above discussion was great to read. It makes me glad to see so much care taken in this decision.

I've got a use case where [[Define]] makes things (seemingly) impossible, where [[set]] would work.

Observable Elements using Proxy

For CanJS 6, we are trying to make class-based HTML elements observable. For example, I'd like something like the following to work:

// Define a custom element:
class HelloWorld extends StacheElement {
  static view = `<h1>Hello {{this.subject}}</h1>`;
  subject = "World";
}
customElements.define("hello-world", HelloWorld);

// Set the subject and have it update the HTML:
var el = new HelloWorld();
document.body.append(el);
el.subject = "Earth";

The idea is that we want to create an event when subject changes and automatically update the DOM. We want people to write "normal" classes. For us, decorators are (unfortunately) a non-starter as they have gone through so many revisions, I'm wary of designing a framework around something that changes so much.

We use proxies to detect changing arbitrary properties. The problem is that custom element instances can not be proxied. So we proxy StacheElement's prototype. This allows us to trap gets and sets. But we can not trap [[Define]].

[rdking]

@justinbmeyer If fields had been properly designed, you could just use a getter/setter for subject and your problem would be solved. However, TC39 is currently writing this off as an edge case that doesn't really warrant consideration in the face of the artificial need to "be able to reason about the result of a definition assignment". Where does this leave you and others who write similar code (myself included)? Stuck.

The only options you have are to either completely avoid using public fields and/or go back to initializing them in the constructor. The reality is that they (assuming I understood what I was told correctly) knowingly chased conflicting concerns, and their choice of resolution blocks seemingly unrelated use cases.

[mbrowne]

@justinbmeyer I haven't looked into it in detail, but it might be possible to use the current babel plugin for decorators to implement what will hopefully eventually be possible natively (https://github.com/tc39/proposal-decorators#set) — a decorator to specify that specific fields should use Set rather than Define.
While the decorators proposal may still change, I think it's a pretty safe bet at this point that it will provide some way to do this with a decorator.

[justinbmeyer]

@mbrowne Thanks for the tip! Unfortunately, decorators are a non-starter for us from a design perspective. Our goal was to make the framework/technology work with "today's" browsers. We want something that works without a build step (ie babel).

As the decorators spec is under its third revision, it seems like it's not relevant for even "tomorrow's" browsers. It might be available in the browsers "the day after tomorrow".

[rdking]

@justinbmeyer How about an alternate solution? It's ugly, and loaded with boilerplate, but it will give you exactly what you want. This is how I used to write classes before I started making Proxy-based solutions. It's also the basis of the SweetJS macro I'm working on:

let HelloWorld = (function() {
   //Put private members in a `this`-keyed object
   const pvt = new WeakMap;
   //Define static private members as variables here
   let _view;

   // Define a custom element:
   class HelloWorld extends StacheElement {
      constructor() {
         super();
         let p = {
            subject: "World"
         }
         pvt.set(this, p);
         _view = `<h1>Hello {{this.subject}}</h1>`
      }
      static get view() { return _view; }
      static set view(v) { _view = v }
      get subject() {
         let p = pvt.get(this) || {};
         return p.subject;
      }
      set subject(v) {
         if (pvt.has(this)) {
            let p = pvt.get(this);
            p.subject = v;
            //fireEvent(....);
         }
         else {
            this.subject = v;
         }
      }
   }
   return HelloWorld;
})();

[anatoliyarkhipov]

Look, the @set boi didn't age well:

[Venryx]

Just wanted to add that there is a performance advantage to the [[Set]] semantics, which can be relevant for some cases.

JSBench test: https://jsbench.me/o6kt78il9e/1

Screenshot:

Purely theoretical? Well, in my case, it was the root cause underlying a mysterious change where a frequently-called section of my code was changing from taking 10ms to >300ms to complete.

The cause? I had enabled the TypeScript useDefineForClassField flag, which switches Typescript from [[Set]] to [[Define]] semantics.

It took me a while to track down, because I didn't expect that something like switching to the new version of class fields would make my performance-critical code 30x slower. (now that I know the cause, I can rewrite that part to avoid classes of course; but it's a drawback to keep in mind, especially for other library authors using classes in large for-loops)

[michaelficarra]

@Venryx That's not an inherent difference between set and define. That's a difference between TypeScript's output. For example, the define path looks up Object.defineProperty every time it defines a property instead of just dereferencing a local.

[mbrowne]

@Venryx what if you compile to native class fields (i.e. with a target of ESNext)? Or is that not an option for your app right now for compatibility reasons? (All major browsers already support public class fields natively.) And at some point soon, there will be a stable target of ES2022 that includes all class fields features (including private fields).

[Venryx]

@Venryx what if you compile to native class fields (i.e. with a target of ESNext)? Or is that not an option for your app right now for compatibility reasons? (All major browsers already support public class fields natively.) And at some point soon, there will be a stable target of ES2022 that includes all class fields features (including private fields).

I added that to the benchmarking test. While native class fields is faster than a manual Object.defineProperty call, it's interestingly still substantially slower than the [[Set]]-based semantics: https://jsbench.me/o6kt78il9e/2

[nicolo-ribaudo]

@Venryx That benchmark uses function with [[Set]] semantics, not class with [[Set]] semantics.

Anyway, these benchmarks show that this.x = y has been optimized in engines for 20 years while class fields have not. They don't show a specific aspect of the proposal, but just where engines can implement things more.

The proper way to test this would be to implement class fields with [[Set]] semantics in an engine, and then benchmark them.

[dgreensp]

The fact that TypeScript (and presumably other tools), given the code:

class Point {
    x: number;
    y: number;
    constructor(x: number, y: number) {
        this.x = x;
        this.y = y;
    }
}

... have to emit this monstrosity (if you supply suitable compiler flags, which people are already using, e.g. Deno hardcodes it):

"use strict";
class Point {
    constructor(x, y) {
        Object.defineProperty(this, "x", {
            enumerable: true,
            configurable: true,
            writable: true,
            value: void 0
        });
        Object.defineProperty(this, "y", {
            enumerable: true,
            configurable: true,
            writable: true,
            value: void 0
        });
        this.x = x;
        this.y = y;
    }
}

instead of:

"use strict";
class Point {
    constructor(x, y) {
        this.x = x;
        this.y = y;
    }
}

... will definitely have far-reaching performance impact on real-world codebases. That sucks. To me, instance field initializers are just a nice syntactic sugar. I never wanted them to have semantics that can't be performantly transpiled, raising complex trade-offs between performance and correctness.

At least the browser support is quite high: https://caniuse.com/mdn-javascript_classes_public_class_fields

I think what's different for people using TypeScript is that defining your "fields" is the norm for every class, because that's where the types go. It's been a core part of the language since day 1. Whereas for JS programmers, it's a new-fangled language feature they opt into. Even if native fields are too slow for a Point class that you are constructing thousands of, why would you insist on using "fields" in that Point class, anyway? Whereas with TS, even your most performance-sensitive code already uses fields. So your existing code gets slower when you switch to the ES semantics, and then you have to wait a few years for browsers to iterate on their JS engines to bring your code back to the performance it had, and for browser support to be truly universal, all for some semantics you probably didn't want in the first place.