Skip to content

Comments

fix(hmr): import.meta.hot.accept in patch file should work#5823

Merged
sapphi-red merged 1 commit intomainfrom
08-21-test_hmr_add_test_cases_for_changing_accept_module
Aug 22, 2025
Merged

fix(hmr): import.meta.hot.accept in patch file should work#5823
sapphi-red merged 1 commit intomainfrom
08-21-test_hmr_add_test_cases_for_changing_accept_module

Conversation

@sapphi-red
Copy link
Member

@sapphi-red sapphi-red commented Aug 21, 2025

Aligned the implementation with

fn visit_call_expression(&mut self, expr: &mut ast::CallExpression<'ast>) {
self.rewrite_hot_accept_call_deps(expr);
if let Some(new_expr) = expr
.callee
.as_identifier_mut()
.and_then(|ident_ref| self.try_rewrite_identifier_reference_expr(ident_ref, true))
{
expr.callee = new_expr;
}
walk_mut::walk_call_expression(self, expr);
}

to make import.meta.hot.accept in patch files work.

@sapphi-red sapphi-red force-pushed the 08-21-test_hmr_add_test_cases_for_changing_accept_module branch 2 times, most recently from 46086ae to d5d5bfc Compare August 21, 2025 07:31
@sapphi-red sapphi-red changed the title test(hmr): add test cases for changing accept module fix(hmr): import.meta.hot.accept in patch file should work Aug 21, 2025
@sapphi-red sapphi-red changed the title fix(hmr): import.meta.hot.accept in patch file should work fix(hmr): import.meta.hot.accept in patch file should work Aug 21, 2025
@github-actions
Copy link
Contributor

Benchmarks Rust

  • target: 08-21-fix_hmr_re-execute_the_boundary_module_rather_than_the_accepted_module(e2ca5c8)
  • pr: 08-21-test_hmr_add_test_cases_for_changing_accept_module(d5d5bfc)
group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.00     83.7±3.02ms        ? ?/sec    1.02     85.6±2.53ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.01     94.0±2.47ms        ? ?/sec    1.00     93.4±1.89ms        ? ?/sec
bundle/bundle@rome_ts                                        1.02    124.8±3.42ms        ? ?/sec    1.00    121.8±1.95ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.01    144.3±3.21ms        ? ?/sec    1.00    142.8±3.21ms        ? ?/sec
bundle/bundle@threejs                                        1.00     46.5±1.43ms        ? ?/sec    1.00     46.7±1.23ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.01     54.4±2.35ms        ? ?/sec    1.00     53.8±0.94ms        ? ?/sec
bundle/bundle@threejs10x                                     1.01    480.1±6.41ms        ? ?/sec    1.00    475.4±7.22ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.01    560.1±6.11ms        ? ?/sec    1.00    554.4±5.06ms        ? ?/sec
scan/scan@rome_ts                                            1.00     98.1±1.97ms        ? ?/sec    1.01     98.8±2.41ms        ? ?/sec
scan/scan@threejs                                            1.00     34.6±0.66ms        ? ?/sec    1.00     34.8±1.53ms        ? ?/sec
scan/scan@threejs10x                                         1.00    357.2±4.48ms        ? ?/sec    1.00    357.1±3.52ms        ? ?/sec

@sapphi-red sapphi-red changed the base branch from 08-21-fix_hmr_re-execute_the_boundary_module_rather_than_the_accepted_module to graphite-base/5823 August 21, 2025 09:56
@sapphi-red sapphi-red force-pushed the 08-21-test_hmr_add_test_cases_for_changing_accept_module branch from d5d5bfc to e4cdb29 Compare August 21, 2025 09:56
@sapphi-red sapphi-red changed the base branch from graphite-base/5823 to 08-21-fix_hmr_ensure_patch_file_name_to_be_unique August 21, 2025 09:56
@sapphi-red sapphi-red marked this pull request as ready for review August 21, 2025 10:02
@sapphi-red sapphi-red requested a review from hyf0 August 21, 2025 10:03
@graphite-app graphite-app bot changed the base branch from 08-21-fix_hmr_ensure_patch_file_name_to_be_unique to graphite-base/5823 August 21, 2025 16:58
github-merge-queue bot pushed a commit that referenced this pull request Aug 21, 2025
Patch file names were not ensured to be unique and caused flaky failures
in #5823.
The failure was caused by overriding a patch file with a newer one.

