Bug/missing references after crawl

[regiontog]

Q	A
Fixed Issues?	Fixes #10896
Patch: Bug Fix?	Yes
Major: Breaking Change?	Uncertain
Tests Added + Pass?	Yes
Documentation PR Link
Any Dependency Changes?	No
License	MIT

When traversing during a crawl() babel currently sets the class identifier inside the class to the old binding. This has caused a number of issues in parcel(parcel-bundler/parcel#3932, parcel-bundler/parcel#2513, parcel-bundler/parcel#2981, parcel-bundler/parcel#3664). This PR sets the binding to the binding in the parent. See #10896 for in depth explanation.

In addition the assumption that a Declaration visitor is visited before ClassDeclaration in crawl() was removed by merging the visitors.

[JLHwung]

Thanks!

Added to v7.8.4 milestone because it is blocking downstream tools.

[jridgewell]

Isn't the following change all that's necessary?

-    path.scope.bindings[name] = path.scope.getBinding(name);
+    path.scope.bindings[name] = path.scope.parent.getBinding(name);

[nicolo-ribaudo]

I agree. parent is the nearest function scope, but classes are block-scoped:

let A;
function fn() {
  {
    class A {}
  }
  A; // This references the let declaration, not the path.
}

[JLHwung]

This change

-    path.scope.bindings[name] = path.scope.getBinding(name);
+    path.scope.bindings[name] = path.scope.parent.getBinding(name);

implicitly depends on the fact that the Declaration visitor runs before ClassDeclaration visitor, so that path.scope.parent.getBinding always points to the new binding which may be initialized in Declaration visitor or BlockScoped visitor.

See #10896 (comment)

Besides that, I agree we should do path.scope.parent instead of parent.

[regiontog]

implicitly depends on the fact that the Declaration visitor runs before ClassDeclaration visitor, so that path.scope.parent.getBinding always points to the new binding which may be initialized in Declaration visitor or BlockScoped visitor.

If the class binding may be(always is?) initialized in BlockScoped won't this stop the identifier from being visible inside the class scope if the visitors are merged?

babel/packages/babel-traverse/src/scope/index.js

Lines 62 to 64 in 63a12f2

	Declaration(path) {
	// delegate block scope handling to the `BlockScoped` method
	if (path.isBlockScoped()) return;

[JLHwung]

@regiontog Ah you are right! The ClassDeclaration binding is registered in BlockScoped and BlockScoped runs before ClassDeclaration. I am curious why the test can even get passed.

[nicolo-ribaudo]

Thanks!

[JLHwung]

Thanks!