-
Notifications
You must be signed in to change notification settings - Fork 106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
can private accessor
s finally solve private fields being undefined before field initializtion?
#480
Comments
accessor
s finally solve private fields being undefined before field initializtion?
I don't think it's an oversight at all, like 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 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 |
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,
});
}
} |
Sidenote, there is another discussion related to this in the ES Discourse group: Why can't |
Oops, I overlooked that. Maybe
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 Furthermore, if the engine can make information like I mean, TC39 members could have designed anything. There is no limitation here. Back to the topicI'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
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 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! |
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. |
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 function Klazz() {
this.field = "blah";
}
Klazz.prototype.method = function method() {
// ...
} Now the THen ES6 comes along, wanting to have a more obvious way of creating classes the new Now at the same time, another object is to be introduced, the 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
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
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 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
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).
What do you mean the "original 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();
}
}
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?
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 |
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 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. |
I see, so
I bet people will change class shapes in the initializers, but that's totally another topic.
I meant to make that a getter only (readonly). Can't the engine simply return It seems like a one-line code change. |
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 |
Not if |
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. |
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 |
The Problem
With public getters, public class fields, or properties-in-constructors, the following sort of thing works fine:
Paste in Chrome console, and the output is:
(live demo: https://jsfiddle.net/4uaofd0t/)
Now, if we wish to convert
foo
to a readonlyfoo
with private#foo
field, like so,then we get a runtime error like so:
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 theFoo
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, butaccessor
was here in this proposal.Can
accessor
be lazy, or returnundefined
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):
New code:
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,
is "roughly desugared" to
There is an opportunity here to specify that the backing storage for an
accessor
returnsundefined
, just like getters/setters typically would in pre-class-fields JavaScript of the formTypeScript
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 handleundefined
cases.Hmm, but then again, that idea does not cover
undefined
in subclass methods that extend from a method likesomethingSpecial
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
accessor
s. 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.
The text was updated successfully, but these errors were encountered: