Closed jridgewell closed 6 years ago
It is indeed an error, for exactly the reason you give.
Is this something that should be considered for a feature? It seems like a footgun that people could easily not think about, and it's a potentially unexpected difference between instance privates and static privates.
Mmmaybe? @littledan
I don't think of it as a difference, though. Private fields are not inherited through prototypes. That's true for static fields just as for instance fields.
Yeah I'm on the fence. I could kind of see it as expected since the constructor extends the parent, the same way the instance does, it's just that class instances have an established chained initialization process, so it's easy to support in there, where class constructors don't have that. On the other hand, I bet it'd be a pain to make this work and or support in Babel.
I don't feel super strongly either way, but I figured it was worth asking.
I had a prototype crawler originally, and it worked fine. I do see it as a difference between instance and static privates that non-spec-writers will definitely trip over.
class Base {
#instance;
static #static;
get() {
return this.#instance;
}
static get() {
return this.#static;
}
}
class Sub extends Base {};
// This works
new Sub().get();
// This throws
Sub.get();
Well, I think this is the sort of question that led static private fields to be left out of the earlier private fields proposal by @zenparsing. @erights has brought up the question about whether subclasses should have a separate private field for subclass static fields. If the answer is "no" to that question (as we settled on), I don't see why users would use this.#static
rather than Base.#static
(which will work just fine).
Do you think we should have separate static private fields for each subclass? Or, do you think this.#static
is much more intuitive than Base.#static
to use within static methods? I would've thought that it would be more common to use the class name within static methods, generally.
I agree with @bakkot that, in a sort of spec-formal way, the current semantics are consistent. But if this would be significantly confusing/underpowered for users, maybe it should be reconsidered.
I was trying to think of some example that would use @@species
with a static field, but I can't seem to come up with a concrete example where I would use this.#static
over Base.#static
.
Maybe just early error on this.#static
for static private fields?
It would be nice if there was a keyword that could be used to s;ways reference the current class, like static
- then you could ergonomically do static#x
for the common use case, and this#x
for when you wanted a method that would throw when inherited.
Maybe just early error on this.#static for static private fields?
It's hard for me to see how we can generalize that, in a few ways:
(this).#static
? For anything besides Base.#static
?#static
is static or on instances?@ljharb Why use that keyword when you can use the name of the class? Such a name exists for class expressions which want it, too.
@littledan to reduce the number of places I have to make a change when I want to rename the class.
@ljharb That doesn't seem like a primary use case. I imagine it will be more work changing the scattered usage sites than what's inside the class definition. Also, you can use class expressions for an internally visible name (though this will leak through the class's .name...).
Yes, I understand the use case isn't highly compelling; but it's incredibly common in the airbnb codebase to call static methods from instance methods, or statics from other statics, and it's very messy having to repeat the class name identifier.
I imagine that you do end up repeating the class name identifier when calling static methods from instance methods...
I've heard the opposite argument, that this
is harmful because it is confusing, and should be avoided whenever possible.
Maybe we'd benefit from searching through code to see how many static methods use this
. My guess would be, not very many of them.
I think the combination of "static methods using this
" and "static or instance methods using the hardcoded class name" and "instance methods using this.constructor
(which is unreliable)" would all indicate where this kind of a keyword would be potentially useful.
Let's break this down:
this
If that's common, well, this is really the target case that is affected, if used together with calling static methods on subclasses.this.constructor
If you're using this.constructor
to call a static method... well, that seems roundabout enough that it seems like you really want the genericity, that your subclass will do something different. If it's that sort of case, maybe you're really looking for different, individual mutable fields for each subclass (which this proposal doesn't provide either).Overall, I see this thread as a reason to walk back on static private fields, but I don't see a clear way forward. I don't think going up the prototype chain makes sense with the rest of the private field design. Maybe we really should be adding "class instance" private static fields.
How often do people even call static methods on subclasses? Outside of ES6 classes, do people even bother setting up the inheritance chain?
Probably not; that's one of the benefits of ES6 classes - that you don't have to remember all the steps.
The case that's already working is not without issue - having to hardcore the class name is the issue.
I very much doubt people using this.constructor
are doing it because they want the roundabout method - in my experience, people do it because they think it's the only way to reference the class name, and because repeating the class name is a very bad practice in other languages, so they assume it's one in JS.
This "repeating the class name is bad practice" idea is new to me. Could you point me to a style guide where it's mentioned (in any language)?
Here's one for Ruby: https://github.com/bbatsov/ruby-style-guide#def-self-class-methods
@ljharb In the linked case, a technique is described for not repeating the class name when defining static methods. In JS syntax, the class name is never repeated in that case. Do you have any other reference that talks about not repeating the class name for another case?
@littledan the overarching principle is DRY - repeating yourself is a bad practice. http://www.korenlc.com/ruby-classes/ mentions this (in the same context; of defining static methods).
In PHP 5.5+, this principle is satisfied by static::staticMethod()
- the existence of this feature serves identical use cases as what I'm suggesting static.staticMethod()
might do in JS.
OK, that's another reference which is recommending the exact same thing--not repeating the class name when defining another static method.
It sounds like there's a reasonable class for adding this static.
feature, but I don't think this proposal needs to block on it. The case in question is pretty narrow--it's calling a static method on a subclass, which might be based on using this
or this.constructor
as the receiver, and then referring to a private field within the method. I haven't seen any evidence that this is widespread currently--the only examples I can think of for calling static methods on subclasses don't refer to any properties. I don't think it's worth complicating this proposal for this edge case.
Oh I totally agree it's not worth complicating this proposal; I brought it up because the future possibility of a syntactic static.foo
lookup could be used to address static private fields in the future, even if we made them early errors for this proposal.
static.foo
is already an early error. Do you think we should ban private static fields because of the runtime errors that they currently cause?
Sorry I'm not being clear :-)
I mean, if we decide to disallow static private fields for the time being, then we could later allow them via static#foo
versus this#foo
inside a static method, and then there'd be a cleaner way to use them.
Similar question for the inverse case (static method on the base class referencing a private name that exists on the subclass but potentially only there, i.e., crossing lexical scopes):
class Base {
static getNode() {
return this.#node;
}
}
class Sub extends Base {
static #node = "sub";
}
Base.getNode() // => throws
Sub.getNode() // => ???
@gibson042 This proposal only coverts truly private properties, not "protected"-like behavior, so your example would be an error. When you do .#node
, you're asking for the #node
declared in the current scope, which has none in the case of Base
. You are right that the this
object in your getNode
does have an internal slot for the #node
declared in Sub
, but it is impossible to access it from another class declaration. If Base
has a static #node = "base";
assignment of its own, that'd also be an entirely separate internal slot that does not relate to the one declared in Sub
.
@gibson042, to add a bit to @loganfsmyth's comment:
The code you've written, under this proposal, would be an early error, because #node
is not declared anywhere that the body of getNode
can see it.
Thank you for the clarification. But it leaves unaddressed the central issue of surprising interactions between static methods and private members.
We discussed this issue at the September 2017 TC39 meeting. Thanks to @jridgewell for bringing it up.
Let's go through a few possibilities here:
this.
. (Thanks to @tschneidereit for this suggestion.) That would eliminate the most common hazard case, but would leave in others (which might be more obscure), such as this.constructor.
.static.
. (Thanks to @ljharb and @rpalmer57 for this suggestion.) This would eliminate all hazard cases, but it would add a dependency on a not-yet-proposed feature.static.
as well. Classes tend to have an immutable binding to the constructor available inside them (unless they are anonymous), so this is typically available. I think we could make this usually be an early error, rather than a runtime error as currently, if we keep track in static semantics of which class name corresponds to which syntactic private name. There will be some corner-cases that would be missed and a runtime error is triggered (e.g., if a method contains a local variable of the same name as the enclosing class), but it should be possible to get everything you'd expect to come in normal code. I don't think the restriction is extremely onerous, but I'd be interested in opinions.To me, at this point, I'm leaning towards option 6. What do you think?
Another option: we could install static fields (and static private methods) on subclasses when they're defined.
I like options 5 or 6. If @bakkot's option would remove the footgun (such that Sub.get() === 'hello'
in the OP), then that seems like it might be better (although I still like static.
as a followon in that case)
We could revisit static private fields and methods being based instead on lexically scoped functions and variables
What does this mean? (Code example would be even better than prose, 😉 )
although I still like
static.
as a followon in that case
I think having a static
that behaves something like super
is worth exploring regardless of our choice here. It just becomes even more valuable if we decide to limit static private usage.
Another option: we could install static fields (and static private methods) on subclasses when they're defined.
I'm hesitant on this approach, since it breaks with inherited public fields. I can later change inheritance chain to something else entirely, and I don't think privates should be preserved in that case (since the publics aren't).
I'm suggesting static public fields get install on subclasses too, so it wouldn't break with that. In general inheriting fields has footguns, and I think it is an advantage of this approach that it avoids inheriting.
@bakkot I'm sorry for the omission above. I'm pretty sceptical of the idea though. When would you actually want this behavior in your program (as opposed for just intuiting it out of a sense of consistency)?
Don't solution 3-7 all smell like special cases hacks that are trying to fix a design wart that shouldn't be there in the first place? None of them are elegant. This is why I eventually abandoned static privates as a concept and championed solution 2, instead.
@littledan argues that solution 2 isn't orthogonal. I disagree and instead will argue that none of the other solution are orthogonal: solution 3 - a special kind of prototype lookup that only occurs for static
slot access that is different from instance slot access; solution 4 - special casing this based accesses to static slots making it different from instance slot access; solution 5 adding special (non-orthogonal ) syntax specifically for accessing static slots; solution 6 - giving the class name binding (if there is one) special semantics that only relate to static slot access; solution 7 - ?? I'm not sure what the "previous" rule is, but the whole concept of requiring use of a linter in this way is certainly a smell.
It's worth stepping back a bit and examine what use cases static slots (and methods) are trying to support. I believe that use case is: provide a singleton binding that is visible throughout the body of a class declaration such that the bound value can be used as a private communication channel among the units of executable code (prototype, instance, and static methods, initializers, etc.) that are defined within the class body. In other words, it is simply a lexical binding that is scoped to the class body.
Solution 2 argues that the orthogonal support for this use case is to simply use one of the existing lexical declaration forms to define the binding. It is not, non-orthogonal with private slots because because it isn't using a private slot to support the use case. The "class local" binding would not use the # names or property access syntax. They would have ordinary identifier based names just like all other lexically scoped bindings.
In summary, I continue to believe that solutions 1 or 2 are the only really alternatives are trying to fix a broken design by introducing special case WTF behaviors to the language.
@ljharb It would remove the footgun, yes. Sub
would have its own #field
(with the same Private Name), which get
would happily read from.
@littledan I think you'd want it for some of the same reasons we install instance fields from superclasses on instances of subclasses, rather than inheriting them.
In particular, the difference between
class Base {
static prop = 0;
}
class Derived extends Base {}
Derived.prop = 1;
Base.prop === 0; // true
and
class Base {
static opts = { prop: 0 };
}
class Derived extends Base {}
Derived.opts.prop = 1;
Base.opts.prop === 0; // false
is I think something which would be surprising to a lot of people - especially given that there is not this dichotomy for instance fields. More generally, prototypical inheritance of fields is weird; the reason I like this solution is that it avoids it.
(Also, it solves the problem in this thread without introducing more weird edge cases.)
@allenwb My proposed solution actually doesn't feel like a special case hack to me. It feels like aligning static fields with instance fields in a pretty natural way.
@bakkot It's still hard for me to think of when you'd have options that you mutate like that. Do you think you could be a little more concrete in the example? TypeScript has Static Properties whose semantics I thought were like the current proposal, not your proposal; is there any evidence (e.g., stack overflow threads, people complaining) that people are running into this issue?
@allenwb I agree that your solution is appropriate in terms of functionality. My biggest concern is that it won't "look private" to ordinary JavaScript programmers. I'm not sure if the lexical scoping intuition is enough; instead, this proposal gives consistent use of #
as the private sigil for classes. As a piece of evidence, there have been several threads on this repository that proposed using let x = y;
as the syntax for public instance field declarations. Jumping from that to private static field declarations makes sense to us as language designers, but maybe not to normal users.
@littledan TypeScript's static properties actually may or may not be like the current proposal depending on whether the environment running the compiled code has the ability to manipulate prototypes directly: if it does static properties are inherited with prototypical inheritance, but if it doesn't static properties are copied to the subclass.
So evidently very few people are using static properties in a way where the difference would be observable. Maybe @RyanCavanaugh would know more?
As to what people would expect - I don't actually know. The difference I describe feels surprising to me; even if it's something you'd rarely run into, I don't like how inheritance differs between static and instance fields. Besides, it solves the problem with static private properties - they'd Just Work. In light of that, do you think my proposal is more surprising than any of your 1-7?
I'm also interested in why you feel skeptical of it more generally.
@littledan Yes, my lexical proposal doesn't look like a private field because it isn't a private field. And that's the whole point. The various proposals for static private fields get into trouble because all other uses of fields, slots, and properties within class definitions have inheritance semantics and so it is a natural expectation that static private fields also have such semantics. But as the various alternatives show, trying to give them such a semantics creates significant issues. Class scoped lexical declarations are a better solution because they support the primary use cases without creating the hard to satisfy expectation (perhaps impossible to satisfy elegantly) that they have inheritance implications.
I think your speculation (it's not really evidence) that such class scoped lexical declarations would be confusing is weak. Just because there has been past proposals to assign other meanings to such declarations when they occur in a class body doesn't mean that most JS programmers would be confused by giving such declarations their normal meaning. In fact, one of the reasons for rejecting let x=y;
as the definitional form for public instance fields is that it would be inconsistent with the normal lexical scoped meaning of that syntactic form.
Pedagogically, consider a likely progression of leaning JS class syntax:
static
means methods and fields are properties of constructor objectAt this point a student might ask, "how do I share a private variable among all the parts of a class definition?" Teacher says: "Put a let
declaration inside the class body". Student says, "Oh, of course. I should have thought of that."
As language designers, we should recognize that when we are having to come up with smelly hacks like solutions 3-7 that we have gone down a poor design path and it's time to go back and reexamine the fundamental assumptions that took us down that path.
I think class scoped lexical declarations are a much better solution. But lacking adoption that or some other clean design, I would favor no support for these use cases (solution 1) over any of the other current alternatives.
OK, from this thread, I think the options 1, 2, 5 and 6 are the most reasonable. My goal for the November meeting (where this question is on the agenda) will be to decide between options 1 and 6 for what we should do right now, as a modification to the existing Stage 3 proposal, and get a feeling for whether to pursue 2 or 5 (or both) as a follow-up.
@bakkot The semantics you're suggesting are really unintuitive to me still. Ultimately, it's a little hard to reason about since it would seem like generally strange design to store mutable state in a property of a class. I imagine it could come up in a situation like this:
class Counted {
static #nextId = 0;
#id;
constructor() {
this.#id = this.constructor.#nextId++;
}
}
class FooCounted extends CountedInstances { }
class BarCounted extends CountedInstances { }
With your semantics, FooCounted and BarCounted each get their own counters. Is this the sort of use case you're thinking of? It just seems unusual to use classes in this way, to get a sort of singleton mutable state container. Does any other language work like this? It also seems less useful in JS than it might be in other languages with JS's single inheritance.
@littledan
With your semantics, FooCounted and BarCounted each get their own counters.
This happens with the current semantics too, is my point, except that they don't get them until the subclasses are first initialized. That is, the counters in FooCounted
and BarCounted
will hold get the value from Counted
until the first time each is initialized, at which point e.g. FooCounted
's count will branch off and then hold "number of times Counted was initialized before the first time FooCounted was initialized + number of times FooCounted has been initialized". That seems way less intuitive.
And while I also agree that putting mutable state in a static field seems like a bad idea, if the language allows it, people are going to do it. There is value in consistency even for features people should not use.
I'm not aware of any language which works like my proposal, but because of the above behavior I'm not aware of any language which works like the current proposal either. If we've got to do something new, at least we should be consistent about it, and ideally do something which will work as expected in as many cases as possible (e.g. for static private methods).Though I am more concerned about people referencing static properties with this
in a static method, or through the class name, than with this.constructor
in a non-static method.
I agree that it seems like "putting mutable state in a static property" is probably an unusual use case. But aside from that case my proposal works very much like the current semantics, so if we're holding that aside as too exceptional to be concerned about, why do we care? And if we're not, it really seems to me that the current proposal's semantics are less intuitive. Either way, mine has the advantage of making static private fields and methods work.
OK, it seems like the argument is not as much based on any particular use case, but instead, "the JS object model works with Set creating a shadowing own property if it's a data property on the prototype; ES6 classes have a prototype chain set up for constructors; the intersection of these two things is ugly, therefore we should create a bunch of extra own properties to prevent it from showing up." Hope that's not an unfair caricature, but if that's what it boils down to, I don't see how we're really solving the problem. You'll still have plenty of other objects which are possible to construct in JS which don't create extra own properties to cover over the weirdness of Set.
@littledan The main argument is "it would be nice if static private fields and methods worked for subclasses", but yes, your description is an accurate summary of my argument for it being a desirable behavior in general rather than just a kludge to get static private to work.
I'm not sold on the idea that we shouldn't worry about that weird behavior because it's present anyway. Yes, it's possible to construct objects which have fields on their prototypes which are absent on instances, but this is the first time we are proposing syntax specifically for adding non-function prototype-placed fields. Previously I would have told anyone setting foo.prototype.field = 0
that this was a bad idea which the language allowed because it didn't really distinguish between fields and methods, but that's no longer the case with the introduction of this feature.
In other words, there are a lot of historical footguns we can't do anything about, but that's no reason to copy them to new features.
Previously I would have told anyone setting
foo.prototype.field = 0
that this was a bad idea which the language allowed because it didn't really distinguish between fields and methods, but that's no longer the case with the introduction of this feature.
We're still not exposing fields on prototypes, or own methods. I think there's a qualitative difference here between prototype fields and static fields, that (I'd hope that) subclassing is less common than base classes, and referring to statics using this
rather than the constructor name is less common, and mutating static fields is less common than mutating instance fields. I don't think we actually want to encourage any of these things. The unlikeliness of the combination makes it less likely to be that we need complicated special case behavior like what you're proposing to give really beautiful behavior for the intersection of everything. Adding complex cases here also adds burden to the mental model. Ultimately, when doing sufficiently advanced things, users will have to understand the object model and what everything does, and we don't have a sufficiently articulated pretty subset where you don't have to understand anything. Anyway, I'll put this topic as part of the presentation for the committee in November so we can get more views than just mine here.
You could solve it as:
class Base {
static prop = 1
static #prop = 1
static get() {
return this.prop + this.#prop
}
}
class Sub extends Base {
static get() {
return super.get.call(Base)
}
}
console.assert(Base.get() === Sub.get())
However, if overwriting the static
prop in Sub
, it won't take it into account. For instance:
class Base {
static prop = 1
static #prop = 1
static get() {
return this.prop + this.#prop
}
}
class Sub extends Base {
static prop = 100 // ignored!
static get() {
return super.get.call(Base)
}
}
No perfect solution, but I guess it's kind of the point of private fields
Per the discussion today, I've been thinking about two possible solutions to the original problem:
Option A is a mechanism that causes the runtime behavior of static private fields to behave like the currently proposed behavior for static public fields:
Given:
class Base {
static #x = 0;
static inc() { return ++this.#x; }
}
class Sub extends Base {}
Lets say we change the spec in the following ways:
33. If superclass is not undefined, then a. Let superclassPrivates be superclass.[[PrivateFieldValues]]. b. For each entry from superclassPrivates, i. Perform ? PrivateFieldAdd(entry.[[PrivateName]], F, entry.[[PrivateField]]).
5. Append { [[PrivateName]]: P, [[PrivateField]]: { [[PrivateValue]]: value } } to O.[[PrivateFieldValues]]
5. Let value be entry.[[PrivateField]].[[PrivateValue]]. 6. While value has a [[PrivateValue]] internal slot, a. Set value to be value.[[PrivateValue]]. 7. Return value.
5. Set entry.[[PrivateField]].[[PrivateValue]] to value.
With these changes, each subclass gets an indirect reference to the private value on the superclass. Now when we call Sub.inc()
and access this.#x
with Sub
as the receiver, we'll read the value 0
from Base
, but when we write the value we replace the indirect reference with a direct value. Further subclasses of Sub
would now have an indirect reference to the value on Sub
.
This approach results in the same observable runtime behavior as public static fields as currently proposed and is only really viable if we decide to move forward with the current behavior rather than re-initializing statics:
Base.inc(); // 1
Sub.inc(); // 2
Sub.inc(); // 3
Base.inc(); // 2
Base.inc(); // 3
However, it has been discussed that the behavior of public static fields may not be desirable, which brings us to Option B.
Alternatively, If instead we decide to re-initialize static public fields we could use a different approach for private static fields and methods.
Option B would be to store a [[PrivateFieldDefinitions]] internal slot on a class constructor that contains Records with [[PrivateName]] and [[PrivateFieldInitializer]] internal slots. Then we modify ClassDefinitionEvaluation to copy the [[PrivateFieldDefinitions]] of superclass to F and then run all of the [[PrivateFieldDefinitions]] of F to initialize the private fields.
To restate what I stated today in committee: it must be possible (and the common, default behavior) that any code whatsoever outside the class
declaration, including superclasses or subclasses, can not observe anything about private fields; including their names, their values, their absence, or their mere existence.
I'm not 100% sure I understand https://github.com/tc39/proposal-class-fields/issues/43#issuecomment-348045445, but anything that involves the prototype chain with privates is imo a nonstarter.
What is the output of:
I want it to equal
'hello'
, but I can't find where the behavior is defined. It's likely just because I'm not familiar enough with the spec text.Maybe it's an error, though? Since I don't think
Sub
has#field
in its[[PrivateFieldValues]]
list, unless there's some super class iteration I'm not seeing.