Skip to content

fix: tree-shake dead dynamic imports to side-effect-free CJS modules#8529

Merged
IWANABETHATGUY merged 1 commit intomainfrom
03-03-test_empty_bits_debug_assertion_failure_case
Apr 3, 2026
Merged

fix: tree-shake dead dynamic imports to side-effect-free CJS modules#8529
IWANABETHATGUY merged 1 commit intomainfrom
03-03-test_empty_bits_debug_assertion_failure_case

Conversation

@sapphi-red
Copy link
Copy Markdown
Member

@sapphi-red sapphi-red commented Mar 3, 2026

Dead dynamic imports to side-effect-free CJS modules were not being tree-shaken, producing an unnecessary chunk. The CJS bailout was marking the module as included even though the dynamic entry would later be removed. The fix skips CJS bailout for top-level pure dynamic imports targeting side-effect-free modules.

Found this case while trying to make a smaller repro for #8522 with https://github.com/sapphi-red/zarara.

Playground link

@netlify
Copy link
Copy Markdown

netlify Bot commented Mar 3, 2026

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 9dab03e
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/69ce7ae0ec8857000927e950

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.

@sapphi-red sapphi-red removed their assignment Mar 3, 2026
@sapphi-red sapphi-red added the bug Something isn't working label Mar 3, 2026
@sapphi-red sapphi-red force-pushed the 03-03-test_empty_bits_debug_assertion_failure_case branch from 160f343 to 8865060 Compare March 16, 2026 13:46
@sapphi-red sapphi-red force-pushed the 03-03-test_empty_bits_debug_assertion_failure_case branch from 8865060 to 551af3b Compare March 24, 2026 02:52
@sapphi-red sapphi-red force-pushed the 03-03-test_empty_bits_debug_assertion_failure_case branch from 551af3b to 69cfc57 Compare April 2, 2026 05:38
@IWANABETHATGUY IWANABETHATGUY force-pushed the 03-03-test_empty_bits_debug_assertion_failure_case branch 2 times, most recently from 0c5c116 to 6f69e14 Compare April 2, 2026 10:31
@IWANABETHATGUY IWANABETHATGUY marked this pull request as ready for review April 2, 2026 14:19
@IWANABETHATGUY IWANABETHATGUY force-pushed the 03-03-test_empty_bits_debug_assertion_failure_case branch from 6f69e14 to 9dab03e Compare April 2, 2026 14:19
@IWANABETHATGUY IWANABETHATGUY requested review from hyf0 and shulaoda April 2, 2026 14:19
@IWANABETHATGUY IWANABETHATGUY self-assigned this Apr 2, 2026
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 2, 2026

Merging this PR will not alter performance

✅ 4 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing 03-03-test_empty_bits_debug_assertion_failure_case (9dab03e) with main (7bcb2e0)

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.

Copy link
Copy Markdown
Member

@shulaoda shulaoda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the title should be updated, as this is actually a bug fix.

@hyf0
Copy link
Copy Markdown
Member

hyf0 commented Apr 2, 2026

@sapphi-red Could you update the PR title and description to add more context?

@IWANABETHATGUY IWANABETHATGUY changed the title test: empty bits debug assertion failure case fix: tree-shake dead dynamic imports to side-effect-free CJS modules Apr 3, 2026
@sapphi-red
Copy link
Copy Markdown
Member Author

sapphi-red commented Apr 3, 2026

(just in case, the test case was made by me but the investigation + fix was done by @IWANABETHATGUY)

Copy link
Copy Markdown
Member

@hyf0 hyf0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just verified, the test will fail without this fix. Good job.

@IWANABETHATGUY IWANABETHATGUY merged commit 37405e7 into main Apr 3, 2026
32 checks passed
@IWANABETHATGUY IWANABETHATGUY deleted the 03-03-test_empty_bits_debug_assertion_failure_case branch April 3, 2026 06:47
This was referenced Apr 8, 2026
shulaoda added a commit that referenced this pull request Apr 8, 2026
## [1.0.0-rc.14] - 2026-04-08

### 🚀 Features

- rust: add `disable_panic_hook` feature to disable the panic hook (#9023) by @sapphi-red
- support inlineConst for CJS exports accessed through module.exports (#8976) by @h-a-n-a

### 🐛 Bug Fixes

- rolldown_plugin_vite_import_glob: normalize resolved alias path to prevent double slashes (#9032) by @shulaoda
- rolldown_plugin_vite_import_glob: follow symlinks in file scanning (#9000) by @Copilot
- wrap CJS entry modules for IIFE/UMD when using exports/module (#8999) by @IWANABETHATGUY
- emit separate __toESM bindings for mixed ESM/CJS external imports (#8987) by @IWANABETHATGUY
- tree-shake dead dynamic imports to side-effect-free CJS modules (#8529) by @sapphi-red
- skip inlining stale CJS export constants on module.exports reassignment (#8990) by @IWANABETHATGUY

### 🚜 Refactor

- generator: migrate ecma formatting from npx oxfmt to vp fmt (#9022) by @shulaoda
- generator: replace npx oxfmt with vp fmt for ecma formatting (#9021) by @shulaoda

### 📚 Documentation

- contrib-guide: mention that running tests on older Node.js version will have different stat results (#8996) by @claude

### ⚙️ Miscellaneous Tasks

- deps: update npm packages (#9002) by @renovate[bot]
- deps: update dependency @napi-rs/cli to v3.6.1 (#9034) by @renovate[bot]
- deps: upgrade oxc to 0.124.0 (#9018) by @shulaoda
- deps: update test262 submodule for tests (#9010) by @sapphi-red
- deps: update dependency oxfmt to ^0.44.0 (#9012) by @renovate[bot]
- deps: update dependency vite to v8.0.5 [security] (#9009) by @renovate[bot]
- deps: update dependency vite-plus to v0.1.16 (#9008) by @renovate[bot]
- deps: update rust crates (#9003) by @renovate[bot]
- deps: update github-actions (#9004) by @renovate[bot]
- deps: update dependency lodash-es to v4.18.1 [security] (#8992) by @renovate[bot]
- deps: update crate-ci/typos action to v1.45.0 (#8988) by @renovate[bot]
- upgrade oxc npm packages to 0.123.0 (#8985) by @shulaoda

### ◀️ Revert

- "chore(deps): update dependency oxfmt to ^0.44.0 (#9012)" (#9019) by @shulaoda

Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants