fix: Warning when export before enum - #18157
liuxingbaoyu wants to merge 2 commits into
Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/61972 |
There was a problem hiding this comment.
馃挕 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b400dc3d6
鈩癸笍 About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 馃憤.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ); | ||
|
|
||
| if (path.scope.hasOwnBinding(name)) { | ||
| if (!(path.scope.getOwnBinding(name)?.kind === "unknown")) { |
There was a problem hiding this comment.
Preserve compatibility with cores that do not bind enums
When this plugin version is loaded by an older but supported @babel/core/@babel/traverse (the plugin's peer range is ^8.0.0), enums have no scope binding, so with optimizeConstEnums an input such as export const enum E { A } takes this branch and becomes Object.assign(E, ...) without ever declaring E, throwing during module evaluation. A missing binding must follow the declaration-creation path, and the paired traverse change must likewise remain compatible with older plugin versions.
Useful? React with 馃憤聽/ 馃憥.
There was a problem hiding this comment.
I am surprised that AI has also discovered this.
| if (path.isTSEnumDeclaration() && !path.node.declare) { | ||
| const id = path.get("id"); | ||
| this.registerBinding(kind, id, path); |
There was a problem hiding this comment.
Clear bindings when inlining local const enums
With optimizeConstEnums, a non-exported const enum merged with a following namespace now leaves this newly registered binding behind when const-enum.ts removes the enum; the generic removal hook cannot clean it because getBindingIdentifiers has no TSEnumDeclaration entry. For a module containing const enum E { A } followed by namespace E { export const B = 1 }, transpileNamespace consequently sees hasOwnBinding("E") and omits its let E declaration, producing an IIFE that reads and assigns undeclared E and throws at runtime. Remove the enum binding before the inlining branch removes the declaration.
Useful? React with 馃憤聽/ 馃憥.
There was a problem hiding this comment.
While adding tests, I discovered that const enum cannot be merged with a namespace, but removing it is fine.
|
commit: |
There was a problem hiding this comment.
馃挕 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1301497ee2
鈩癸笍 About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 馃憤.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (path.isTSEnumDeclaration() && !path.node.declare) { | ||
| const id = path.get("id"); | ||
| this.registerBinding(kind, id, path); |
There was a problem hiding this comment.
Register nested enums in their block scope
When an enum appears in a nested block and an enclosing function/program has a same-named binding, this new special case registers the enum in that enclosing scope because the collector routes non-BlockScoped declarations to the function/program scope. Consequently, valid shadowing such as let E; { enum E {} } throws Duplicate declaration "E" during scope crawling, even though the enum transform emits a block-local let; register enum declarations in their block parent (or classify them as block-scoped) instead.
Useful? React with 馃憤聽/ 馃憥.
What would happen? |
I recall it would cause the generation of unusable code. |
Could you provide an example and elaborate? |
This might not be mergeable, because if users update only
@babel/traversewithout updating@babel/transform-typescript, they will encounter more serious issues.