test(hmr): delete file not used anymore by sapphi-red · Pull Request #5932 · rolldown/rolldown

sapphi-red · 2025-08-27T09:42:49Z

Make HMR work properly when a file is deleted.
This PR adds a test for the case where a file not used anymore is deleted.

sapphi-red · 2025-08-27T09:43:08Z

test(hmr): delete file not used anymore #5932 : 2 dependent PRs (#5933 , #7349 ) 👈 (View in Graphite)
main

How to use the Graphite Merge Queue

Add the label graphite: merge 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 · 2025-08-27T09:43:17Z

✅ Deploy Preview for rolldown-rs canceled.

Name	Link
🔨 Latest commit	`b110890`
🔍 Latest deploy log	https://app.netlify.com/projects/rolldown-rs/deploys/6938fa3a93e2020008c949e4

crates/rolldown/tests/rolldown/topics/hmr/delete_file_not_used_anymore/artifacts.snap

github-actions · 2025-08-27T09:58:05Z

Benchmarks Rust

target: main(8c27e46)
pr: 08-27-test_hmr_delete_file_not_used_anymore(b110890)

group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.00     65.1±1.41ms        ? ?/sec    1.01     65.5±1.89ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.00     71.3±1.49ms        ? ?/sec    1.01     71.8±1.77ms        ? ?/sec
bundle/bundle@rome_ts                                        1.00    107.7±1.76ms        ? ?/sec    1.00    107.7±1.36ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.00    121.1±2.54ms        ? ?/sec    1.00    121.0±2.51ms        ? ?/sec
bundle/bundle@threejs                                        1.00     40.1±2.43ms        ? ?/sec    1.00     40.1±2.46ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.00     43.2±0.64ms        ? ?/sec    1.00     43.1±0.58ms        ? ?/sec
bundle/bundle@threejs10x                                     1.00    396.2±5.53ms        ? ?/sec    1.00    395.6±6.21ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.00    455.2±4.82ms        ? ?/sec    1.00    454.9±4.32ms        ? ?/sec
scan/scan@rome_ts                                            1.03     87.6±1.61ms        ? ?/sec    1.00     85.3±1.65ms        ? ?/sec
scan/scan@threejs                                            1.01     29.0±0.46ms        ? ?/sec    1.00     28.8±0.43ms        ? ?/sec
scan/scan@threejs10x                                         1.01    303.2±4.75ms        ? ?/sec    1.00    299.6±4.46ms        ? ?/sec

sapphi-red

Still WIP but at least it now passes the tests

crates/rolldown/tests/rolldown/topics/hmr/delete_file_not_used_anymore/artifacts.snap

crates/rolldown_dev/src/bundle_coordinator.rs

crates/rolldown_dev/src/bundling_task.rs

Copilot

Pull request overview

This PR enhances the HMR (Hot Module Replacement) system to properly handle file deletions. Previously, the system only tracked file paths as a set, but now it tracks both the file path and the type of change (Create, Update, or Delete) using WatcherChangeKind. The implementation includes a new test case demonstrating HMR behavior when a file that is no longer used is deleted.

Key changes:

Changed data structures from FxIndexSet<PathBuf> to FxIndexMap<PathBuf, WatcherChangeKind> throughout the codebase to track change types
Introduced #__DELETE__ annotation pattern for HMR test files to simulate file deletions
Updated HMR stage logic to propagate changes to importers when a module is deleted (rather than treating the deleted module as changed)

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`crates/rolldown_testing/src/integration_test.rs`	Updated to handle tuple iteration with WatcherChangeKind in changed_files
`crates/rolldown_testing/src/hmr_files.rs`	Added DELETE_ANNOTATION_RE regex pattern and logic to detect and handle file deletions in test scenarios
`crates/rolldown_dev/src/types/task_input.rs`	Changed changed_files from FxIndexSet to FxIndexMap to track WatcherChangeKind
`crates/rolldown_dev/src/dev_engine.rs`	Updated test helper to generate appropriate notify events based on WatcherChangeKind
`crates/rolldown_dev/src/bundling_task.rs`	Updated to iterate over (path, event) tuples and pass WatcherChangeKind to plugin hooks
`crates/rolldown_dev/src/bundle_coordinator.rs`	Enhanced event handling to map different notify event types to corresponding WatcherChangeKind values
`crates/rolldown/src/hmr/hmr_stage.rs`	Added logic to propagate changes to importers when a module is deleted
`crates/rolldown/src/bundler/impl_bundler_hmr.rs`	Updated signature to accept FxIndexMap with WatcherChangeKind
`crates/rolldown/tests/rolldown/topics/hmr/delete_file_not_used_anymore/*`	New test case files demonstrating file deletion HMR behavior
`crates/rolldown/tests/snapshots/integration_rolldown__filename_with_hash.snap`	Updated snapshot with new test case hash

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

crates/rolldown_dev/src/dev_engine.rs

crates/rolldown_testing/src/hmr_files.rs

crates/rolldown/src/bundler/impl_bundler_hmr.rs

hyf0 · 2025-12-09T15:05:24Z

Does the AST of removed modules still exist? If so, I think we need an issue to track it how to clean up properly.

sapphi-red · 2025-12-10T04:27:17Z

Does the AST of removed modules still exist? If so, I think we need an issue to track it how to clean up properly.

I guess so. I'll create an issue that tracks these problems.

graphite-app · 2025-12-10T04:27:32Z

Merge activity

Dec 10, 4:27 AM UTC: hyf0 added this pull request to the Graphite merge queue.
Dec 10, 4:57 AM UTC: The Graphite merge queue couldn't merge this PR because it timed out.
Dec 10, 5:54 AM UTC: sapphi-red added this pull request to the Graphite merge queue.
Dec 10, 5:54 AM UTC: Merged by the Graphite merge queue.

Make HMR work properly when a file is deleted. This PR adds a test for the case where a file not used anymore is deleted.

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

crates/rolldown_dev/src/dev_engine.rs

@shulaoda

## [1.0.0-beta.54] - 2025-12-11 ⏱️ Plugin Timing Analysis - Added `checks.pluginTimings` option to emit warnings when plugins significantly impact build performance - More details: https://rolldown.rs/options/checks#plugintimings <img width="834" height="119" alt="image" src="https://github.com/user-attachments/assets/e819f906-6192-4503-8d5c-854fa92ecb45" /> > [!WARNING] > For hooks using `ctx.resolve()` or `ctx.load()`, the reported time includes waiting for other plugins, which may overestimate that plugin's actual cost. > > Additionally, since plugin hooks execute concurrently, the statistics represent accumulated time rather than wall-clock time. The measured duration also includes Rust-side processing overhead, Tokio async scheduling overhead, NAPI data conversion overhead, and JavaScript event loop overhead. ### 🚀 Features - rolldown_plugin_vite_reporter: add newline after build summary for better log separation (#7458) by @shulaoda - plugin: collect plugin hook execution timings (#7364) by @shulaoda - test-dev-server: reload page when detecting hmr reload message (#7422) by @hyf0 - rolldown_plugin_vite_dynamic_import_vars: add transform-based v2 implementation (#7400) by @shulaoda - rolldown_plugin_vite_import_glob: add transform-based v2 implementation (#7394) by @shulaoda - rolldown_plugin_vite_wasm_helper: add v2 implementation (#7402) by @shulaoda - expose error location and context fields on JS API error objects (#7341) by @Copilot - add `CIRCULAR_REEXPORT` error (#7337) by @Copilot - support emit prebuilt chunk (#7277) by @Copilot ### 🐛 Bug Fixes - preserve object key order when parse json with serde_json (#7443) by @IWANABETHATGUY - improve JSON parsing with serde_json to emit proper diagnostic (#7442) by @IWANABETHATGUY - deconflict external symbols in CJS modules (#7447) by @IWANABETHATGUY - rolldown_plugin_vite_transform,rolldown_plugin_vite_resolve: enable `yarnPnp` option when pnp is detected (#5791) by @sapphi-red - skip deconflicting top-level symbols for CJS modules (#7425) by @IWANABETHATGUY - rolldown_plugin_esm_external_require: run resolveId hook before other plugins (#7426) by @shulaoda - avoid duplicate underscores in legitimized identifiers (#7418) by @IWANABETHATGUY - use `process.on('exit')` instead of `signal-exit` on webcontainers (#7421) by @sapphi-red - dev: remove `imports` when an import is removed (#7348) by @sapphi-red - bench: access latency.mean instead of mean in tinybench result (#7417) by @shulaoda - path compression in symbol linking (#7392) by @IWANABETHATGUY - rolldown_plugin_vite_resolve: add RwLock to avoid clearing cache while resolving (#7386) by @sapphi-red - handle JSON prototype properties correctly (#7383) by @IWANABETHATGUY - preserve entry signature strict chunk merging (#7343) by @IWANABETHATGUY - support eliminating multiple unused dynamic imports in a single statement (#7361) by @IWANABETHATGUY - eliminate unreachable dynamic entry (#7356) by @IWANABETHATGUY - `NormalizedInputOptions#cwd` should always exists (#7360) by @hyf0 - cli: support multiple comma-separated define arguments (#7340) by @Copilot - remove redundant symbol param in __reExport runtime helper (#7346) by @IWANABETHATGUY - always use __export for empty namespace objects when symbols enabled (#7345) by @IWANABETHATGUY - relax the restriction of preserveEntrySignatures when merging chunks (#7339) by @IWANABETHATGUY ### 🚜 Refactor - builtin-plugin: make config parameter required for vite plugins (#7451) by @shulaoda - move esbuild test related scripts to `scripts/src/esbuild-tests` (#7377) by @sapphi-red - rewrite gen-esbuild-test in TypeScript (#7376) by @sapphi-red - remove unnecessary Option in current_stmt_idx (#7359) by @IWANABETHATGUY - move more code into chunk optimizer (#7335) by @IWANABETHATGUY ### 📚 Documentation - checks: clarify pluginTimings measures CPU time due to concurrent execution (#7448) by @shulaoda - checks: add accuracy note to pluginTimings documentation (#7441) by @shulaoda - development-guide: add esbuild test description (#7439) by @sapphi-red - checks: expand `pluginTimings` documentation with detection mechanism (#7428) by @shulaoda - add checks options documentation (#7427) by @shulaoda - development-guide: add test262 integration test description (#7430) by @sapphi-red - guide: fix grammer in getting-started (#7331) by @jakeparis ### 🧪 Testing - rollup-tests: make `--grep` work and document it (#7431) by @sapphi-red - add a way to run some test262 test cases by name (#7429) by @sapphi-red - rolldown_plugin_vite_dynamic_import_vars: add test cases for v2 implementation (#7401) by @shulaoda - rolldown_plugin_vite_import_glob: add test cases for v2 implementation (#7395) by @shulaoda - triage new esbuild tests (#7405) by @sapphi-red - hmr: delete file used (#5933) by @sapphi-red - watch: add `watchChange` hook `create` / `delete` tests (#7349) by @sapphi-red - hmr: delete file not used anymore (#5932) by @sapphi-red - triage esbuild failed reasons (#7391) by @sapphi-red - change esbuild tests stats and diff generation (#7379) by @sapphi-red - dev: add `continuous generate hmr patch` and `debounce bundle` tests (#7368) by @hyf0 - add new esbuild tests (#7353) by @sapphi-red ### ⚙️ Miscellaneous Tasks - setup Node before updating snapshots for test262 update (#7435) by @sapphi-red - run tests when submodules are updated (#7455) by @sapphi-red - exclude NUL path pattern to prevent crash on Windows (#7450) by @IWANABETHATGUY - run cargo-test for esbuild script changes and update esbuild snapshot (#7440) by @sapphi-red - jsx_import_meta tests to `ignoreReasons` due to architectural limitation (#7434) by @Copilot - deps: update dependency oxlint-tsgolint to v0.8.5 (#7433) by @renovate[bot] - update test dependencies automatically (#7432) by @sapphi-red - remove unused code (#7420) by @IWANABETHATGUY - add back `just update-esbuild-diff` and add it to CI (#7404) by @sapphi-red - deps: update oxc-resolver to 11.15.0 (#7415) by @shulaoda - deps: update rust crate napi to v3.7.0 (#7393) by @renovate[bot] - fix esbuild compatibility metrics calculation (#7390) by @sapphi-red - support `tsconfig: true` in rust tests (#7389) by @sapphi-red - hide oxc runtime in snapshots (#7388) by @sapphi-red - deps: update oxc to v0.102.0 (#7385) by @camc314 - improve esbuild tests to download snapshots automatically (#7378) by @sapphi-red - deps: update dependency tinybench to v6 (#7371) by @renovate[bot] - deps: update actions/checkout action to v6 (#7370) by @renovate[bot] - deps: update rust crates (#7367) by @renovate[bot] - deps: update dependency oxlint to v1.32.0 (#7374) by @renovate[bot] - deps: update dependency oxlint-tsgolint to v0.8.4 (#7373) by @renovate[bot] - node/test-dev-server: support using custom html file (#7357) by @hyf0 - test: setup browser-based e2e test for `test-dev-server` (#7351) by @hyf0 - pin pnpm to version 10.23.0 (#7369) by @IWANABETHATGUY - deps: update dependency rolldown-plugin-dts to v0.18.3 (#7372) by @renovate[bot] - deps: update github-actions (#7365) by @renovate[bot] - deps: update npm packages (#7366) by @renovate[bot] - polish the comments about `SymbolRefFlags` (#7358) by @IWANABETHATGUY - deps: update dependency rolldown-plugin-dts to v0.18.2 (#7355) by @renovate[bot] - fix esbuild test generation script (#7352) by @sapphi-red - apply cargo shear suggestion (#7347) by @IWANABETHATGUY - deps: update oxc to v11.15.0 (#7344) by @renovate[bot] - deps: update dependency @napi-rs/cli to v3.5.0 (#7338) by @renovate[bot] - deps: update dependency oxlint to v1.31.0 (#7305) by @renovate[bot] - update rollup-tests results (#7333) by @sapphi-red - update rollup-tests Rollup version (#7332) by @sapphi-red - add ignore config for test262 submodule (#7326) by @Copilot ### ❤️ New Contributors * @jakeparis made their first contribution in [#7331](#7331) Co-authored-by: shulaoda <[email protected]>

sapphi-red mentioned this pull request Aug 27, 2025

test(hmr): delete file used #5933

Merged

sapphi-red commented Aug 27, 2025

View reviewed changes

sapphi-red requested a review from hyf0 August 27, 2025 09:48

sapphi-red force-pushed the 08-27-test_hmr_delete_file_not_used_anymore branch 2 times, most recently from 96ae019 to 7f66c17 Compare November 28, 2025 06:51

sapphi-red commented Nov 28, 2025

View reviewed changes

sapphi-red force-pushed the 08-27-test_hmr_delete_file_not_used_anymore branch 2 times, most recently from 68e118d to 802d7bd Compare December 5, 2025 12:50

sapphi-red mentioned this pull request Dec 5, 2025

fix(dev): remove imports when an import is removed #7348

Merged

sapphi-red force-pushed the 08-27-test_hmr_delete_file_not_used_anymore branch 2 times, most recently from a29b5d8 to 5e0a8fe Compare December 5, 2025 17:38

sapphi-red mentioned this pull request Dec 5, 2025

test(watch): add watchChange hook create / delete tests #7349

Merged

sapphi-red force-pushed the 08-27-test_hmr_delete_file_not_used_anymore branch from 5e0a8fe to 8d960cb Compare December 9, 2025 07:20

