Skip to content

Commit

Permalink
fix: ASI in the concatenated module only when necessary
Browse files Browse the repository at this point in the history
  • Loading branch information
fi3ework committed Aug 26, 2024
1 parent dfd2706 commit 5a1c019
Show file tree
Hide file tree
Showing 16 changed files with 56 additions and 36 deletions.
30 changes: 24 additions & 6 deletions crates/rspack_core/src/concatenated_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ pub struct ConcatenatedModuleInfo {
pub global_scope_ident: Vec<ConcatenatedModuleIdent>,
pub idents: Vec<ConcatenatedModuleIdent>,
pub binding_to_ref: HashMap<(Atom, SyntaxContext), Vec<ConcatenatedModuleIdent>>,
pub prefix_asi: bool,
}

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -892,6 +893,16 @@ impl Module for ConcatenatedModule {
let source = info.source.as_mut().expect("should have source");
// range is extended by 2 chars to cover the appended "._"
// https://github.com/webpack/webpack/blob/ac7e531436b0d47cd88451f497cdfd0dad41535d/lib/optimize/ConcatenatedModule.js#L1411-L1412

let add_prefix_asi = (&final_name).to_string().starts_with(|c: char| match c {

Check failure on line 897 in crates/rspack_core/src/concatenated_module.rs

View workflow job for this annotation

GitHub Actions / Rust check

this expression borrows a value the compiler would automatically borrow

Check failure on line 897 in crates/rspack_core/src/concatenated_module.rs

View workflow job for this annotation

GitHub Actions / Rust check

match expression looks like `matches!` macro
'[' | '(' | '+' | '-' | '/' => true,
_ => false,
});

if add_prefix_asi {
info.prefix_asi = true;
}

source.replace(low, high + 2, &final_name, None);
}
}
Expand Down Expand Up @@ -1166,12 +1177,13 @@ impl Module for ConcatenatedModule {
match info {
ModuleInfo::Concatenated(info) => {
result.add(RawSource::from(
format!(
"\n;// CONCATENATED MODULE: {}\n",
module_readable_identifier
)
.as_str(),
format!("\n// CONCATENATED MODULE: {}\n", module_readable_identifier).as_str(),
));

if info.prefix_asi {
result.add(RawSource::from(";"));
}

// https://github.com/webpack/webpack/blob/ac7e531436b0d47cd88451f497cdfd0dad41535d/lib/optimize/ConcatenatedModule.js#L1582
result.add(info.source.clone().expect("should have source"));

Expand Down Expand Up @@ -1694,12 +1706,13 @@ impl ConcatenatedModule {
let source_code = source.source();

let cm: Arc<swc_core::common::SourceMap> = Default::default();
let source_code_clone = source_code.clone();
let fm = cm.new_source_file(
Arc::new(FileName::Custom(format!(
"{}",
self.readable_identifier(&compilation.options.context),
))),
source_code.into(),
source_code_clone.into(),
);
let comments = SwcComments::default();
let mut module_info = concatenation_scope.current_module;
Expand Down Expand Up @@ -1747,11 +1760,16 @@ impl ConcatenatedModule {
));
});

let prefix = (&source_code).to_string().starts_with(|c: char| match c {

Check failure on line 1763 in crates/rspack_core/src/concatenated_module.rs

View workflow job for this annotation

GitHub Actions / Rust check

this expression borrows a value the compiler would automatically borrow

Check failure on line 1763 in crates/rspack_core/src/concatenated_module.rs

View workflow job for this annotation

GitHub Actions / Rust check

match expression looks like `matches!` macro
'[' | '(' | '+' | '-' | '/' => true,
_ => false,
});
let result_source = ReplaceSource::new(source.clone());
module_info.module_ctxt = module_ctxt;
module_info.global_ctxt = global_ctxt;
module_info.ast = Some(ast);
module_info.runtime_requirements = runtime_requirements;
module_info.prefix_asi = prefix;
module_info.internal_source = Some(source);
module_info.source = Some(result_source);
module_info.chunk_init_fragments = chunk_init_fragments;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -439,13 +439,13 @@ __webpack_require__.o = function (obj, prop) {
})();
/************************************************************************/
;// CONCATENATED MODULE: ./e/foo.js
// CONCATENATED MODULE: ./e/foo.js
const foo = 'foo'
// EXTERNAL MODULE: ./e/bar.cjs
var bar = __webpack_require__(\\"997\\");
var bar_default = /*#__PURE__*/__webpack_require__.n(bar);
;// CONCATENATED MODULE: ./e/index.js
// CONCATENATED MODULE: ./e/index.js
Expand All @@ -458,7 +458,7 @@ export { foo, __webpack_exports__bar as bar };
exports[`config config/library/modern-module-force-concaten step should pass: .mjs should concat 1`] = `
"
;// CONCATENATED MODULE: ./d.mjs
// CONCATENATED MODULE: ./d.mjs
const d = 'd'
"
`;
Expand Down Expand Up @@ -541,11 +541,11 @@ __webpack_require__.o = function (obj, prop) {
// EXTERNAL MODULE: ./f/bar.cjs
var bar = __webpack_require__(\\"441\\");
var bar_default = /*#__PURE__*/__webpack_require__.n(bar);
;// CONCATENATED MODULE: ./f/foo.js
// CONCATENATED MODULE: ./f/foo.js
const foo = 'foo'
;// CONCATENATED MODULE: ./f/index.js
// CONCATENATED MODULE: ./f/index.js
;
const value = foo + (bar_default())
Expand All @@ -558,7 +558,7 @@ export { value };
exports[`config config/library/modern-module-force-concaten step should pass: harmony export should concat 1`] = `
"
;// CONCATENATED MODULE: ./a.js
// CONCATENATED MODULE: ./a.js
const a = 'a'
export { a };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ Rspack compiled with 1 error"
`;
exports[`statsOutput statsOutput/concatenated-modules should print correct stats for 1`] = `
"asset main.js 234 bytes [emitted] (name: main)
"asset main.js 230 bytes [emitted] (name: main)
orphan modules 141 bytes [orphan] 4 modules
./index.js + 3 modules 141 bytes [code generated]
Rspack x.x.x compiled successfully in X s"
Expand Down Expand Up @@ -428,7 +428,7 @@ Rspack x.x.x compiled successfully in X s"
`;
exports[`statsOutput statsOutput/side-effects-bailouts should print correct stats for 1`] = `
"asset main.js 177 bytes {909} [emitted] (name: main)
"asset main.js 175 bytes {909} [emitted] (name: main)
chunk {909} (runtime: main) main.js (main) 91 bytes [entry] [rendered]
orphan modules 91 bytes [orphan] 2 modules
./index.js + 1 modules [911] 91 bytes {909} [code generated]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
(self['webpackChunkwebpack'] = self['webpackChunkwebpack'] || []).push([["main"], {
"./index.js": (function () {

;// CONCATENATED MODULE: ./answer.js
// CONCATENATED MODULE: ./answer.js
const answer = 103330;

;// CONCATENATED MODULE: ./index.js
// CONCATENATED MODULE: ./index.js

answer;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,11 @@ var a_default = /*#__PURE__*/__webpack_require__.n(a);
// EXTERNAL MODULE: ../../normalModuleFactory​#afterResolve/request/c.js
var c = __webpack_require__("../../normalModuleFactory​#afterResolve/request/c.js");
var c_default = /*#__PURE__*/__webpack_require__.n(c);
;// CONCATENATED MODULE: external "fs"
// CONCATENATED MODULE: external "fs"
var external_fs_namespaceObject = require("fs");
var external_fs_default = /*#__PURE__*/__webpack_require__.n(external_fs_namespaceObject);
;// CONCATENATED MODULE: ../../normalModuleFactory​#afterResolve/request/request.js

// CONCATENATED MODULE: ../../normalModuleFactory​#afterResolve/request/request.js
;



Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,11 @@ var a_default = /*#__PURE__*/__webpack_require__.n(a);
// EXTERNAL MODULE: ../../normalModuleFactory​#afterResolve/resource/b.js
var b = __webpack_require__("../../normalModuleFactory​#afterResolve/resource/b.js");
var b_default = /*#__PURE__*/__webpack_require__.n(b);
;// CONCATENATED MODULE: external "fs"
// CONCATENATED MODULE: external "fs"
var external_fs_namespaceObject = require("fs");
var external_fs_default = /*#__PURE__*/__webpack_require__.n(external_fs_namespaceObject);
;// CONCATENATED MODULE: ../../normalModuleFactory​#afterResolve/resource/resource.js

// CONCATENATED MODULE: ../../normalModuleFactory​#afterResolve/resource/resource.js
;



Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,11 @@ var a_default = /*#__PURE__*/__webpack_require__.n(a);
// EXTERNAL MODULE: ../../normalModuleFactory​#resolve/resource/b.js
var b = __webpack_require__("../../normalModuleFactory​#resolve/resource/b.js");
var b_default = /*#__PURE__*/__webpack_require__.n(b);
;// CONCATENATED MODULE: external "fs"
// CONCATENATED MODULE: external "fs"
var external_fs_namespaceObject = require("fs");
var external_fs_default = /*#__PURE__*/__webpack_require__.n(external_fs_namespaceObject);
;// CONCATENATED MODULE: ../../normalModuleFactory​#resolve/resource/resource.js

// CONCATENATED MODULE: ../../normalModuleFactory​#resolve/resource/resource.js
;



Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ module.exports = {
"isOverSizeLimit": false,
"name": "main.js",
"related": Array [],
"size": 2763,
"size": 2761,
"type": "asset",
},
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ chunk (runtime: main) trees.js (trees) 215 bytes [rendered]
`;

exports[`StatsTestCases should print correct stats for ignore-warnings 1`] = `
"asset main.js 1.83 KiB [emitted] (name: main)
"asset main.js 1.82 KiB [emitted] (name: main)
orphan modules 794 bytes [orphan] 10 modules
./index.js + 9 modules 794 bytes [code generated]
Rspack x.x.x compiled successfully in X.23"
Expand Down Expand Up @@ -904,7 +904,7 @@ Rspack x.x.x compiled successfully in X.23"
`;
exports[`StatsTestCases should print correct stats for parse-error 1`] = `
"assets by status 1.41 KiB [cached] 1 asset
"assets by status 1.4 KiB [cached] 1 asset
orphan modules 30 bytes [orphan] 2 modules
cacheable modules 85 bytes
./index.js + 1 modules 30 bytes [code generated]
Expand Down Expand Up @@ -1628,7 +1628,7 @@ Rspack x.x.x compiled successfully in X.23"
`;
exports[`StatsTestCases should print correct stats for side-effects-simple-unused 1`] = `
"asset main.js 297 bytes [emitted] (name: main)
"asset main.js 294 bytes [emitted] (name: main)
modules by path ./node_modules/pmodule/*.js 232 bytes
./node_modules/pmodule/index.js 75 bytes [orphan] [built]
[only some exports used: default]
Expand Down Expand Up @@ -2278,8 +2278,8 @@ exports[`StatsTestCases should print correct stats for split-chunks-runtime-spec
asset used-exports-c.js 4.6 KiB [emitted] (name: c)
asset used-exports-b.js 4.6 KiB [emitted] (name: b)
asset used-exports-572.js 370 bytes [emitted]
asset used-exports-a.js 249 bytes [emitted] (name: a)
Entrypoint a 249 bytes = used-exports-a.js
asset used-exports-a.js 247 bytes [emitted] (name: a)
Entrypoint a 247 bytes = used-exports-a.js
Entrypoint b 4.96 KiB = used-exports-572.js 370 bytes used-exports-b.js 4.6 KiB
Entrypoint c 4.96 KiB = used-exports-572.js 370 bytes used-exports-c.js 4.6 KiB
chunk (runtime: b, c) used-exports-572.js (id hint: ) 72 bytes [initial] [rendered] split chunk (cache group: default)
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
/** @type {import("@rspack/core").Configuration} */
module.exports = {
optimization: {
concatenateModules: true
}
};

0 comments on commit 5a1c019

Please sign in to comment.