Skip to content

fix: extracted comments should be behind the shebang#12380

Merged
CPunisher merged 3 commits intomainfrom
12-05-fix/extract-comments-with-hashbang
Dec 5, 2025
Merged

fix: extracted comments should be behind the shebang#12380
CPunisher merged 3 commits intomainfrom
12-05-fix/extract-comments-with-hashbang

Conversation

@CPunisher
Copy link
Copy Markdown
Contributor

Summary

Currently, rspack may produce the code where extracted comments are on the first line before shebang:

/*! LICENSE */
#!/usr/bin/env node

We should check if the first line is shebang, and find the property place to insert extracted comments.

Ref: https://github.com/webpack/terser-webpack-plugin/blob/30182a343893ef2b56d755260d697d9aa7db4060/src/index.js#L603-L616

cc @Timeless0911 for visibility.

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).

Copilot AI review requested due to automatic review settings December 5, 2025 07:31
@netlify
Copy link
Copy Markdown

netlify bot commented Dec 5, 2025

Deploy Preview for rspack canceled.

Name Link
🔨 Latest commit 6e6d664
🔍 Latest deploy log https://app.netlify.com/projects/rspack/deploys/69328bb13d6d73000835b90f

@github-actions github-actions bot added release: bug fix release: bug related release(mr only) team The issue/pr is created by the member of Rspack. labels Dec 5, 2025
Copy link
Copy Markdown
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 fixes an issue where extracted comments (license headers) were incorrectly placed before the shebang line in minified JavaScript files. The fix ensures that when both a shebang (#!/usr/bin/env node) and extracted comments exist, the shebang remains on the first line followed by the banner containing extracted comments.

Key Changes:

  • Modified the minimizer to detect and extract shebangs before prepending comment banners
  • Restructured the source concatenation logic to handle shebang positioning correctly
  • Added comprehensive test coverage for CJS and ESM output formats with extracted comments

Reviewed changes

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

Show a summary per file
File Description
crates/rspack_plugin_swc_js_minimizer/src/lib.rs Implements shebang detection and proper ordering logic - extracts shebang from minified code when banner exists and prepends it before the banner
tests/rspack-test/configCases/rslib/hashbang-and-extract-comments/test.js Adds test cases to verify shebang appears at the first line for CJS and ESM outputs
tests/rspack-test/configCases/rslib/hashbang-and-extract-comments/test.config.js Configures test runner to use bundle3.js as the test executor
tests/rspack-test/configCases/rslib/hashbang-and-extract-comments/rspack.config.js Sets up three compilation configurations (CJS, ESM without EsmLibraryPlugin, ESM with EsmLibraryPlugin) with extractComments enabled
tests/rspack-test/configCases/rslib/hashbang-and-extract-comments/index.js Provides test source file with shebang that imports module with license comments
tests/rspack-test/configCases/rslib/hashbang-and-extract-comments/a.js Provides module with various license comment formats to test comment extraction

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@CPunisher CPunisher enabled auto-merge (squash) December 5, 2025 07:43
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Dec 5, 2025

📦 Binary Size-limit

Comparing 6e6d664 to refactor: render javascript runtime globals according to compiler options (#12371) by harpsealjs

❌ Size increased by 1.75KB from 47.69MB to 47.69MB (⬆️0.00%)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Dec 5, 2025

Rsdoctor Bundle Diff Analysis

Found 5 project(s) in monorepo.

📁 react-10k

Path: ../build-tools-performance/cases/react-10k/dist/rsdoctor-data.json

📌 Baseline Commit: cd22df7788 | PR: #12371

Metric Current Baseline Change
📊 Total Size 5.7 MB 5.7 MB 0 B (0.0%)
📄 JavaScript 5.7 MB 5.7 MB 0 B (0.0%)
🎨 CSS 21.0 B 21.0 B 0 B (0.0%)
🌐 HTML 0 B 0 B N/A
📁 Other Assets 0 B 0 B N/A

📦 Download Diff Report: react-10k Bundle Diff

📁 react-1k

Path: ../build-tools-performance/cases/react-1k/dist/rsdoctor-data.json

📌 Baseline Commit: cd22df7788 | PR: #12371

Metric Current Baseline Change
📊 Total Size 823.6 KB 823.6 KB 0 B (0.0%)
📄 JavaScript 823.6 KB 823.6 KB 0 B (0.0%)
🎨 CSS 0 B 0 B N/A
🌐 HTML 0 B 0 B N/A
📁 Other Assets 0 B 0 B N/A

📦 Download Diff Report: react-1k Bundle Diff

📁 ui-components

Path: ../build-tools-performance/cases/ui-components/dist/rsdoctor-data.json

📌 Baseline Commit: cd22df7788 | PR: #12371

Metric Current Baseline Change
📊 Total Size 2.1 MB 2.1 MB 0 B (0.0%)
📄 JavaScript 2.0 MB 2.0 MB 0 B (0.0%)
🎨 CSS 83.0 KB 83.0 KB 0 B (0.0%)
🌐 HTML 0 B 0 B N/A
📁 Other Assets 0 B 0 B N/A

📦 Download Diff Report: ui-components Bundle Diff

📁 rome

Path: ../build-tools-performance/cases/rome/dist/rsdoctor-data.json

📌 Baseline Commit: cd22df7788 | PR: #12371

Metric Current Baseline Change
📊 Total Size 984.3 KB 984.3 KB 0 B (0.0%)
📄 JavaScript 984.3 KB 984.3 KB 0 B (0.0%)
🎨 CSS 0 B 0 B N/A
🌐 HTML 0 B 0 B N/A
📁 Other Assets 0 B 0 B N/A

📦 Download Diff Report: rome Bundle Diff

📁 react-5k

Path: ../build-tools-performance/cases/react-5k/dist/rsdoctor-data.json

📌 Baseline Commit: cd22df7788 | PR: #12371

Metric Current Baseline Change
📊 Total Size 2.7 MB 2.7 MB 0 B (0.0%)
📄 JavaScript 2.7 MB 2.7 MB 0 B (0.0%)
🎨 CSS 21.0 B 21.0 B 0 B (0.0%)
🌐 HTML 0 B 0 B N/A
📁 Other Assets 0 B 0 B N/A

📦 Download Diff Report: react-5k Bundle Diff

Generated by Rsdoctor GitHub Action

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Dec 5, 2025

CodSpeed Performance Report

Merging #12380 will not alter performance

Comparing 12-05-fix/extract-comments-with-hashbang (6e6d664) with main (cd22df7)

Summary

✅ 17 untouched

@CPunisher CPunisher merged commit 5041023 into main Dec 5, 2025
145 of 156 checks passed
@CPunisher CPunisher deleted the 12-05-fix/extract-comments-with-hashbang branch December 5, 2025 09:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release: bug fix release: bug related release(mr only) team The issue/pr is created by the member of Rspack.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants