fix(dev): revert cache mutations when a partial scan fails#10110
Conversation
How to use the Graphite Merge QueueAdd 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. |
Merging this PR will not alter performance
Comparing Footnotes
|
|
@codex[agent] review |
|
To use Codex here, create a Codex account and connect to github. |
814b555 to
2390768
Compare
1811447 to
306de8b
Compare
9d2b9c6 to
2d18156
Compare
306de8b to
1a94977
Compare
2d18156 to
5c787cc
Compare
1a94977 to
5b4dade
Compare
5c787cc to
00250db
Compare
✅ Deploy Preview for rolldown-rs canceled.
|
Merge activity
|
### Description Part of #7416. Stacked on #10107. This PR makes a failed scan harmless to the incremental cache. The remaining items of #7416 are listed at the end. #### Problem A partial scan applies task results as they arrive. When the scan aborts (the typical case is a syntax error in an edited file), the cache is left half updated while the snapshot receives nothing, because `merge` never runs on the error path: 1. Re-scanned modules were flipped to `Seen` in `module_id_to_idx`, so they are treated as fresh even though their new content never landed. The next build silently serves their old code and the error disappears. 2. Completed tasks already rewrote parts of the `importers` edge list, which then disagrees with the snapshot. 3. Newly discovered modules got an index and a `module_id_to_idx` entry, but no slot in the snapshot table. Since new indexes are allocated from the map length and `merge` appends at the table length, a leftover entry shifts every later module by one slot and corrupts all index based lookups. Concrete reproduction (the new fixture): break `b.js`, then edit `a.js`. The rebuild reports success, ships the pre-break `b.js`, and the error overlay is gone. #### Fix A failed partial scan now reverts its cache mutations, so the cache holds a single invariant: between builds it is either empty (no snapshot, only a full scan is possible) or a valid graph any partial scan can build on. There is no third "poisoned" state and no consumer has to reason about failed builds. `ModuleLoader::revert_partial_scan` runs at the scan error exit. The snapshot is never touched by a scan, so it acts as the clean master copy everything else is recovered from: - `importers` is re-derived from the snapshot (`ScanStageCache::derive_importers_from_snapshot`), since every edge is a resolved import record of some module. Slot order is not preserved, and consumers treat slots as sets. The rebuild cost is paid only on failure, the happy path pays nothing. - Existing `module_id_to_idx` entries end the scan at their pre-scan values (`Seen` to `Invalidate` back to `Seen` with the same index), so only the keys inserted for newly discovered modules need removal. The loader records them in `new_module_ids`. - `modules_with_changed_importers` is cleared, matching the reverted edge list. - `barrel_state` is restored from an upfront clone, taken only when lazy barrel is enabled. When disabled the state is empty and never mutated. - The scanned files go into `ScanStageCache::pending_rescans`, a work queue the next partial scan drains. Reverting alone would lose the knowledge that these files changed, and the next build would silently serve their old content. The queue keeps their errors surfacing until the files are fixed. Only files the graph still needs are queued: configured or emitted entries, and files something still imports on the freshest edge state. A broken file whose last import was removed by the very scan that failed is dropped; otherwise its retry would fail every later partial build that a fresh build of the same tree would pass, wedging the session until a restart (new fixture `hmr/error_recovery/remove_import_of_broken_file`). - `transform_dependencies` entries of the newly allocated (and now freed) module indices are dropped, so a later module reusing an index does not inherit them. `Bundler::incremental_bundle` now picks the scan mode from `has_snapshot()` alone. A failed full scan needs no revert, its cache reset already leaves the empty state that forces a full retry. One adjacent panic became reachable and is now handled: a client calling `import.meta.hot.invalidate()` for a module the current graph does not know (for example after a failed full build reset the cache) gets a full reload instead of a server panic. The same applies to `compile_lazy_entry`: a lazy chunk request arriving after a failed full build now returns an error instead of panicking on the missing snapshot. Two consumers of the retry queue needed matching fixes: - `HmrStage::compute_hmr_update` folds the queued files into its stale and changed sets before computing client boundaries. Without this, an edit rolled back by a failed scan reached the server graph on the successful retry but never a client patch, leaving connected clients silently running the old module. The new fixture `hmr/error_recovery/fix_broken_file_delivers_other_edits` asserts the recovery patch contains both the fixed file and the edit made while broken. - The import chain enrichment of unresolved import errors now runs before the revert, while the failed scan's modules still exist in the lookup structures the trace walks. Running it after erased the chain from dev mode diagnostics. Plugin side state (`module_infos`, JS side caches) is not reverted. Hooks that ran during the aborted scan already updated them. This residue is informational, keyed by module id, and overwritten by the next successful scan. The exception is `transform_dependencies`, which is keyed by module index: the entries of indices freed by the revert are dropped, as described above. #### Behavior - The existing error recovery fixtures are unchanged. Breaking a file and fixing the same file still recovers with a plain HMR patch. - New fixture `hmr/error_recovery/remove_import_of_broken_file`: break `b.js`, then remove the `import './b.js'` from `a.js`. That build still surfaces the queued error once, then drops `b.js` from the queue since nothing imports it anymore, and the following edit builds normally again. - New fixture `hmr/error_recovery/edit_other_file_while_broken`: break `b.js`, edit `a.js` (the error surfaces again instead of a fake success with stale code), fix `b.js` (builds succeed again, the queued edit of `a.js` lands in the same scan), edit `a.js` once more (a normal patch, proving the cache stayed incremental through the whole episode). #### Docs The "Cache integrity on a failed build" section of `internal-docs/bundler-data-lifecycle/implementation.md` now documents the revert and the two state invariant. `internal-docs/cache/implementation.md` is updated for the new field and methods. #### Remaining items of #7416 - The state still holds removed modules. Editing an orphaned module triggers a full reload instead of a noop, and its module and AST stay in memory. - There is no general check yet that an incremental build and a full build produce the same states. A dedicated comparison in the test harness would also guard the revert introduced here. - Dynamic import entry points are never removed from the cached entry list when the dynamic import goes away. <!-- - What is this PR solving? Write a clear and concise description. - Reference the issues it solves (e.g. `fixes #123`). - What other alternatives have you explored? - Are there any parts you think require more attention from reviewers? Also, please make sure you do the following: - Read the Contributing Guidelines at https://rolldown.rs/contribution-guide/. - Check that there isn't already a PR that solves the problem the same way. If you find a duplicate, please help us review it. - Update the corresponding documentation if needed. - Include relevant tests that fail without this PR but pass with it. If the tests are not included, explain why. Thank you for contributing to Rolldown! -->
00250db to
472971e
Compare
…#10112) ### Description Addresses the review feedback on #10107. Stacked on #10110. Two of the four comments need changes and they live here. The third comment (a failed scan leaves marks that later materialize edge list drift into the HMR importer sets) is fixed by #10110, which reverts all cache mutations of an aborted partial scan. The fourth comment (rescans mark every dependency even when the import set did not change) stays a follow-up, with one note for whoever picks it up: skipping scan output indexes in `merge` is not a pure win, because the loader refreshes `ModuleInfo` only for modules with non empty importers and the merge loop currently covers the empty case. #### Stale doc references `internal-docs/cache/implementation.md` still pointed at three `merge` call sites in `hmr_stage.rs`. The site near `:621` no longer exists. The `Callers` section and the writers table rows for `merge` and `update_defer_sync_data` now cite the two real HMR call sites (the update path and the lazy compile path). #### Restore runtime assertions of `delete_file_not_used_anymore` `"expectExecuted": false` is a fixture wide switch. Only the orphan recreation step needs it, but it also turned off the runtime assertions of the initial build and of every other step, making the whole fixture snapshot only. The fixture is now split: - `delete_file_not_used_anymore` keeps the executing steps (drop the import, then delete the file) and runs its runtime assertions again. - The new `hmr/recreate_orphan_module` fixture carries the full four step sequence (drop the import, delete the file, recreate it, re-import it) as snapshot only. It documents the current limitation that editing or recreating an orphaned module triggers a full reload until the orphan cleanup tracked in #7416 lands. The recreation step cannot stay in the executing fixture, because the re-import step needs the recreated file on disk, so the sequence only makes sense as a whole. <!-- - What is this PR solving? Write a clear and concise description. - Reference the issues it solves (e.g. `fixes #123`). - What other alternatives have you explored? - Are there any parts you think require more attention from reviewers? Also, please make sure you do the following: - Read the Contributing Guidelines at https://rolldown.rs/contribution-guide/. - Check that there isn't already a PR that solves the problem the same way. If you find a duplicate, please help us review it. - Update the corresponding documentation if needed. - Include relevant tests that fail without this PR but pass with it. If the tests are not included, explain why. Thank you for contributing to Rolldown! -->
## [1.1.5] - 2026-07-08 ### 🚀 Features - detect top-level import-binding reads as execution-order sensitive (#10180) by @hyf0 - sourcemap_filenames: add a sourcemapFileNames option (#9271) by @V1OL3TF0X - binding: record plugin hook result kind in tracing spans (#10154) by @IWANABETHATGUY - linking: skip side-effect-free modules in per-entry reachability (#10111) by @IWANABETHATGUY - improve error message for unresolved virtual imports (#10156) by @sapphi-red - add descriptive metadata to plugin API (#10106) by @sapphi-red - add `--configLoader=native` option (#10118) by @sapphi-red ### 🐛 Bug Fixes - improve invalid annotation warnings (#10185) by @hyf0 - keep deduplicated asset filenames stable once they can be observed (#10191) by @shulaoda - sourcemap_filenames: use public option name in pattern errors (#10188) by @IWANABETHATGUY - sourcemap_filenames: hash prepared sourcemap content (#10178) by @hyf0 - tree-shake unused circular declarators exported via export list (#10166) by @IWANABETHATGUY - dev: don't panic when an HMR rebuild hits an unresolved import (#10162) by @shulaoda - propagate errors from output.globals function (#9880) by @shulaoda - dev: revert cache mutations when a partial scan fails (#10110) by @shulaoda - dev: update importer relationships of cached modules in incremental build (#10107) by @shulaoda - hmr: fall back to full reload when a changed module is not registered as executed (#10132) by @shulaoda - chunk-optimizer: follow entry facade edges in runtime placement cycle check (#10101) by @hyf0 - dev: ignore watcher events after close (#10113) by @hyf0 - emit async wrapper for TLA modules under onDemandWrapping (#10086) by @IWANABETHATGUY - gate sideEffects:false modules' side effects on body demand (#10080) by @IWANABETHATGUY - rolldown_plugin_vite_resolve: return empty object for `browser: false` mapped modules (#10082) by @sapphi-red - reset the word-boundary state on newline in Hires::Boundary sourcemaps (#10025) by @shulaoda - trim an emptied chunk's outro/intro instead of skipping past it (#10029) by @shulaoda - test each edited chunk's own start against indent exclude ranges (#10026) by @shulaoda - preserve sourcemap mappings for indented lines when a CJS module shares the chunk (#10074) by @hyf0 ### 🚜 Refactor - separate tree-shaking side effects from execution order sensitivity (#10168) by @hyf0 - type construct_vite_preload_call to take an ObjectPattern (#10135) by @shulaoda - treeshake: single-source the own-export classification shared with the lazy-barrel loader (#10098) by @IWANABETHATGUY - dev: reuse Vite's bundledDev server (#10081) by @h-a-n-a - clippy: ban std HashMap/HashSet in favour of FxHashMap/FxHashSet (#10108) by @Boshen - treeshake: make body demand a second module bit instead of a stmt multimap (#10097) by @IWANABETHATGUY - seal used_symbol_refs by construction after its last writer (#10091) by @hyf0 - treeshake: replace inclusion mutual recursion with a worklist engine (#10096) by @IWANABETHATGUY - treeshake: split include_statements.rs into focused modules (#10095) by @IWANABETHATGUY - drop redundant is_user_defined filter on partitioned entries (#10050) by @shulaoda - project the retained export interface out of used_symbol_refs (#10089) by @hyf0 - track used external symbols separately from used_symbol_refs (#10088) by @hyf0 - make module namespace inclusion an explicit linking metadata field (#10087) by @hyf0 - rename statement evaluation metadata (#10078) by @hyf0 ### 📚 Documentation - virtual modules user-facing id convention (#10155) by @sapphi-red - cli: clarify disabling boolean/object flags like codeSplitting (#10153) by @IWANABETHATGUY - chore: remove Vite+ alpha banner (#10105) by @mdong1909 - write down the used_symbol_refs contract (#10090) by @hyf0 - dev/lazy: update design and implementation (#10079) by @h-a-n-a ### ⚡ Performance - ast_scanner: stop order-sensitivity checks once a module is flagged (#10190) by @IWANABETHATGUY - return impl ExactSizeIterator from slice-backed accessors (#10133) by @Boshen - binding: box dev and watcher napi futures (#10103) by @Boshen ### 🧪 Testing - move string_wizard replace unit tests to the JS magic-string suite (#10176) by @IWANABETHATGUY - dev: assert incremental scan state matches a fresh full build after each HMR step (#10115) by @shulaoda - dev: restore runtime assertions of delete_file_not_used_anymore (#10112) by @shulaoda - dev: fix flaky dev server tests in CI (#10152) by @h-a-n-a - add regression test for #10099 (lazyBarrel drops default-import binding but keeps its property reads) (#10109) by @IWANABETHATGUY ### ⚙️ Miscellaneous Tasks - deploy website to Void via GitHub OIDC (#10192) by @Boshen - deps: update oxc to 0.139.0 (#10161) by @shulaoda - deps: update test262 submodule for tests (#10160) by @rolldown-guard[bot] - rolldown_plugin_utils: remove dead asset-url and css scaffolding (#10131) by @shulaoda - deps: revert vite-plus to v0.2.1 (#10148) by @shulaoda - deps: update github actions (#10141) by @renovate[bot] - deps: update dependency rust to v1.96.1 (#10145) by @renovate[bot] - deps: update npm packages (#10142) by @renovate[bot] - deps: update rust crates (#10143) by @renovate[bot] - deps: update napi to v3.10.3 (#10121) by @renovate[bot] - rolldown_utils: remove unused time module (#10138) by @shulaoda - remove dead CopyModulePlugin::is_active method (#10129) by @shulaoda - remove dead LazyCompilationContext::is_lazy_module method (#10128) by @shulaoda - remove dead BuildDiagnostic::downcast_ref method (#10127) by @shulaoda - deps: update dependency vite-plus to v0.2.2 (#10084) by @renovate[bot] - deps: update rust crate oxc_sourcemap to v8.1.0 (#10122) by @renovate[bot] - deps: update crate-ci/typos action to v1.48.0 (#10124) by @renovate[bot] - enable more clippy restriction lints (#10114) by @Boshen - deps: update rust dependencies (#10100) by @Boshen - deps: update oxc resolver to v11.23.0 (#10083) by @renovate[bot] ###◀️ Revert - Revert "chore(deps): revert vite-plus to v0.2.1" (#10157) by @h-a-n-a - "fix(hmr): fall back to full reload when a changed module is not registered as executed (#10132)" (#10151) by @shulaoda ### ❤️ New Contributors * @V1OL3TF0X made their first contribution in [#9271](#9271) Co-authored-by: shulaoda <[email protected]>

