refactor: replace take_in-then-write-back with ReplaceWith and by-value moves#10285
Merged
Merged
Conversation
✅ Deploy Preview for rolldown-rs canceled.
|
Merging this PR will not alter performance
Comparing 
Footnotes
|
Member
Merge activity
|
…ue moves (#10285) ## What Converts the `take_in` → build → write-back patterns in the module finalizer, scan pre-processor, HMR finalizer, and lazy-export generation to `oxc_allocator::ReplaceWith` (available since oxc 0.139) or plain by-value moves. ## Why Each `take_in` writes a `Dummy` node into the vacated slot; for AST enums that dummy is a real arena allocation (16 bytes boxed) that is overwritten an instant later but stays in the pooled module arena until the generate stage ends. It advances the bump pointer (raising the pool's retained high-water mark) and lowers live-node density for every later walk over the same arena — scanner, DCE, finalizer, codegen. oxc's own parser (oxc-project/oxc#24018) and minifier (oxc-project/oxc#24017) made the same move in 0.140.0. ## How - **Slot-level `replace_with`**: keep-name wraps (finalizer ×3), both dynamic-`import()`-in-CJS rewrites, the `.then((m) => __toESM(...))` interop wrapper, `replace_first_expr_stmt`, and the PreProcessor's `require(a?b:c)` / `import(a?b:c)` transposes (those destructure the conditional by value — 3 dummies each, now 0). - **By-value with box reuse**: the finalizer's export-default block and the HMR finalizer's `handle_top_level_stmt` now match the owned statement directly, so `export default function/class` reuses the existing `Box<Function>`/`Box<Class>` instead of take-dummy-rebox (also saves the ~200-byte struct copy). `get_transformed_class_decl` takes the class box by value and returns `Result<Declaration, Box<Class>>`, giving the box back when no transform applies. - **Plain moves where no trait is needed**: var-hoisting drains `declarations` by value (`Vec`'s dummy is an empty vec — no allocation), JSON prop inlining uses `Option::take`, and paren unwrapping in lazy export works by value. Left as-is: the `Vec` drain-and-rebuild `take_in`s (free dummy, structurally required) and the `take_in_box` in the dynamic-import-to-other-chunk path, whose `Option`-returning protocol tolerates a partially consumed node on the warn paths. One behavior note: the finalizer's export-default fallback arm is now `unreachable!()` for `TSInterfaceDeclaration` (TS is stripped before finalization), matching the convention already used by `visit_declaration` and the HMR finalizer for TS variants. Output is byte-identical: the snapshot suite passes with zero snapshot updates.
graphite-app
Bot
force-pushed
the
refactor/take-in-to-replace-with
branch
from
July 15, 2026 03:24
2936fc3 to
f21f92f
Compare
IWANABETHATGUY
added a commit
that referenced
this pull request
Jul 15, 2026
## What Readability follow-ups to #10285, which migrated the `take_in`-then-write-back patterns to `ReplaceWith`. No behavior change — this only touches how the code reads. ## How - **`matches!(…)` + `let … else { unreachable!() }` → `if let`** (module finalizer, ×2: the `export default` arm and the `top_level_var` class rewrite). Inside a `replace_with` closure that double-destructure is unavoidable — the closure is handed the whole node, so it has to re-narrow, and that's the shape oxc's own docs use. But `top_stmt` is a plain owned local. `if let` moves the binding out only when the pattern matches, and every arm there already reassigns `top_stmt` or returns, so the second discriminant test and its `unreachable!()` were pure ceremony. Two of them go away. - **Say what the HMR catch-all arm actually does.** #10285 flattened the inner `match module_decl` into the outer `match node`, which merged two arms: one catching only `TSExportAssignment`/`TSNamespaceExportDeclaration` (carrying the TypeScript comment), and an uncommented `_` catching everything else. Only the TypeScript comment survived the merge, so the arm now reads as though it is about TypeScript when in practice nearly everything reaching it is an ordinary statement. - **Re-indent the `exports.extend` closure in the HMR finalizer.** rustfmt bails out on this expression, so its body had been stranded ~11 columns right of its header since before #10285, and the flattening widened that to ~15. `cargo fmt --check` passes either way, so CI never flagged it. At the shallower nesting the block now fits `max_width`, so rustfmt formats it again instead of giving up — it should stop drifting. - **`first_mut()` → `first()`** in `json_object_expr_to_esm`, now that the binding is only read; the two-step destructure folds into one `let … else`. - **Drop `.expect("init is checked to be `Some` above")`** in `try_inline_json_module_prop`. It re-checked at runtime an invariant established a few lines above, and only existed to dodge a `&mut` that the `Some` arm no longer needs. Each arm now reads `first_decl.init` directly. ## Tests No new tests. Nothing here changes behavior, so there is no test that would fail before and pass after. What needs checking is that the output is unchanged, and the snapshot suite is the check for that — but it auto-accepts (as `just test-update` puts it: "Rust tests will update snapshots automatically"), so a green run proves nothing by itself. The real check is what the working tree looks like afterwards: - `cargo test -p rolldown` → 1869 passed, and **zero `.snap` files modified**. - `cargo clippy -p rolldown --all-targets -- --deny warnings` and `cargo fmt --all --check` both clean.
Merged
shulaoda
added a commit
that referenced
this pull request
Jul 15, 2026
## [1.2.0] - 2026-07-15 ### 🚀 Features - dev: skip shipping factories for newly imported top-level modules (#10223) by @h-a-n-a - dev: per-client ship map for HMR patch sizing (#10208) by @h-a-n-a - dev: client-side HMR (#10164) by @h-a-n-a - dev: send a full-reload update to clients when a tsconfig changes (#10262) by @shulaoda - treat `import.meta['url']` and `import.meta['ROLLUP_FILE_URL_*']` as side-effect free (#10267) by @sapphi-red - rewrite `import.meta['url']` (#10251) by @sapphi-red - add `FILE_NOT_FOUND` error (#10220) by @sapphi-red - treat `import.meta.ROLLUP_FILE_URL_*` as side-effect free (#10217) by @sapphi-red ### 🐛 Bug Fixes - sourcemap: preserve unmapped boundaries during composition (#10254) by @hyfdev - `[format]` in `*FileNames` option for ESM format should be `es` instead of `esm` (#10214) by @sapphi-red - sourcemap: preserve coarse mappings during composition (#10249) by @hyfdev - rolldown_plugin_vite_import_glob: support tsconfig paths with `import.meta.glob` (#10167) by @sapphi-red - dev: clear tsconfig caches for bare full builds (#10276) by @shulaoda - dev: force a full rebuild when a tsconfig changes (#10261) by @shulaoda - treat rooted drive-less module ids as absolute in preserveModules naming (#10235) by @IWANABETHATGUY - watch: rebuild when tsconfig files change (#10258) by @shulaoda - watch: drop tsconfig-merged transform options on each rebuild (#10257) by @shulaoda - incorrect `EMPTY_IMPORT_META` warning for `import.meta.ROLLUP_FILE_URL_*` for CJS output (#10221) by @sapphi-red - deconflict: rename CJS locals shadowing wrapped-ESM namespace objects (#9970) by @IWANABETHATGUY - rolldown: drop the unused runtime module after entry-level external flattening (#10237) by @IWANABETHATGUY - rolldown: re-propagate has_dynamic_exports to transitive star importers (#10239) by @IWANABETHATGUY - tree-shaking: tree-shake destructured dynamic import namespace bindings (#10213) by @logaretm - s390x: use json-escape-simd 3.1.1 for big-endian JSON escaping fix (#10211) by @satyamg1620 ### 🚜 Refactor - dev: move full-reload to client side (#10207) by @h-a-n-a - readability follow-ups to the ReplaceWith migration (#10286) by @IWANABETHATGUY - replace take_in-then-write-back with ReplaceWith and by-value moves (#10285) by @Boshen - share the main resolver's cache with the transformer's tsconfig lookups (#10205) by @shulaoda - rolldown: extract the ns star-external __reExport emission rule into LinkingMetadata (#10238) by @IWANABETHATGUY - rolldown: unify link/generate diagnostics into a Diagnostics accumulator (#10234) by @IWANABETHATGUY - sourcemap_filenames: drop dead sourcemap-filename plumbing (#10189) by @IWANABETHATGUY - extract external import symbol merging into a method (#10224) by @IWANABETHATGUY - rolldown: skip CJS namespace merging under strict execution order (#10203) by @hyfdev - resolve the manual tsconfig per file instead of once at startup (#10200) by @shulaoda - rolldown: route interop ESM init emission through a shared init-target view (#10202) by @hyfdev - rolldown: collapse vestigial wrap-kind state and share chunk sort helper (#10201) by @hyfdev ### 📚 Documentation - show plugin kinds in JSDoc and each hook's description (#10218) by @sapphi-red - add an explanation about removing imports from external modules without any messages (#10215) by @sapphi-red ### ⚡ Performance - sourcemap: owned merge in SourceJoiner::join (4005->5 allocs/chunk) (#10250) by @Boshen - avoid redundant sourcemap string copies in collapse and minify paths (#10093) by @Boshen ### 🧪 Testing - code-splitting: establish strict-order review baselines (#10287) by @hyfdev - dev: add hot API test cases (#10181) by @h-a-n-a - code-splitting: normalize strict execution order variants (#10277) by @hyfdev - code-splitting: harden strict execution order coverage (#10252) by @hyfdev - code-splitting: add strict execution order regressions (#10253) by @hyfdev ### ⚙️ Miscellaneous Tasks - deps: update github actions (#10241) by @renovate[bot] - deps: update oxc to 0.140.0 (#10274) by @shulaoda - update Yunfei's GitHub username (#10275) by @hyfdev - deps: update napi (#10260) by @renovate[bot] - deps: update test262 submodule for tests (#10266) by @rolldown-guard[bot] - deps: update dependency vite-plus to v0.2.4 (#10256) by @renovate[bot] - deps: update napi (#10240) by @renovate[bot] - deps: update oxc resolver to v11.24.2 (#10245) by @renovate[bot] - deps: update rust crates (#10244) by @renovate[bot] - disable Renovate updates for idna_adapter (#10248) by @shulaoda - deps: update oxc resolver to v11.24.1 (#10232) by @renovate[bot] - deps: update rust crate oxc_sourcemap to v8.1.1 (#10233) by @renovate[bot] - deps: update dependency rolldown-plugin-dts to ^0.27.0 (#10206) by @renovate[bot] - deps: upgrade sugar_path to v3 (#10230) by @hyfdev - add `dist-*` to `.gitignore` in sourcemap-filenames/hash-final-content fixture (#10216) by @sapphi-red - deps: update dependency rust to v1.97.0 (#10209) by @renovate[bot] ### ❤️ New Contributors * @satyamg1620 made their first contribution in [#10211](#10211) Co-authored-by: shulaoda <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Converts the
take_in→ build → write-back patterns in the module finalizer, scan pre-processor, HMR finalizer, and lazy-export generation tooxc_allocator::ReplaceWith(available since oxc 0.139) or plain by-value moves.Why
Each
take_inwrites aDummynode into the vacated slot; for AST enums that dummy is a real arena allocation (16 bytes boxed) that is overwritten an instant later but stays in the pooled module arena until the generate stage ends. It advances the bump pointer (raising the pool's retained high-water mark) and lowers live-node density for every later walk over the same arena — scanner, DCE, finalizer, codegen. oxc's own parser (oxc-project/oxc#24018) and minifier (oxc-project/oxc#24017) made the same move in 0.140.0.How
replace_with: keep-name wraps (finalizer ×3), both dynamic-import()-in-CJS rewrites, the.then((m) => __toESM(...))interop wrapper,replace_first_expr_stmt, and the PreProcessor'srequire(a?b:c)/import(a?b:c)transposes (those destructure the conditional by value — 3 dummies each, now 0).handle_top_level_stmtnow match the owned statement directly, soexport default function/classreuses the existingBox<Function>/Box<Class>instead of take-dummy-rebox (also saves the ~200-byte struct copy).get_transformed_class_decltakes the class box by value and returnsResult<Declaration, Box<Class>>, giving the box back when no transform applies.declarationsby value (Vec's dummy is an empty vec — no allocation), JSON prop inlining usesOption::take, and paren unwrapping in lazy export works by value.Left as-is: the
Vecdrain-and-rebuildtake_ins (free dummy, structurally required) and thetake_in_boxin the dynamic-import-to-other-chunk path, whoseOption-returning protocol tolerates a partially consumed node on the warn paths.One behavior note: the finalizer's export-default fallback arm is now
unreachable!()forTSInterfaceDeclaration(TS is stripped before finalization), matching the convention already used byvisit_declarationand the HMR finalizer for TS variants.Output is byte-identical: the snapshot suite passes with zero snapshot updates.