Skip to content

Comments

fix!: align directive rendering with esbuild#4557

Merged
IWANABETHATGUY merged 2 commits intomainfrom
05-15-fix_align_direct_rendering_with_esbuild
May 15, 2025
Merged

fix!: align directive rendering with esbuild#4557
IWANABETHATGUY merged 2 commits intomainfrom
05-15-fix_align_direct_rendering_with_esbuild

Conversation

@IWANABETHATGUY
Copy link
Member

@IWANABETHATGUY IWANABETHATGUY commented May 15, 2025

Description

@IWANABETHATGUY Graphite App
Copy link
Member Author

IWANABETHATGUY commented May 15, 2025

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

@netlify
Copy link

netlify bot commented May 15, 2025 •
edited
Loading

✅ Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit bf0d574
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6825c48bd616590008863e6f

@IWANABETHATGUY IWANABETHATGUY force-pushed the 05-15-fix_align_direct_rendering_with_esbuild branch from 29a65c5 to 0a493da Compare May 15, 2025 09:21
@IWANABETHATGUY IWANABETHATGUY marked this pull request as ready for review May 15, 2025 09:28
Copilot AI review requested due to automatic review settings May 15, 2025 09:28
Copilot AI reviewed May 15, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR aligns the direct rendering behavior with esbuild by removing the redundant insertion and tracking of the "use strict" directive.

  • Removed "use strict" from test snapshots and source outputs in various module formats (UMD, IIFE, CJS, etc.).
  • Eliminated internal flags and functions related to handling the "use strict" directive to simplify the code generation process.
  • Updated related test snapshots to reflect these changes and maintain consistency with esbuild’s behavior.

Reviewed Changes

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

Show a summary per file
File Description
crates/rolldown/tests/esbuild/default/import_namespace_this_value/artifacts.snap Removed unnecessary "use strict" directive from the snapshot.
crates/rolldown/tests/esbuild/default/import_fs_node_common_js/bypass.md Removed "use strict" directive from the inline code sample.
crates/rolldown/tests/esbuild/default/import_fs_node_common_js/artifacts.snap Removed "use strict" directive from the snapshot.
crates/rolldown/tests/esbuild/default/hashbang_banner_use_strict_order/bypass.md Adjusted the placement and format of the "use strict" directive in the banner section.
crates/rolldown/tests/esbuild/default/hashbang_banner_use_strict_order/artifacts.snap Updated snapshot to reflect the new banner order with "use strict".
crates/rolldown/tests/esbuild/default/export_wildcard_fs_node_common_js/bypass.md Removed "use strict" from the inline JavaScript.
crates/rolldown/tests/esbuild/default/export_wildcard_fs_node_common_js/artifacts.snap Removed "use strict" from the snapshot.
crates/rolldown/tests/esbuild/default/export_forms_iife/bypass.md Removed the "use strict" directive from the IIFE wrapper.
crates/rolldown/tests/esbuild/default/export_forms_iife/artifacts.snap Removed "use strict" from the snapshot.
crates/rolldown/tests/esbuild/dce/preserve_directives_minify_bundle/bypass.md Removed "use strict" directive from the code sample.
crates/rolldown/tests/esbuild/dce/preserve_directives_minify_bundle/artifacts.snap Removed "use strict" from the snapshot.
crates/rolldown/src/utils/tweak_ast_for_scanning.rs Removed internal tracking for "use strict" in the preprocessor.
crates/rolldown/src/utils/pre_process_ecma_ast.rs Removed the assignment of the "contains_use_strict" flag.
crates/rolldown/src/module_loader/runtime_module_task.rs Removed updating of the "contains_use_strict" flag on the AST.
crates/rolldown/src/ecmascript/format/umd.rs Removed insertion of "use strict" during UMD generation.
crates/rolldown/src/ecmascript/format/iife.rs Removed insertion of "use strict" during IIFE generation.
crates/rolldown/src/ecmascript/format/esm.rs Filtered out the "use strict" directive from rendered directives.
crates/rolldown/src/ecmascript/format/cjs.rs Removed the logic to conditionally append "use strict" during CJS rendering.
crates/rolldown/src/types/generator.rs Removed the unused method that referenced "use strict".

@IWANABETHATGUY IWANABETHATGUY force-pushed the 05-15-fix_align_direct_rendering_with_esbuild branch from 6a7a2fb to 9b202bd Compare May 15, 2025 09:32
@IWANABETHATGUY IWANABETHATGUY changed the title fix: align direct rendering with esbuild fix: align directive rendering with esbuild May 15, 2025
@github-actions
Copy link
Contributor

github-actions bot commented May 15, 2025 •
edited
Loading

