fix(transformer): clean up erased namespace bindings#24302
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a87e3bc13c
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
This PR keeps Scoping consistent with the transformed AST when the TypeScript namespace transform fully erases namespace declarations (empty/type-only/ambient), preventing bindings and references from diverging when semantics are rebuilt from the output program.
Changes:
- Track namespace bindings that are erased during the namespace transform, deduplicate by
SymbolId, and finalize scoping after traversal. - Remove fully-erased bindings from their scope and convert any previously-resolved references to those symbols into root unresolved references.
- Update conformance/coverage snapshots to reflect the improved semantic consistency (fewer binding/reference mismatches).
Reviewed changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crates/oxc_transformer/src/typescript/namespace.rs | Records erased namespace bindings during traversal and applies scoping cleanup in a post-pass. |
| crates/oxc_transformer/src/typescript/mod.rs | Adds a TypeScript finalize step to apply namespace scoping cleanup. |
| crates/oxc_transformer/src/lib.rs | Calls the TypeScript finalize hook after traversal and before returning TransformerReturn. |
| crates/oxc_semantic/src/scoping.rs | Adds unresolve_symbol_references to move resolved refs for a removed symbol into root unresolved refs. |
| crates/oxc_syntax/src/reference.rs | Adds Reference::clear_symbol_id to support unresolving references. |
| tasks/transform_conformance/snapshots/oxc.snap.md | Snapshot updates reflecting fixed scoping mismatches in transform conformance. |
| tasks/transform_conformance/snapshots/babel.snap.md | Snapshot updates reflecting improved results in Babel transform conformance. |
| tasks/coverage/snapshots/semantic_typescript.snap | Snapshot updates reflecting improved semantic consistency on TypeScript coverage cases. |
| tasks/coverage/snapshots/semantic_babel.snap | Snapshot updates reflecting improved semantic consistency on Babel coverage cases. |
0fa4694 to
5b107df
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b107df7b9
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58b04f3f8b
ℹ️ 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".
58b04f3 to
72eb055
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72eb055b5f
ℹ️ 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".
72eb055 to
b76f3cf
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b76f3cf8cc
ℹ️ 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".
| self.record_removed_binding(&ident); | ||
| return; |
There was a problem hiding this comment.
Record erased nested namespace bindings
For an erased namespace that contains only nested type-only namespaces, e.g. namespace Foo { namespace Bar { export type T = number } } (or the dotted form namespace Foo.Bar { ... }), this branch records only Foo and returns before visiting the nested TSModuleDeclaration that the semantic builder already bound as Bar in Foo's module scope. The transformed AST contains neither namespace, but the returned Scoping still retains the inner Bar binding in an erased scope, so consumers that inspect symbol bindings after transform can still observe TS-only namespace bindings; please collect removed bindings from nested module declarations before returning.
Useful? React with 👍 / 👎.
Summary
Keep semantic scoping in sync when the TypeScript namespace transform erases a namespace declaration.
Empty, type-only, and ambient namespaces can be removed from the output while their original binding and resolved references remain in
Scoping. Rebuilding semantics from the transformed AST therefore produces different bindings and unresolved references.This PR:
Why finalize after traversal?
The binding cannot always be removed while visiting the namespace.
Later namespace declarations may merge with the same symbol and emit a runtime value. Removing the binding immediately would make the decision before all declarations have been transformed. It can also interfere with later TypeScript processing, such as export removal, which still relies on the original semantic information.
The namespace transform therefore records candidate bindings as declarations are erased, then finalizes scoping after traversal. Candidates are deduplicated by symbol, and only symbols without a surviving runtime declaration are removed.
This operates only on the recorded namespace symbols and their existing reference lists; it does not require another full-program AST traversal.