Skip to content

Commit

Permalink
fix: should keep comments of esm export (#8168)
Browse files Browse the repository at this point in the history
  • Loading branch information
LingyuCoder authored Oct 18, 2024
1 parent 728a6f4 commit 539b10f
Show file tree
Hide file tree
Showing 8 changed files with 78 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,19 +39,22 @@ pub struct ESMExportExpressionDependency {
id: DependencyId,
range: RealDependencyLocation,
range_stmt: RealDependencyLocation,
prefix: String,
declaration: Option<DeclarationId>,
}

impl ESMExportExpressionDependency {
pub fn new(
range: RealDependencyLocation,
range_stmt: RealDependencyLocation,
prefix: String,
declaration: Option<DeclarationId>,
) -> Self {
Self {
range,
range_stmt,
declaration,
prefix,
id: DependencyId::default(),
}
}
Expand Down Expand Up @@ -171,7 +174,7 @@ impl DependencyTemplate for ESMExportExpressionDependency {
source.replace(
self.range_stmt.start,
self.range.start,
"/* ESM default export */ ",
format!("/* ESM default export */ {}", self.prefix).as_str(),
None,
);
} else {
Expand Down Expand Up @@ -227,7 +230,7 @@ impl DependencyTemplate for ESMExportExpressionDependency {
source.replace(
self.range_stmt.start,
self.range.start,
&format!("{}(", content),
&format!("{}({}", content, self.prefix),
None,
);
source.replace_with_enforce(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use itertools::Itertools;
use rspack_core::{
BoxDependency, ConstDependency, DependencyType, RealDependencyLocation, SpanExt,
};
use swc_core::atoms::Atom;
use swc_core::common::comments::CommentKind;
use swc_core::common::Spanned;

use super::esm_import_dependency_parser_plugin::{ESMSpecifierData, ESM_SPECIFIER_TAG};
Expand Down Expand Up @@ -158,6 +160,20 @@ impl JavascriptParserPlugin for ESMExportDependencyParserPlugin {
let dep: ESMExportExpressionDependency = ESMExportExpressionDependency::new(
range.with_source(parser.source_map.clone()),
statement_span.into(),
parser
.comments
.and_then(|c| c.get_leading(expr_span.lo))
.map(|c| {
c.iter()
.dedup()
.map(|c| match c.kind {
CommentKind::Block => format!("/*{}*/", c.text),
CommentKind::Line => format!("//{}\n", c.text),
})
.collect_vec()
.join("")
})
.unwrap_or_default(),
match expr {
ExportDefaultExpression::FnDecl(f) => {
let start = f.span().real_lo();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,13 @@ exports[`config config/library/modern-module-force-concaten step should pass: .
const d = 'd'
`;
exports[`config config/library/modern-module-force-concaten step should pass: ESM export should concat 1`] = `
;// CONCATENATED MODULE: ./a.js
const a = 'a'
export { a };
`;
exports[`config config/library/modern-module-force-concaten step should pass: external module should bail out when bundling 1`] = `
import { createRequire as __WEBPACK_EXTERNAL_createRequire } from "module";
var __webpack_modules__ = ({
Expand Down Expand Up @@ -551,13 +558,6 @@ const value = foo + (bar_default())
export { value };
`;
exports[`config config/library/modern-module-force-concaten step should pass: ESM export should concat 1`] = `
;// CONCATENATED MODULE: ./a.js
const a = 'a'
export { a };
`;
exports[`config config/library/modern-module-force-concaten step should pass: unambiguous should bail out 1`] = `const c = 'c'`;
exports[`config config/optimization/minimizer-esm-asset exported tests minimizing an asset file of esm type should success 1`] = `console.log(import.meta.url);export const a=1;`;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { requestEngine } from './share';
import used from './used';

used;

const content = __non_webpack_require__("fs").readFileSync(__filename, 'utf-8');
const pureComment = "/* #__PURE__ */";
it("should keep pure comment of unused export default", () => {
const methodName = "createRequest";
expect(content).toContain(`${pureComment}${methodName}`)
});

it("should keep pure comment of used export default", () => {
const methodName = "function __WEBPACK_DEFAULT_EXPORT__()";
expect(content).toContain(`${pureComment}${methodName}`)
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/**
* @type {import('@rspack/core').Configuration}
*/
module.exports = {
entry: "./index.js",
node: {
__dirname: false,
__filename: false
},
optimization: {
sideEffects: false,
concatenateModules: false,
innerGraph: false
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
function createRequest() {
}
function requestEngine() {

}

export default /* #__PURE__ */ createRequest();

export { requestEngine };
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@

function fun2() {

}

export default /* #__PURE__ */ function () {
}

export { fun2 };
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ function res() {
answer;
}
const a = 3;
/* unused ESM default export */ var __WEBPACK_DEFAULT_EXPORT__ = ((/* unused pure expression or super */ null && (res())));
/* unused ESM default export */ var __WEBPACK_DEFAULT_EXPORT__ = (/*#__PURE__*/(/* unused pure expression or super */ null && (res())));


}),
Expand Down

2 comments on commit 539b10f

@rspack-bot
Copy link

Choose a reason for hiding this comment

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

📝 Benchmark detail: Open

Name Base (2024-10-18 d01e85c) Current Change
10000_development-mode + exec 2.14 s ± 16 ms 2.11 s ± 36 ms -1.10 %
10000_development-mode_hmr + exec 673 ms ± 11 ms 668 ms ± 7.7 ms -0.68 %
10000_production-mode + exec 2.68 s ± 42 ms 2.69 s ± 43 ms +0.34 %
arco-pro_development-mode + exec 1.79 s ± 90 ms 1.8 s ± 60 ms +0.51 %
arco-pro_development-mode_hmr + exec 427 ms ± 1.4 ms 427 ms ± 0.31 ms -0.18 %
arco-pro_production-mode + exec 3.18 s ± 64 ms 3.23 s ± 61 ms +1.76 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.17 s ± 81 ms 3.26 s ± 89 ms +2.95 %
threejs_development-mode_10x + exec 1.61 s ± 18 ms 1.61 s ± 17 ms +0.03 %
threejs_development-mode_10x_hmr + exec 756 ms ± 12 ms 756 ms ± 4.8 ms +0.01 %
threejs_production-mode_10x + exec 4.94 s ± 35 ms 5 s ± 40 ms +1.27 %

@rspack-bot
Copy link

Choose a reason for hiding this comment

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

📝 Ran ecosystem CI: Open

suite result
modernjs ✅ success
_selftest ✅ success
rspress ✅ success
rslib ❌ failure
rsbuild ✅ success
examples ✅ success
devserver ✅ success

Please sign in to comment.