https://github.com/rolldown/rolldown/actions/runs/17120246519/job/48560035074#step:6:1526

The patch file name generation logic did not generate a unique name
because

- It uses
[`std::time::SystemTime`](https://doc.rust-lang.org/std/time/struct.SystemTime.html),
which is not monotonic.
- It uses millisecond precision.

This PR changes that logic to use a simple counter instead.
@sapphi-red sapphi-red force-pushed the 08-21-test_hmr_add_test_cases_for_changing_accept_module branch from e4cdb29 to 3d1ee2a Compare August 22, 2025 00:41
@graphite-app graphite-app bot changed the base branch from graphite-base/5823 to main August 22, 2025 00:41
@graphite-app
Copy link
Contributor

graphite-app bot commented Aug 22, 2025

Merge activity

  • Aug 22, 12:41 AM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.

@sapphi-red sapphi-red added this pull request to the merge queue Aug 22, 2025
Merged via the queue into main with commit 0078e84 Aug 22, 2025
26 of 31 checks passed
@sapphi-red sapphi-red deleted the 08-21-test_hmr_add_test_cases_for_changing_accept_module branch August 22, 2025 00:55
shulaoda added a commit that referenced this pull request Aug 25, 2025
## [1.0.0-beta.34] - 2025-08-25

### 💥 BREAKING CHANGES

- improve merging of top-level `transform` option with `tsconfig` (#5882) by @shulaoda
- support top-level `tsconfig` (#5842) by @shulaoda

### 🚀 Features

- rolldown_plugin_esm_external_require: export namespace directly for Node builtin modules (#5885) by @shulaoda
- rolldown_plugin_dynamic_import_vars: warn by default when errors occur (#5866) by @shulaoda
- support adding note for diagnostic (#5864) by @IWANABETHATGUY
- rolldown_plugin_vite_css_post: align partial css chunk logic (#5861) by @shulaoda
- setup `rolldown_binding_watcher` (#5859) by @hyf0
- rolldown_plugin_vite_css_post: align css url replace logic (#5857) by @shulaoda
- node/dev: binding for `DevEngine` (#5852) by @hyf0
- dev: introduce `DevEngine` to support build for devlopement scenario (#5808) by @hyf0
- Show a information log to tell built-in features if a plugin covered by built-in features are used (#5845) by @IWANABETHATGUY
- rolldown_plugin_vite_css_post: align partial css url replace logic (#5837) by @shulaoda
- rolldown_plugin_vite_css_post: align CSS chunk concatenation logic (#5836) by @shulaoda
- rolldown_plugin_transform: support `useDefineForClassFields=false` with `target>=es2022` (#5841) by @sapphi-red
- expose `esmExternalRequirePlugin` (#5810) by @shulaoda
- rolldown_plugin_esm_external_require: basic implementation (#5809) by @shulaoda
- rolldown: oxc v0.82.3 (#5807) by @Boshen
- expose `oxc_minifier` options (#5804) by @IWANABETHATGUY
- rolldown_plugin_require_to_import: initialize (#5797) by @shulaoda
- adjust codegen `initial_indent` for concatenateWrappedModule (#5779) by @IWANABETHATGUY
- rolldown: align default value of `option.context` (#5745) by @situ2001

### 🐛 Bug Fixes

- topLevelVar option removes class name, causing a TypeError in static block (#5888) by @IWANABETHATGUY
- a esm module required by other module can't be a inner module of concatenateModuleGroup (#5875) by @IWANABETHATGUY
- rolldown_plugin_transform: align `lang` logic correctly (#5874) by @shulaoda
- Runtime error occurs depending on the lazy import order. (#5873) by @IWANABETHATGUY
- when importee is ts or tsx adding potential false positive note for `missing_export` diagnostic (#5862) by @IWANABETHATGUY
- hmr: boundary and accept_via was reversed (#5843) by @sapphi-red
- returning result of this.resolve in resolveId hook impacts bundle size (#5851) by @IWANABETHATGUY
- rolldown_plugin_transform: set `typescript.removeClassFieldsWithoutInitializer` for `useDefineForClassFields=false` (#5840) by @sapphi-red
- hmr: `import.meta.hot.accept` in patch file should work (#5823) by @sapphi-red
- proper handle pife and profiler_names for concatenate_wrapped_modules (#5835) by @IWANABETHATGUY
- jsx preserve break component which is default export (#5751) by @shulaoda
- hmr: ensure patch file name to be unique (#5825) by @sapphi-red
- hmr: re-execute the boundary module rather than the accepted module (#5822) by @sapphi-red
- Rolldown cannot treeshake unused branch (#5829) by @IWANABETHATGUY
- `inlineConst` with constant propagation (#5826) by @IWANABETHATGUY
- don't mangle variable names when `minify: 'dce-only'` is used (#5830) by @sapphi-red
- return actual normalized minify options (#5818) by @IWANABETHATGUY
- plugin/vite-resolve: try non-prefixed index before prefixed index (#5801) by @sapphi-red
- throw a semantic error message instead of panic (#5796) by @AliceLanniste
- rolldown: run DCE on chunk when `minify: dce-only` (#5792) by @Boshen
- the import attribute has been removed. (#5794) by @IWANABETHATGUY
- don't match CRLF for `/./` (#5790) by @IWANABETHATGUY
- node: allow `output.topLevelVar` by options validator (#5789) by @sapphi-red
- node: allow `transform.jsx: 'preserve'` by options validator (#5781) by @sapphi-red
- preserve default export for `preserveModules` (#5780) by @shulaoda
- browser: sync exports (#5776) by @sxzz
- rolldown_plugin_transform: skip builtin transform for module id with null byte (#5775) by @hi-ogawa

### 🚜 Refactor

- move common transform types into rolldown_common (#5876) by @shulaoda
- rolldown_plugin_vite_css_post: extract `finalize_vite_css_urls` (#5860) by @shulaoda
- rust: make `NotifyWatcher` WASM-compatible (#5855) by @hyf0
- rust/dev: remove improper deref for `BuildDriver` (#5854) by @hyf0
- incremental: use `clone_in_with_semantic_ids` for program cloning (#5853) by @shulaoda
- rolldown_plugin_esm_external_require: tweak code (#5824) by @shulaoda
- improve `resolve_dependencies` (#5795) by @shulaoda
- simplify method calls in `PluginContext` (#5785) by @situ2001

### 📚 Documentation

- builtin-plugins: clarify behavior of `esmExternalRequirePlugin` (#5886) by @sapphi-red
- builtin-plugins: add documentation for `esmExternalRequirePlugin` (#5813) by @shulaoda
- support extracting doc for reference type (#5834) by @IWANABETHATGUY
- optimization.inlineConst (#5831) by @IWANABETHATGUY
- update description for output.minify (#5816) by @IWANABETHATGUY
- plugins: extract plugins into a separate section (#5812) by @shulaoda

### ⚡ Performance

- string_wizard: reduce allocation (#5793) by @Brooooooklyn

### 🧪 Testing

- hmr: import.meta.hot.accept cases (#5821) by @sapphi-red
- hmr: static import cases (#5820) by @sapphi-red
- hmr: no boundary full reload case (#5819) by @sapphi-red

### ⚙️ Miscellaneous Tasks

- deps: update `oxc-resolver` to v11.7.0 (#5889) by @shulaoda
- deps: lock file maintenance (#5880) by @renovate[bot]
- deps: lock file maintenance rust crates (#5881) by @renovate[bot]
- deps: update github-actions (#5877) by @renovate[bot]
- deps: lock file maintenance rust crates (#5879) by @renovate[bot]
- deps: lock file maintenance npm packages (#5878) by @renovate[bot]
- bump rolldown-ariadne (#5863) by @IWANABETHATGUY
- remove usage of `quote_expr`, `quote_stmt` (#5858) by @IWANABETHATGUY
- vite-tests: run all test suites even if some test suites failed (#5828) by @sapphi-red
- vite-tests: set `_VITE_TEST_JS_PLUGIN` instead of `_VITE_TEST_NATIVE_PLUGIN` (#5827) by @sapphi-red
- deps: use `vitepress@^2.0.0-alpha.12` instead (#5811) by @shulaoda
- deps: update dependency rolldown-plugin-dts to v0.15.7 (#5814) by @renovate[bot]
- add example with oxc transform styled components plugin (#5800) by @IWANABETHATGUY
- deprecate top-level `jsx` option in favor of `transform.jsx` (#5783) by @shulaoda
- deps: update crate-ci/typos action to v1.35.5 (#5786) by @renovate[bot]
- update default value of options.context (#5777) by @IWANABETHATGUY

### ◀️ Revert

- "fix: jsx preserve break component which is default export (#5764)" (#5856) by @shulaoda

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.

2 participants