Skip to content

refactor(hmr): migrate hmr finalizer to AstFactory#9695

Merged
graphite-app[bot] merged 1 commit into
mainfrom
astfactory-migrate-hmr
Jun 10, 2026
Merged

refactor(hmr): migrate hmr finalizer to AstFactory#9695
graphite-app[bot] merged 1 commit into
mainfrom
astfactory-migrate-hmr

Conversation

@hyf0

@hyf0 hyf0 commented Jun 9, 2026

Copy link
Copy Markdown
Member

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)

🤖 Generated with Claude Code

hyf0 commented Jun 9, 2026

Copy link
Copy Markdown
Member Author

How to use the Graphite Merge Queue

Add the label graphite: merge-when-ready to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

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

This PR refactors the HMR AST “finalizer” codepath to use the newer AstFactory newtype instead of the legacy AstSnippet facade, consolidating construction state and adding shared AstFactory::make_* helpers for common AST patterns used by HMR.

Changes:

  • Replace HmrAstFinalizer’s alloc/snippet/builder fields with a single ast_factory: AstFactory.
  • Update HmrAstBuilder::builder() to return AstBuilder by value (Copy) and adjust its implementors accordingly.
  • Add several reusable AstFactory::make_* helpers (member access, call-with-arg, Promise.resolve().then(...), import-star stmt, __toESM wrapping, parenthesized seq).

Reviewed changes

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

Show a summary per file
File Description
crates/rolldown/src/hmr/utils.rs Updates HmrAstBuilder::builder() API to return AstBuilder by value; adapts HMR and scope-hoisting implementors.
crates/rolldown/src/hmr/impl_traverse_for_hmr_ast_finalizer.rs Migrates HMR traversal/finalization construction calls from AstSnippet/allocator fields to AstFactory.
crates/rolldown/src/hmr/hmr_stage.rs Switches HMR finalizer construction sites to build and store AstFactory::new(allocator).
crates/rolldown/src/hmr/hmr_ast_finalizer.rs Rewrites HMR AST finalizer logic to use AstFactory and new shared make_* helpers.
crates/rolldown_ecmascript_utils/src/ast_factory.rs Adds new shared make_* construction helpers to support the migration and future reuse.

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

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comment thread crates/rolldown/src/hmr/hmr_ast_finalizer.rs Outdated
@hyf0 hyf0 force-pushed the astfactory-migrate-hmr branch from 87bda68 to be74dba Compare June 10, 2026 02:07
@hyf0 hyf0 force-pushed the astfactory-migrate-vite-build-import-analysis branch from 24f78e3 to 2299c2e Compare June 10, 2026 02:07
@hyf0 hyf0 marked this pull request as ready for review June 10, 2026 02:16
@hyf0 hyf0 requested review from Copilot, h-a-n-a and shulaoda June 10, 2026 02:16

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

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

@codspeed-hq

codspeed-hq Bot commented Jun 10, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 4 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing astfactory-migrate-hmr (be74dba) with astfactory-migrate-vite-build-import-analysis (24f78e3)2

Open in CodSpeed

Footnotes

  1. 10 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 astfactory-migrate-vite-build-import-analysis (2299c2e) during the generation of this report, so ccc9659 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

graphite-app Bot pushed a commit that referenced this pull request Jun 10, 2026
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)
@graphite-app graphite-app Bot force-pushed the astfactory-migrate-vite-build-import-analysis branch from 2299c2e to 132d844 Compare June 10, 2026 02:29
@graphite-app graphite-app Bot force-pushed the astfactory-migrate-vite-build-import-analysis branch from 132d844 to ba76527 Compare June 10, 2026 02:30
@graphite-app graphite-app Bot force-pushed the astfactory-migrate-hmr branch from be74dba to 03d409d Compare June 10, 2026 02:30
@graphite-app

graphite-app Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Merge activity

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)
graphite-app Bot pushed a commit that referenced this pull request Jun 10, 2026
…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)
@graphite-app graphite-app Bot force-pushed the astfactory-migrate-vite-build-import-analysis branch from ba76527 to f588477 Compare June 10, 2026 02:57
@graphite-app graphite-app Bot force-pushed the astfactory-migrate-hmr branch from 03d409d to f5e2849 Compare June 10, 2026 02:57
graphite-app Bot pushed a commit that referenced this pull request Jun 10, 2026
…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)
graphite-app Bot pushed a commit that referenced this pull request Jun 10, 2026
…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)
graphite-app Bot pushed a commit that referenced this pull request Jun 10, 2026
…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)
Base automatically changed from astfactory-migrate-vite-build-import-analysis to main June 10, 2026 03:02
@graphite-app graphite-app Bot merged commit f5e2849 into main Jun 10, 2026
31 checks passed
@graphite-app graphite-app Bot deleted the astfactory-migrate-hmr branch June 10, 2026 03:03
@shulaoda shulaoda mentioned this pull request Jun 11, 2026
@rolldown-guard rolldown-guard Bot mentioned this pull request Jun 11, 2026
shulaoda pushed a commit that referenced this pull request Jun 11, 2026
## [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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants