Skip to content

fix(treeshaking): require should consider as having side effects#1949

Merged
hyf0 merged 1 commit intomainfrom
08-12-fix_treeshaking_require_should_consider_as_having_side_effects
Aug 11, 2024
Merged

fix(treeshaking): require should consider as having side effects#1949
hyf0 merged 1 commit intomainfrom
08-12-fix_treeshaking_require_should_consider_as_having_side_effects

Conversation

@hyf0
Copy link
Copy Markdown
Member

@hyf0 hyf0 commented Aug 11, 2024 •
edited
Loading

Description

require should consider having side-effect no matter what in current codebase.

require('no-side-effect'); // Shouldn't consider having side-effect but we haven't handled such cases.

@hyf0 Graphite App
Copy link
Copy Markdown
Member Author

hyf0 commented Aug 11, 2024 •
edited
Loading

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

Join @hyf0 and the rest of your teammates on Graphite Graphite

@netlify
Copy link
Copy Markdown

netlify Bot commented Aug 11, 2024 •
edited
Loading

✅ Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit e837829
🔍 Latest deploy log https://app.netlify.com/sites/rolldown-rs/deploys/66b8f9a1f338270008ad392f

@hyf0 hyf0 marked this pull request as ready for review August 11, 2024 17:46
@hyf0 hyf0 force-pushed the 08-12-fix_treeshaking_require_should_consider_as_having_side_effects branch from ea89c2c to e837829 Compare August 11, 2024 17:49
@hyf0 hyf0 mentioned this pull request Aug 11, 2024
Merged
@hyf0 hyf0 linked an issue Aug 11, 2024 that may be closed by this pull request
[Bug]: cjs dependency ends up empty when treeshake.moduleSideEffects: false is set #1917
Closed
@hyf0 hyf0 self-assigned this Aug 11, 2024
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 11, 2024

Benchmarks Rust

  • target: main(4dc05c3)
  • pr: 08-12-fix_treeshaking_require_should_consider_as_having_side_effects(e837829)
group                                                               pr                                     target
-----                                                               --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol                     1.00     51.6±0.78ms        ? ?/sec    1.02     52.8±1.65ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-minify              1.00     77.7±0.76ms        ? ?/sec    1.00     77.6±0.77ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-minify-sourcemap    1.00     94.6±1.12ms        ? ?/sec    1.00     94.6±1.32ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap           1.00     59.9±0.58ms        ? ?/sec    1.00     60.0±0.51ms        ? ?/sec
bundle/bundle@rome-ts                                               1.01    102.0±0.78ms        ? ?/sec    1.00    101.4±1.01ms        ? ?/sec
bundle/bundle@rome-ts-minify                                        1.00    183.1±2.34ms        ? ?/sec    1.00    182.8±1.62ms        ? ?/sec
bundle/bundle@rome-ts-minify-sourcemap                              1.01    225.8±6.05ms        ? ?/sec    1.00    223.8±3.83ms        ? ?/sec
bundle/bundle@rome-ts-sourcemap                                     1.01    117.5±1.25ms        ? ?/sec    1.00    116.6±0.88ms        ? ?/sec
bundle/bundle@threejs                                               1.00     36.2±1.03ms        ? ?/sec    1.00     36.2±1.55ms        ? ?/sec
bundle/bundle@threejs-minify                                        1.00     81.4±1.67ms        ? ?/sec    1.01     82.1±1.76ms        ? ?/sec
bundle/bundle@threejs-minify-sourcemap                              1.01     99.5±2.12ms        ? ?/sec    1.00     98.3±2.06ms        ? ?/sec
bundle/bundle@threejs-sourcemap                                     1.01     45.6±0.37ms        ? ?/sec    1.00     45.2±0.44ms        ? ?/sec
bundle/bundle@threejs10x                                            1.00    391.3±3.41ms        ? ?/sec    1.00    392.4±3.28ms        ? ?/sec
bundle/bundle@threejs10x-minify                                     1.00    990.4±7.51ms        ? ?/sec    1.00    992.3±9.67ms        ? ?/sec
bundle/bundle@threejs10x-minify-sourcemap                           1.00  1225.4±11.20ms        ? ?/sec    1.01  1238.9±15.49ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                                  1.00    459.5±6.38ms        ? ?/sec    1.00    458.4±1.94ms        ? ?/sec
remapping/remapping                                                 1.00     32.3±1.09ms        ? ?/sec    1.00     32.3±0.15ms        ? ?/sec
remapping/render-chunk-remapping                                    1.03     81.4±0.32ms        ? ?/sec    1.00     79.1±0.27ms        ? ?/sec
scan/scan@rome-ts                                                   1.00     81.7±1.05ms        ? ?/sec    1.00     82.0±1.05ms        ? ?/sec
scan/scan@threejs                                                   1.00     28.3±0.14ms        ? ?/sec    1.02     29.0±0.75ms        ? ?/sec
scan/scan@threejs10x                                                1.00    293.9±5.43ms        ? ?/sec    1.00    293.8±2.83ms        ? ?/sec

@hyf0 hyf0 added this pull request to the merge queue Aug 11, 2024
Merged via the queue into main with commit 28e53d6 Aug 11, 2024
@hyf0 hyf0 deleted the 08-12-fix_treeshaking_require_should_consider_as_having_side_effects branch August 11, 2024 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@github-actions github-actions[bot] github-actions[bot] approved these changes

Assignees

@hyf0 hyf0

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: cjs dependency ends up empty when treeshake.moduleSideEffects: false is set

1 participant

@hyf0