Usage of private member inside computed properties

[caiolima]

Last week I noticed that current spec is throwing different types of Error depending on ordering of declaration and usage of private members. While I understand the reason behind both evaluations, I'm not convinced they are very intuitive. See examples below:

a)

class C {
    // This is going to throw ReferenceError during evaluation of class
    [this.#f] = 'Test262';
    #f = 'foo';
  }

and

b)

class C {
    #f = 'foo';
    // This is going to throw TypeError during evaluation of class
    [this.#f] = 'Test262';
  }

The difference is: in a), The PrivateIdentifier #f is not initialized yet and we fail when trying to get the binding of an unitialized id. in b), the id is already initialized and we fail because there is no such private field into this. It would be simpler (to understand and to implement) if we throw the same error in both cases.

[rdking]

Isn't that construction bad from the outset? It almost appears as if you want this to represent the current instance of C. Only problem is that calculated field names must be fully resolvable at the time the class definition is parsed. So in this case, this has to be the context of the function in which C is declared, not an instance of C.

Given that, I agree. Unless that context contains a private field #f, this should always be a TypeError, just as you would get when trying to use a non-instance object as the context of a private field using member function. Having a ReferenceError scenario fosters the errant opinion that this can refer to an instance of class C.

[caiolima]

Isn't that construction bad from the outset? It almost appears as if you want this to represent the current instance of C. Only problem is that calculated field names must be fully resolvable at the time the class definition is parsed. So in this case, this has to be the context of the function in which C is declared, not an instance of C.

Yes, it is a very bad construction and I can't see any case where such construction would execute without throwing an error (maybe I'm wrong here). But since it is a valid JS program, we need to have this covered with tests =).

The major problem of current spec is that it is not intuitive to figure out why a) is ReferenceError and why b) is TypeError. It requires a detailed read into the Spec, and that's is a burden to developers. It potentially can be a source of uncatched exceptions, and this is very bad.

[rdking]

Yes, it is a very bad construction and I can't see any case where such construction would execute without throwing an error (maybe I'm wrong here).

It would work in a scenario like this:

class B {
  #f = "foo";
  C = class C {
    [this.#f] = "Test262"
  };
}

Unless I'm misunderstanding something, class C would get created in the scope of the constructor of B. So this would be an instance of B. There's no reasonable scenario I can think of that would allow for C to have an #f of its own and also use it in a calculated field name of its own declaration.

Given how private fields have been described, it's somewhat intuitive why you'd get a TypeError, but that ReferenceError makes absolutely no sense at all unless it is somehow intended that the declarations of a class can no longer be thought of as an atomic action. But that would be asking for trouble.

[joyeecheung]

FWIW, with the current design of implementation in V8 I believe a TypeError would introduce less overhead than ReferenceError. With the example in the OP:

class C {
    [this.#f] = 'Test262';
    #f = 'foo';
}

If it's a TypeError, V8 can just pre-initialize all the private names (I believe that's also what the spec change would look like) so that it only needs to check that the object does not contain that private field at runtime. If it's a ReferenceError, currently V8 needs to insert a hole check for every access to obj.#key to make sure it throws in TDZ before it moves on checking whether that the object does not contain the private field - this means that every obj.#key access (whether in the computed property key or in method bodies) would be slower.

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=9611

This overhead does not seem to only apply to V8, @caitp and @caiolima would know more about what this entails in JSC. But I think it's reasonable to assume that differentiating the error types based on the order of the access and declaration comes with a cost in implementations.

[caitp]

I’ve said before that it would be a bit easier/faster to emit a single error for “property does not exist” if the private name isn’t present on an object. Otherwise, we need to pre-define all fields on an object in the TDZ, and emit distinct errors if the field is accessed in the TDZ or not defined at all. But I do acknowledge the user gets a slightly better experience (error message) with the current design

This might look like

new class {
  [this.#f] = ‘test262’; // TypeError: invalid private field #f
  #f = ‘foo’;
}

as opposed to a ReferenceError indicating #f is still TDZ’d

[ljharb]

I’m confused why this is a problem; referencing something that doesn’t exist should be a reference error and using a field on the wrong type of object should be a type error.

[caitp]

it's not a problem, it's just more complicated (and a little more expensive) than doing it differently, for arguably little gain (how little is debatable, as some value good error messages higher than others).

[ljharb]

I think consistency with the rest of the language is pretty valuable.

[caiolima]

I’m confused why this is a problem; referencing something that doesn’t exist should be a reference error and using a field on the wrong type of object should be a type error.

In my specific case, this was very confusing because I had the mental model where classes declarations are not affected by ordering. However, I don't think it can be used as a strong argument, since my mental model is just wrong.

The problem is in the fact that it takes quite some effort to understand why it is a) throws ReferenceError and b) throws TypeError. Digesting ECMA262 spec to understand a specific behaviour doesn't sound very productive IMHO.

