Should class declarations contain a binding for class name in scope for the class?

[overlookmotel]

Should class declarations contain a binding for class name in the scope for the class?

Klass in example below has 2 bindings - 1st at top level, 2nd within the class body.

class Klass {
    static tryWrite() {
        Klass = 123;
    }
}

const CopyOfKlass = Klass;

// Reassignment of `Klass` in top level scope succeeds
Klass = 123;
console.log(Klass); // Logs 123

// Reassignment of `Klass` within class fails.
// `TypeError: Assignment to constant variable at Klass.tryWrite`
CopyOfKlass.tryWrite();

The outer binding is reassignable, whereas the binding within the class is read-only.

[Dunqing]

In the spec. The class doesn't have a scope.

In the TypeScript container. Will create a container for class. But doesn't add a binding for class name to the container

[overlookmotel]

In the spec. The class doesn't have a scope.

Is it possible for the spec to be wrong?? (or wrong in terms of what we need). I do think in this case there are 2 bindings. The above example demonstrates this.

[Dunqing]

Is it possible for the spec to be wrong?? (or wrong in terms of what we need).

Unlikely. But we are aligning with the TypeScript container now.

I do think in this case there are 2 bindings. The above example demonstrates this.

IMO, we don't need to add a binding to the class scope. The error in the above example is a runtime error that we don't need to care about

[rzvxa]

I think the class identifier much like the this keyword means something special in the context of class and that's why you can't reassign it, Similar to how you can't redefine the same class twice.

class Klass {}
class Klass {} // error can not redefine!

[overlookmotel]

Maybe the spec has some "special" way of thinking about this. But for our purposes, you have 2 variables with the same name which are independent of each other (can contain different values), and one shadows the other - that sounds a lot like scopes to me!

[overlookmotel]

The error in the above example is a runtime error that we don't need to care about

I think we will need to care about it in future. e.g. a transform which converts a class into a function, or moves class methods outside of the class body (maybe private methods transform will). And minifier may be able to perform optimizations if it understands that one binding is read-only.

[rzvxa]

But it will cause the scopes to be wrong in some cases.

class Klass {
  y() {
    class Foo {
      y() {
        console.log(Klass, Foo);
      }
    }
    return new Foo();
  }
}

new Klass().y().y();

What should Klass and Foo point to in the console.log line?

[overlookmotel]

What should Klass and Foo point to in the console.log line?

Equivalent to:

let KlassOuter = class KlassInner {
  y() {
    let FooOuter = class FooInner {
      y() {
        console.log(KlassInner, FooInner);
      }
    };
    return new FooOuter();
  }
};

new KlassOuter().y().y();

I did originally open this issue to pose a question, but now I've become convinced of the answer!

[rzvxa]

I'm not 100% sure if it is right or wrong but please investigate it more before doing anything. I feel TS people had a good reason to not create a scope here while they could do it easily. What are our other options?

let KlassOuter = class KlassInner {
  y() {
    let FooOuter = class FooInner {
      y() {
        console.log(KlassInner, FooInner);
        console.log(KlassInner, FooInner);
        console.log(KlassInner, FooInner);
        console.log(KlassInner, FooInner);
      }
    };
    return new FooOuter();
  }
};

new KlassOuter().y().y();

In the example above how many times KlassOuter should be read? With this, we have to propagate shadowed class identifiers throughout our scope analysis.

[overlookmotel]

I feel TS people had a good reason to not create a scope here while they could do it easily.

It could be they take a different approach, because they're trying to do something different. Their emphasis is on type checking, and the types of KlassOuter and KlassInner are the same, so no point differentiating them. But we are doing something else - e.g. minification, where whether bindings are read-only or not matters.

In the example above how many times KlassOuter should be read?

Once I guess in new KlassOuter().

With this, we have to propagate shadowed class identifiers throughout our scope analysis.

How do you mean?

NB: Babel's transform of class private methods seems to support my view: Babel REPL - _X in output represents the inner binding.

[rzvxa]

I was AFK for a few hours. But I'd like to investigate it when I find the time. This Babel REPL suggests that it only uses alias variables for lowering private members to prevent the Class identifier being shadowed by others.

[rzvxa]

Here is the same example without private members which doesn't create a _X anymore (while it still being used in public methods)

This seems to be their renaming in action not how they differentiate between the bindings. This example seems to be the primary reason why they are using it.

[overlookmotel]

Yes, it only creates the _X alias when the method is converted to a function outside of the class. That makes sense - when the method remains inside the class, it doesn't need to create an alias, because X inside the class refers to the inner binding already.

