Static private member with same private name as a private instance member

[tjcrowder]

Apologies in advance if this has been covered in discussions I haven't found.

The current @babel/plugin-proposal-class-properties plugin supports both private static fields and private instance fields. It doesn't allow using the same private name string for both:

class Example {
    static #x = 1;
    #x = 2; // SyntaxError ("Duplicate private field")

    method() {
        console.log(`Example.#x = ${Example.#x}`);
        console.log(`this.#x = ${this.#x}`);
    }
}
new Example().method();

Is that an artifact of the plugin, or is it how this proposal is defined? I couldn't tell from the current spec text.

If it's an aspect of the proposal, this is a mis-match with public fields, where there's no name conflict, and with private members of other similar languages (Java, C#) where having a private static member and a private instance member with the same name is allowed. (I'm not going to say doing it is a good idea...)

It seems like it can be avoided if there's a will/desire to do so, but possibly with a runtime cost. Here's one way (using terminology and structures described in the class fields proposal's spec text):

1. Have "instance" and "static" parts in [[PrivateNameEnvironment]].
2. Allow a name to be duplicated across them (e.g., #x in one doesn't conflict with #x in the other).
3. In AllPrivateNamesValid, check both parts.
4. Have an internal slot on objects saying which part relates to them. The slot on a class constructor says it uses the static part, the slot on all other objects says it uses the instance part.
5. When resolving the name string to a private name object (for instance, in GetValue's step 5.b.ii, quoted below), use the slot to determine which part of the [[PrivateNameEnvironment]] to use.

For clarity, GetValue step 5.b.ii in the class fields spec is currently:

Let field be ? ResolveBinding(GetReferencedName(V), env)

...where env is the relevant PrivateNameEnvironment.

Alternately, a naming convention could be used rather than two "parts" of a [[PrivateNameEnvironment]]. This might be both simpler (since having two "parts" of a [[PrivateNameEnvironment]] makes it no longer a LexicalEnvironment object, making more work) and perhaps more general-purpose for use in the future. With a naming convention:

1. In [[PrivateNameEnvironment]], prefix the binding name of a static private member with the string "static". E.g., static#x vs. #x.
2. (unnecessary)
3. In AllPrivateNamesValid, for the name #x, check both for the instance name (#x) and the static name (static#x).
4. Have a slot on objects saying whether it uses "static" prefixed names (perhaps the slot could contain a generic prefix, defaulting to "", for future expansion). The slot on a class constructor says it uses "static", the slot on all other objects says it doesn't.
5. When resolving the name string to a private name object (for instance, in GetValue's step 5.b.ii), use the object's slot to determine whether to use the name as given (#x) or with a prefix (static#x).

WIth that sort of mechanism, the code block at the beginning of this post would be valid, and would output

Example.#x = 1
this.#x = 2

I don't know the internals of how engines are implementing private members, but I suspect they're able to optimize away the "Let field be ? ResolveBinding(GetReferencedName(V), env)" part of GetValue, which is why I mention a possible runtime cost. If I'm right in my assumption, they wouldn't be able to with this new mechanism (or at least, they could only narrow it down to two possibilities).

I'm not necessarily advocating ensuring that static #x; and #x could both exist in a class, but I thought it worth raising, and ideally when raising a possible issue I prefer to outline a possible solution if I can.

Again, though, the proposal's team may well already have covered this off.

[littledan]

For simplicity, these shared name cases are disallowed with a syntax error. Do you think that's OK?

[tjcrowder]

@littledan -

As long as it's on purpose, I don't see it as a problem, despite the discrepancies with public fields and other languages I noted above. On the rare occasions I've seen static and instance fields with the same name, I don't ever recall thinking it was a good choice.

And, if it's a syntax error now (as it is), that means if there's a strong use case for it later, it can be added later.

(BTW: Thank you -- and all the team -- for all the hard and oftentimes-unrewarding work of bringing these two proposals to fruition [almost there!]. Class fields was particularly tricky and the team have found a really good way to do it. In particular I like the two-stage resolution, Private Names which can be built on later, and not dropping this. when accessing private fields. It all hangs together well, and anyone with eyes open knows how challenging that was. Your patience in the issues list on class fields does not go unnoticed. ;-) )

[littledan]

Yes, this was on purpose. I didn't see any important use cases, and allowing name sharing would increase complexity a lot. Do you think we should document this reasoning better?

[tjcrowder]

@littledan - Maybe just a short paragraph in the proposal's readme? Something like:

Private name identifiers for static members share the PrivateNameEnvironment used by private instance members, so it's a syntax error to have a static private member (static #x) with the same private name identifier as an instance member (#x). While some languages allow this, the use cases don't justify the complexity. Since it's a syntax error, if compelling use cases are put forward, this can be the subject of a subsequent proposal.

[littledan]

I think the issue is a little more fundamental than that: a static method can be .call()ed with an instance as the receiver, so you don't know whether a reference to a shared name is talking about the static or instance class element.

I am thinking the note to put in the readme should maybe be a little bit higher level, whereas maybe a note in the specification could go into mechanics.

[littledan]

@tjcrowder Is there more that we need to do about this issue, or should we close it? PRs to the README to clarify would be welcome.

[tjcrowder]

@littledan - I take your point about it being bigger than that. I'll do a PR for the README and separately one for a note in the spec soonish (have a couple of other higher-priority ones to finish off first).