Skip to content
This repository has been archived by the owner on Jan 25, 2022. It is now read-only.

Uninitialized properties should still exist #57

Closed
mbrowne opened this issue Jan 29, 2017 · 12 comments
Closed

Uninitialized properties should still exist #57

mbrowne opened this issue Jan 29, 2017 · 12 comments

Comments

@mbrowne
Copy link

mbrowne commented Jan 29, 2017

First of all, I hope that opening an issue here is the correct starting point for suggesting a change to the proposal. @loganfsmyth (one of the Babel maintainers) suggested that I open a new issue, since my suggestion is kind of independent of this issue that I commented on. *

Currently, the proposal specifies that properties that aren't given default values should be completely ignored, i.e. this:

class Demo {
	foo;
}

translates to this:

function Demo() {
    //no mention of foo property whatsoever
}

Beyond the fact that one would expect the runtime environment to be aware of the property even if it hasn't been initialized yet, this is problematic because you can't enumerate over the property or check for its existence in any way before it is given a value:

let d = new Demo();
('d' in foo); //false

As I explained here, this makes some common and useful patterns impossible to implement using class properties.

I propose changing the spec such that the above example would transpile to this:

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

Or the more spec-conformant version:

var Demo = function Demo() {
  ...
  Object.defineProperty(this, "foo", {
    enumerable: true,
    writable: true,
    value: undefined
  });
};

Logan pointed out here that this would mean that uninitialized properties in subclasses would be set to undefined even if the parent class specified a default value, for example:

class Base {
    foo = 4;
}
class Child extends Base {
    foo;
}

let c = new Child();
c.foo; //undefined

After considering it and comparing with the behavior of Java and PHP, I'm thinking that the above behavior is how it should work, even if it's initially surprising to some users. As I said in my reply to Logan:

If I'm explicitly declaring a property in a subclass, I don't think I'd want the addition of a default value for that property in the parent class to change the behavior of the child class -- especially if the parent class is maintained by a 3rd party.

However, I think this is a topic that should be discussed to weigh the pros and cons. If it's decided that uninitialized properties shouldn't overwrite the default value specified in a parent class, then the implementation could be modified accordingly, for example:

function Child() {
    Base.apply(this, arguments);
    this.foo = ('foo' in this) ? this.foo: undefined;
}

Or more succinctly:

function Child() {
    Base.apply(this, arguments);
    this.foo = this.foo;
}

(Technically this alteration would only be needed for subclasses, but perhaps it would be simpler to be consistent.)

I look forward to hearing people's thoughts on this - thank you.


* But it's still related, and I think it would be ideal for instance properties to exist on the prototype somehow (as long as non-primitive default values are defined in the constructor rather than the proprotype).

@mbrowne
Copy link
Author

mbrowne commented Jan 29, 2017

P.S. This is how it works in Java:

class Base {
    public int foo = 4;
}

class Child extends Base {
    public int foo;
}

class Program {
    public static void main(String[] args) {
        Child c = new Child();
        System.out.println(c.foo); //outputs "0"
    }
}

@ljharb
Copy link
Member

ljharb commented Jan 29, 2017

This is already the plan of record, as I understand it.

@bakkot
Copy link
Contributor

bakkot commented Jan 29, 2017

Strongly agree that foo; should be equivalent to foo = undefined;. I think it's currently the intent of the authors too; at least, it's what @littledan said in his slides at the January TC39 meeting.

I think it's actually a bug that it's not what's currently specified. This commit introduced the behavior you propose for static fields, but not instance fields.

--

Edit for pedantry:

Technically this alteration would only be needed for subclasses, but perhaps it would be simpler to be consistent

It's possible to detect the difference between a property which is present with a value of undefined and a missing property, so in fact even base classes and classes which extend null need this.

@mbrowne
Copy link
Author

mbrowne commented Jan 29, 2017

Thanks! This is good news.

It's possible to detect the difference between a property which is present with a value of undefined and a missing property, so in fact even base classes and classes which extend null need this.

It probably doesn't matter now, but to be clear, I was saying that this.foo = ('foo' in this) ? this.foo: undefined would only be needed for subclasses. Agreed, base classes would still need to set this.foo = undefined.

@ljharb
Copy link
Member

ljharb commented Jan 29, 2017

Base classes still inherit from Function.prototype and Object.prototype.

@mbrowne
Copy link
Author

mbrowne commented Feb 1, 2017

So what's the next step; are we just waiting for the authors to confirm this change? Note that I already implemented this in Babel (pull request here) but I imagine that the spec will need to be officially changed before the pull request can be accepted.

@littledan
Copy link
Member

I don't know if there's any change to make. I believe the semantics you're talking about here are already what the draft specification requires.

@bakkot
Copy link
Contributor

bakkot commented Feb 6, 2017

@littledan I think it's missing for instance fields. Per 9.a.iii, nothing happens if initializer is empty. Should be a simple fix but I can't make it right now

@mbrowne
Copy link
Author

mbrowne commented Feb 12, 2017

I was going to submit a pull request for this, but it looks like it's already updated to set uninitialized properties to undefined in the master branch: https://github.com/tc39/proposal-class-public-fields/blob/master/spec/new-productions.htm. Maybe someone already changed it but didn't update the gh-pages branch so the github page is out of date? Another difference is that the source file in the master branch doesn't mention steps 3-7 here.

@jeffmo
Copy link
Member

jeffmo commented Feb 12, 2017 via email

@jeffmo
Copy link
Member

jeffmo commented Feb 13, 2017

gh-pages updated now

@mbrowne
Copy link
Author

mbrowne commented Feb 13, 2017

Great! Thank you.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants