Skip to content

fix(transformer): clean up semantics for stripped TypeScript syntax#24180

Merged
graphite-app[bot] merged 1 commit into
mainfrom
codex/transformer-ts-stripped-references
Jul 6, 2026
Merged

fix(transformer): clean up semantics for stripped TypeScript syntax#24180
graphite-app[bot] merged 1 commit into
mainfrom
codex/transformer-ts-stripped-references

Conversation

@camc314

@camc314 camc314 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Keep transformer semantic data in sync when TypeScript-only syntax is removed.

This change:

  • removes resolved and unresolved references belonging only to stripped TypeScript syntax
  • removes bindings for erased type-only and unused imports
  • preserves runtime bindings when a type import is merged with a value declaration
  • promotes the surviving declaration’s symbol flags, span, and redeclaration metadata
  • adds coverage for type imports merged with runtime declarations
  • refreshes semantic and transformer conformance snapshots

Motivation

We are preparing to change the AST shape, including adding and rearranging scopes.

Currently, transformer semantic snapshots contain thousands of lines of mismatches caused by stale bindings and references from TypeScript syntax that has already been removed from the AST. That creates a large amount of unrelated snapshot noise.

With that noise present, snapshot changes caused by the upcoming AST work would be difficult to review: a real scope or semantic regression could easily be hidden among mechanical churn.

This PR fixes the underlying semantic inconsistencies first, giving us a much cleaner baseline for reviewing future AST and scope changes. It is not intended to change emitted JavaScript.

@camc314

This comment was marked as resolved.

@github-actions github-actions Bot added A-semantic Area - Semantic A-transformer Area - Transformer / Transpiler labels Jul 5, 2026
chatgpt-codex-connector[bot]

This comment was marked as resolved.

@camc314
camc314 force-pushed the codex/transformer-ts-stripped-references branch from d443afc to 189bc84 Compare July 5, 2026 14:12
@codspeed-hq

codspeed-hq Bot commented Jul 5, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 4.81%

❌ 1 regressed benchmark
✅ 56 untouched benchmarks
⏩ 14 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation transformer[binder.ts] 1.7 ms 1.8 ms -4.81%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing codex/transformer-ts-stripped-references (5df8cc9) with main (bf1a151)2

Open in CodSpeed

Footnotes

  1. 14 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on main (dad4287) during the generation of this report, so bf1a151 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@camc314
camc314 force-pushed the codex/transformer-ts-stripped-references branch 4 times, most recently from f801b6c to 29a6c00 Compare July 5, 2026 14:39
@camc314

camc314 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

This should be ready to go. There will be some followups needed, but those are out of scope for now.

@Dunqing do you mind having a look? I'd like to get this in today.

In regards to the perf regression, I think it's acceptable - we're inching towards correctness here, and it'll allow rolldown to avoid a semantic rebuild in future.

@camc314
camc314 marked this pull request as ready for review July 6, 2026 07:40
@camc314
camc314 requested a review from Dunqing as a code owner July 6, 2026 07:40
Copilot AI review requested due to automatic review settings July 6, 2026 07:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Cleans up semantic data produced/kept by the transformer when stripping TypeScript-only syntax, so transformer “after transform” semantics stay aligned with semantics rebuilt from the transformed AST (reducing snapshot noise ahead of upcoming AST/scope work).

Changes:

  • Remove type-space-only resolved/unresolved references and delete bindings for stripped TS-only declarations/imports.
  • Preserve runtime bindings when a type import is merged with a value declaration by removing just the import’s redeclaration and promoting surviving symbol metadata.
  • Add coverage for type-import + value redeclaration, and refresh transformer/semantic conformance snapshots.

Reviewed changes

Copilot reviewed 4 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
crates/oxc_transformer/src/typescript/annotations.rs Removes bindings when erasing TS imports/specifiers; preserves bindings for merged runtime declarations.
crates/oxc_semantic/src/scoping.rs Adds remove_symbol_declaration and expands delete_typescript_bindings to prune TS-only references as well as bindings.
tasks/coverage/misc/pass/type-import-value-redeclaration.ts New coverage case for type-import merged with runtime function declarations.
tasks/transform_conformance/snapshots/oxc.snap.md Updated transformer conformance snapshot after semantic cleanup.
tasks/transform_conformance/snapshots/babel.snap.md Updated Babel transformer conformance snapshot after semantic cleanup.
tasks/coverage/snapshots/transformer_misc.snap Snapshot refresh due to added misc coverage case.
tasks/coverage/snapshots/semantic_misc.snap Snapshot refresh reflecting reduced semantic mismatches after TS reference cleanup.
tasks/coverage/snapshots/semantic_babel.snap Snapshot refresh reflecting reduced semantic mismatches after TS reference cleanup.
tasks/coverage/snapshots/parser_misc.snap Snapshot refresh due to added misc coverage case.
tasks/coverage/snapshots/formatter_misc.snap Snapshot refresh due to added misc coverage case.
tasks/coverage/snapshots/codegen_misc.snap Snapshot refresh due to added misc coverage case.

@Dunqing Dunqing left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, it looks good, and the performance regression is expected. Just a small improvement we could make is in the follow-up.

Comment thread crates/oxc_semantic/src/scoping.rs
@camc314
camc314 force-pushed the codex/transformer-ts-stripped-references branch from 29a6c00 to e06692c Compare July 6, 2026 08:02
@Dunqing Dunqing added the 0-merge Merge with Graphite Merge Queue label Jul 6, 2026

Dunqing commented Jul 6, 2026

Copy link
Copy Markdown
Member

Merge activity

…24180)

## Summary

Keep transformer semantic data in sync when TypeScript-only syntax is removed.

This change:

- removes resolved and unresolved references belonging only to stripped TypeScript syntax
- removes bindings for erased type-only and unused imports
- preserves runtime bindings when a type import is merged with a value declaration
- promotes the surviving declaration’s symbol flags, span, and redeclaration metadata
- adds coverage for type imports merged with runtime declarations
- refreshes semantic and transformer conformance snapshots

## Motivation

We are preparing to change the AST shape, including adding and rearranging scopes.

Currently, transformer semantic snapshots contain thousands of lines of mismatches caused by stale bindings and references from TypeScript syntax that has already been removed from the AST. That creates a large amount of unrelated snapshot noise.

With that noise present, snapshot changes caused by the upcoming AST work would be difficult to review: a real scope or semantic regression could easily be hidden among mechanical churn.

This PR fixes the underlying semantic inconsistencies first, giving us a much cleaner baseline for reviewing future AST and scope changes. It is not intended to change emitted JavaScript.
@graphite-app
graphite-app Bot force-pushed the codex/transformer-ts-stripped-references branch from 5df8cc9 to e8b50ee Compare July 6, 2026 08:11

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5df8cc9eda

ℹ️ 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".

Comment thread crates/oxc_semantic/src/scoping.rs
@graphite-app
graphite-app Bot merged commit e8b50ee into main Jul 6, 2026
29 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Jul 6, 2026
@graphite-app
graphite-app Bot deleted the codex/transformer-ts-stripped-references branch July 6, 2026 08:17
Boshen added a commit that referenced this pull request Jul 6, 2026
### 🚀 Features

