-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Preserve class id when transforming using declarations with exported class #17257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/59193 |
// If localName is not found in the bindingKindLookup generated | ||
// from top level declarations, it must be a reference to a var | ||
// declaration defined within block statement or switch case | ||
const kind = bindingKindLookup.get(localName) ?? "var"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even with export { A }; { var A }
, the programPath.scope.get("A")
properly returns the var
declaration. Could we just query the scope properly, so that we still catch invalid exports?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The invalid exports have been caught by the parser. Assuming the previous check somehow detects nested var, now if it fails, there should be a plugin injecting new exports without defining them.
If we remove bindingKindLookup
and query from the program scope, chances are plugins that injects export { A }; var A;
will fail because the plugin does not update the program scope. So we should keep bindingKindLookup
and fallback to scope query.
Node.js 6 is still unhappy :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the scope-based update!
This PR fixed two issues:
The amd/cjs/umd transform will throw when the local exports reference nested var declarations:
This pattern is not very popular in real-world codebase but the explicit-resource-management transform will generate such pattern.
Preserve class id for inner class binding references, as suggested by @overlookmotel in
plugin-proposal-explicit-resource-management
incorrect transform for exported class #17172. Fixing this issue relies on the first fix.