improvement: add reference from definition of inner class variable to outer one

[sokra]

The first 3 commits bring this repo up-to-date:

• require-dir is broken with node 8 -> update gulp-git
• node LTS minimium is 4 -> adjust travis
• ({a}) = b is incorrect syntax -> ({a} = b) instead

The last commit is the important one.

Before a ClassDeclaration creates two variables, one in the outer scope and one in the inner scope. This results in weird behavior when the class is referenced from inside the class. i. e. when renaming a variable.

After the change only a single variable is created and all references are attached to this one.

Note: that a comment on one test assumes it's the correct behavior to create to two variables. I'm not this opinion. FunctionDeclarations are similar and also create only a single variable.

fixes webpack/webpack#5000

[michaelficarra]

@sokra Classes actually do create two bindings: one in the surrounding scope and one in the inner scope. This is because the outer binding is mutable, but the inner binding is not. So a reference to a class name from one of its methods will always refer to that class, while a reference to a class name from outside of the class may be changed by anything else that closes over it.

class C { constructor() { console.log(C === alwaysC); } }
const alwaysC = C;
new alwaysC; // true
C = null;
new alwaysC; // still true
C === alwaysC; // false

But I'd be happy to accept your other commits!

[bakkot]

@sokra As @michaelficarra says, there are indeed two variables. You can compare against shift-scope, for example, by pasting his snippet here, or read the spec: the outer binding is created as step 7 of BindingClassDeclarationEvaluation, and the inner as step 4a of ClassDefinitionEvaluation.

[michaelficarra]

@bakkot That demo actually supports shareable links.

[sokra]

The same is true for FunctionDeclarations and here escope also creates a single variable.

function Foo() {
  Foo // 1
}
Foo // 2

ModuleScope
  variables
    Foo
      references
        Foo // 1
        Foo // 2
FunctionScope
  variables
    arguments

---

ClassDeclaration currently

class Foo {
  constructor() {
    Foo // 1
  }
}
Foo // 2

ModuleScope
  variables
    Foo
      references
        Foo // 2
ClassScope
  variables
    Foo
      references
        Foo // 1
FunctionScope
  variables
    arguments

---

I wanted to align the behavior. If the intended behavior is that two variables are created, this should be changed for FunctionDeclarations too.

In this case I would propose to add a cross referencing property to the variables to be able to find the inner scope variable from the outer scope variable. This is important for the renaming operation.

[sokra]

or am I wrong about the FunctionDeclaration? ... need to check the spec here.

[sokra]

ok I was wrong here. FunctionDeclaration behaves different.

[sokra]

I will update to PR to keep the behavior and include a cross-reference instead.

[michaelficarra]

@sokra Can you rebase?

[brettz9]

As it looks like master was merged in, is this ready for merging now?

[michaelficarra]

I'm not really convinced that this is useful. The two bindings aren't related, other than that they're declared by the same node. And if you want to look for any variables declared by the same node, you already have that ability. A similar quirk occurs with references in catch blocks. What's the problem this is trying to solve?