sapphi-red marked this pull request as ready for review December 9, 2025 08:53

Copilot AI review requested due to automatic review settings December 9, 2025 08:53

Copilot started reviewing on behalf of sapphi-red December 9, 2025 08:53 View session

Copilot AI reviewed Dec 9, 2025

View reviewed changes

sapphi-red requested review from hyf0 and removed request for hyf0 December 9, 2025 09:28

sapphi-red self-assigned this Dec 9, 2025

hyf0 approved these changes Dec 9, 2025

View reviewed changes

hyf0 reviewed Dec 9, 2025

View reviewed changes

crates/rolldown/src/bundler/impl_bundler_hmr.rs Show resolved Hide resolved

hyf0 approved these changes Dec 9, 2025

View reviewed changes

graphite-app bot added the graphite: merge label Dec 9, 2025

graphite-app bot pushed a commit that referenced this pull request Dec 10, 2025

test(hmr): delete file not used anymore (#5932)

d33f424

Make HMR work properly when a file is deleted. This PR adds a test for the case where a file not used anymore is deleted.

graphite-app bot force-pushed the 08-27-test_hmr_delete_file_not_used_anymore branch from 8d960cb to d33f424 Compare December 10, 2025 04:27

sapphi-red mentioned this pull request Dec 10, 2025

Properly update internal states for incremental builds #7416

Open

4 tasks

test(hmr): delete file not used anymore (#5932)

b110890

Make HMR work properly when a file is deleted. This PR adds a test for the case where a file not used anymore is deleted.

Copilot AI review requested due to automatic review settings December 10, 2025 04:42

graphite-app bot force-pushed the 08-27-test_hmr_delete_file_not_used_anymore branch from d33f424 to b110890 Compare December 10, 2025 04:42

Copilot AI reviewed Dec 10, 2025

View reviewed changes

graphite-app bot removed the graphite: merge label Dec 10, 2025

graphite-app bot reviewed Dec 10, 2025

View reviewed changes

crates/rolldown_dev/src/dev_engine.rs Show resolved Hide resolved

graphite-app bot merged commit b110890 into main Dec 10, 2025
29 checks passed

graphite-app bot deleted the 08-27-test_hmr_delete_file_not_used_anymore branch December 10, 2025 05:54

This was referenced Dec 11, 2025

release: v1.0.0-beta.54 #7454

Closed

release: v1.0.0-beta.54 #7461

Merged

Comments

Conversation

sapphi-red commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sapphi-red commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to use the Graphite Merge Queue

Uh oh!

netlify bot commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for rolldown-rs canceled.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Aug 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarks Rust

Uh oh!

sapphi-red left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hyf0 commented Dec 9, 2025

Uh oh!

sapphi-red commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

graphite-app bot commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sapphi-red commented Aug 27, 2025 •

edited

Loading

sapphi-red commented Aug 27, 2025 •

edited

Loading

netlify bot commented Aug 27, 2025 •

edited

Loading

github-actions bot commented Aug 27, 2025 •

edited

Loading

sapphi-red commented Dec 10, 2025 •

edited

Loading

graphite-app bot commented Dec 10, 2025 •

edited

Loading