can private `accessor`s finally solve private fields being undefined before field initializtion?

Question

can private `accessor`s finally solve private fields being undefined before field initializtion?

trusktr opened this issue 2 years ago · 13 comments

The Problem

With public getters, public class fields, or properties-in-constructors, the following sort of thing works fine:

const specialDefault = 456

class Base {
  constructor() {
    this.somethingSpecial()
  }

  // abstract foo: number

  somethingSpecial() {
    console.log(this.foo ?? specialDefault)  
  }
}

class Foo extends Base {
  foo = this.foo ?? 123

  constructor() {
    super()
    console.log('construct Foo')
  }
}

new Foo()

Paste in Chrome console, and the output is:

456
construct Foo

(live demo: https://jsfiddle.net/4uaofd0t/)

Now, if we wish to convert foo to a readonly foo with private #foo field, like so,

const specialDefault = 456

class Base {
  constructor() {
    this.somethingSpecial()
  }

  somethingSpecial() {
    console.log(this.foo ?? specialDefault)  
  }
}

class Foo extends Base {
  #foo = 123
  get foo() { return this.#foo }

  constructor() {
    super()
    console.log('construct Foo')
  }
}

new Foo()

then we get a runtime error like so:

Uncaught TypeError: Cannot read private member #foo from an object whose class did not declare it

This sort of issue easily pops up in large pre-class-fields code bases. I've seen it with my own eyes. Wanting to use new features (to make code actually semantic of intent with private fields instead of __underscored properties), but not being able to without refactoring due to this issue, has been painful and annoying.

The error is also unintuitive, because the only code accessing the #foo field is inside the Foo class.

The Solution?

It seems like accessor could (or should) somehow help with this, but maybe not. And maybe this isn't the right place for the discussion, but accessor was here in this proposal.

Can accessor be lazy, or return undefined for uninitialized accessors, so that converting large codebases from __private properties to #private fields simply works with no issues?

For example, old code (very simplified compared to what I've seen in the real world in huge code bases, including at NASA):

// ...class Base like before...

class Foo extends Base {
  __foo = 123 // private variable, do not use this outside of this class!!!
  get foo() { return this.__foo }
}

New code:

// ...class Base like before...

class Foo extends Base {
  accessor #foo = 123
  get foo() { return this.#foo } // it works, no error!!
}

The main idea here, is that, similar to old-school getters/setters (accessors) which would just return undefined in old code, this would match that pattern (even if internally the implementation is different). This is why I brought this up here, because old-school accessors are what have this behavior.

In the README, this example,

class C {
  accessor x = 1;
}

is "roughly desugared" to

class C {
  #x = 1;

  get x() {
    return this.#x;
  }

  set x(val) {
    this.#x = val;
  }
}

There is an opportunity here to specify that the backing storage for an accessor returns undefined, just like getters/setters typically would in pre-class-fields JavaScript of the form

class C {
  __x = 1;

  get x() {
    return this.__x;
  }

  set x(val) {
    this.__x = val;
  }
}

TypeScript

As for TypeScript, there can be a new rule for accessors such that accessing a subclass accessor that is abstract in a base class always has the additional type | undefined unioned with it. The solution seems very simple, and would require base classes to handle undefined cases.

Hmm, but then again, that idea does not cover undefined in subclass methods that extend from a method like somethingSpecial in the above example. Would the solution then be to always include | undefined in the type? It would be less ergonomic, but it would also be proper.

Or maybe type safety in TypeScript isn't so important here. I mean, this example also fails in TypeScript, because TypeScript isn't completely type safe being guide rails for JS:

https://tsplay.dev/m3XyLW

Regardless, JavaScript can do what it needs to, and TypeScript can decide how to handle this separately in some way.

Is it too late?

It seems like an oversight to not fix this issue with accessors. This proposal is stage 3, but there are no native engines with this shipped yet, as far as I know, not even behind a flag:

https://caniuse.com/decorators

It would be amazing to be able to port old code without quite huge issues and refactoring needs.

I'm wishing for this out of experience.

Answer 1 · 2022-08-21T14:54:28.000Z

Can accessor be lazy, or return undefined for uninitialized accessors, so that converting large codebases from __private properties to #private fields simply works with no issues?

It seems like an oversight to not fix this issue with accessors. This proposal is stage 3, but there are no native engines with this shipped yet, as far as I know, not even behind a flag:

I don't think it's an oversight at all, like accessor shouldn't be the place to fix a behaviour of private fields. Like if such a fix was to be implemented, why would accessor work but not get/set?

And as long as private fields have weakmap semantics (and I highly doubt such a decision would be reconsidered) this problem is unlikely to ever be fixed. Ultimately because of constructor return, the only time private fields can be installed is at the end of super() because this the only time when the actual value of this that the subclass will receive is known.

i.e. Consider this:

class Other {}

class Base {
    constructor() {
        console.log(this.x);
        return new Other(); 
    }
}

class Sub extends Base {
    #x = 10;
    
    constructor() {
        super();
    }
    
    get x() {
        return this.#x;
    }
}

When invoking this.x in Base the actual this value isn't even the same value that private fields will be installed upon, so naturally this.#x fails. The actual object that acts as this in Sub is the new Other() object.

Answer 2 · 2022-08-21T15:19:54.000Z

Designs that run into this problem seem to be when the superclass constructor is trying to communicate with/interact with a subclass - imo this just isn’t the way classes are supposed to work, so it’s fine that these examples run afoul of private fields.

Answer 3 · 2022-08-21T15:37:05.000Z

Designs that run into this problem seem to be when the superclass constructor is trying to communicate with/interact with a subclass - imo this just isn’t the way classes are supposed to work, so it’s fine that these examples run afoul of private fields.

Even if communication is desired, a considerably less fragile way to do this is to separate communication from subclassing altogether by providing an explicit controller:

class Base {
    constructor({ controller }) {
        controller.doWhatever();
    }
}

class Sub extends Base {
    constructor() {
        super({
            controller: someControllerForSub,
        });
    }
}

Answer 4 · 2022-08-23T02:40:20.000Z

Sidenote, there is another discussion related to this in the ES Discourse group: Why can't #private fields return undefined before initialization?

Answer 5 · 2022-08-23T03:18:23.000Z

why would accessor work but not get/set?

Oops, I overlooked that. Maybe get/set would do that too in this case.

And as long as private fields have weakmap semantics

We can't use "WeakMap semantics" as an argument against class fields returning undefined before being initialized, because here's an example that uses "WeakMap semantics" that allows a super class to get undefined from a subclass private field:

// INPUT CODE
class Base {
  constructor() {
    this.log()
  }
  log() { console.log(this.value) }
}

class Sub extends Base {
  #value = 123
  get value() { return this.#value }
}

new Base() // logs "undefined"

// OUTPUT CODE
class Base {
  constructor() {
    this.log()
  }
  log() { console.log(this.value) }
}

const value = new WeakMap

class Sub extends Base {
  constructor() {
    super()
    value.set(this, 123)
  }
  get value() { return value.get(this) }
}

new Sub() // logs "undefined"

In your example, the object this.#x runs on is an entirely different object. With the "WeakMap semantics" of my example, this.#x still returns undefined regardless which object it runs on.

Furthermore, if the engine can make information like new.target availabe across the whole constructor stack, it could also make internal information like the original new.target object available, such that private access on any foreign object throws an error instead of undefined, and undefined can be a behavior exclusively for non-foreign objects.

I mean, TC39 members could have designed anything. There is no limitation here.

Back to the topic

I'm just pointing out an unfortunate undesirable behavior that makes old code non-future compatible, and wondering if there is a space here to fix it.

If not using accessor and get/set, then what? That's the real question here.

imo this just isn’t the way classes are supposed to work

Your opinion on base classes strictly not knowing about subclasses is just that: an opinion. I don't think that is a fair way to design this language, especially when that opinion had no place in JavaScript this whole time until class fields landed and was not just syntax sugar.

A base class with a limited set of final sub classes is a perfectly good example, for example. Or, for example, an abstract base class that serves as a backbone for several other base classes, where the user is clearly guided on using one of the non-abstract base classes.

Your opinion here literally hampers totally valid code patterns just because you don't like them.

That opinion of yours should, in my humblest opinion, be your purview in the code bases you personally manage, or the purview of those who subscribe and follow you, but not the purview of the language.

JavaScript was always great as a flexible language. That has always been its strength (for people willing to learn how to wield the power).

Please take that into consideration!

Answer 6 · 2022-08-23T03:20:15.000Z

Even if communication is desired, a considerably less fragile way to do this is to separate communication from subclassing altogether by providing an explicit controller:

That's true, but abstracting state outside of classes just for a parent-child class relationship can be an unwanted new level of indirection and verbosity.

Answer 7 · 2022-08-23T19:33:08.000Z

We can't use "WeakMap semantics" as an argument against class fields returning undefined before being initialized, because here's an example that uses "WeakMap semantics" that allows a super class to get undefined from a subclass private field:

Okay "weakmap" semantics is a bit of an oversimplication of private fields behaviour. In fact understanding the reasons behind private field semantics requires a bit of a history lesson to get the larger picture of reasons for the semantics.

So, consider back to the times of ES5, in these times there were no classes, proxies, weakmaps, symbols, etc. At this time "classes" were essentially a pattern around some behaviour of new Fn():

function Klazz() {
    this.field = "blah";
}

Klazz.prototype.method = function method() {
    // ...
}

Now the new operator had some special behaviour, specifically that it would call the function setting the this value as a an Object with [[Prototype]] set to `Klazz.prototype.

THen ES6 comes along, wanting to have a more obvious way of creating classes the new class syntax allowing this pattern to work more directly, however due to the existence of legacy classes there is a need to be able to integrate with those classes, in addition to builtin classes.

Now at the same time, another object is to be introduced, the Proxy object. This object is meant to be able to model any JS object that has special behaviour. One of these behaviours is the behaviour of the new operator. This behaviour was previously defined by a simplified version of [[Construct]] that merely took a list of args like any other function. However the new class feature is also trying to support subclassing, so how can a subclass instance appear in the superclass constructor? Well the answer is introduce an additional value new.target which is also passed into the [[Construct]] operation. Now proxies can simulate such behaviour by invoking Object.create on the passed newTarget value.

Also at the same time, the weakmap object is introduced which allows for weakly associating data to any object. Something important to know about the WeakMap object is that it can actually be implemented in two ways. The first way is simply as some kind've weak hash table, the alternative is known as the inverted representation where each object maintains a list of weakrefs to weakmaps it is a member of. This inverted representation has some advantages, however not all engines implemented it (to this day) for a couple reasons:

One is that hashmap-like implementations are already fairly common for such structures
All objects would need to have a field for storing such weakmaps, whether they were ever added to a weakmap or not
- This includes host objects which would further complicate implementation

Now fast-forward to the private fields proposal, at this point a decision needed to be made as to how private fields would be hidden from regular fields. There were basically two possible styles, either symbol-like or weakmap-like. One of the considerations was that the intended extension of private methods would only have a single method per class compared to one-to-one of fields. In favour of symbol-like behaviour was that private methods could just look up to obj.[[GetPrototypeOf]]() for the private method rather than having a per-instance thing. However weakmap-like semantics ultimately won out for a few reasons:

obj.[[GetPrototypeOf]]() means that private field/method lookup is partially observable partially breaking the whole "private" part
weakmap-like semantics can be implemented as the inverted-representation (where each object has a pointer to each), this has a considerable implementation advantage that private fields can be implemented basically as structs with private fields packed just like other members

An additional behaviour that private fields have is that trying to access a non-existent one throws. This behaviour was added for various reasons, however one important point is that it directly impacts how struct-like implementation can work. In particular consider some code like:

class Foo {
   #field;
   
   setField() {
       this.#field = 3;
   }
}

If throwing behaviour didn't exist, and we used actual weakmap semantics, then implementations would always need to be prepared to store such fields on objects other than instances of Foo.

Now because of constructor-return it is possible to install private fields on other objects as if private fields were weakmaps, however such behaviour complicates implementations and in fact just a couple months ago a change was added to allow host objects to specifically reject private fields which shows you that even such fields are not really weakmaps at all (they're just weakmap-like).

The thing is though, with throwing semantics, cases where struct-like implementation are doable is still statically determinable. If this restriction were loosened like you're proposing suddenly every this.#field = 3 would become a full weakmap as people could just call Foo.prototype.setField.call(anyValue) with anyValue they like and the engine would need to do something with it.

Your opinion here literally hampers totally valid code patterns just because you don't like them.

That opinion of yours should, in my humblest opinion, be your purview in the code bases you personally manage, or the purview of those who subscribe and follow you, but not the purview of the language.

JavaScript was always great as a flexible language. That has always been its strength (for people willing to learn how to wield the power).

Please take that into consideration!

Even history and implementation aside, the whole "flexible language" just isn't as much the case with modern features. Since ES6 highly dynamic features simply haven't been accepted, this is most obvious with well the decorator proposal.

Like decorators itself used to be highly dynamic with the whole feature being about returning descriptors, then static decorators came along with a bit more of a static design but that turned out to not really be optimizable at all and multiple implemeters (all?) refused to implement such a thing. The current design of decorators specifically came about because it doesn't change class-shapes in unpredictable ways so can be highly optimized. (Like you might think it's fairly dynamic because you can return arbitrary things, but ultimately shapes don't change so fields stay fields, getters stay getters, methods stay methods, etc).

Furthermore, if the engine can make information like new.target availabe across the whole constructor stack, it could also make internal information like the original new.target object available, such that private access on any foreign object throws an error instead of undefined, and undefined can be a behavior exclusively for non-foreign objects.

What do you mean the "original new.target"? There's only one new.target, that is whatever the function that comes after new. If you mean the "original instance", this object is simply created by calling Object.create(new.target.prototype), such an object has never even been observed by the subclass. i.e. A superclass really works like:

const Base = new Proxy({}, {
    construct(args, newTarget) {
        return Object.create(newTarget.prototype);
    },
});

class Sub extends Base {
    constructor() {
        // There is no "original instance", the [[Construct]] hook of Base
        // could return literally anything, that is the whole reason
        // "this" is inaccessible until AFTER super() is called
        super();
    }
}

I mean, TC39 members could have designed anything. There is no limitation here.

There is limitation here, if you can't convince implemeters to implement such a feature then standardizing a feature is about as useful as doing nothing. Implemeters have indicated they don't want to introduce new highly dynamic features, so why would TC39 waste effort standardizing something that wouldn't be implemented?

I'm just pointing out an unfortunate undesirable behavior that makes old code non-future compatible, and wondering if there is a space here to fix it.

Here is definitely the wrong place regardless of whether it's worth fixing or not. Your issue here ultimately stems from two historical decisions, first is that this is inaccessible before super() (a neccessary consequence of constructor-return), and secondly that private fields throw on objects that haven't been received by the constructor. Neither of these problems has anything to do with decorators or even accessor (which is basically just sugar for a pair of get/set).

Answer 8 · 2022-08-23T20:31:36.000Z

I actually think this will make the error worse (as you see it), because the getter's underlying data will act like a private field:

const specialDefault = 456

class Base {
  constructor() {
    this.somethingSpecial()
  }

  somethingSpecial() {
    console.log(this.foo ?? specialDefault)  
  }
}

class Foo extends Base {
  // notice this is a "public" field
  accessor foo = 123

  constructor() {
    super()
    console.log('construct Foo')
  }
}

new Foo()

This will transform into something like:

class Foo extends Base {
  #_foo = 123;
  get foo() {
    return this.#_foo;
  }
  set foo(v) {
    return this.#_foo = v;
  }

  constructor() {
    super()
    console.log('construct Foo')
  }
}

new Foo()

So using an accessor on a public field will internally store the data in a private field, which will not be initialized until after super() constructor completes. So accessing the "public" field in the super constructor will now throw, because it internally accesses an uninitialized private field.

Whether this is a better or worse is debatable. I see accessing a subclass's field (that I expect to be initialized but hasn't yet been) to be an error, so throwing an error is appropriate.

Answer 9 · 2022-08-24T21:14:14.000Z

Foo.prototype.setField.call(anyValue)

I see, so Foo.prototype.setField.call(anyValue) is a deoptimization that is entirely avoided (it would make it really WeakMap like, losing the more optimized version you described).

(Like you might think it's fairly dynamic because you can return arbitrary things, but ultimately shapes don't change so fields stay fields, getters stay getters, methods stay methods, etc).

I bet people will change class shapes in the initializers, but that's totally another topic.

I actually think this will make the error worse (as you see it), because the getter's underlying data will act like a private field:
...
  // notice this is a "public" field
  accessor foo = 123

I meant to make that a getter only (readonly).

Can't the engine simply return undefined instead of throwing? It doesn't seem that anything in the implementation has to be modified at all, apart from that one thing, so all the optimization described above would remain exactly the same.

It seems like a one-line code change.

Answer 10 · 2022-08-24T21:19:28.000Z

I'm pretty sure that would break code that's shipped that uses try/catch to detect whether something has a field - ie, anything transpiled, either written prior to #x in y existing that has since stopped transpiling, or the transpilation of the throwing behavior itself.

Answer 11 · 2022-08-26T17:58:50.000Z

Not if accessor (or something else) is the new way to make that case return undefined. #x in foo could be left as is?

Answer 12 · 2022-08-26T18:25:43.000Z

Oh sure, I meant if it was changed by default. If there's an explicit opt-in it would be fine - but that would be an entirely distinct proposal from this one, that would need sufficient motivation.

Answer 13 · 2022-11-25T15:23:21.000Z

As noted above, this change is outside of the scope of this proposal and should be opened as a separate proposal. Ideally such a proposal would address both private fields and accessor, because ultimately they should likely work similarly if not exactly the same. I'm going to close this issue because I don't think it's relevant to the current decorators proposal.