Initialized instance fields/properties need to be defined as own properties of the prototype

[silkentrance]

For more information, see

https://phabricator.babeljs.io/T7301

and referenced other issues

The main problem with the current specification is that it does prevent certain decoration use cases and also that it gives the user some hard time finding bugs when overriding such properties. Besides of that it also imposes additional runtime overhead as such properties will be defined during instancing and not during construction of the class.

In addition, these fields should be made configurable, so that one can decorate and override them or altogether remove them.

And, to be frank, ECMAScript should be an all purpose language and not one that is designed around an idiom that is commonly shared by or that is meant to target a few frameworks.

What this basically means is that the following part of the specification should be revised.

Instance Field Initialization Process

The process for executing a field initializer happens at class construction time. The following describes the process for initializing each class field initializer (intended to run once for each field in the order they are declared):

Let prototype be the prototype object
Let fieldName be the name for the current field (as stored in the slot on the constructor function).
Let fieldInitializerExpression be the thunked initializer expression for the current field (as stored in the slot on the constructor function).
Let initializerResult be the result of evaluating fieldInitializerExpression
Let initializerResult be the result of evaluating fieldInitializerExpression with this equal to instance.
Let propertyDescriptor be PropertyDescriptor{[[Value]]: initializerResult, [[Writable]]: true, [[Enumerable]]: true, [[Configurable]]: true}.
Call prototype.defineOwnProperty(fieldName, propertyDescriptor).

[fatfisz]

This is related: #16

[loganfsmyth]

Putting the assigned values on the prototype solves this specific edge case and exposes others though. With existing frameworks like Backbone, you end up with hacks to ensure that you don't accidentally place objects in the prototype chain. You'd get the same thing here for instance

class SomeThing {
    someThing = {
        prop: null,
    };

    constructor(options){
        this.someThing.prop = options.prop;
    }
}

The someThing object will be shared across all instance of the object, which seems like a worse issue than the one you're describing because it's more likely to go unnoticed, and it seems more likely than the case where you shadow a prototype method with a class property.

[silkentrance]

Is a duplicate of #16, thanks @fatfisz for pointing this out. @jeffmo Feel free to close this when no longer needed.

[silkentrance]

@loganfsmyth Yes, you are right.

There is a solution of course, and this involves making the initializer function a getter, e.g.

class Foo {
    a = { o: 1 };
}

and desugared

class Foo {
     defineProperty(Foo.prototype, 'a', { 
        get: function () { 
            if (!this['_a']) {
              this._a = {o:1};
            }
            return this._a;
        }, 
        set: function(v) { this._a = v; }})
}

Of course, one could instead use symbols for the slot used for storage.

[silkentrance]

@loganfsmyth @lukescott

Reconsidering #38 (comment) and also the proposal for a truly private state over at https://github.com/wycats/javascript-decorators/blob/master/interop/private-state.md

How about desugaring this to

var initializedPrivateInstanceProperties = {};
var initializedPrivateStaticProperties = {};

var Foo = function Foo() {
    // apply class call check
};

var _symA = Symbol('a');
defineProperty(Foo.prototype, 'a', { 
        get: function () { 
            if (!(_symA in initializedPrivateInstanceProperties)) {
              initializedPrivateInstanceProperties[_symA] = initializer.call(this);
            }
            return initializedPrivateInstanceProperties[_symA];
        }, 
        set: function(v) { initializedPrivateInstanceProperties[_symA] = v;}
})

That way, decorating initialized instance properties is no longer a special case.

[lukescott]

Using get / set makes the initialization of the variable lazy. It really isn't much different than the current initializer, besides the initializer is always run in the constructor. I don't see how using a getter/setter make it any better.

[silkentrance]

@lukescott the reason for doing so would be the use case of decorating such initialized instance properties, which is currently impossible to achieve along a line of inherited classes, say

class A
{
   initialized = 'A';
}

class B extends A
{
   @decorate
   initialized = 'B';
}

or even

@initializedInstancePropertyDecorator({decorate:'initialized'})
class B extends A
{}

See also jayphelps/core-decorators#62 for a more concrete example.

[amiller-gh]

@lukescott – Going to throw in my two cents here I suppose.

Many frameworks (echem, Backbone) allow sub-classes to modify default properties used in the parent's constructor. With the proposal as is, this common pattern is currently impossible:

class Model {
  constructor(data){
    this.set(this.defaults);
    this.set(data);
  }

  // Default defaults are empty by default. Default...default..
  defaults = {};

  set(data){
    // Assume this.set does what you'd expect
  }
  get(data){
    // Assume this.get does what you'd expect
  }

}

class Test extends Model {
  // Objects of type Test should have this as the default values
  defaults = { foo: 'bar' };
}

var obj = new Test();

obj.get('foo'); // undefined

Because they aren't on the prototype, the user provided defaults are set after the parent constructor is run.

The same extension can be written like this and work just fine:

class Test extends Model {
  get defaults(){ 
    return this._defaults || (this._defaults = { foo: 'bar' }); // Replace with private state method
  }
}

var obj = new Test();

obj.get('foo'); // 'bar'

As of right now, I agree with @silkentrance that class fields should be syntactic sugar for a property getter.

[mbrowne]

I have a question that's somewhat related to this so I thought I'd start by posting a comment here. Reading the current proposal, it's currently unclear to me whether it would be in accordance with the proposal to have properties explicitly initialized to undefined if they don't have default values rather than simply ignoring them as Babel's transform-class-properties plugin currently does. I opened a Babel issue about this with some more detail. I'm interested in implementing patterns similar to what @EpicMiller mentioned and I think this would be a step in that direction.

To summarize, I'm proposing that this:

class Demo {
	foo;
}

Should transpile to this:

Demo = function Demo() {
	_classCallCheck(this, Demo);
	this.foo = undefined;
};

I actually just successfully made this change to the source code for the transform-class-properties plugin and am ready to submit a pull request for it. But first I wanted to verify that this isn't a violation of the current proposal, or if it is, whether you all would be open to changing it to the above behavior? I think the current behavior of Babel's transform-class-properties plugin of completely ignoring properties without default values is quite problematic. I think any property declared with a property initializer should be enumerable; it's quite surprising that properties without default values don't even appear in the transpiled code at all.

[mbrowne]

Here's an example of an implementation that's currently doesn't work due to the ignored properties:

class Entity {
	assignAttributes(attributes) {
		for (let key in attributes) {
			if (key in this) {
				this[key] = attributes[key];
			}
		}
	}
}

class User extends Entity {
	id;
	email;

	constructor(attributes) {
		super();
		this.assignAttributes(attributes);
	}
}

It would of course be easy to fix it manually by doing this:

class User extends Entity {
	id = undefined; // (or null would work too)
	email = undefined;
}

But I think it's surprising that the first way doesn't work, and personally I'm not a fan of boilerplate code like = undefined that would just be syntactic noise in cases like this.