[caitp]

I think consistency with the rest of the language is pretty valuable.

I don't think this is really consistency with the rest of the language, since private fields aren't really comparable to lexical variables.

Ordinarily, if you access a property which hasn't yet been defined, but eventually will be, you get "undefined". Where do we ever throw a ReferenceError in this case, except when the base object is TDZ'd?

[ljharb]

This is akin to const and let, where accessing it prior to its lexical declaration throws a reference error.

Ordering of class fields absolutely matters, and was a major debate point during the advancement of the proposal - consider a = this.b and b = {}. If the order is as i indicated, you’d not get this.a equaling this.b; if you flipped it, you would.

[caitp]

This logic doesn't apply to non-private fields, so I don't really agree

new class {
  [this.f] = ‘test262’; // this.undefined = 'test262'
  f = this.undefined; // this.f = 'test262'
}

If we were going to make the argument that class fields behave like lexical variables, we'd have some consistency across private and non-private there, IMHO.

Really, I think private fields are their own thing, distinct from lexical variables and public fields, and we have a choice of whether to throw a ReferenceError and make implementations a bit more complicated, or go for simplicity (with a minor usability cost)

[caiolima]

This is akin to const and let, where accessing it prior to its lexical declaration throws a reference error.

Ordering of class fields absolutely matters, and was a major debate point during the advancement of the proposal - consider a = this.b and b = {}. If the order is as i indicated, you’d not get this.a equaling this.b; if you flipped it, you would.

The problem here is related with when we initialize the binding of a PrivateName. I don't understand how it would affect the ordering of fields.

[ljharb]

@caitp totally fair point; i agree that private fields is something new so that perhaps “consistency” doesn’t tie our hands.

@caiolima const and let can’t be used until the binding is initialized; my intuition is that private fields should work the same, or else they’d be behaving more like var.

[caiolima]

@ljharb I get it now. Thanks for clarifying.

[jridgewell]

Consider:

new (class Outer {
  f = 'outer';

  constructor() {
    return new (class Inner {
      [this.f] = 'Test262';
      f = 'inner';
    });
  }
});
// { outer: 'Test262', f: 'inner' }

In this example, this.f in the computed property key evaluates using the Outer's this. Given that, I think using a private field inside a computed key should be evaluating using the Outer's private environment:

new (class Outer {
  #f = 'outer';

  constructor() {
    return new (class Inner {
      [this.#f] = 'Test262';
      #f = 'inner';
    });
  }
});
// { outer: 'Test262', #f: 'inner' }

// Equivalent
new (class Outer {
  #f = 'outer';

  constructor() {
    const key = this.#f;
    return new (class Inner {
      [key] = 'Test262';
      #f = 'inner';
    });
  }
});

With that change, both of OP's examples should be throwing ReferenceErrors because there is no #f in the outer scope to look up.

This conveniently matches Babel's implementation.

[joyeecheung]

I’m confused why this is a problem; referencing something that doesn’t exist should be a reference error and using a field on the wrong type of object should be a type error.

I have been wondering whether it is actually possible to create a valid reference to a private field in the computed property key in the same class. e.g.

class C {
    [this.#f] = 'Test262';
    #f = 'foo';
}

Since you can't reference C in the property keys, you can't create an object that actually contains any of those private names in the same class anyway no matter where they are defined - at least I can't think of a way to do that. Unlike const and let where you might correct your code with a different ordering, even if you can create a valid reference to the private names, you'll still end up with a TypeError in the end. If that's just impossible and always going to error at runtime, how much different would that be for users who do not read the spec and understand the ordering of these abstract operations?

[ljharb]

The ordering here is observable and doesn’t require knowing the spec. I think it’s very instructive for users to see a ReferenceError and get a hint that something different is happening - ie, it can help them form their mental model about what “this” means in a computed property key.

[caiolima]

I don't agree that to understand the difference between those errors, the user don't need to know the spec. At least to me, it was not very intuitive and I couldn't find any place where we state something like the biding of private names happen in the order that they are declared.
Anyway, I think @jridgewell's solution is even better, and if we decide to move it further, this issue wouldn't matter anymore.

[ljharb]

The way i think about it is, the binding of everything happens in the order it’s declared, and everything nonlegacy is in tdz until it’s lexically initialized.

[caitp]

Anyway, I think @jridgewell's solution is even better, and if we decide to move it further, this issue wouldn't matter anymore.

I'm not sure it's a "solution" so much as a just "the actual way this is spec'd" --- since computed property names are evaluated during class evaluation, not during instantiation. I think we've been using them just to illustrate the ordering and it isn't really central to the point that's been discussed in this thread (but, maybe I'm missing something?)

[caiolima]

The point I’m making is that if we change current PrivateName resolution and use outer scope instead of current class scope, there is no ReferenceError or TypeError when using private members inside computed property on a) and b). In the case of the absence of the PrivateName into the outer scope, it would then be a SyntaxError, since we are using a private name that is not defined. That’s why I think it is an alternative to solve this issue, since it wouldn’t exist anymore.

[jridgewell]

I'm not sure it's a "solution" so much as a just "the actual way this is spec'd" --- since computed property names are evaluated during class evaluation, not during instantiation.

It sidesteps the issue observable by using computed property keys by making it always fail the same regardless of the fields' order.

---

I think we've been using them just to illustrate the ordering and it isn't really central to the point that's been discussed in this thread (but, maybe I'm missing something?)

I think what you're discussing here can be demonstrated with initializer ordering:

class Ex {
  #foo = 1;
  #bar = this.#foo;
}

// Vs
class Ex {
  #bar = this.#foo;
  #foo = 1;
}

Which is then directly translatable to block scoping:

{
    const foo = 1;
    const bar = foo;
}

// Vs
{
    const bar = foo;
    const foo = 1;
}

Whether this translates to a TypeError (current spec) or a ReferenceError is up for debate. But, it's throwing a TypeError which is the easier one to do?

[rdking]

I think you just missed one of the points @caitp is trying to make:

In:

