refactor(ecmascript_utils): fold construction ext traits onto AstFactory and delete AstSnippet#9702
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors rolldown’s ECMAScript AST construction utilities to rely solely on AstFactory (the unified oxc AstBuilder wrapper), updating the remaining construction-oriented extension traits accordingly and removing the now-unused AstSnippet facade.
Changes:
- Updated
BindingPatternExt::into_assignment_target/BindingPropertyExt::into_assignment_target_propertyto accept&AstFactoryinstead of constructingAstBuilderinternally. - Removed
AstSnippet(type, module, andlib.rsexport) after confirming it has no remaining Rust users. - Trimmed the AST construction design doc by removing the completed temporary “Plan” section.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| meta/design/ast-construction.md | Removes the temporary migration plan section now that the work is complete. |
| crates/rolldown/src/module_finalizers/mod.rs | Updates binding-pattern assignment-target conversion to pass &AstFactory. |
| crates/rolldown_ecmascript_utils/src/lib.rs | Drops the AstSnippet module and public re-export. |
| crates/rolldown_ecmascript_utils/src/extensions/ast_ext/binding_property_ext.rs | Refactors assignment-target-property construction to use the shared AstFactory handle. |
| crates/rolldown_ecmascript_utils/src/extensions/ast_ext/binding_pattern_ext.rs | Refactors assignment-target conversion to use &AstFactory and removes AstSnippet usage. |
| crates/rolldown_ecmascript_utils/src/ast_snippet.rs | Deletes the unused legacy AST construction helper facade. |
34bb840 to
2de0302
Compare
4181924 to
080f411
Compare
Merging this PR will not alter performance
Comparing Footnotes
|
2de0302 to
96a8b9c
Compare
080f411 to
08566cd
Compare
Merge activity
|
96a8b9c to
5b21bed
Compare
1fac435 to
a7074b3
Compare
Migrates the `PreProcessor` scanner pre-pass off `AstSnippet` to `AstFactory` (handle `ast_factory`): `.builder.*` reach-throughs → deref'd calls, `.alloc()` → `.allocator`. Pure mechanical migration — no new `make_*`. Behavior-preserving; clippy/fmt clean + adversarial reviews. ### AstFactory migration progress (`meta/design/ast-construction.md`) - [x] Design doc + conventions — #9673 (merged) - [x] Introduce `AstFactory` + `vite_web_worker_post` + `generate_lazy_export` — #9682 (merged) - [x] `tweak_ast_for_scanning` — **#9683 ← this PR** - [x] `vite_build_import_analysis` — #9693 - [x] hmr finalizer + `hmr_stage` (+ 6 shared `make_*`, `builder()` by-value) — #9695 - [x] `module_finalizers/mod.rs` (+ the `make_*` ports it consumes; transitional dual field) — #9700 - [x] rest of `ScopeHoistingFinalizer` (+ wrapper `make_*` ports; `snippet` field removed) — #9701 - [x] fold construction ext traits onto `AstFactory`; delete `AstSnippet` — #9702 🤖 Generated with [Claude Code](https://claude.com/claude-code)
5b21bed to
15bbfcc
Compare
a7074b3 to
416206b
Compare
…ory and delete AstSnippet (#9702) Final step of the migration. `BindingPatternExt::into_assignment_target` / `BindingPropertyExt::into_assignment_target_property` now take `&AstFactory` (matching the `into_expression` precedent from #9693) instead of constructing `AstBuilder::new(alloc)` / `AstSnippet::new(alloc)` internally; the sole external caller (`module_finalizers/mod.rs`) passes `&self.ast_factory`. With zero users left, `AstSnippet` is deleted — `ast_snippet.rs` (1030 lines) and the `lib.rs` export — and the temporary `## Plan` section in `meta/design/ast-construction.md` is removed per its own instruction (`## Current state` retitled `## Prior state` to match the new reality; one stale method-name doc comment fixed). Behavior-preserving: full-fixture snapshot suite byte-identical (incl. the `wrapped_esm` fixture, which exercises every branch of the rewritten destructuring-assignment conversion) + 2 adversarial reviews + 1 CodeX review, all clean. ### AstFactory migration progress (`meta/design/ast-construction.md`) - [x] Design doc + conventions — #9673 (merged) - [x] Introduce `AstFactory` + `vite_web_worker_post` + `generate_lazy_export` — #9682 (merged) - [x] `tweak_ast_for_scanning` — #9683 - [x] `vite_build_import_analysis` — #9693 - [x] hmr finalizer + `hmr_stage` (+ 6 shared `make_*`, `builder()` by-value) — #9695 - [x] `module_finalizers/mod.rs` (+ the `make_*` ports it consumes; transitional dual field) — #9700 - [x] rest of `ScopeHoistingFinalizer` (+ wrapper `make_*` ports; `snippet` field removed) — #9701 - [x] fold construction ext traits onto `AstFactory`; delete `AstSnippet` — **#9702 ← this PR** 🤖 Generated with [Claude Code](https://claude.com/claude-code)
…9693) Migrates `vite_build_import_analysis` off `AstSnippet` onto `AstFactory`. Adds `AstFactory::make_arrow_returning` **and the identifier shortcuts `make_id` / `make_id_ref_expr` / `make_id_name`** (high-frequency one-liners that read poorly when inlined — per review feedback); migrates `BindingPatternExt::into_expression` to `&AstFactory` (its only external caller is here; `into_assignment_target` deferred). Remaining thin wrappers inlined. Behavior-preserving; 2 adversarial + 1 CodeX review, all clean. ### AstFactory migration progress (`meta/design/ast-construction.md`) - [x] Design doc + conventions — #9673 (merged) - [x] Introduce `AstFactory` + `vite_web_worker_post` + `generate_lazy_export` — #9682 (merged) - [x] `tweak_ast_for_scanning` — #9683 - [x] `vite_build_import_analysis` — **#9693 ← this PR** - [x] hmr finalizer + `hmr_stage` (+ 6 shared `make_*`, `builder()` by-value) — #9695 - [x] `module_finalizers/mod.rs` (+ the `make_*` ports it consumes; transitional dual field) — #9700 - [x] rest of `ScopeHoistingFinalizer` (+ wrapper `make_*` ports; `snippet` field removed) — #9701 - [x] fold construction ext traits onto `AstFactory`; delete `AstSnippet` — #9702 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Migrates the HMR finalizer area (`crates/rolldown/src/hmr/`) off the legacy `AstSnippet` facade onto the `AstFactory` newtype. Behavior-preserving. - **Consolidates 3 redundant construction fields** on `HmrAstFinalizer` (`alloc: &Allocator`, `snippet: AstSnippet`, `builder: &AstBuilder`) into a single `ast_factory: AstFactory`. The 3 construction sites in `hmr_stage.rs` build `AstFactory::new(allocator)`. - **`HmrAstBuilder::builder()`** (shared trait, also impl'd for the not-yet-migrated `ScopeHoistingFinalizer`) now returns `AstBuilder` **by value** (it is `Copy`) instead of `&AstBuilder` — the one non-mechanical fix the design doc flagged. `HmrAstFinalizer` returns `*self.ast_factory`; `ScopeHoistingFinalizer` returns `self.snippet.builder` and keeps compiling unchanged. - **Shared `make_*` additions on `AstFactory`** (incl. `make_member_access` -> `MemberExpression`, which `make_member_access_expr` delegates to) (faithful ports of `AstSnippet` patterns, reused later by `module_finalizers`): `make_member_access_expr`, `make_call_with_arg`, `make_promise_resolve_then`, `make_lazy_export_property` (renamed from `object_property_kind_object_property` to avoid shadowing the oxc builder method reached via `Deref`), `make_import_star_stmt`, `make_to_esm_call_with_interop`, `make_seq_in_parens`. - Thin wrappers inlined (`id`/`id_ref_expr`/`id_name`/`atom`/`string_literal_expr`/`call_expr_expr`/…); `var_decl_stmt` → existing `make_var_decl`; `.builder.*` → deref'd `ast_factory.*`; `.alloc()` → `.allocator`. ### AstFactory migration progress (`meta/design/ast-construction.md`) - [x] Design doc + conventions — #9673 (merged) - [x] Introduce `AstFactory` + `vite_web_worker_post` + `generate_lazy_export` — #9682 (merged) - [x] `tweak_ast_for_scanning` — #9683 - [x] `vite_build_import_analysis` — #9693 - [x] hmr finalizer + `hmr_stage` (+ 6 shared `make_*`, `builder()` by-value) — **#9695 ← this PR** - [x] `module_finalizers/mod.rs` (+ the `make_*` ports it consumes; transitional dual field) — #9700 - [x] rest of `ScopeHoistingFinalizer` (+ wrapper `make_*` ports; `snippet` field removed) — #9701 - [x] fold construction ext traits onto `AstFactory`; delete `AstSnippet` — #9702 🤖 Generated with [Claude Code](https://claude.com/claude-code)
…9700) Migrates all 179 `self.snippet.*` call sites in `module_finalizers/mod.rs` onto the `AstFactory` handle, **adding the 12 `make_*` methods (+2 private helpers) this file consumes** in the same PR (behavior-identical ports of the corresponding `AstSnippet` patterns): `make_to_esm_wrapper`, `make_keep_name_call`, `make_static_block_keep_name`, `make_re_export_call`, `make_member_expr_or_ident_ref`, `make_member_expr_with_void_zero_object`, `make_callee_then_call`, `make_then_extract_property`, `make_then_call_esm_wrapper_with_namespace`, `make_then_call_cjs_wrapper_with_to_esm`, plus `arrow_function_extract_property` / `then_with_arrow_callback`. Thin wrappers are inlined to deref'd oxc constructor calls, `.builder.*` reach-throughs become deref'd `ast_factory.*`, and `AstBuilder::new(self.alloc)` / `self.builder()` become `*self.ast_factory`. The one-use `object_freeze_dynamic_import_polyfill` is inlined at its call site. Adds a **transitional** `ast_factory` field alongside `snippet` on `ScopeHoistingFinalizer` — the sibling impl files (`impl_visit_mut.rs` / `rename.rs` / `hmr.rs`) still read `snippet` and migrate in #9701, which removes the field. Behavior-preserving: two independent full-fixture snapshot differential runs against the parent commit (byte-identical emitted JS, identical pass/fail sets) + 2 adversarial reviews + 1 CodeX review + Copilot, all clean. ### AstFactory migration progress (`meta/design/ast-construction.md`) - [x] Design doc + conventions — #9673 (merged) - [x] Introduce `AstFactory` + `vite_web_worker_post` + `generate_lazy_export` — #9682 (merged) - [x] `tweak_ast_for_scanning` — #9683 - [x] `vite_build_import_analysis` — #9693 - [x] hmr finalizer + `hmr_stage` (+ 6 shared `make_*`, `builder()` by-value) — #9695 - [x] `module_finalizers/mod.rs` (+ the `make_*` ports it consumes; transitional dual field) — **#9700 ← this PR** - [x] rest of `ScopeHoistingFinalizer` (+ wrapper `make_*` ports; `snippet` field removed) — #9701 - [x] fold construction ext traits onto `AstFactory`; delete `AstSnippet` — #9702 🤖 Generated with [Claude Code](https://claude.com/claude-code)
15bbfcc to
e9f7d42
Compare
…ctory (#9701) Finishes the `ScopeHoistingFinalizer` migration: the remaining `impl_visit_mut.rs` (16 sites) / `rename.rs` (6) / `hmr.rs` (3) call sites move off `AstSnippet`, **adding the `make_commonjs_wrapper_stmt` / `make_esm_wrapper_stmt` ports alongside their first callers**. The transitional `snippet` field from #9700 is removed (struct + construction + imports), and the `HmrAstBuilder::builder()` impl for `ScopeHoistingFinalizer` now returns `*self.ast_factory`. `literal_prop_access_member_expr` (the `MemberExpression`-returning variant) and other thin wrappers are inlined. Behavior-preserving: full-fixture snapshot suite byte-identical + 2 adversarial reviews + 1 CodeX review + Copilot, all clean. ### AstFactory migration progress (`meta/design/ast-construction.md`) - [x] Design doc + conventions — #9673 (merged) - [x] Introduce `AstFactory` + `vite_web_worker_post` + `generate_lazy_export` — #9682 (merged) - [x] `tweak_ast_for_scanning` — #9683 - [x] `vite_build_import_analysis` — #9693 - [x] hmr finalizer + `hmr_stage` (+ 6 shared `make_*`, `builder()` by-value) — #9695 - [x] `module_finalizers/mod.rs` (+ the `make_*` ports it consumes; transitional dual field) — #9700 - [x] rest of `ScopeHoistingFinalizer` (+ wrapper `make_*` ports; `snippet` field removed) — **#9701 ← this PR** - [x] fold construction ext traits onto `AstFactory`; delete `AstSnippet` — #9702 🤖 Generated with [Claude Code](https://claude.com/claude-code)
416206b to
fad6176
Compare
## [1.1.1] - 2026-06-11 ### 🚀 Features - ecmascript_utils: introduce AstFactory for AST construction (#9682) by @hyf0 - drop no-op init calls for empty wrapped-ESM modules (#9678) by @IWANABETHATGUY ### 🐛 Bug Fixes - resolver: honor package.json#type for .jsx/.tsx extensions (#9690) (#9699) by @IWANABETHATGUY - hmr: report the full-reload reason for the invalidate-loop case (#9708) by @hyf0 - explicit `moduleSideEffects` from a hook must take priority over the `package.json#sideEffects` (#9688) by @sapphi-red - keep the `rolldown-runtime` name for the standalone runtime chunk (#9685) by @shulaoda - lazy-barrel: request all exports for entry barrels on first encounter (#9672) by @shulaoda - finalizer: skip init_*() for tree-shaken wrapped ESM owners (#9669) by @IWANABETHATGUY - order `chunk.imports` by execution order (#9654) by @chuganzy - vite-resolve: preserve slash separators for Sass partial exports (#9546) by @cjc0013 - cross-chunk CJS wrapper shadowed by author-local binding (#9648) by @IWANABETHATGUY ### 🚜 Refactor - precompute wrapped-ESM init metadata in generate stage (#9712) by @IWANABETHATGUY - ecmascript_utils: fold construction ext traits onto AstFactory and delete AstSnippet (#9702) by @hyf0 - finalizer: finish ScopeHoistingFinalizer migration to AstFactory (#9701) by @hyf0 - finalizer: migrate module_finalizers/mod.rs to AstFactory (#9700) by @hyf0 - hmr: migrate hmr finalizer to AstFactory (#9695) by @hyf0 - plugin: migrate vite_build_import_analysis to AstFactory (#9693) by @hyf0 - scanner: migrate tweak_ast_for_scanning to AstFactory (#9683) by @hyf0 - always split runtime module first (#9419) by @IWANABETHATGUY ### 📚 Documentation - tsconfig: correct auto-discovery resolution to match TypeScript (#9714) by @shulaoda - design: plan to unify all internal AST construction (#9673) by @hyf0 - tsconfig: align reference resolution docs with TypeScript behavior (#9641) by @shulaoda ### ⚡ Performance - avoid per-module join Strings in scope-hoisting concatenation (#9645) by @Boshen - avoid intermediate Strings in the ESM export clause (#9644) by @Boshen - reuse a scratch buffer for facade namespace names in the scanner (#9642) by @Boshen - reuse the import-matching tracker stack across named imports (#9643) by @Boshen - avoid cloning the per-chunk export-items map in render_chunk_exports (#9639) by @Boshen - avoid CompactStr allocation in sorted-exports membership check (#9640) by @Boshen ### 🧪 Testing - add more `moduleSideEffects` precedence tests (#9689) by @sapphi-red - 9651: drop shimMissingExports from regression fixture (#9674) by @IWANABETHATGUY ### ⚙️ Miscellaneous Tasks - update react repo links (#9711) by @iiio2 - remove outdated pnpm configurations (#9666) by @btea - deps: update github actions to v2.81.5 (#9665) by @renovate[bot] - testing: migrate test harness off deprecated inlineDynamicImports (#9710) by @IWANABETHATGUY - hmr: delete commented-out dead code left from old register-module codegen (#9707) by @hyf0 - deps: update napi-rs toolchain (#9706) by @shulaoda - deps: update rollup submodule for tests to v4.61.1 (#9676) by @rolldown-guard[bot] - deps: update test262 submodule for tests (#9677) by @rolldown-guard[bot] - deps: update oxc to 0.135.0 (#9670) by @Boshen - pass ALGOLIA_APP_ID and ALGOLIA_API_KEY to void deploy (#9667) by @Boshen - deps: update github actions (#9662) by @renovate[bot] - deps: update rust crates (#9663) by @renovate[bot] - deps: update rolldown-plugin-dts to v0.25.2 (#9661) by @renovate[bot] - deps: update typos to v1.47.2 (#9660) by @renovate[bot] - deps: update docs dependencies (#9657) by @bddjr - deps: update typos to v1.47.1 (#9655) by @renovate[bot] - deploy website in its own workflow, only on docs output change (#9650) by @Boshen - publish to pkg.pr.new add pm and `commentWithDev` option (#9638) by @btea ### ❤️ New Contributors * @chuganzy made their first contribution in [#9654](#9654) * @cjc0013 made their first contribution in [#9546](#9546)

Final step of the migration.
BindingPatternExt::into_assignment_target/BindingPropertyExt::into_assignment_target_propertynow take&AstFactory(matching theinto_expressionprecedent from #9693) instead of constructingAstBuilder::new(alloc)/AstSnippet::new(alloc)internally; the sole external caller (module_finalizers/mod.rs) passes&self.ast_factory.With zero users left,
AstSnippetis deleted —ast_snippet.rs(1030 lines) and thelib.rsexport — and the temporary## Plansection inmeta/design/ast-construction.mdis removed per its own instruction (## Current stateretitled## Prior stateto match the new reality; one stale method-name doc comment fixed).Behavior-preserving: full-fixture snapshot suite byte-identical (incl. the
wrapped_esmfixture, which exercises every branch of the rewritten destructuring-assignment conversion) + 2 adversarial reviews + 1 CodeX review, all clean.AstFactory migration progress (
meta/design/ast-construction.md)AstFactory+vite_web_worker_post+generate_lazy_export— feat(ecmascript_utils): introduce AstFactory for AST construction #9682 (merged)tweak_ast_for_scanning— refactor(scanner): migrate tweak_ast_for_scanning to AstFactory #9683vite_build_import_analysis— refactor(plugin): migrate vite_build_import_analysis to AstFactory #9693hmr_stage(+ 6 sharedmake_*,builder()by-value) — refactor(hmr): migrate hmr finalizer to AstFactory #9695module_finalizers/mod.rs(+ themake_*ports it consumes; transitional dual field) — refactor(finalizer): migrate module_finalizers/mod.rs to AstFactory #9700ScopeHoistingFinalizer(+ wrappermake_*ports;snippetfield removed) — refactor(finalizer): finish ScopeHoistingFinalizer migration to AstFactory #9701AstFactory; deleteAstSnippet— refactor(ecmascript_utils): fold construction ext traits onto AstFactory and delete AstSnippet #9702 ← this PR🤖 Generated with Claude Code