Hey @regiontog! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite."

Hi @regiontog, thanks for your report!

This issue comes from how babel registers bindings for the id of ClassDeclaration

As you can see the binding variable is referencing the returned result from path.scope.getBinding - That means if the binding is not defined in this scope, it will walk up until it finds one.

Now let's take a look at your example

const path = getPath("class a { build() { return new a(); } }");
path.scope.crawl();

Before crawling, the binding a can be accessed in the following two nodes:

Program.bindings.a
ClassDeclaration.bindings.a ==> Program.bindings.a

Keep in mind that scope.crawl() will reset Program.bindings to {}, but the old Program.bindings.a would not be GCed because it is still referenced in ClassDeclaration.bindings.a. Therefore after crawling the binding a can still be accessed in the following two nodes.

Program.bindings.a
ClassDeclaration.bindings.a ==> ClassDeclaration.bindings.a before crawl
                              (getBinding returns existent old binding info)
==> Program.bindings.a before crawl

But ClassDeclaration.bindings.a refers to the binding generated before scope.crawl(), so the referencePaths is screwed up because it is assigned to the binding before crawl.

We can walk up from path.scope.parent instead of path.scope, so that we avoid accessing obsolete binding information.

@JLHwung sidenote, fantastic explanation :)

I'll close my old issue #9327 as a duplicate.

friendly ping @regiontog, let me know if you are working on this. Should you have questions on developing a fix, please ask in the #development slack channel.

Thanks, I've been busy this week, but I was planning on taking a look tomorrow since I have the day off. I don't mind if someone else wants to take a look either.

@JLHwung You were right, the fix is exactly as you suggested. The only thing that is not entirely clear to me is when the parent scope gets the a binding. I assume previously in the traverse if it is top-to-bottom. But I don't see a Program visitor.

Nevertheless my test passes now, I'll check if the bug in parcel is resolved by the same change. I can submit a PR with the fix and the test if you guys are ok with the change.

Edit: https://github.com/regiontog/parcel2-bug builds fine with this fix applied to babel.

The parent scope will get the a binding in the Declaration visitor

Looking into this code I think it maybe more clear to merge ClassDeclaration visitor with Declaration visitor, so we define an extra binding in the class scope for the class id so that the class method can reference it.

Looking into this code I think it maybe more clear to merge ClassDeclaration visitor with Declaration visitor, so we define an extra binding in the class scope for the class id so that the class method can reference it.

My only problem with the current approach that it presumably depends on the Declaration visitor being called before the ClassDeclaration visitor. I don't know if that is by design since Declaration is less specific or not. But if that's not a contract traverse must uphold then depending on it might not be a good idea.

My only problem with the current approach that it presumably depends on the Declaration visitor being called before the ClassDeclaration visitor.

The Declaration visitor is called before ClassDeclaration because babel-traverse explode the visitors according to the Object.keys:

I admit that this is too subtle and essentially the object keys are unordered. So here we are on the same page. I propose to merge the ClassDeclaration visitor with Declaration visitor:

Declaration(path) {
  // register binding on parent scope
  // ...
  // register class identifier binding in class scope
  if (t.isClassDeclaration(path)) {
    // ...
  }
}

and then we don't have to worry about the execution order of Declaration and ClassDeclaration.