fix(strict_execution_order): wrapped module should be included on demand by hyf0 · Pull Request #4687 · rolldown/rolldown

hyf0 · 2025-05-26T13:03:40Z

Description

This PR fixes the size problem of enabling strictExecutionOrder at #3981.

Fixes #3981.

hyf0 · 2025-05-26T13:03:56Z

fix(strict_execution_order): wrapped module should be included on demand #4687 👈 (View in Graphite)
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

netlify · 2025-05-26T13:04:23Z

✅ Deploy Preview for rolldown-rs canceled.

Name	Link
🔨 Latest commit	`bfca2c5`
🔍 Latest deploy log	https://app.netlify.com/projects/rolldown-rs/deploys/683558ab4ac87b00088e0895

Copilot

Pull Request Overview

This PR fixes the size issue when enabling strictExecutionOrder by changing when and how wrapped modules are pulled on demand. Key changes include:

Modifying variable naming and duplicate assertions in test files for module import order.
Updating side‐effect flags in the linking stages (wrapping.rs and reference_needed_symbols.rs) to ensure correct module initialization.
Introducing and propagating a new symbol chain map (normal_symbol_exports_chain_map) in several stages for improved export resolution.

Reviewed Changes

Copilot reviewed 63 out of 63 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
crates/rolldown/tests/esbuild/packagejson/package_json_browser_index_no_ext/bypass.md	Adjusts variable naming and assert order in test scenarios.
crates/rolldown/tests/esbuild/packagejson/package_json_browser_index_no_ext/artifacts.snap	Updates snapshot file in line with bypass.md changes.
crates/rolldown/tests/esbuild/importstar_ts/ts_import_star_and_common_js/diff.md	Removes a duplicate initialization call and adjusts logging of module export values.
crates/rolldown/tests/esbuild/importstar/import_star_and_common_js/diff.md	Switches from direct logging to assertions for module export validation.
crates/rolldown/tests/esbuild/default/top_level_await_forbidden_require*_bypass.md/artifacts.snap	Removes dead branch initializations to prevent unnecessary code execution.
crates/rolldown/tests/esbuild/default/minified_exports_and_module_format_common_js/*	Reorders and renames export variables for consistency in CommonJS formats.
crates/rolldown/tests/esbuild/default/exports_and_module_format_common_js/*	Fixes variable naming order and updates assertions to ensure expected output.
crates/rolldown/tests/esbuild/dce/package_json_side_effects_/	Removes duplicate module initialization calls and aligns side-effect handling.
crates/rolldown/src/stages/link_stage/wrapping.rs	Changes the side_effect flag from false to true to trigger module wrapping on demand.
crates/rolldown/src/stages/link_stage/tree_shaking/include_statements.rs	Extends symbol resolution by chaining normal symbol exports for statement inclusion.
crates/rolldown/src/stages/link_stage/reference_needed_symbols.rs	Updates side-effect condition assignment with added tracing info for reexport conditions.
crates/rolldown/src/stages/link_stage/mod.rs	Adds a new field for normal_symbol_exports_chain_map to support export chaining.
crates/rolldown/src/stages/link_stage/bind_imports_and_exports.rs	Assigns the newly created normal_symbol_exports_chain_map to the LinkStage instance.

Comments suppressed due to low confidence (3)

crates/rolldown/src/stages/link_stage/reference_needed_symbols.rs:178

The side-effect determination logic has been updated; please verify that the new condition correctly reflects the intended module initialization behavior.

stmt_info.side_effect = is_reexport_all || importee.side_effects.has_side_effects();

crates/rolldown/src/stages/link_stage/wrapping.rs:199

Changing the 'side_effect' flag to true may affect module initialization order. Please ensure that this change is thoroughly validated with integration tests.

side_effect: true,

crates/rolldown/tests/esbuild/packagejson/package_json_browser_index_no_ext/bypass.md:42

Duplicate assertion for browser$1 detected; please verify if two identical assertions are intentional.

assert.equal(browser$1, "browser");

crates/rolldown/src/stages/link_stage/bind_imports_and_exports.rs

packages/rolldown/package.json

crates/rolldown/src/stages/link_stage/bind_imports_and_exports.rs

...tests/esbuild/dce/package_json_side_effects_false_keep_bare_import_and_require_es6/bypass.md

crates/rolldown/tests/esbuild/default/exports_and_module_format_common_js/artifacts.snap

packages/rollup-tests/src/ignored-tests.js

It's getting rate limited, and I also read issues actively, so remove it.

github-actions · 2025-05-27T06:31:41Z

Benchmarks Rust

target: main(a002b24)
pr: 05-26-fix_strict_execution_order_wrapped_module_should_be_pulled_on_demand(bfca2c5)

group                                                               pr                                     target
-----                                                               --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol                     1.01     75.7±1.54ms        ? ?/sec    1.00     75.2±1.78ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-minify              1.00     94.8±1.06ms        ? ?/sec    1.03     98.1±2.91ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-minify-sourcemap    1.00    108.5±0.77ms        ? ?/sec    1.01    109.8±2.98ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap           1.00     84.9±0.82ms        ? ?/sec    1.00     85.1±1.57ms        ? ?/sec
bundle/bundle@rome-ts                                               1.00    115.5±0.95ms        ? ?/sec    1.01    116.5±1.29ms        ? ?/sec
bundle/bundle@rome-ts-minify                                        1.00    188.6±2.24ms        ? ?/sec    1.03    194.0±4.11ms        ? ?/sec
bundle/bundle@rome-ts-minify-sourcemap                              1.00    230.9±4.23ms        ? ?/sec    1.01    232.3±6.97ms        ? ?/sec
bundle/bundle@rome-ts-sourcemap                                     1.00    127.8±1.90ms        ? ?/sec    1.01    129.5±1.85ms        ? ?/sec
bundle/bundle@threejs                                               1.01     40.0±1.36ms        ? ?/sec    1.00     39.6±0.46ms        ? ?/sec
bundle/bundle@threejs-minify                                        1.02     82.8±1.53ms        ? ?/sec    1.00     81.2±0.90ms        ? ?/sec
bundle/bundle@threejs-minify-sourcemap                              1.07    102.6±1.54ms        ? ?/sec    1.00     96.3±0.83ms        ? ?/sec
bundle/bundle@threejs-sourcemap                                     1.02     46.7±0.60ms        ? ?/sec    1.00     46.0±0.35ms        ? ?/sec
bundle/bundle@threejs10x                                            1.02    421.7±2.36ms        ? ?/sec    1.00    414.2±4.38ms        ? ?/sec
bundle/bundle@threejs10x-minify                                     1.00   1005.2±4.03ms        ? ?/sec    1.00   1003.8±5.84ms        ? ?/sec
bundle/bundle@threejs10x-minify-sourcemap                           1.01   1190.7±6.11ms        ? ?/sec    1.00  1179.4±11.45ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                                  1.01    484.3±2.24ms        ? ?/sec    1.00    479.3±2.95ms        ? ?/sec
remapping/remapping                                                 1.01     27.2±0.94ms        ? ?/sec    1.00     27.0±0.64ms        ? ?/sec
remapping/render-chunk-remapping                                    1.05     73.1±5.27ms        ? ?/sec    1.00     69.8±5.00ms        ? ?/sec
scan/scan@rome-ts                                                   1.03     92.7±1.32ms        ? ?/sec    1.00     90.0±0.89ms        ? ?/sec
scan/scan@threejs                                                   1.00     31.1±0.96ms        ? ?/sec    1.03     31.9±1.38ms        ? ?/sec
scan/scan@threejs10x                                                1.00    307.9±2.91ms        ? ?/sec    1.00    308.7±3.62ms        ? ?/sec

github-actions bot added the needs-triage label May 26, 2025

hyf0 marked this pull request as ready for review May 26, 2025 13:05

Copilot AI review requested due to automatic review settings May 26, 2025 13:05

Copilot AI reviewed May 26, 2025

View reviewed changes

crates/rolldown/src/stages/link_stage/bind_imports_and_exports.rs Outdated Show resolved Hide resolved

graphite-app bot reviewed May 26, 2025

View reviewed changes

packages/rolldown/package.json Outdated Show resolved Hide resolved

hyf0 changed the title ~~fix(strict_execution_order): wrapped module should be pulled on demand~~ fix(strict_execution_order): wrapped module should be included on demand May 26, 2025

hyf0 force-pushed the 05-26-fix_strict_execution_order_wrapped_module_should_be_pulled_on_demand branch 3 times, most recently from f297242 to ec34a68 Compare May 26, 2025 17:42

hyf0 commented May 26, 2025

View reviewed changes

crates/rolldown/src/stages/link_stage/bind_imports_and_exports.rs Show resolved Hide resolved

hyf0 commented May 26, 2025

View reviewed changes

...tests/esbuild/dce/package_json_side_effects_false_keep_bare_import_and_require_es6/bypass.md Show resolved Hide resolved

hyf0 commented May 26, 2025

View reviewed changes

crates/rolldown/tests/esbuild/default/exports_and_module_format_common_js/artifacts.snap Show resolved Hide resolved

hyf0 mentioned this pull request May 26, 2025

Advanced chunking seems to revert any treeshaking #3981

Closed

Boshen force-pushed the 05-26-fix_strict_execution_order_wrapped_module_should_be_pulled_on_demand branch from ec34a68 to aa8f7e6 Compare May 27, 2025 04:00

hyf0 force-pushed the 05-26-fix_strict_execution_order_wrapped_module_should_be_pulled_on_demand branch 2 times, most recently from 5cb4012 to bcdfe7d Compare May 27, 2025 06:11

hyf0 commented May 27, 2025

View reviewed changes

packages/rollup-tests/src/ignored-tests.js Outdated Show resolved Hide resolved

chore(ci): remove stale bot

bfca2c5

It's getting rate limited, and I also read issues actively, so remove it.

hyf0 force-pushed the 05-26-fix_strict_execution_order_wrapped_module_should_be_pulled_on_demand branch from bcdfe7d to bfca2c5 Compare May 27, 2025 06:16

hyf0 requested a review from IWANABETHATGUY May 27, 2025 06:22

IWANABETHATGUY approved these changes May 27, 2025

View reviewed changes

Boshen added this pull request to the merge queue May 27, 2025

Merged via the queue into main with commit 7c21036 May 27, 2025
26 checks passed

Boshen deleted the 05-26-fix_strict_execution_order_wrapped_module_should_be_pulled_on_demand branch May 27, 2025 07:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(strict_execution_order): wrapped module should be included on demand#4687

fix(strict_execution_order): wrapped module should be included on demand#4687
Boshen merged 1 commit intomainfrom
05-26-fix_strict_execution_order_wrapped_module_should_be_pulled_on_demand

hyf0 commented May 26, 2025 •

edited

Loading

Uh oh!

hyf0 commented May 26, 2025

Uh oh!

netlify bot commented May 26, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented May 27, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

hyf0 commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

hyf0 commented May 26, 2025

Uh oh!

netlify bot commented May 26, 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!

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!

github-actions bot commented May 27, 2025

Benchmarks Rust

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hyf0 commented May 26, 2025 •

edited

Loading

netlify bot commented May 26, 2025 •

edited

Loading