Skip to content

fix(hmr): report the full-reload reason for the invalidate-loop case#9708

Merged
graphite-app[bot] merged 1 commit into
mainfrom
hmr-invalidate-full-reload-reason
Jun 10, 2026
Merged

fix(hmr): report the full-reload reason for the invalidate-loop case#9708
graphite-app[bot] merged 1 commit into
mainfrom
hmr-invalidate-full-reload-reason

Conversation

@hyf0

@hyf0 hyf0 commented Jun 10, 2026

Copy link
Copy Markdown
Member

Re-enables the missing full-reload reason for the invalidate-loop case.

Problem

  • When an HMR update propagates back to the module that already called import.meta.hot.invalidate() for the same update, rolldown falls back to a full reload.
  • Unlike the other two full-reload branches (circular import chain: ..., no hmr boundary found for module `...` ), this one reports "Unknown reason".

History

Fix

  • Re-enable the assignment, upgrading the message to carry the invalidating module's id (matching the sibling branches' detail level):

    update propagated back to `<module id>`, which already called `import.meta.hot.invalidate()`
    

Blast radius

  • Exactly one string — no consumer parses the reason text, and nothing in-repo asserts on it.
  • The branch is only reachable when an integrator passes firstInvalidatedBy to devEngine.invalidate() (e.g. rolldown-vite), so there is zero in-repo test/snapshot impact.

🤖 Generated with Claude Code

hyf0 commented Jun 10, 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.

@netlify

netlify Bot commented Jun 10, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs canceled.

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

@hyf0 hyf0 marked this pull request as ready for review June 10, 2026 07:36
Copilot AI review requested due to automatic review settings June 10, 2026 07:36

hyf0 commented Jun 10, 2026

Copy link
Copy Markdown
Member Author

Merge activity

  • Jun 10, 7:36 AM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jun 10, 9:58 AM UTC: hyf0 added this pull request to the Graphite merge queue.
  • Jun 10, 10:04 AM UTC: Merged by the Graphite merge queue.

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 fixes HMR full-reload reporting for the “invalidate-loop” scenario: when an update propagates back to the module that already called import.meta.hot.invalidate() for the same update, rolldown correctly falls back to a full reload but now provides a specific reason string instead of defaulting to “Unknown reason”.

Changes:

  • Re-introduces assignment of full_reload_reason for the invalidate-loop branch to include the invalidating module’s stable id in the message.

@hyf0 hyf0 requested a review from h-a-n-a June 10, 2026 09:53
…9708)

Re-enables the missing full-reload reason for the invalidate-loop case.

### Problem

- When an HMR update propagates back to the module that already called `import.meta.hot.invalidate()` for the same update, rolldown falls back to a full reload.
- Unlike the other two full-reload branches (`circular import chain: ...`, ``no hmr boundary found for module `...` ``), this one reports **"Unknown reason"**.

### History

- #4339 introduced the branch with the reason assignment **live**.
- #5400 extracted the logic into a helper and rewrote the sibling branches onto the new `reason: &mut Option<String>` out-param — this one assignment was commented out instead of rewritten.
- The comment then survived two further refactors (#5748, #6442) even after `full_reload_reason` came back into scope.

### Fix

- Re-enable the assignment, upgrading the message to carry the invalidating module's id (matching the sibling branches' detail level):

  ```
  update propagated back to `<module id>`, which already called `import.meta.hot.invalidate()`
  ```

### Blast radius

- Exactly one string — no consumer parses the reason text, and nothing in-repo asserts on it.
- The branch is only reachable when an integrator passes `firstInvalidatedBy` to `devEngine.invalidate()` (e.g. rolldown-vite), so there is zero in-repo test/snapshot impact.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@graphite-app graphite-app Bot force-pushed the hmr-invalidate-full-reload-reason branch from f2c32a4 to eec3506 Compare June 10, 2026 09:58
@graphite-app graphite-app Bot merged commit eec3506 into main Jun 10, 2026
30 of 31 checks passed
@graphite-app graphite-app Bot deleted the hmr-invalidate-full-reload-reason branch June 10, 2026 10:04
@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