class Ex {
  #foo = 'bar';
  [this.#foo] = 1;
}

this.#foo is evaluated at the time the class expression is evaluated. Therefore #foo must both have been previously defined or it's already an error. It doesn't matter whether or not Ex has a #foo declaration because it is impossible for this to be an instance of Ex. So order is not even an issue in this case. But if it's not an issue, then why are there 2 different errors?

On the other hand, with

class Ex {
  #foo = 1;
  #bar = this.#foo;
}

this.#foo is evaluated during instance construction, causing this to actually be an instance of Ex. So now order matters. Introducing this version that doesn't use a private field as the value of a calculated name fails to illustrate the issue the OP points out.

[ljharb]

(only talking about your first example) In the case where this.#foo appears after #foo is declared, then obviously the issue is that this can't have a #foo field. When the order is reversed, then the issue is that #foo doesn't exist. I understand that both result in an error, but I find it important why they result in different errors, and I think that differentiation is a potential important part of folks' mental models.

[syg]

First, I'm not a fan of referring to the outer private name scope (#263 (comment)) for roughly the same reasons as @ljharb stated. It is true, as @caitp says, that private fields are not lexical bindings. However, that #-names are statically lexically scoped was very explicit in their design. Having lexical things available in their scope before the declaration itself is executed is such an ingrained part of the language, I feel like breaking that for private names is inconsistent. This isn't compelling reason to disallow users from writing mutual letrec functions with private methods or fields.

To me there is the separate question of whether it is reasonable to expect the runtime semantics (to wit, TDZ) of "lexical things" in the language to extend to #-names. I've always kinda thought TDZ is a mistake, and separate errors for the private fields seems like not a language consistency I care too much about. So I'm weakly fine with unifying errors.

[rdking]

@ljharb (only talking about your first example) In the case where this.#foo appears after #foo is declared, then obviously the issue is that this can't have a #foo field.

There's only one problem I have with your assessment: the fact that you even consider Ex->#foo as a valid candidate for the #foo in this.#foo when it is used for a calculated name. If that were the case, then the code below would also always fail.

class Outer {
   #foo = "outerField";
   Inner = class Inner {
      #foo = "innerField";
      [this.#foo] = "bar"; //TypeError
   }
}

The reason is that if Inner->#foo is a valid candidate for use in the calculated name [this.#foo], then the true field name of Outer->#foo would always be shadowed. So even though the above is syntactically valid, your analysis makes the above impossible.

It get's even more curious though. According to everything that's been pushed about private fields, Inner->#foo should not be available outside any method of Inner. If that's true, then your evaluation makes even less sense considering that the resolution of [this.#foo] must be performed during class evaluation, which is most certainly outside of any method of the class being evaluated.

Long story short, there is no circumstance for which the #foo in [this.#foo] in the given examples could refer to the same #foo private name that exists in the class being evaluated. As such, this is always either a ReferenceError or a TypeError (up to you guys to choose), and there is no 2nd condition to produce the other.

[littledan]

Thanks for filing this issue. I like to explain private fields and methods as, "the name can be accessed only inside the curly braces for the class body". This means, logically, that we should hoist the definition of the private name to the beginning of when the class scope is created, rather than when the method or field declaration is reached. It was simply a drafting error of mine to not do this hoisting earlier.

Think about what the temporal dead zone is for: TDZ gives us a useful error when a binding does not yet logically have a value. But private names always have a logical value when they are in scope, since we are lexically inside the class.

Methods are logically added to the class "all at once, at the beginning", rather than one by one. So it's pretty surprising if the private names are defined one by one, based on the linear execution of the class. Even for private instance fields, it's the same name for all instances, so it also makes sense for the name to be defined "at the beginning".

The ease of implementation is a nice side-benefit, but it's not the motivation for this fix.

[rdking]

@littledan Just for clarification:
Are you saying that the intent is to make the private names immediately available upon entry of the declarative lexical environment of the class so that in OP use case a), there is a TDZ error, and use case b) simply works?

If so, this still cannot work. While the private name will indeed be available to the class, there is still no this such that this instanceof C === true. As such, the pr above only serves to complicate the mental model with no benefit to the developer.

[littledan]

@rdking You're right that there will be no such instance. So we will get a TypeError. I just don't think we have a good reason to use a TDZ here. Note that, in your above code sample, the inner #foo will shadow the outer defined #foo, making it not a very interesting test case.

[rdking]

@littledan Not only is it not interesting, it's not useful. That's my point. If Inner->#foo didn't exist, then it would be an interesting use case. The entire Inner class would be defined in the context of the Outer class constructor, and therefore have a this such that this instanceof Outer === true. That would allow inner to use this.#foo successfully.

