Skip to content

fix(hmr): fall back to full reload when a changed module is not registered as executed#10132

Merged
shulaoda merged 5 commits into
mainfrom
07-04-fix_hmr_fall_back_to_full_reload_when_a_changed_module_is_not_registered_as_executed
Jul 6, 2026
Merged

fix(hmr): fall back to full reload when a changed module is not registered as executed#10132
shulaoda merged 5 commits into
mainfrom
07-04-fix_hmr_fall_back_to_full_reload_when_a_changed_module_is_not_registered_as_executed

Conversation

@shulaoda

@shulaoda shulaoda commented Jul 4, 2026

Copy link
Copy Markdown
Member

closes #10149

Problem

Editing a file right after a page loads can be silently lost in full bundle mode. The dev engine decides whether a client needs an HMR update by checking the client's executed_modules set, but that set is reported asynchronously by the browser runtime (batched hmr:module-registered messages over the socket) and can lag what the client actually ran. When the edit is processed before the reports arrive, compute_out_hmr_prerequisites skipped the changed module for that client and produced an empty update. Vite's bundled dev ignores updates that carry no patches, and the HMR module cache had already advanced past the edit, so nothing ever re-delivers it. The page stays stale forever with no error.

This is what turned main red after #10084. The vite-plus 0.2.2 bump added CPU contention on the 4-core CI runners, which stretched the report lag past the watcher debounce window, so the first edit in playground/hmr-full-bundle-mode started losing the race in roughly one of five jobs (example run). The three test failures in that suite are a cascade from the single lost edit.

Fix

compute_out_hmr_prerequisites now receives the existing changed_modules set. When a changed module is not registered as executed by a client, the update for that client falls back to a full reload with an explicit reason instead of being skipped. The default RebuildStrategy::Auto upgrades the task to a rebuild, and Vite defers the reload until onOutput, so the reload always lands on fresh output.

The invalidate flow passes an empty changed_modules set and keeps the old skip. Nothing changed on disk there, so an unexecuted importer has nothing to re-run. The importer pruning inside propagate_update is also untouched, since that behavior is pinned by the client-module-execution-status fixture.

Known cost: with lazy compilation, a client that never fetched the changed lazy module now reloads unnecessarily. We cannot tell it apart from a client whose loaded bundle bakes in the now stale initializer without a bundle epoch in the protocol.

Tests

A new snapshot fixture topics/hmr/changed_module_not_executed_by_client drives an HMR step against a client with an empty executed_modules set, enabled by a new dev.unregisteredClient test config flag. On the old code the same fixture produces an empty patch (applyUpdates([])) instead of the full reload, so the snapshot pins the fix. All existing HMR snapshots, the full integration suite, and the node dev server fixtures pass unchanged.

Follow ups

Two narrower windows of the same class remain and are left for separate issues. A changed module that is already registered while its accept boundary importer is not can still yield an empty patch. And when no client is registered at all, the update list is empty and Vite still ignores it, which needs a check on the Vite side when a client first registers.

@netlify

netlify Bot commented Jul 4, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 0a5f8e0
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6a4b2ce9dfece50008d74294

@codspeed-hq

codspeed-hq Bot commented Jul 4, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 7 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing 07-04-fix_hmr_fall_back_to_full_reload_when_a_changed_module_is_not_registered_as_executed (0a5f8e0) with main (04fcbc1)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 main (99aae0e) during the generation of this report, so 04fcbc1 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

…a_changed_module_is_not_registered_as_executed
shulaoda added 2 commits July 6, 2026 09:56
…a_changed_module_is_not_registered_as_executed
…a_changed_module_is_not_registered_as_executed
shulaoda added a commit that referenced this pull request Jul 6, 2026
It exposed a potential issue (#10132), so revert it to avoid blocking CI.

Reverts the vite-plus `0.2.2` upgrade from #10084 (df05eba).

This pins `vite-plus` and its bundled oxc toolchain back to the `0.2.1`-era versions:

- `vite-plus` / `@voidzero-dev/vite-plus-*`: `0.2.2` -> `0.2.1`
- `oxfmt`: `0.57.0` -> `0.55.0`
- `oxlint`: `1.72.0` -> `1.70.0`
- `oxlint-tsgolint`: `0.24.0` -> `0.23.0`

Only the `vite-plus` dependency closure changed; no unrelated packages are touched.
…a_changed_module_is_not_registered_as_executed
@shulaoda
shulaoda merged commit c387fc5 into main Jul 6, 2026
35 checks passed
@shulaoda
shulaoda deleted the 07-04-fix_hmr_fall_back_to_full_reload_when_a_changed_module_is_not_registered_as_executed branch July 6, 2026 04:30
@h-a-n-a

h-a-n-a commented Jul 6, 2026

Copy link
Copy Markdown
Member

I would merge this PR as a workaround. The new design of HMR is going to fix this, which removes the lag and executed_modules itself.

h-a-n-a pushed a commit that referenced this pull request Jul 6, 2026
…not registered as executed (#10132)" (#10151)

Reverts #10132 (commit c387fc5).

## Why

The full-reload fallback added in #10132 did not actually solve the
intended problem, so it is being reverted rather than kept as dead
complexity. This also removes the extra `dev.unregisteredClient` test
config flag and its snapshot fixture.

The tree after this revert is identical to the state at 99aae0e (the
parent of #10132), so nothing else on `main` is affected.

## What is reverted

- The `changed_modules` fall-back-to-full-reload path in
`compute_out_hmr_prerequisites` (`crates/rolldown/src/hmr/hmr_stage.rs`)
- The `dev.unregisteredClient` test config flag (`_config.schema.json`,
`dev_test_meta.rs`, `integration_test.rs`, `dev_engine.rs`)
- The `topics/hmr/changed_module_not_executed_by_client` snapshot
fixture

The underlying issue #10149 remains open for a follow-up.
@rolldown-guard rolldown-guard Bot mentioned this pull request Jul 8, 2026
shulaoda added a commit that referenced this pull request Jul 8, 2026
## [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]>
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.

Infra: Flaky node dev server test workflow

2 participants