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 https://github.com/tc39/proposal-class-fields/issues/150#issuecomment-431121659:

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 https://github.com/tc39/proposal-decorators/issues/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.

[mbrowne]

+1 to @rdking’s idea. Using an @override decorator would actually be arguably better for the sake of libraries that need this because it would make the code more explicit and easy to follow. By contrast, making [[Define]] the default is likely to surprise a lot of unsuspecting developers who will assume that it’s using [[Set]] when they see the =. So using decorators for the reverse scenario is a far inferior option.

These prototype issues are subtle and don’t come up all the time, so developers won’t necessarily be bitten by them right away but sooner or later they will unless they’ve read all the details about how field initialization works (this reminds me of how developers are initially unaware of the issues with using arrow functions in class properties for binding purposes) ...unless the current proposal matches their expectations, but I would be very surprised if a developer survey showed [[Define]] to be the expectation of any more than a small minority of developers. The vast majority will assume that public field declarations are just sugar for what we’ve been doing with ES6 classes and before that, constructor functions. Why introduce a new normal at this point?

I realize that the objections to [[Define]] are based on edge cases, but they’re confusing and difficult-to-debug edge cases for the uninitiated. Shouldn’t that take precedence over mere convenience in what’s probably even more of an edge case—overriding a getter/setter with a data property?

[hax]

About react, even React official documentation only use public field for handler of autobind/arrow functions and always init this.state in the constructor (which means you should not use state = as public field), in the wild, some React users just use state = .... For example, https://github.com/Frank1e0927/redux-demo/blob/bd32f7e1bdff3f500e7ed576fdd3a5498eabcd61/src/container/contextPage/Page.jsx , https://github.com/michal-karbasz/explorea-event-manager/blob/b35a30a3b2dc84779131fa911c92570d5b430351/js/components/event.jsx ...

So I would say, the risk is very hard to avoid if using current confusing syntax.

[hax]

For all the authors of frameworks :