Benchmarks Rust

  • target: main(69aebee)
  • pr: 05-15-fix_align_direct_rendering_with_esbuild(bf0d574)
group                                                               pr                                     target
-----                                                               --                                     ------
bundle/bundle@multi-duplicated-top-level-symbol                     1.03     74.9±2.52ms        ? ?/sec    1.00     72.9±1.27ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-minify              1.03     96.6±2.18ms        ? ?/sec    1.00     93.7±0.93ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-minify-sourcemap    1.03    110.7±3.56ms        ? ?/sec    1.00    107.2±1.20ms        ? ?/sec
bundle/bundle@multi-duplicated-top-level-symbol-sourcemap           1.04     86.0±2.91ms        ? ?/sec    1.00     82.9±0.90ms        ? ?/sec
bundle/bundle@rome-ts                                               1.00    115.9±1.49ms        ? ?/sec    1.00    116.1±1.07ms        ? ?/sec
bundle/bundle@rome-ts-minify                                        1.01    192.8±3.21ms        ? ?/sec    1.00    190.2±3.01ms        ? ?/sec
bundle/bundle@rome-ts-minify-sourcemap                              1.03    231.8±5.70ms        ? ?/sec    1.00    224.9±2.84ms        ? ?/sec
bundle/bundle@rome-ts-sourcemap                                     1.07   137.8±14.51ms        ? ?/sec    1.00    128.9±1.50ms        ? ?/sec
bundle/bundle@threejs                                               1.02     40.9±0.47ms        ? ?/sec    1.00     40.0±0.53ms        ? ?/sec
bundle/bundle@threejs-minify                                        1.00     83.6±0.70ms        ? ?/sec    1.00     83.3±1.27ms        ? ?/sec
bundle/bundle@threejs-minify-sourcemap                              1.01     99.2±1.60ms        ? ?/sec    1.00     98.4±1.08ms        ? ?/sec
bundle/bundle@threejs-sourcemap                                     1.02     48.1±0.87ms        ? ?/sec    1.00     47.0±0.32ms        ? ?/sec
bundle/bundle@threejs10x                                            1.02   425.0±19.30ms        ? ?/sec    1.00    416.3±2.99ms        ? ?/sec
bundle/bundle@threejs10x-minify                                     1.01   1036.3±7.22ms        ? ?/sec    1.00   1027.3±3.46ms        ? ?/sec
bundle/bundle@threejs10x-minify-sourcemap                           1.00  1199.5±14.82ms        ? ?/sec    1.00  1199.2±11.17ms        ? ?/sec
bundle/bundle@threejs10x-sourcemap                                  1.01    490.7±6.28ms        ? ?/sec    1.00    486.1±2.73ms        ? ?/sec
remapping/remapping                                                 1.00     25.0±0.20ms        ? ?/sec    1.02     25.6±0.41ms        ? ?/sec
remapping/render-chunk-remapping                                    1.02     67.8±3.87ms        ? ?/sec    1.00     66.3±4.08ms        ? ?/sec
scan/scan@rome-ts                                                   1.00     90.6±0.80ms        ? ?/sec    1.01     91.7±1.69ms        ? ?/sec
scan/scan@threejs                                                   1.00     31.1±1.28ms        ? ?/sec    1.02     31.6±1.95ms        ? ?/sec
scan/scan@threejs10x                                                1.00    311.2±3.95ms        ? ?/sec    1.01    312.9±4.15ms        ? ?/sec

@IWANABETHATGUY IWANABETHATGUY force-pushed the 05-15-fix_align_direct_rendering_with_esbuild branch 2 times, most recently from 55287da to c7c48ff Compare May 15, 2025 10:20
@IWANABETHATGUY IWANABETHATGUY force-pushed the 05-15-fix_align_direct_rendering_with_esbuild branch from c7c48ff to bf0d574 Compare May 15, 2025 10:40
@IWANABETHATGUY IWANABETHATGUY changed the title fix: align directive rendering with esbuild fix!: align directive rendering with esbuild May 15, 2025
@IWANABETHATGUY IWANABETHATGUY added this pull request to the merge queue May 15, 2025
Merged via the queue into main with commit 709eb63 May 15, 2025
32 checks passed
@IWANABETHATGUY IWANABETHATGUY deleted the 05-15-fix_align_direct_rendering_with_esbuild branch May 15, 2025 11:25
@sapphi-red sapphi-red mentioned this pull request May 19, 2025
Closed
@IWANABETHATGUY IWANABETHATGUY mentioned this pull request Jun 12, 2025
Closed
@sapphi-red sapphi-red mentioned this pull request Nov 7, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@hyf0 hyf0 hyf0 approved these changes

Assignees

No one assigned

Labels

needs-triage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@IWANABETHATGUY @hyf0