But that just goes back to what I was saying about there being no possible way for a private field in any class to have an accessible value during that same class's definition evaluation. The private name may be defined, but no value would exist anywhere at that time. This is where we get back to the OP. Since there is no circumstance (not even with the new PR) under which it would be possible to retrieve the value of a non-existent private field for use in the calculated name of a sibling field, there's only 1 possible error instead of 2. Considering the validity of a lexically available private name in this scenario is pointless. As such, the only place where it make sense to consider the validity of any private name is within the scope of a lexically declared method of the class.

So the PR did not change anything useful... unless I missed something.

[littledan]

I don't think we will change the scope to each of the methods and initializers, rather than the whole class body. This would be much more complicated. I agree with you that it's not useful in a computed property name, but we still need to define how the error gets there, and which error it is.

[rdking]

I don't think we will change the scope to each of the methods and initializers, rather than the whole class body.

I'm not asking that you do this either. That would limit what future proposal addon's to this are capable of. I'm merely speaking to the effective limit of the proposal as it currently exists. Even the PR doesn't change that limit.

I agree with you that it's not useful in a computed property name, but we still need to define how the error gets there, and which error it is.

How the error gets there is pretty straight forward. I think I've explained it twice already. All that's really needed is for you to decide on what kind of error it is. From where I stand, it looks like a TypeError. For all intents, this in the OP scenario can only be either global/window, the context object of the current function, or undefined.

Given cases a) and b) from the OP, the PR eliminates case a), but still causes a TypeError since the private name of the class being constructed is available, but cannot be found on this when this is an object. When this is not an object, it's just the standard TypeError: Cannot read property '#f' of undefined at .... So it seems there's never going to be cause for a ReferenceError.

Side Note: Unless I'm mistaken, the PR will suddenly become very handy if some means of keyword-based class self-referencing becomes available in the future.

[littledan]

Sounds like you are in favor of this throwing a TypeError. Me too. You can find an explanation of why the current spec actually throws a ReferenceError upthread. So it seems like we are all in agreement that we should fix this issue.

[syg]

Recap of discussion with @jridgewell for some illustrative examples of TDZ in class scopes

{ let C = 1; C = class C { [C](){} } }

This is a TDZ error because [C] inside class C refers to the const binding generated by the class expression.

{ let C = 1; C = class C extends C {} }

This is a TDZ error because the heritage expression is oddly enclosed, lexical scope wise, under the class C scope itself, so the C in heritage position refers to the uninitialized const binding generated by the class expression.

[rdking]

@syg Close, but not a fit. Remember that everything on the right side of the = in a field declaration is actually in a function of its own. That's the only scope where any newly declared, non-static fields exist. The left side of the = in the same field declaration does not have access to any instance of the class being declared, and therefore doesn't have access to any such definitions. In the example you gave, [C] ends up in a TDZ because C was in the process of being defined in the parent lexical scope of the class's body.

So even while it seem's like a similar case, the semantics involved are very different.

[ljharb]

@rdking that’s a block, not a class. There’s no field in that example.

[rdking]

@ljharb I'm fully aware of that. My point was that the semantics of the example @syg gave are significantly different for the semantics of the OP. So the TDZ that occurs in his example is perfectly valid, but a different case entirely from the main discussion of this thread.

[syg]

Remember that everything on the right side of the = in a field declaration is actually in a function of its own.

There aren't fields in that example. Sorry those aren't really examples of this particular discussion with fields, but instead illustrative TDZ examples that @jridgewell requested be documented in the thread.

In the example you gave, [C] ends up in a TDZ because C was in the process of being defined in the parent lexical scope of the class's body.

@rdking That's not what's going on. The C in [C] that throws the TDZ is in the class scope itself. It's the const binding as declared in step 4.a here, which is initially in TDZ and gets initialized at the end.

[rdking]

Ok. So I learned something new. The name of the class is a member of the declarative lexical scope of the class being declared. Seems like an odd choice, but oh well. In either case, the result is the same, the definition isn't yet complete so C is uninitialized at the time when [C] is encountered.