Skip to content

Commit 5041023

Browse files
CPunisherCopilot
andauthored
fix: extracted comments should be behind the shebang (#12380)
* Add tests * Fix * Update crates/rspack_plugin_swc_js_minimizer/src/lib.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent cd22df7 commit 5041023

File tree

6 files changed

+214
-25
lines changed

6 files changed

+214
-25
lines changed

crates/rspack_plugin_swc_js_minimizer/src/lib.rs

Lines changed: 72 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,7 @@ async fn process_assets(&self, compilation: &mut Compilation) -> Result<()> {
298298
}
299299
};
300300

301-
let output = match javascript_compiler.minify(
301+
let mut output = match javascript_compiler.minify(
302302
swc_core::common::FileName::Custom(filename.to_string()),
303303
input,
304304
js_minify_options,
@@ -316,33 +316,80 @@ async fn process_assets(&self, compilation: &mut Compilation) -> Result<()> {
316316
},
317317
};
318318

319-
let source = if let Some(source_map) = output.map {
320-
SourceMapSource::new(SourceMapSourceOptions {
321-
value: output.code,
322-
name: filename,
323-
source_map,
324-
original_source: None,
325-
inner_source_map: input_source_map,
326-
remove_original_source: true,
327-
})
328-
.boxed()
329-
} else {
330-
RawStringSource::from(output.code).boxed()
331-
};
332-
let source = if let Some(Some(banner)) = extract_comments_option.map(|option| option.banner)
333-
&& all_extracted_comments
319+
let banner = if all_extracted_comments
334320
.lock()
335321
.expect("all_extract_comments lock failed")
336-
.contains_key(filename)
337-
{
338-
ConcatSource::new([
339-
RawStringSource::from(banner).boxed(),
340-
RawStringSource::from_static("\n").boxed(),
341-
source
342-
]).boxed()
343-
} else {
344-
source
322+
.contains_key(filename) {
323+
extract_comments_option.and_then(|option| option.banner)
324+
} else {
325+
None
326+
};
327+
328+
let source = match banner {
329+
Some(banner) => {
330+
// There are two cases with banner:
331+
// 1. There's no shebang, we just prepend the banner to the code.
332+
// 2. There's a shebang, we prepend the shebang, then the banner, then the code.
333+
334+
let mut shebang = None;
335+
if output.code.starts_with("#!") {
336+
if let Some(line_pos) = output.code.find('\n') {
337+
shebang = Some(output.code[0..line_pos + 1].to_string());
338+
output.code = output.code[line_pos + 1..].to_string();
339+
} else {
340+
// Handle shebang without newline - treat entire content as shebang
341+
shebang = Some(output.code.clone());
342+
output.code = String::new();
343+
}
344+
}
345+
346+
let source = if let Some(source_map) = output.map {
347+
SourceMapSource::new(SourceMapSourceOptions {
348+
value: output.code,
349+
name: filename,
350+
source_map,
351+
original_source: None,
352+
inner_source_map: input_source_map,
353+
remove_original_source: true,
354+
})
355+
.boxed()
356+
} else {
357+
RawStringSource::from(output.code).boxed()
358+
};
359+
360+
if let Some(shebang) = shebang {
361+
ConcatSource::new([
362+
RawStringSource::from(shebang).boxed(),
363+
RawStringSource::from(banner).boxed(),
364+
RawStringSource::from_static("\n").boxed(),
365+
source
366+
]).boxed()
367+
} else {
368+
ConcatSource::new([
369+
RawStringSource::from(banner).boxed(),
370+
RawStringSource::from_static("\n").boxed(),
371+
source
372+
]).boxed()
373+
}
374+
},
375+
None => {
376+
// If there's no banner, we don't need to handle `output.code` at all.
377+
if let Some(source_map) = output.map {
378+
SourceMapSource::new(SourceMapSourceOptions {
379+
value: output.code,
380+
name: filename,
381+
source_map,
382+
original_source: None,
383+
inner_source_map: input_source_map,
384+
remove_original_source: true,
385+
})
386+
.boxed()
387+
} else {
388+
RawStringSource::from(output.code).boxed()
389+
}
390+
},
345391
};
392+
346393
original.set_source(Some(source));
347394
original.get_info_mut().minimized.replace(true);
348395
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/*! Legal Comment */
2+
3+
/**
4+
* @preserve Copyright 2009 SomeThirdParty.
5+
* Here is the full license text and copyright
6+
* notice for this file. Note that the notice can span several
7+
* lines and is only terminated by the closing star and slash:
8+
*/
9+
10+
/**
11+
* Utility functions for the foo package.
12+
* @license Apache-2.0
13+
*/
14+
15+
/*! Legal Foo */
16+
17+
// Foo
18+
19+
/*
20+
Foo Bar
21+
*/
22+
23+
// @license
24+
25+
/*
26+
* Foo
27+
*/
28+
// @lic
29+
30+
module.exports = "aaaa";
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#!/usr/bin/env node
2+
3+
import os from "os";
4+
import a from "./a.js";
5+
6+
export function hello() {
7+
console.log(a);
8+
return "hello from rslib hashbang" + os.platform();
9+
}
Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
const {
2+
rspack,
3+
experiments: { RslibPlugin, EsmLibraryPlugin }
4+
} = require("@rspack/core");
5+
6+
/** @type {import("@rspack/core").Configuration} */
7+
const baseConfig = {
8+
entry: {
9+
index: "./index.js"
10+
},
11+
target: "node",
12+
node: {
13+
__filename: false,
14+
__dirname: false
15+
},
16+
optimization: {
17+
minimize: true,
18+
minimizer: [
19+
new rspack.SwcJsMinimizerRspackPlugin({
20+
extractComments: true
21+
})
22+
]
23+
}
24+
};
25+
26+
module.exports = [
27+
// CJS output
28+
{
29+
...baseConfig,
30+
output: {
31+
library: {
32+
type: "commonjs"
33+
}
34+
},
35+
plugins: [new RslibPlugin()]
36+
},
37+
// ESM output (without EsmLibraryPlugin)
38+
{
39+
...baseConfig,
40+
experiments: {
41+
outputModule: true
42+
},
43+
externals: {
44+
os: "module os"
45+
},
46+
output: {
47+
module: true,
48+
library: {
49+
type: "modern-module"
50+
}
51+
},
52+
plugins: [new RslibPlugin()]
53+
},
54+
// ESM output (with EsmLibraryPlugin)
55+
{
56+
...baseConfig,
57+
experiments: {
58+
outputModule: true
59+
},
60+
externals: {
61+
os: "module os"
62+
},
63+
output: {
64+
module: true,
65+
library: {
66+
type: "modern-module"
67+
}
68+
},
69+
plugins: [new RslibPlugin(), new EsmLibraryPlugin()]
70+
},
71+
// Test entry
72+
{
73+
entry: "./test.js",
74+
target: "node",
75+
node: {
76+
__filename: false,
77+
__dirname: false
78+
}
79+
}
80+
];
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
/** @type {import("../../../..").TConfigCaseConfig} */
2+
module.exports = {
3+
findBundle: function (i, options) {
4+
return ["./bundle3.js"];
5+
}
6+
};
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
const path = require('path');
2+
const fs = require('fs');
3+
4+
const testCases = [
5+
{ name: 'CJS', file: 'bundle0.js' },
6+
{ name: 'ESM', file: 'bundle1.mjs' },
7+
{ name: 'ESM (with EsmLibraryPlugin)', file: 'bundle2.mjs' }
8+
];
9+
10+
testCases.forEach(({ name, file }) => {
11+
it(`should include hashbang at the first line (${name})`, () => {
12+
const filePath = path.resolve(__dirname, file);
13+
const content = fs.readFileSync(filePath, 'utf-8');
14+
15+
expect(content.startsWith('#!/usr/bin/env node\n')).toBe(true);
16+
});
17+
});

0 commit comments

Comments
 (0)