Simply speaking, you have two options to solve this footgun problem (to see why it's footgun, check the previous comments in the thread).

1. Use [[Set]] semantic by default, require decorator like @define to provide [[Define]] semantic if you want
2. Change the syntax, for example, add keyword like public, declare, define... or change the notation from = to :=.

You definitely can support use [[Define]] by default, but if that, you must also support change the syntax. Or you will leave the footguns to your users.

[mbrowne]

@hax The React docs have a policy of not using syntax from future proposals in their code examples. They broke their own policy by showing the example of autobind/arrow functions, presumably because manual binding was a pain point for developers. (But recommending the use of arrow functions for this was unfortunate, as has been discussed at great length in the decorators repo and in #80).

[mbrowne]

Anyway, React specifics weren't the point. My point (in #144) was that if you're relying on a library or framework that requires the definition of public properties, you can't just decide that in your own code you're going to use private properties instead (e.g. #state instead of state); it won't work.

[mbrowne]

P.S. It would probably make more sense to continue the discussion about public properties in #144 rather than here.

[hax]

@mbrowne

The React docs have a policy of not using syntax from future proposals in their code examples. They broke their own policy by showing the example of autobind/arrow functions, presumably because manual binding was a pain point for developers. (But recommending the use of arrow functions for this was unfortunate, as has been discussed at great length in the decorators repo and in #80).

Whether using arrow function is good idea is not my point. I just showed that even a framework has official doc, users still use public field in a different way, which we could get the idea that we can not expect the users of the framework can always aware the semantic difference and choose the right one. Actually they are more likely to write state = ... just for avoid writing constructor.

[ljharb]

They are and do, which to me helps show the importance of providing a way to avoid having to write the constructor for common use cases.

[hax]

@mbrowne

you can't just decide that in your own code you're going to use private properties instead (e.g. #state instead of state); it won't work.

Obviously, isn't it? 😂

I never said you can change to private if your framework not. The point is, framework, just like all other project, can have new version which leverage the new feature to improve encapsulation and user experience. For example, React could make the state be private and only expose getter, which make sure newcomer will not accidently write this.state = ... in event handler, and force them follow initState()/setState() usage. If that, public field which lure the users writing class C extends React.Component { state = ... } which nullify the getter/setter on base class just shoot the users and the framework.

[hax]

@ljharb

They are and do, which to me helps show the importance of providing a way to avoid having to write the constructor for common use cases.

So you are writing React component in the way which React official documentation do not suggest?

[ljharb]

Absolutely, as are most of the React community. The official docs for a project guide, but do not dictate, use of it - whatever is possible will be done, which coincidentally is also why private fields being undetectable is so important.

[mbrowne]

This React discussion seems like it might veer off-topic, but I guess the only way to put this to rest is to point you to the contributing guidelines for the React docs: https://github.com/reactjs/reactjs.org/blob/master/CONTRIBUTING.md#dont-use-features-that-arent-standardized-yet. As I pointed out above, they broke their own rule with arrow functions since I guess they considered it a special case, but anyway, like @ljharb and much of the React community my team and I are already using this form in our code:

class MyComponent extends React.Component {
  state = {value: ''}

Obviously I have voiced my support for [[Set]], but it's actually not much of an issue for React components since inheritance is not generally used (aside from extending from React.Component or React.PureComponent). This is simply a shorter and more convenient syntax that accomplishes exactly the same thing as initializing state in the constructor.

[sophiebits]

I don't believe we do use class fields in our docs for React except in a few rare places where we explain it as an experimental syntax and as one of many alternatives. I don't believe we are "breaking our own rule". (I just committed https://github.com/reactjs/reactjs.org/commit/026edff33c2f427f17f5cfde761a661630fcd9f4 to fix one use that seems to have slipped in; let me know if you're aware of others.)

The broader reason that we avoid recommending experimental features is we don't want users to be left holding the bag if the proposal changes or is abandoned. We've felt more comfortable offering class fields in particular as an option because we use it extensively in our own codebases (both the state = {...} and onClick = () => {...} forms), so we feel confident that if the proposal changes, we will write an automated codemod to convert our codebases to the new semantics (or simply desugar them to not use it), and we will release that for our users too.

[mbrowne]

@sophiebits Sorry for overstating the issue...I'm pretty sure I saw one of the pages in the "Main Concepts" section of the docs using that syntax as recently as a year ago (without a disclaimer), but in any case it looks like there is indeed a disclaimer now everywhere that syntax is used (now that you've fixed the one that slipped through unintentionally). BTW, regarding the usage of a decorator for binding, I opened an issue here: https://github.com/reactjs/reactjs.org/issues/1276.

[hax]

@sophiebits https://github.com/tc39/proposal-class-fields/issues/151#issuecomment-431679515 explained how class { state = ... } pattern could limit the future API design of React. There is a trick which could workaround it in the framework, but may introduce other side-effects.

[mbrowne]

@rbuckton made a good point at https://github.com/tc39/proposal-class-fields/issues/144#issuecomment-431751077. And I would add that the base class might be from an external library that you'd rather not have to fork. So this means that with [[Define]], it would be very awkward to intercept attempts to get/set that property in your subclass. Decorators on subclasses have no access to change elements in their parent classes. So the only option would be to make the subclass getter/setter own properties instead of prototype properties, and even then the setter would be ignored here:

class Base {
  x = 1;
}

[mbrowne]

We have already seen in this thread that the opinions of library authors on this issue are not so cut-and-dried...quote from @yyx990803 above:

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.

So I question @littledan's claim that there are "stable differences of opinion for this tradeoff". Given that an important defense of the # syntax is that people need to hear an explanation of the tradeoffs before putting too much weight on their reaction, it's only fair that we apply the same standard here. Knowing that an @override or @define decorator is an option and seeing the concrete examples of foot-guns shown in this thread could very well change some opinions. There have been plenty of times when I've had to reassess my own opinions in response to concrete examples given by others, and I'm sure everyone here has had similar experiences. Yes I'm aware that this was discussed quite a bit in the past, but it seems like those past discussions and solicitations of feedback might not have framed the issue as clearly as it has been revealed in this repo in the past week or so.

I'm honestly not trying to be unreasonably pushy or repetitive. In fact I hope I'm wrong and that [[Define]] will actually be better for users on the whole, despite violating most users' expectations of how class properties will work. I just don't get why @littledan, @bakkot, @ljharb, and presumably others are so adamant about this, with statements by @littledan implying that the proposal is nearly set in stone at this point. I can understand preferring [[Define]] (well, actually I'd still like to understand the reasoning better), but I don't understand the attitude that changing the proposal to use [[Set]] is a near impossibility at this point.

Here is the situation as I see it:

• Committee and library authors are mostly neutral on this issue, but have decided on [[Define]] based on some edge cases they deem important
• Library authors might be willing to have [[Set]] as the default as long as they have an easy way to override it
• The rest of the community is almost entirely unaware that a change in behavior has even been proposed from what they've become accustomed to with Babel 6 and TypeScript (or how they will probably expect it to work, for those not currently using public fields). When they learn about the change, the vast majority are baffled by it and generally against it, as can be seen in this repo. See also the link posted earlier with reactions on Twitter: https://twitter.com/SeaRyanC/status/1052621631594553344

If this is an accurate description of the situation, then you can't blame the community for believing the committee is dismissing their feedback (especially combined with the response to feedback on private fields), however untrue that is. So many red flags that this issue should be revisited....

[mbrowne]

There's also a potential downside of [[Define]] for decorators:

class Base {
  @observed x = 1
}

class Sub extends Base {
  x = 2
}

Would the@observed decorator still apply to x in Sub? The intent here is clearly only to change the default value.

[rdking]

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.

It's likewise also fine for declarations to use [[Set]] semantics because define is also always available in the constructor. So that's not really a good argument. Further, standard practice, like in the example code you gave, is for a [[Set]] against a property on the prototype to either

• call a set accessor existing on the prototype, or
• define a new property on the instance that shadows the prototype.

What I'm saying is that with [[Define]] semantics, you break consistency with how prototypes are designed to work. They might as well not be there if you're going to blithely cover them up with notation that doesn't make it clear this is what you're doing.

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.

I hope this makes it clear. The behavior of class is such that it builds a prototype and (if needed) a default constructor. Both of these objects are products of the class. It is free to [[Define]] anything it wants on these. However, an instance object is not a product of class. Allowing class to directly affect the construction of the behavior of the instance beyond the prototype is a reversal. However, there's a bigger one. The fact that = is used in declaring these fields is a much bigger issue. The = means [[Set]] everywhere in the language currently. Switching this to mean [[Define]] on something that is not a product of class is the bigger, more confusing, and more dangerous reversal.

[rdking]

@mbrowne Nice catch.

There's also a potential downside of [[Define]] for decorators:

That's not a "potential" downside, but a definite. Decorators work by rewriting property definitions. No subclass that overrides a base class field will ever be able to take advantage of the parent's decorators without superfluous use of super.

[mweststrate]

@littledan

Personally, I am still strongly in favor of [[set]], as [[define]], as with [[define]]. Using initializers to default fields introduced introduced in super classes will become anti pattern with [[define]], while it is quite a common case currently (we use React's state = example a lot), and it is non-trivial to migrate away from.

Furthermore it removes some tools from the toolbox of superclass authors, such as deprecation, warning and invariant checks at the earliest place possible.

Most concerning to me are the few, but very hard to find potentially breaking changes (like the one I started of with), they only way to statically find them in a babel code base would be to disallow all field assignment in subclasses.

TypeScript was also represented in TC39 at the time and, (...) my understanding was that they were OK with either outcome.

I am not entirely sure about that one, given @RyanCavanaugh's comments?

I am quite in favor of @yyx990803's earlier proposal:

Use [[Set]] semantics for field initializers 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.

(I doubt whether there are many actual use cases for the additional syntax, but at least it helps with the 'completeness' of the language?)

That all being said, I think we just arrived at re-iterating arguments, so I think it is fine to close this issue from my side and leave the rest up to the wisdom of TC-39. In either case we'll most probably survive :).