Babel's class-to-function transform similarly produces 2 independent bindings.

My point is:

1. The private method example demonstrates that there are 2 independent bindings.
2. We are going to have to implement the class private methods transform, so we need this represented in our scope tree.

[rzvxa]

I'm thinking about it the other way around. Instead of creating a dummy scope in the semantics which potentially can cause issues in other use cases like linters, We may want to identify the private members and reassign them to a newly created symbol as we are doing the transformation.

I'm not saying having 2 bindings is wrong as the matter of fact it might be the right thing to do, but It may cause unnecessary issues in other parts so that's why I'm saying we should be more careful about the implications of doing it this way.

Even If it works but isn't up to spec it can cause difficulties for people when they are trying to use the semantics. Sadly I don't have the time to investigate it at the moment. So here are my 2 cents, IMHO it is one of those things that might pay off if we go with the spec. You guys have a much clearer picture of it. I'll let you decide on this one. I believe you'd go with the best approach⚓.

Personal wish list: It would be awesome if we go with something close to typescript in case we someday find the funding for a ts checker written in Rust. There is clearly a need for it and with STC(dudykr/stc not the collections) going silent there is a void to fill. Having the infrastructure for handling types correctly can open up opportunities for other tools to use our parser and binder in the future.

[overlookmotel]

Oh lord. I really didn't think this would be so controversial!

OK, I'm not great at reading the spec, but I've done my best. Here goes...

15.7.15

ClassDeclaration : class BindingIdentifier ClassTail

1. Let className be the StringValue of BindingIdentifier.
2. Let value be ? ClassDefinitionEvaluation of ClassTail with arguments className and className.

15.7.14

The syntax-directed operation ClassDefinitionEvaluation takes arguments classBinding (a String or undefined) and className ...

3. If classBinding is not undefined, then a. Perform ! classEnv.CreateImmutableBinding(classBinding, true).

9.1.1

CreateImmutableBinding(N, S) Create a new but uninitialized immutable binding in an Environment Record. The String value N is the text of the bound name. If S is true then attempts to set it after it has been initialized will always throw an exception, regardless of the strict mode setting of operations that reference that binding.

Translating this into pseudo-code:

fn BindingClassDeclarationEvaluation(className: String) {
    ClassDefinitionEvaluation(className, className);
}

fn ClassDefinitionEvaluation(classBinding: String | undefined, className: String) {
    if classBinding != undefined {
        classEnv.CreateImmutableBinding(classBinding, true);
    }
}

fn CreateImmutableBinding(self: Env, N: String, S: bool) {
    // Create binding with name `N` in `self` (environment).
    // If `S` is true, assigning to the binding will throw.
}

So, following the logic, a class definition generates a binding in classEnv. And that binding is immutable and throws if you assign to it.

This cannot be referring to the outer binding because the outer binding is not immutable, and does not throw when you assign to it.

Like I said, I find it pretty hard reading the spec, so maybe I am interpreting it wrongly, but it does seem to me to suggest there is an inner binding for the class name in class declarations. If I have it wrong, please correct me.

[rzvxa]

Wow! Thanks for summarizing the spec so greatly. Talking about it in terms of spec gives me more confidence about this change. Sorry, it took me a few hours to respond to your comment, Had to read the section 15.7 to be sure.

---

I'm not sure about this but might be worth investigating later on. The only caveat that I could find with the limited time I've spent on it is a class heritage should only have access to the LexicalEnvironment (classEnv) and its outer PrivateEnvironment which is a bit odd, I can't exactly put my finger on why this happens here my first guess was to prevent stuff like this but I might very well be wrong.

class Klass extends ((this.x = "NO")) { x = "YES"; } 
// or maybe this?
class Klass extends ((x = String)) { y = "YES"; } 

[overlookmotel]

The example you give is interesting.

function outer() {
    class Klass extends ((this.x = "NO"), class {}) {}
}

const obj = {};
outer.call(obj);
console.log(obj.x); // Logs "NO"

i.e. this in the extends clause refers to this in the enclosing environment.

But...

class Klass {}
{
    class Klass extends Klass {}
}

throws an error ReferenceError: Cannot access 'Klass' before initialization.

Conclusion: Class name is within scope of the extends clause of that class (but in TDZ). But the this binding is bound within a further nested scope (the class body).

Sorry, it took me a few hours to respond to your comment

Please don't apologize! We don't have to resolve every question in quick fire. Have sleep, have life!

[Dunqing]

Maybe we can learn more from boa.

[Boshen]

Do we have a consensus on this issue?