Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ASI in the concatenated module only when necessary #7688

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions crates/rspack_core/src/concatenated_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ use crate::{
type ExportsDefinitionArgs = Vec<(String, String)>;
define_hook!(ConcatenatedModuleExportsDefinitions: SyncSeriesBail(exports_definitions: &mut ExportsDefinitionArgs) -> bool);

const CHARS_REQUIRING_SEMICOLON: [char; 5] = ['[', '(', '+', '-', '/'];

#[derive(Debug, Default)]
pub struct ConcatenatedModuleHooks {
pub exports_definitions: ConcatenatedModuleExportsDefinitionsHook,
Expand Down Expand Up @@ -185,6 +187,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 +895,14 @@ 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 =
matches!(final_name.chars().next(), Some(c) if CHARS_REQUIRING_SEMICOLON.contains(&c));

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 @@ -1692,6 +1704,8 @@ impl ConcatenatedModule {
.remove(&SourceType::JavaScript)
.expect("should have javascript source");
let source_code = source.source();
let prefix_asi =
matches!(source_code.chars().next(), Some(c) if CHARS_REQUIRING_SEMICOLON.contains(&c));

let cm: Arc<swc_core::common::SourceMap> = Default::default();
let fm = cm.new_source_file(
Expand Down Expand Up @@ -1752,6 +1766,7 @@ impl ConcatenatedModule {
module_info.global_ctxt = global_ctxt;
module_info.ast = Some(ast);
module_info.runtime_requirements = runtime_requirements;
module_info.prefix_asi = prefix_asi;
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
}
};
Loading