Skip to content

Comments

fix(hmr): avoid panic after syntax error#4898

Merged
shulaoda merged 1 commit intomainfrom
fix/hmr-panic-after-syntax-error
Jun 12, 2025
Merged

fix(hmr): avoid panic after syntax error#4898
shulaoda merged 1 commit intomainfrom
fix/hmr-panic-after-syntax-error

Conversation

@sapphi-red
Copy link
Member

@sapphi-red sapphi-red commented Jun 10, 2025

Fixes the panic that happens after generating a HMR patch after syntax error.

@netlify
Copy link

netlify bot commented Jun 10, 2025

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 66aa027
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/684b05db4cf3920008028d1b

@github-actions
Copy link
Contributor

github-actions bot commented Jun 10, 2025

Benchmarks Rust

group                                                        pr                                     target
-----                                                        --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol              1.05     90.1±3.82ms        ? ?/sec    1.00     85.9±3.02ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap    1.06    101.5±2.18ms        ? ?/sec    1.00     95.5±2.14ms        ? ?/sec
bundle/bundle@rome_ts                                        1.02    124.9±2.35ms        ? ?/sec    1.00    122.1±1.68ms        ? ?/sec
bundle/bundle@rome_ts-sourcemap                              1.02    138.5±2.32ms        ? ?/sec    1.00    136.3±1.85ms        ? ?/sec
bundle/bundle@threejs                                        1.00     44.4±2.50ms        ? ?/sec    1.01     44.9±2.49ms        ? ?/sec
bundle/bundle@threejs-sourcemap                              1.01     50.7±2.34ms        ? ?/sec    1.00     50.3±0.71ms        ? ?/sec
bundle/bundle@threejs10x                                     1.03    469.5±7.78ms        ? ?/sec    1.00    457.0±4.61ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                           1.03    542.8±6.11ms        ? ?/sec    1.00    529.2±7.60ms        ? ?/sec
scan/scan@rome_ts                                            1.00     94.0±1.73ms        ? ?/sec    1.01     95.0±2.13ms        ? ?/sec
scan/scan@threejs                                            1.00     32.8±0.61ms        ? ?/sec    1.00     32.7±0.53ms        ? ?/sec
scan/scan@threejs10x                                         1.00    335.1±4.50ms        ? ?/sec    1.01    339.5±6.27ms        ? ?/sec

@sapphi-red sapphi-red changed the base branch from main to graphite-base/4898 June 10, 2025 13:16
@sapphi-red sapphi-red force-pushed the fix/hmr-panic-after-syntax-error branch from 01a794d to 78934fb Compare June 10, 2025 13:16
@sapphi-red sapphi-red changed the base branch from graphite-base/4898 to 06-10-test_include_error_in_hmr_test_snapshots June 10, 2025 13:16
Copy link
Member Author

sapphi-red commented Jun 10, 2025

Base automatically changed from 06-10-test_include_error_in_hmr_test_snapshots to main June 11, 2025 01:40
@sapphi-red sapphi-red force-pushed the fix/hmr-panic-after-syntax-error branch from 78934fb to c0d596f Compare June 11, 2025 10:22
@IWANABETHATGUY
Copy link
Member

I need some time to understand your code,maybe I should try to fix the panic in watch with incremental build enabled.

@sapphi-red
Copy link
Member Author

FYI the only essential change in this PR is #4898 (comment). The other parts are to make the compilation pass.

@sapphi-red sapphi-red changed the base branch from main to graphite-base/4898 June 12, 2025 09:56
@sapphi-red sapphi-red force-pushed the fix/hmr-panic-after-syntax-error branch from c0d596f to 705e545 Compare June 12, 2025 09:56
@sapphi-red sapphi-red changed the base branch from graphite-base/4898 to 06-12-fix_should_not_panic_when_meet_a_syntax_error_in_watch_mode June 12, 2025 09:56
@IWANABETHATGUY IWANABETHATGUY force-pushed the fix/hmr-panic-after-syntax-error branch from 705e545 to d685cb7 Compare June 12, 2025 10:20
github-merge-queue bot pushed a commit that referenced this pull request Jun 12, 2025
I thought using a **mutable reference** may be better than returning the
`cache` struct, so that we don't need to worry about forgetting to
reassign `cache` in the future.

realted to #4898
Base automatically changed from 06-12-fix_should_not_panic_when_meet_a_syntax_error_in_watch_mode to main June 12, 2025 10:21
@IWANABETHATGUY IWANABETHATGUY force-pushed the fix/hmr-panic-after-syntax-error branch from d685cb7 to 82d49e8 Compare June 12, 2025 10:22
@sapphi-red sapphi-red marked this pull request as ready for review June 12, 2025 11:01
@sapphi-red
Copy link
Member Author

sapphi-red commented Jun 12, 2025

Looks good to me. @IWANABETHATGUY would you approve this PR as I cannot approve it myself?