- 260425f semantic/examples: Include unresolved references (#24214)
(camc314)
- 2d9b0b3 minifier: Fold boolean-literal ternary branches in value
contexts (#24110) (Dunqing)
- 61fbf10 ast: Implement `ReplaceWith` on all AST types (#24013)
(overlookmotel)
- 7db7a29 allocator: Add `ReplaceWith` trait (#24012) (overlookmotel)
- 4eb074e mangler: Add `reserved` option for names that must not be
mangled (#24041) (Dunqing)
- 2e62012 data_structures: Add `StringExt` trait (#24006)
(overlookmotel)
- 60e7160 minifier: Drop side-effect-free IIFEs whose result is unused
(#23967) (Dunqing)
- 26dd9e2 ast: Add method to widen inherited enum ref to parent ref
(#23961) (overlookmotel)

### 🐛 Bug Fixes

- e8b50ee transformer: Clean up semantics for stripped TypeScript syntax
(#24180) (camc314)
- d966d0b react_compiler: Remove clippy allows (#24168) (Boshen)
- 854ef8d react_compiler: Compile generic functions instead of
over-bailing on type-param hoisting (#24158) (Boshen)
- 093586c react_compiler: Align memoization cache-slot allocation with
Babel (#24157) (Boshen)
- 09c8f59 react_compiler: Normalize snapshot fixture paths (#24142)
(camc314)
- f13df97 react_compiler: Drop stray empty statement from catch bindings
(#24133) (Boshen)
- cb2a505 react_compiler: Codegen destructuring reassignment targets
(#24131) (Boshen)
- b82c394 react_compiler: Propagate codegen invariants instead of
emitting empty bodies (#24128) (Boshen)
- 5771982 react_compiler: Render unchanged programs as source in fixture
snapshots (#24129) (Boshen)
- 4b16e1a transformer/async-to-generator: Preserve direct eval scope
flags (#24136) (camc314)
- 4e9194f react_compiler: Lower `delete obj.prop` to
Property/ComputedDelete (#24123) (Boshen)
- 0b25582 ast: Type binding node `typeAnnotation` as `TSTypeAnnotation |
null` (#23113) (Boshen)
- 018c0e5 transformer: Hoist lowered async declarations (#22770)
(camc314)
- 652fbaf mangler: Keep names of destructured exported bindings (#24036)
(Dunqing)
- e274415 minifier: Don't drop global calls that throw despite pure
arguments (#23917) (Dunqing)
- 59abb30 minifier: Only merge string literals in `try_fold_add` when
the inner operator is `+` (#23622) (Jerry Zhao)

### ⚡ Performance

- c5ca77b transformer: Avoid cloning refresh options (#24191) (camc314)
- bf1a151 react_compiler: Compile out debug printers (#24184) (Boshen)
- abb44a0 transformer: Build fixed object-rest arguments (#24190)
(camc314)
- a4db731 isolated_declarations: Use `ReplaceWith` instead of `TakeIn`
(#24016) (overlookmotel)
- ff10855 transformer: Use `ReplaceWith` instead of `TakeIn` (#24015)
(overlookmotel)
- bd49aff ecmascript: Avoid heap-allocating Math.min/max/imul operands
(#23941) (Lawrence Lin)
- e4b708b react_compiler: Skip compiled files before prefilters (#24171)
(Boshen)
- c59f2fe rust: Return impl ExactSizeIterator from slice-backed
accessors (#24144) (Boshen)
- 5d6d04a codegen: SWAR-skip boring byte runs in sourcemap line/column
scan (#24023) (Boshen)
- a55e0be traverse: Reduce string operations in `get_var_name_from_node`
(#24007) (overlookmotel)
- e6d48e1 transformer/nullish_coalescing: Move cold path into separate
function (#23989) (overlookmotel)
- c4e35b5 transformer/object_rest_spread: Pre-allocate capacity in `Vec`
(#23988) (overlookmotel)
- 527b8e5 transformer/decorators: Narrow type earlier (#23987)
(overlookmotel)

### 📚 Documentation

- 30d17f5 allocator: Clarify docs for `TakeIn::take_in_box` (#24093)
(overlookmotel)
- 675e6a8 ast: Correct doc comment for `PrivateFieldExpression` (#24008)
(overlookmotel)
- e4c30e6 minifier: Explain what `dce` mode means (#23994) (Dunqing)
- 37cbf88 ast_macros: Document fields of `StructDetails` (#23959)
(overlookmotel)
- 4de3e54 ast: Correct doc comment (#23948) (overlookmotel)

Co-authored-by: Boshen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-semantic Area - Semantic A-transformer Area - Transformer / Transpiler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants