From 5a1c019f3a07689f31c5f23ef040cd29105a33b9 Mon Sep 17 00:00:00 2001 From: fi3ework Date: Mon, 26 Aug 2024 18:49:19 +0800 Subject: [PATCH] fix: ASI in the concatenated module only when necessary --- crates/rspack_core/src/concatenated_module.rs | 30 +++++++++++++++---- .../tests/__snapshots__/Config.test.js.snap | 14 ++++----- .../__snapshots__/StatsOutput.test.js.snap | 4 +-- .../__snapshots__/output.snap.txt | 4 +-- .../request/output.snap.txt | 6 ++-- .../resource/output.snap.txt | 6 ++-- .../resource/output.snap.txt | 6 ++-- .../tests/statsAPICases/asset-info.js | 2 +- .../StatsTestCases.basictest.js.snap | 10 +++---- .../scope-hoisting/issue-11897/test.filter.js | 4 --- .../scope-hoisting/issue-11897/cjs.js | 0 .../scope-hoisting/issue-11897/iife.js | 0 .../scope-hoisting/issue-11897/index.js | 0 .../scope-hoisting/issue-11897/module.js | 0 .../scope-hoisting/issue-11897/test.config.js | 0 .../issue-11897/webpack.config.js | 6 ++++ 16 files changed, 56 insertions(+), 36 deletions(-) delete mode 100644 tests/webpack-test/cases/scope-hoisting/issue-11897/test.filter.js rename tests/webpack-test/{cases => configCases}/scope-hoisting/issue-11897/cjs.js (100%) rename tests/webpack-test/{cases => configCases}/scope-hoisting/issue-11897/iife.js (100%) rename tests/webpack-test/{cases => configCases}/scope-hoisting/issue-11897/index.js (100%) rename tests/webpack-test/{cases => configCases}/scope-hoisting/issue-11897/module.js (100%) rename tests/webpack-test/{cases => configCases}/scope-hoisting/issue-11897/test.config.js (100%) create mode 100644 tests/webpack-test/configCases/scope-hoisting/issue-11897/webpack.config.js diff --git a/crates/rspack_core/src/concatenated_module.rs b/crates/rspack_core/src/concatenated_module.rs index 2ae5fe992344..ce3cf05b6e9e 100644 --- a/crates/rspack_core/src/concatenated_module.rs +++ b/crates/rspack_core/src/concatenated_module.rs @@ -185,6 +185,7 @@ pub struct ConcatenatedModuleInfo { pub global_scope_ident: Vec, pub idents: Vec, pub binding_to_ref: HashMap<(Atom, SyntaxContext), Vec>, + pub prefix_asi: bool, } #[derive(Debug, Clone)] @@ -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 { + '[' | '(' | '+' | '-' | '/' => true, + _ => false, + }); + + if add_prefix_asi { + info.prefix_asi = true; + } + source.replace(low, high + 2, &final_name, None); } } @@ -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")); @@ -1694,12 +1706,13 @@ impl ConcatenatedModule { let source_code = source.source(); let cm: Arc = 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; @@ -1747,11 +1760,16 @@ impl ConcatenatedModule { )); }); + let prefix = (&source_code).to_string().starts_with(|c: char| match c { + '[' | '(' | '+' | '-' | '/' => 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; diff --git a/packages/rspack-test-tools/tests/__snapshots__/Config.test.js.snap b/packages/rspack-test-tools/tests/__snapshots__/Config.test.js.snap index fc542c9a9d13..fb53bac6f2ae 100644 --- a/packages/rspack-test-tools/tests/__snapshots__/Config.test.js.snap +++ b/packages/rspack-test-tools/tests/__snapshots__/Config.test.js.snap @@ -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 @@ -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' " `; @@ -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()) @@ -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 }; diff --git a/packages/rspack-test-tools/tests/__snapshots__/StatsOutput.test.js.snap b/packages/rspack-test-tools/tests/__snapshots__/StatsOutput.test.js.snap index d32cb2fe93e3..e21ca5ac102a 100644 --- a/packages/rspack-test-tools/tests/__snapshots__/StatsOutput.test.js.snap +++ b/packages/rspack-test-tools/tests/__snapshots__/StatsOutput.test.js.snap @@ -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" @@ -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] diff --git a/packages/rspack-test-tools/tests/builtinCases/samples/concatenate-modules/__snapshots__/output.snap.txt b/packages/rspack-test-tools/tests/builtinCases/samples/concatenate-modules/__snapshots__/output.snap.txt index 3e2f36796a56..64342a7244c9 100644 --- a/packages/rspack-test-tools/tests/builtinCases/samples/concatenate-modules/__snapshots__/output.snap.txt +++ b/packages/rspack-test-tools/tests/builtinCases/samples/concatenate-modules/__snapshots__/output.snap.txt @@ -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; diff --git a/packages/rspack-test-tools/tests/hookCases/normalModuleFactory#afterResolve/request/output.snap.txt b/packages/rspack-test-tools/tests/hookCases/normalModuleFactory#afterResolve/request/output.snap.txt index 347659048a1a..44360c31d137 100644 --- a/packages/rspack-test-tools/tests/hookCases/normalModuleFactory#afterResolve/request/output.snap.txt +++ b/packages/rspack-test-tools/tests/hookCases/normalModuleFactory#afterResolve/request/output.snap.txt @@ -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 +; diff --git a/packages/rspack-test-tools/tests/hookCases/normalModuleFactory#afterResolve/resource/output.snap.txt b/packages/rspack-test-tools/tests/hookCases/normalModuleFactory#afterResolve/resource/output.snap.txt index 5ae4c7c9e7c3..310dbea7e2e4 100644 --- a/packages/rspack-test-tools/tests/hookCases/normalModuleFactory#afterResolve/resource/output.snap.txt +++ b/packages/rspack-test-tools/tests/hookCases/normalModuleFactory#afterResolve/resource/output.snap.txt @@ -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 +; diff --git a/packages/rspack-test-tools/tests/hookCases/normalModuleFactory#resolve/resource/output.snap.txt b/packages/rspack-test-tools/tests/hookCases/normalModuleFactory#resolve/resource/output.snap.txt index 18fc0f6d7d6f..91be61f3b06b 100644 --- a/packages/rspack-test-tools/tests/hookCases/normalModuleFactory#resolve/resource/output.snap.txt +++ b/packages/rspack-test-tools/tests/hookCases/normalModuleFactory#resolve/resource/output.snap.txt @@ -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 +; diff --git a/packages/rspack-test-tools/tests/statsAPICases/asset-info.js b/packages/rspack-test-tools/tests/statsAPICases/asset-info.js index 2190bba45a62..02da1fbd740f 100644 --- a/packages/rspack-test-tools/tests/statsAPICases/asset-info.js +++ b/packages/rspack-test-tools/tests/statsAPICases/asset-info.js @@ -98,7 +98,7 @@ module.exports = { "isOverSizeLimit": false, "name": "main.js", "related": Array [], - "size": 2763, + "size": 2761, "type": "asset", }, ] diff --git a/tests/webpack-test/__snapshots__/StatsTestCases.basictest.js.snap b/tests/webpack-test/__snapshots__/StatsTestCases.basictest.js.snap index 51f8092262e9..6dbeaafbf2b6 100644 --- a/tests/webpack-test/__snapshots__/StatsTestCases.basictest.js.snap +++ b/tests/webpack-test/__snapshots__/StatsTestCases.basictest.js.snap @@ -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" @@ -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] @@ -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] @@ -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) diff --git a/tests/webpack-test/cases/scope-hoisting/issue-11897/test.filter.js b/tests/webpack-test/cases/scope-hoisting/issue-11897/test.filter.js deleted file mode 100644 index ad8d560b5acd..000000000000 --- a/tests/webpack-test/cases/scope-hoisting/issue-11897/test.filter.js +++ /dev/null @@ -1,4 +0,0 @@ - -module.exports = () => {return true} - - diff --git a/tests/webpack-test/cases/scope-hoisting/issue-11897/cjs.js b/tests/webpack-test/configCases/scope-hoisting/issue-11897/cjs.js similarity index 100% rename from tests/webpack-test/cases/scope-hoisting/issue-11897/cjs.js rename to tests/webpack-test/configCases/scope-hoisting/issue-11897/cjs.js diff --git a/tests/webpack-test/cases/scope-hoisting/issue-11897/iife.js b/tests/webpack-test/configCases/scope-hoisting/issue-11897/iife.js similarity index 100% rename from tests/webpack-test/cases/scope-hoisting/issue-11897/iife.js rename to tests/webpack-test/configCases/scope-hoisting/issue-11897/iife.js diff --git a/tests/webpack-test/cases/scope-hoisting/issue-11897/index.js b/tests/webpack-test/configCases/scope-hoisting/issue-11897/index.js similarity index 100% rename from tests/webpack-test/cases/scope-hoisting/issue-11897/index.js rename to tests/webpack-test/configCases/scope-hoisting/issue-11897/index.js diff --git a/tests/webpack-test/cases/scope-hoisting/issue-11897/module.js b/tests/webpack-test/configCases/scope-hoisting/issue-11897/module.js similarity index 100% rename from tests/webpack-test/cases/scope-hoisting/issue-11897/module.js rename to tests/webpack-test/configCases/scope-hoisting/issue-11897/module.js diff --git a/tests/webpack-test/cases/scope-hoisting/issue-11897/test.config.js b/tests/webpack-test/configCases/scope-hoisting/issue-11897/test.config.js similarity index 100% rename from tests/webpack-test/cases/scope-hoisting/issue-11897/test.config.js rename to tests/webpack-test/configCases/scope-hoisting/issue-11897/test.config.js diff --git a/tests/webpack-test/configCases/scope-hoisting/issue-11897/webpack.config.js b/tests/webpack-test/configCases/scope-hoisting/issue-11897/webpack.config.js new file mode 100644 index 000000000000..7d4cc233bfe4 --- /dev/null +++ b/tests/webpack-test/configCases/scope-hoisting/issue-11897/webpack.config.js @@ -0,0 +1,6 @@ +/** @type {import("@rspack/core").Configuration} */ +module.exports = { + optimization: { + concatenateModules: true + } +};