@IWANABETHATGUY IWANABETHATGUY added this pull request to the merge queue Jun 12, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 12, 2025
@IWANABETHATGUY IWANABETHATGUY added this pull request to the merge queue Jun 12, 2025
github-merge-queue bot pushed a commit that referenced this pull request Jun 12, 2025
Fixes the panic that happens after generating a HMR patch after syntax
error.
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 12, 2025
@IWANABETHATGUY IWANABETHATGUY force-pushed the fix/hmr-panic-after-syntax-error branch from 82d49e8 to 66aa027 Compare June 12, 2025 16:52
@shulaoda shulaoda enabled auto-merge June 12, 2025 16:55
@shulaoda shulaoda added this pull request to the merge queue Jun 12, 2025
Merged via the queue into main with commit b43cefb Jun 12, 2025
23 checks passed
@shulaoda shulaoda deleted the fix/hmr-panic-after-syntax-error branch June 12, 2025 17:11
github-merge-queue bot pushed a commit that referenced this pull request Jun 13, 2025
~~probably related to #4898,~~ found while trying
#4898 (comment)

This test fails only when tree shaking is enabled. Enabling tree shaking
with HMR is not supported and is disabled now. So I decided not to try
fixing it and only keep the tests for now.
github-actions bot pushed a commit that referenced this pull request Jun 15, 2025
## [1.0.0-beta.16] - 2025-06-15

### 🚀 Features

- rolldown: feat(rolldown): oxc v0.73.0 by @Boshen in [#4948](#4948)
- types: feat(types): generate HMR runtime types by @sapphi-red in [#4928](#4928)
- rolldown_plugin_build_import_analysis: feat(rolldown_plugin_build_import_analysis): align with rolldown-vite by @shulaoda in [#4931](#4931)
- feat: allow to configure`experimental#attachDebugInfo: 'none'` to disable generating `#region` comments by @TheAlexLichter in [#4918](#4918)

### 🐛 Bug Fixes

- fix: deduplicate entry point by @IWANABETHATGUY in [#4956](#4956)
- fix: diagnostic message on minified code is too long to be readable by @IWANABETHATGUY in [#4950](#4950)
- fix: deduplicated entry points when have same `id`, `name`, and `filename` by @IWANABETHATGUY in [#4947](#4947)
- fix: unbundle does not work with outExtensions by @IWANABETHATGUY in [#4945](#4945)
- types: fix(types): remove `undefined` from return type of `generateHmrPatch` and `hmrInvalidate` by @sapphi-red in [#4943](#4943)
- fix: handle error in `hmrInvalidate` by @sapphi-red in [#4942](#4942)
- rolldown_plugin_build_import_analysis: fix(rolldown_plugin_build_import_analysis): preload helper is imported even if it’s not needed by @shulaoda in [#4936](#4936)
- hmr: fix(hmr): avoid panic after syntax error by @sapphi-red in [#4898](#4898)
- strictExecutionOrder: fix(strictExecutionOrder): ensure initialization chain of entry exports by @hyf0 in [#4933](#4933)
- fix: should not panic when meet a syntax error in watch mode. by @IWANABETHATGUY in [#4926](#4926)
- fix: regression, id filter normalization for windows is not working by @IWANABETHATGUY in [#4922](#4922)
- rolldown_plugin_json: fix(rolldown_plugin_json): incorrect default JSON export by @shulaoda in [#4924](#4924)

### 🚜 Refactor

- refactor: avoid unnecessary collect by @IWANABETHATGUY in [#4949](#4949)
- refactor: get normalized options via binding by @IWANABETHATGUY in [#4938](#4938)
- refactor: include DevRuntime base class even if custom HMR implementation is passed by @sapphi-red in [#4929](#4929)

### 📚 Documentation

- docs: clarify minifier state by @TheAlexLichter in [#4953](#4953)
- docs: drafting code splitting by @hyf0 in [#4951](#4951)
- docs: add link to repl by @sxzz in [#4917](#4917)
- pluginutils: docs(pluginutils): add example to simple filter functions by @sapphi-red in [#4915](#4915)

### 🧪 Testing

- test: add testcase for normalized output options by @IWANABETHATGUY in [#4939](#4939)
- hmr: test(hmr): enable `expectExecuted` for `generatePatchError` test by @sapphi-red in [#4937](#4937)
- test: evaluate hmr tests in node by @sapphi-red in [#4930](#4930)
- test: add "non used export" hmr test by @sapphi-red in [#4900](#4900)
- test: enable expectExecuted in HMR tests by @sapphi-red in [#4927](#4927)

### ⚙️ Miscellaneous Tasks

- deps: chore(deps): lock file maintenance npm packages by @renovate[bot] in [#4866](#4866)
- infra: chore(infra): remove comments related to `cspell` by @shulaoda in [#4940](#4940)
- ci: remove cargo bench test by @IWANABETHATGUY in [#4925](#4925)
@Boshen Boshen mentioned this pull request Jun 15, 2025
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.

4 participants