Description
Part of #7416. Stacked on #10107. This PR makes a failed scan harmless to the incremental cache. The remaining items of #7416 are listed at the end.
Problem
A partial scan applies task results as they arrive. When the scan aborts (the typical case is a syntax error in an edited file), the cache is left half updated while the snapshot receives nothing, because
mergenever runs on the error path:Seeninmodule_id_to_idx, so they are treated as fresh even though their new content never landed. The next build silently serves their old code and the error disappears.importersedge list, which then disagrees with the snapshot.module_id_to_idxentry, but no slot in the snapshot table. Since new indexes are allocated from the map length andmergeappends at the table length, a leftover entry shifts every later module by one slot and corrupts all index based lookups.Concrete reproduction (the new fixture): break
b.js, then edita.js. The rebuild reports success, ships the pre-breakb.js, and the error overlay is gone.Fix
A failed partial scan now reverts its cache mutations, so the cache holds a single invariant: between builds it is either empty (no snapshot, only a full scan is possible) or a valid graph any partial scan can build on. There is no third "poisoned" state and no consumer has to reason about failed builds.
ModuleLoader::revert_partial_scanruns at the scan error exit. The snapshot is never touched by a scan, so it acts as the clean master copy everything else is recovered from:importersis re-derived from the snapshot (ScanStageCache::derive_importers_from_snapshot), since every edge is a resolved import record of some module. Slot order is not preserved, and consumers treat slots as sets. The rebuild cost is paid only on failure, the happy path pays nothing.module_id_to_idxentries end the scan at their pre-scan values (SeentoInvalidateback toSeenwith the same index), so only the keys inserted for newly discovered modules need removal. The loader records them innew_module_ids.modules_with_changed_importersis cleared, matching the reverted edge list.barrel_stateis restored from an upfront clone, taken only when lazy barrel is enabled. When disabled the state is empty and never mutated.ScanStageCache::pending_rescans, a work queue the next partial scan drains. Reverting alone would lose the knowledge that these files changed, and the next build would silently serve their old content. The queue keeps their errors surfacing until the files are fixed. Only files the graph still needs are queued: configured or emitted entries, and files something still imports on the freshest edge state. A broken file whose last import was removed by the very scan that failed is dropped; otherwise its retry would fail every later partial build that a fresh build of the same tree would pass, wedging the session until a restart (new fixturehmr/error_recovery/remove_import_of_broken_file).transform_dependenciesentries of the newly allocated (and now freed) module indices are dropped, so a later module reusing an index does not inherit them.Bundler::incremental_bundlenow picks the scan mode fromhas_snapshot()alone. A failed full scan needs no revert, its cache reset already leaves the empty state that forces a full retry.One adjacent panic became reachable and is now handled: a client calling
import.meta.hot.invalidate()for a module the current graph does not know (for example after a failed full build reset the cache) gets a full reload instead of a server panic. The same applies tocompile_lazy_entry: a lazy chunk request arriving after a failed full build now returns an error instead of panicking on the missing snapshot.Two consumers of the retry queue needed matching fixes:
HmrStage::compute_hmr_updatefolds the queued files into its stale and changed sets before computing client boundaries. Without this, an edit rolled back by a failed scan reached the server graph on the successful retry but never a client patch, leaving connected clients silently running the old module. The new fixturehmr/error_recovery/fix_broken_file_delivers_other_editsasserts the recovery patch contains both the fixed file and the edit made while broken.Plugin side state (
module_infos, JS side caches) is not reverted. Hooks that ran during the aborted scan already updated them. This residue is informational, keyed by module id, and overwritten by the next successful scan. The exception istransform_dependencies, which is keyed by module index: the entries of indices freed by the revert are dropped, as described above.Behavior
hmr/error_recovery/remove_import_of_broken_file: breakb.js, then remove theimport './b.js'froma.js. That build still surfaces the queued error once, then dropsb.jsfrom the queue since nothing imports it anymore, and the following edit builds normally again.hmr/error_recovery/edit_other_file_while_broken: breakb.js, edita.js(the error surfaces again instead of a fake success with stale code), fixb.js(builds succeed again, the queued edit ofa.jslands in the same scan), edita.jsonce more (a normal patch, proving the cache stayed incremental through the whole episode).Docs
The "Cache integrity on a failed build" section of
internal-docs/bundler-data-lifecycle/implementation.mdnow documents the revert and the two state invariant.internal-docs/cache/implementation.mdis updated for the new field and methods.Remaining items of #7416