From 41335dbb99f9b23bc282741eb8adad38308cc139 Mon Sep 17 00:00:00 2001 From: LingyuCoder <--global> Date: Mon, 20 Nov 2023 19:58:27 +0800 Subject: [PATCH 1/2] chore: commonjs exports dependency --- .../snapshot/new_treeshaking.snap | 4 +- .../snapshot/output.snap | 4 +- .../snapshot/new_treeshaking.snap | 2 +- .../snapshot/output.snap | 2 +- .../snapshot/new_treeshaking.snap | 2 +- .../tree-shaking-interop/snapshot/output.snap | 2 +- .../src/dependency/dependency_type.rs | 3 + .../src/dependency/runtime_template.rs | 2 +- crates/rspack_core/src/exports_info.rs | 7 +- crates/rspack_core/src/init_fragment.rs | 12 +- crates/rspack_core/src/runtime_globals.rs | 3 + crates/rspack_core/src/utils/visitor.rs | 4 + .../src/dependency/commonjs/mod.rs | 3 + .../esm/harmony_compatibility_dependency.rs | 2 +- .../src/plugin/mod.rs | 14 +- .../rspack_plugin_javascript/src/runtime.rs | 37 +++-- .../dependency/common_js_export_scanner.rs | 156 ++++++++++++++---- .../src/visitors/dependency/mod.rs | 1 + .../src/visitors/dependency/util.rs | 3 - packages/rspack/tests/Stats.test.ts | 2 +- .../tests/__snapshots__/Stats.test.ts.snap | 2 +- .../cjs-tree-shaking/non-root-this/arrow.js | 8 + .../cjs-tree-shaking/non-root-this/class.js | 9 + .../non-root-this/function.js | 7 + .../cjs-tree-shaking/non-root-this/index.js | 15 ++ .../sub-properties/exports.js | 16 ++ .../cjs-tree-shaking/sub-properties/index.js | 21 +++ .../sub-properties/module-exports.js | 16 ++ .../cjs-tree-shaking/sub-properties/this.js | 16 ++ .../cjs-tree-shaking/bailouts/test.filter.js | 3 +- .../cjs-tree-shaking/exports/test.filter.js | 5 - .../test.filter.js | 4 - 32 files changed, 305 insertions(+), 82 deletions(-) create mode 100644 packages/rspack/tests/cases/cjs-tree-shaking/non-root-this/arrow.js create mode 100644 packages/rspack/tests/cases/cjs-tree-shaking/non-root-this/class.js create mode 100644 packages/rspack/tests/cases/cjs-tree-shaking/non-root-this/function.js create mode 100644 packages/rspack/tests/cases/cjs-tree-shaking/non-root-this/index.js create mode 100644 packages/rspack/tests/cases/cjs-tree-shaking/sub-properties/exports.js create mode 100644 packages/rspack/tests/cases/cjs-tree-shaking/sub-properties/index.js create mode 100644 packages/rspack/tests/cases/cjs-tree-shaking/sub-properties/module-exports.js create mode 100644 packages/rspack/tests/cases/cjs-tree-shaking/sub-properties/this.js delete mode 100644 webpack-test/cases/cjs-tree-shaking/exports/test.filter.js delete mode 100644 webpack-test/cases/cjs-tree-shaking/object-define-property-replace/test.filter.js diff --git a/crates/rspack/tests/tree-shaking/cjs-export-computed-property/snapshot/new_treeshaking.snap b/crates/rspack/tests/tree-shaking/cjs-export-computed-property/snapshot/new_treeshaking.snap index 7fbbccd3da4..cd5c6738cdc 100644 --- a/crates/rspack/tests/tree-shaking/cjs-export-computed-property/snapshot/new_treeshaking.snap +++ b/crates/rspack/tests/tree-shaking/cjs-export-computed-property/snapshot/new_treeshaking.snap @@ -5,9 +5,9 @@ source: crates/rspack_testing/src/run_fixture.rs (self['webpackChunkwebpack'] = self['webpackChunkwebpack'] || []).push([["main"], { "./zh_locale.js": (function (__unused_webpack_module, exports, __webpack_require__) { "use strict"; -Object.defineProperty(exports, "__esModule", { +Object.defineProperty(exports, "__esModule", ({ value: true -}); +})); exports["default"] = void 0; /* eslint-disable no-template-curly-in-string */ var _default = {}; exports["default"] = _default; diff --git a/crates/rspack/tests/tree-shaking/cjs-export-computed-property/snapshot/output.snap b/crates/rspack/tests/tree-shaking/cjs-export-computed-property/snapshot/output.snap index e707f90a30b..4040c312c3a 100644 --- a/crates/rspack/tests/tree-shaking/cjs-export-computed-property/snapshot/output.snap +++ b/crates/rspack/tests/tree-shaking/cjs-export-computed-property/snapshot/output.snap @@ -5,9 +5,9 @@ source: crates/rspack_testing/src/run_fixture.rs (self['webpackChunkwebpack'] = self['webpackChunkwebpack'] || []).push([["main"], { "./zh_locale.js": (function (__unused_webpack_module, exports, __webpack_require__) { "use strict"; -Object.defineProperty(exports, "__esModule", { +Object.defineProperty(exports, "__esModule", ({ value: true -}); +})); exports["default"] = void 0; /* eslint-disable no-template-curly-in-string */ var _default = {}; exports["default"] = _default; diff --git a/crates/rspack/tests/tree-shaking/export-all-from-side-effects-free-commonjs/snapshot/new_treeshaking.snap b/crates/rspack/tests/tree-shaking/export-all-from-side-effects-free-commonjs/snapshot/new_treeshaking.snap index 29afaa4f455..90f4ba75b59 100644 --- a/crates/rspack/tests/tree-shaking/export-all-from-side-effects-free-commonjs/snapshot/new_treeshaking.snap +++ b/crates/rspack/tests/tree-shaking/export-all-from-side-effects-free-commonjs/snapshot/new_treeshaking.snap @@ -12,7 +12,7 @@ __webpack_require__.r(__webpack_exports__); /*#__PURE__*/ (_lib__WEBPACK_IMPORTED_MODULE_0___namespace_cache || (_lib__WEBPACK_IMPORTED_MODULE_0___namespace_cache = __webpack_require__.t(_lib__WEBPACK_IMPORTED_MODULE_0__, 2))); }), "./lib.js": (function (__unused_webpack_module, exports, __webpack_require__) { -exports['a'] = 100000; +exports.a = 100000; }), },function(__webpack_require__) { diff --git a/crates/rspack/tests/tree-shaking/export-all-from-side-effects-free-commonjs/snapshot/output.snap b/crates/rspack/tests/tree-shaking/export-all-from-side-effects-free-commonjs/snapshot/output.snap index 29afaa4f455..90f4ba75b59 100644 --- a/crates/rspack/tests/tree-shaking/export-all-from-side-effects-free-commonjs/snapshot/output.snap +++ b/crates/rspack/tests/tree-shaking/export-all-from-side-effects-free-commonjs/snapshot/output.snap @@ -12,7 +12,7 @@ __webpack_require__.r(__webpack_exports__); /*#__PURE__*/ (_lib__WEBPACK_IMPORTED_MODULE_0___namespace_cache || (_lib__WEBPACK_IMPORTED_MODULE_0___namespace_cache = __webpack_require__.t(_lib__WEBPACK_IMPORTED_MODULE_0__, 2))); }), "./lib.js": (function (__unused_webpack_module, exports, __webpack_require__) { -exports['a'] = 100000; +exports.a = 100000; }), },function(__webpack_require__) { diff --git a/crates/rspack/tests/tree-shaking/tree-shaking-interop/snapshot/new_treeshaking.snap b/crates/rspack/tests/tree-shaking/tree-shaking-interop/snapshot/new_treeshaking.snap index c44edd8c131..99c3f135dc6 100644 --- a/crates/rspack/tests/tree-shaking/tree-shaking-interop/snapshot/new_treeshaking.snap +++ b/crates/rspack/tests/tree-shaking/tree-shaking-interop/snapshot/new_treeshaking.snap @@ -27,7 +27,7 @@ __webpack_require__.el(/* ./bar */"./bar.js").then(__webpack_require__.bind(__we console.log(mod); }); const a = "a"; -exports.test = 30; +__webpack_exports__.test = 30; }), "./foo.js": (function (module, exports, __webpack_require__) { { diff --git a/crates/rspack/tests/tree-shaking/tree-shaking-interop/snapshot/output.snap b/crates/rspack/tests/tree-shaking/tree-shaking-interop/snapshot/output.snap index c44edd8c131..99c3f135dc6 100644 --- a/crates/rspack/tests/tree-shaking/tree-shaking-interop/snapshot/output.snap +++ b/crates/rspack/tests/tree-shaking/tree-shaking-interop/snapshot/output.snap @@ -27,7 +27,7 @@ __webpack_require__.el(/* ./bar */"./bar.js").then(__webpack_require__.bind(__we console.log(mod); }); const a = "a"; -exports.test = 30; +__webpack_exports__.test = 30; }), "./foo.js": (function (module, exports, __webpack_require__) { { diff --git a/crates/rspack_core/src/dependency/dependency_type.rs b/crates/rspack_core/src/dependency/dependency_type.rs index 2c6a15766b2..692da09ae4e 100644 --- a/crates/rspack_core/src/dependency/dependency_type.rs +++ b/crates/rspack_core/src/dependency/dependency_type.rs @@ -24,6 +24,8 @@ pub enum DependencyType { DynamicImportEager, // cjs require CjsRequire, + // cjs exports + CjsExports, // new URL("./foo", import.meta.url) NewUrl, // new Worker() @@ -75,6 +77,7 @@ impl DependencyType { DependencyType::EsmImportSpecifier => Cow::Borrowed("esm import specifier"), DependencyType::DynamicImport => Cow::Borrowed("dynamic import"), DependencyType::CjsRequire => Cow::Borrowed("cjs require"), + DependencyType::CjsExports => Cow::Borrowed("cjs exports"), DependencyType::NewUrl => Cow::Borrowed("new URL()"), DependencyType::NewWorker => Cow::Borrowed("new Worker()"), DependencyType::ImportMetaHotAccept => Cow::Borrowed("import.meta.webpackHot.accept"), diff --git a/crates/rspack_core/src/dependency/runtime_template.rs b/crates/rspack_core/src/dependency/runtime_template.rs index f10458046d0..59ee7eb3e76 100644 --- a/crates/rspack_core/src/dependency/runtime_template.rs +++ b/crates/rspack_core/src/dependency/runtime_template.rs @@ -61,7 +61,7 @@ pub fn export_from_import( format!("var {import_var}_namespace_cache;\n",), InitFragmentStage::StageHarmonyExports, -1, - InitFragmentKey::uniqie(), + InitFragmentKey::unique(), None, ) .boxed(), diff --git a/crates/rspack_core/src/exports_info.rs b/crates/rspack_core/src/exports_info.rs index c234eb1a6ca..0bb2b9feea0 100644 --- a/crates/rspack_core/src/exports_info.rs +++ b/crates/rspack_core/src/exports_info.rs @@ -348,7 +348,10 @@ impl ExportsInfoId { let info = self.get_read_only_export_info(&name, mg); info.get_used_name(&name, runtime).map(UsedName::Str) } - UsedName::Vec(_) => todo!(), + UsedName::Vec(_) => { + // TODO + Some(name.clone()) + } } } @@ -514,7 +517,7 @@ impl ExportsInfo { } } -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum UsedName { Str(JsWord), Vec(Vec), diff --git a/crates/rspack_core/src/init_fragment.rs b/crates/rspack_core/src/init_fragment.rs index 781d987391a..3bbee1470ef 100644 --- a/crates/rspack_core/src/init_fragment.rs +++ b/crates/rspack_core/src/init_fragment.rs @@ -19,8 +19,8 @@ pub struct InitFragmentContents { pub end: Option, } -pub struct InitFragmentKeyUniqie; -pub type InitFragmentKeyUKey = rspack_database::Ukey; +pub struct InitFragmentKeyUnique; +pub type InitFragmentKeyUKey = rspack_database::Ukey; #[derive(Debug, Clone, Hash, PartialEq, Eq)] pub enum InitFragmentKey { @@ -31,12 +31,12 @@ pub enum InitFragmentKey { AwaitDependencies, HarmonyCompatibility, ModuleDecorator(String /* module_id */), - Uniqie(InitFragmentKeyUKey), + Unique(InitFragmentKeyUKey), } impl InitFragmentKey { - pub fn uniqie() -> Self { - Self::Uniqie(rspack_database::Ukey::new()) + pub fn unique() -> Self { + Self::Unique(rspack_database::Ukey::new()) } } @@ -79,7 +79,7 @@ impl InitFragmentKey { | InitFragmentKey::HarmonyExportStar(_) | InitFragmentKey::ExternalModule(_) | InitFragmentKey::ModuleDecorator(_) => first(fragments), - InitFragmentKey::HarmonyCompatibility | InitFragmentKey::Uniqie(_) => { + InitFragmentKey::HarmonyCompatibility | InitFragmentKey::Unique(_) => { debug_assert!(fragments.len() == 1, "fragment = {:?}", self); first(fragments) } diff --git a/crates/rspack_core/src/runtime_globals.rs b/crates/rspack_core/src/runtime_globals.rs index d971ea87ee9..99b820145ce 100644 --- a/crates/rspack_core/src/runtime_globals.rs +++ b/crates/rspack_core/src/runtime_globals.rs @@ -220,6 +220,8 @@ bitflags! { * the System.register context object */ const SYSTEM_CONTEXT = 1 << 49; + + const THIS_AS_EXPORTS = 1 << 50; } } @@ -290,6 +292,7 @@ impl RuntimeGlobals { R::HARMONY_MODULE_DECORATOR => "__webpack_require__.hmd", R::NODE_MODULE_DECORATOR => "__webpack_require__.nmd", R::SYSTEM_CONTEXT => "__webpack_require__.y", + R::THIS_AS_EXPORTS => "top-level-this-exports", r => panic!( "Unexpected flag `{r:?}`. RuntimeGlobals should only be printed for one single flag." ), diff --git a/crates/rspack_core/src/utils/visitor.rs b/crates/rspack_core/src/utils/visitor.rs index 2d8a44e8a6b..c1af8e246fd 100644 --- a/crates/rspack_core/src/utils/visitor.rs +++ b/crates/rspack_core/src/utils/visitor.rs @@ -225,6 +225,10 @@ pub fn extract_member_expression_chain<'e, T: Into>>( Expr::Member(ref expr) => { walk_member_expr(expr, false, members, members_optionals, members_spans, kind) } + Expr::This(ref this_expr) => { + members.push_front((JsWord::from("this"), this_expr.span.ctxt)); + members_spans.push_front(this_expr.span); + } Expr::Ident(ref ident) => { members.push_front((ident.sym.clone(), ident.span.ctxt)); members_spans.push_front(ident.span); diff --git a/crates/rspack_plugin_javascript/src/dependency/commonjs/mod.rs b/crates/rspack_plugin_javascript/src/dependency/commonjs/mod.rs index 40a3cda0b2f..5a8a34e0d09 100644 --- a/crates/rspack_plugin_javascript/src/dependency/commonjs/mod.rs +++ b/crates/rspack_plugin_javascript/src/dependency/commonjs/mod.rs @@ -1,4 +1,7 @@ +mod common_js_exports_dependency; mod common_js_require_dependency; +pub use common_js_exports_dependency::CommonJsExportsDependency; +pub use common_js_exports_dependency::ExportsBase; pub use common_js_require_dependency::CommonJsRequireDependency; mod require_resolve_dependency; pub use require_resolve_dependency::RequireResolveDependency; diff --git a/crates/rspack_plugin_javascript/src/dependency/esm/harmony_compatibility_dependency.rs b/crates/rspack_plugin_javascript/src/dependency/esm/harmony_compatibility_dependency.rs index 4bfd3d5473f..9b790c1ab61 100644 --- a/crates/rspack_plugin_javascript/src/dependency/esm/harmony_compatibility_dependency.rs +++ b/crates/rspack_plugin_javascript/src/dependency/esm/harmony_compatibility_dependency.rs @@ -58,7 +58,7 @@ impl DependencyTemplate for HarmonyCompatibilityDependency { ), InitFragmentStage::StageAsyncBoundary, 0, - InitFragmentKey::uniqie(), + InitFragmentKey::unique(), Some(format!("\n__webpack_async_result__();\n}} catch(e) {{ __webpack_async_result__(e); }} }}{});", if matches!(mgm.build_meta.as_ref().map(|meta| meta.has_top_level_await), Some(true)) { ", 1" } else { "" })), ))); } diff --git a/crates/rspack_plugin_javascript/src/plugin/mod.rs b/crates/rspack_plugin_javascript/src/plugin/mod.rs index e8f22f9acb8..3ade24f4da3 100644 --- a/crates/rspack_plugin_javascript/src/plugin/mod.rs +++ b/crates/rspack_plugin_javascript/src/plugin/mod.rs @@ -86,9 +86,17 @@ impl JsPlugin { execOptions.factory.call(module.exports, module, module.exports, execOptions.require); "#, ), - false => RawSource::from( - "__webpack_modules__[moduleId](module, module.exports, __webpack_require__);\n", - ), + false => { + if runtime_requirements.contains(RuntimeGlobals::THIS_AS_EXPORTS) { + RawSource::from( + "__webpack_modules__[moduleId].call(module.exports, module, module.exports, __webpack_require__);\n", + ) + } else { + RawSource::from( + "__webpack_modules__[moduleId](module, module.exports, __webpack_require__);\n", + ) + } + } }; if strict_module_error_handling { diff --git a/crates/rspack_plugin_javascript/src/runtime.rs b/crates/rspack_plugin_javascript/src/runtime.rs index a9aff442722..8d317002372 100644 --- a/crates/rspack_plugin_javascript/src/runtime.rs +++ b/crates/rspack_plugin_javascript/src/runtime.rs @@ -104,18 +104,35 @@ fn render_module( runtime_requirements: Option<&RuntimeGlobals>, module_id: &str, ) -> Result { - // TODO unused exports_argument - let module_argument = { + let need_module = runtime_requirements.is_some_and(|r| r.contains(RuntimeGlobals::MODULE)); + // TODO: determine arguments by runtime requirements after aligning commonjs dependencies with webpack + // let need_exports = runtime_requirements.is_some_and(|r| r.contains(RuntimeGlobals::EXPORTS)); + // let need_require = runtime_requirements.is_some_and(|r| { + // r.contains(RuntimeGlobals::REQUIRE) || r.contains(RuntimeGlobals::REQUIRE_SCOPE) + // }); + let need_exports = true; + let need_require = true; + + let mut args = Vec::new(); + if need_module || need_exports || need_require { let module_argument = mgm.get_module_argument(); - if let Some(runtime_requirements) = runtime_requirements - && runtime_requirements.contains(RuntimeGlobals::MODULE) - { + args.push(if need_module { module_argument.to_string() } else { format!("__unused_webpack_{module_argument}") - } - }; - let exports_argument = mgm.get_exports_argument(); + }); + } + if need_exports || need_require { + let exports_argument = mgm.get_exports_argument(); + args.push(if need_exports { + exports_argument.to_string() + } else { + format!("__unused_webpack_{exports_argument}") + }); + } + if need_require { + args.push(RuntimeGlobals::REQUIRE.to_string()); + } let mut sources = ConcatSource::new([ RawSource::from(serde_json::to_string(module_id).map_err(|e| internal_error!(e.to_string()))?), RawSource::from(": "), @@ -124,8 +141,8 @@ fn render_module( sources.add(RawSource::from(format!("\n/* start::{} */\n", module_id))); } sources.add(RawSource::from(format!( - "(function ({module_argument}, {exports_argument}, {}) {{\n", - RuntimeGlobals::REQUIRE + "(function ({}) {{\n", + args.join(", ") ))); if let Some(build_info) = &mgm.build_info && build_info.strict diff --git a/crates/rspack_plugin_javascript/src/visitors/dependency/common_js_export_scanner.rs b/crates/rspack_plugin_javascript/src/visitors/dependency/common_js_export_scanner.rs index 81b00a1c659..ae897ffd511 100644 --- a/crates/rspack_plugin_javascript/src/visitors/dependency/common_js_export_scanner.rs +++ b/crates/rspack_plugin_javascript/src/visitors/dependency/common_js_export_scanner.rs @@ -1,9 +1,9 @@ use rspack_core::{ - BuildMeta, BuildMetaDefaultObject, BuildMetaExportsType, DependencyTemplate, ModuleType, - RuntimeGlobals, + extract_member_expression_chain, BoxDependency, BuildMeta, BuildMetaDefaultObject, + BuildMetaExportsType, DependencyTemplate, ModuleType, RuntimeGlobals, SpanExt, UsedName, }; use swc_core::{ - common::SyntaxContext, + common::{Spanned, SyntaxContext}, ecma::{ ast::{ AssignExpr, CallExpr, Callee, Expr, ExprOrSpread, Ident, Lit, MemberExpr, ModuleItem, @@ -14,9 +14,10 @@ use swc_core::{ }; use super::{expr_matcher, is_require_call_expr}; -use crate::dependency::ModuleDecoratorDependency; +use crate::dependency::{CommonJsExportsDependency, ExportsBase, ModuleDecoratorDependency}; pub struct CommonJsExportDependencyScanner<'a> { + dependencies: &'a mut Vec, presentational_dependencies: &'a mut Vec>, unresolved_ctxt: SyntaxContext, build_meta: &'a mut BuildMeta, @@ -30,6 +31,7 @@ pub struct CommonJsExportDependencyScanner<'a> { impl<'a> CommonJsExportDependencyScanner<'a> { pub fn new( + dependencies: &'a mut Vec, presentational_dependencies: &'a mut Vec>, unresolved_ctxt: SyntaxContext, build_meta: &'a mut BuildMeta, @@ -37,6 +39,7 @@ impl<'a> CommonJsExportDependencyScanner<'a> { parser_exports_state: &'a mut Option, ) -> Self { Self { + dependencies, presentational_dependencies, unresolved_ctxt, build_meta, @@ -107,32 +110,69 @@ impl Visit for CommonJsExportDependencyScanner<'_> { fn visit_assign_expr(&mut self, assign_expr: &AssignExpr) { if let PatOrExpr::Pat(box Pat::Expr(box expr)) = &assign_expr.left { - // exports.__esModule = true; - // module.exports.__esModule = true; - // this.__esModule = true; - if expr_matcher::is_module_exports_esmodule(expr) - || expr_matcher::is_exports_esmodule(expr) - || expr_matcher::is_this_esmodule(expr) - { - self.enable(); - self.check_namespace( - // const flagIt = () => (exports.__esModule = true); => stmt_level = 1, last_stmt_is_expr_stmt = false - // const flagIt = () => { exports.__esModule = true }; => stmt_level = 2, last_stmt_is_expr_stmt = true - // (exports.__esModule = true); => stmt_level = 1, last_stmt_is_expr_stmt = true - self.stmt_level == 1 && self.last_stmt_is_expr_stmt, - Some(&assign_expr.right), - ); - assign_expr.right.visit_children_with(self); - return; - } // exports.xxx = 1; - if self.is_exports_member_expr_start(expr) { - self.enable(); + // module.exports.xxx = 1; + // this.xxx = 1; + let is_exports_start = self.is_exports_member_expr_start(expr); + let is_module_exports_start = self.is_module_exports_member_expr_start(expr); + let is_this_start: bool = self.is_this_member_expr_start(expr); + + if is_exports_start || is_module_exports_start || is_this_start { + if is_exports_start { + self.enable(); + } + + let remaining_members = expr.as_member().map(|expr: &MemberExpr| { + extract_member_expression_chain(expr) + .members() + .iter() + .skip(if is_module_exports_start { 2 } else { 1 }) + .map(|n| n.0.clone()) + .collect::>() + }); + + if let Some(remaining_members) = remaining_members + && !remaining_members.is_empty() + { + // exports.__esModule = true; + // module.exports.__esModule = true; + // this.__esModule = true; + if let Some(first_member) = remaining_members.first() + && first_member == "__esModule" + { + self.check_namespace( + // const flagIt = () => (exports.__esModule = true); => stmt_level = 1, last_stmt_is_expr_stmt = false + // const flagIt = () => { exports.__esModule = true }; => stmt_level = 2, last_stmt_is_expr_stmt = true + // (exports.__esModule = true); => stmt_level = 1, last_stmt_is_expr_stmt = true + self.stmt_level == 1 && self.last_stmt_is_expr_stmt, + Some(&assign_expr.right), + ); + } + + self + .dependencies + .push(Box::new(CommonJsExportsDependency::new( + (expr.span().real_lo(), expr.span().real_hi()), + None, + if is_exports_start { + ExportsBase::Exports + } else if is_module_exports_start { + ExportsBase::ModuleExports + } else if is_this_start { + ExportsBase::This + } else { + panic!("Unexpected expr type"); + }, + UsedName::Vec(remaining_members), + ))); + } + assign_expr.right.visit_children_with(self); return; } if self.is_exports_or_module_exports_or_this_expr(expr) { self.enable(); + if is_require_call_expr(&assign_expr.right, self.unresolved_ctxt) { // exports = require('xx'); // module.exports = require('xx'); @@ -154,9 +194,9 @@ impl Visit for CommonJsExportDependencyScanner<'_> { fn visit_call_expr(&mut self, call_expr: &CallExpr) { if let Callee::Expr(expr) = &call_expr.callee { - // Object.defineProperty(exports, "__esModule", { value: true }); - // Object.defineProperty(module.exports, "__esModule", { value: true }); - // Object.defineProperty(this, "__esModule", { value: true }); + // Object.defineProperty(exports, "xxx", { value: 1 }); + // Object.defineProperty(module.exports, "xxx", { value: 1 }); + // Object.defineProperty(this, "xxx", { value: 1 }); if expr_matcher::is_object_define_property(expr) && let Some(ExprOrSpread { expr, .. }) = call_expr.args.first() && self.is_exports_or_module_exports_or_this_expr(expr) @@ -168,12 +208,33 @@ impl Visit for CommonJsExportDependencyScanner<'_> { expr: box Expr::Lit(Lit::Str(str)), .. }) = call_expr.args.get(1) - && str.value == "__esModule" { - self.check_namespace( - self.stmt_level == 1, - get_value_of_property_description(arg2), - ); + // Object.defineProperty(exports, "__esModule", { value: true }); + // Object.defineProperty(module.exports, "__esModule", { value: true }); + // Object.defineProperty(this, "__esModule", { value: true }); + if str.value == "__esModule" { + self.check_namespace( + self.stmt_level == 1, + get_value_of_property_description(arg2), + ); + } + + self + .dependencies + .push(Box::new(CommonJsExportsDependency::new( + (call_expr.span.real_lo(), call_expr.span.real_hi()), + Some((arg2.span().real_lo(), arg2.span().real_hi())), + if self.is_exports_expr(expr) { + ExportsBase::DefinePropertyExports + } else if expr_matcher::is_module_exports(expr) { + ExportsBase::DefinePropertyModuleExports + } else if self.is_this_expr(expr) { + ExportsBase::DefinePropertyThis + } else { + panic!("Unexpected expr type"); + }, + UsedName::Vec(vec![str.value.clone()]), + ))); } self.enter_call += 1; @@ -209,16 +270,41 @@ impl<'a> CommonJsExportDependencyScanner<'a> { } } + fn is_module_exports_member_expr_start(&self, mut expr: &Expr) -> bool { + loop { + match expr { + _ if expr_matcher::is_module_exports(expr) => return true, + Expr::Member(MemberExpr { obj, .. }) => expr = obj.as_ref(), + _ => return false, + } + } + } + + fn is_this_member_expr_start(&self, mut expr: &Expr) -> bool { + if self.enter_call != 0 { + return false; + } + loop { + match expr { + _ if self.is_this_expr(expr) => return true, + Expr::Member(MemberExpr { obj, .. }) => expr = obj.as_ref(), + _ => return false, + } + } + } + fn is_exports_or_module_exports_or_this_expr(&self, expr: &Expr) -> bool { - matches!(expr, Expr::Ident(ident) if &ident.sym == "exports" && ident.span.ctxt == self.unresolved_ctxt) - || expr_matcher::is_module_exports(expr) - || matches!(expr, Expr::This(_) if self.enter_call == 0) + self.is_exports_expr(expr) || expr_matcher::is_module_exports(expr) || self.is_this_expr(expr) } fn is_exports_expr(&self, expr: &Expr) -> bool { matches!(expr, Expr::Ident(ident) if &ident.sym == "exports" && ident.span.ctxt == self.unresolved_ctxt) } + fn is_this_expr(&self, expr: &Expr) -> bool { + matches!(expr, Expr::This(_) if self.enter_call == 0 && self.stmt_level == 1) + } + fn check_namespace(&mut self, top_level: bool, value_expr: Option<&Expr>) { if matches!(self.parser_exports_state, Some(false)) || self.parser_exports_state.is_none() { return; diff --git a/crates/rspack_plugin_javascript/src/visitors/dependency/mod.rs b/crates/rspack_plugin_javascript/src/visitors/dependency/mod.rs index 9a41035852c..ce6171ea0f7 100644 --- a/crates/rspack_plugin_javascript/src/visitors/dependency/mod.rs +++ b/crates/rspack_plugin_javascript/src/visitors/dependency/mod.rs @@ -117,6 +117,7 @@ pub fn scan_dependencies( )); program.visit_with(&mut RequireContextScanner::new(&mut dependencies)); program.visit_with(&mut CommonJsExportDependencyScanner::new( + &mut dependencies, &mut presentational_dependencies, unresolved_ctxt, build_meta, diff --git a/crates/rspack_plugin_javascript/src/visitors/dependency/util.rs b/crates/rspack_plugin_javascript/src/visitors/dependency/util.rs index de28fd0f0e9..73a41f7fb22 100644 --- a/crates/rspack_plugin_javascript/src/visitors/dependency/util.rs +++ b/crates/rspack_plugin_javascript/src/visitors/dependency/util.rs @@ -97,9 +97,6 @@ pub(crate) mod expr_matcher { is_import_meta_webpack_context: "import.meta.webpackContext", is_import_meta_url: "import.meta.url", is_import_meta: "import.meta", - is_exports_esmodule: "exports.__esModule", - is_this_esmodule: "this.__esModule", - is_module_exports_esmodule: "module.exports.__esModule", is_object_define_property: "Object.defineProperty", }); } diff --git a/packages/rspack/tests/Stats.test.ts b/packages/rspack/tests/Stats.test.ts index ad93a04ecec..973afcbb7b7 100644 --- a/packages/rspack/tests/Stats.test.ts +++ b/packages/rspack/tests/Stats.test.ts @@ -79,7 +79,7 @@ describe("Stats", () => { - Rspack compiled with 1 error (720d278abb396fd4623f)" + Rspack compiled with 1 error (22193973344376247dbc)" `); }); diff --git a/packages/rspack/tests/__snapshots__/Stats.test.ts.snap b/packages/rspack/tests/__snapshots__/Stats.test.ts.snap index 6d30e8f5d12..97a6f1f2325 100644 --- a/packages/rspack/tests/__snapshots__/Stats.test.ts.snap +++ b/packages/rspack/tests/__snapshots__/Stats.test.ts.snap @@ -421,7 +421,7 @@ exports.c = require("./c?c=3"); "errors": [], "errorsCount": 0, "filteredModules": undefined, - "hash": "592ad362eb9d0efc897b", + "hash": "f141b204a28da3455a73", "logging": {}, "modules": [ { diff --git a/packages/rspack/tests/cases/cjs-tree-shaking/non-root-this/arrow.js b/packages/rspack/tests/cases/cjs-tree-shaking/non-root-this/arrow.js new file mode 100644 index 00000000000..cb0d70003cb --- /dev/null +++ b/packages/rspack/tests/cases/cjs-tree-shaking/non-root-this/arrow.js @@ -0,0 +1,8 @@ +const F = () => { + this.test1 = true; + Object.defineProperty(this, "test2", { + value: true + }); + return this; +}; +exports.fff = F(); diff --git a/packages/rspack/tests/cases/cjs-tree-shaking/non-root-this/class.js b/packages/rspack/tests/cases/cjs-tree-shaking/non-root-this/class.js new file mode 100644 index 00000000000..b2c1364bb42 --- /dev/null +++ b/packages/rspack/tests/cases/cjs-tree-shaking/non-root-this/class.js @@ -0,0 +1,9 @@ +class F { + constructor() { + this.test1 = true; + Object.defineProperty(this, "test2", { + value: true + }); + } +} +exports.fff = new F(); diff --git a/packages/rspack/tests/cases/cjs-tree-shaking/non-root-this/function.js b/packages/rspack/tests/cases/cjs-tree-shaking/non-root-this/function.js new file mode 100644 index 00000000000..fe838288ccf --- /dev/null +++ b/packages/rspack/tests/cases/cjs-tree-shaking/non-root-this/function.js @@ -0,0 +1,7 @@ +function F() { + this.test1 = true; + Object.defineProperty(this, "test2", { + value: true + }); +} +exports.fff = new F(); diff --git a/packages/rspack/tests/cases/cjs-tree-shaking/non-root-this/index.js b/packages/rspack/tests/cases/cjs-tree-shaking/non-root-this/index.js new file mode 100644 index 00000000000..204dbd09f50 --- /dev/null +++ b/packages/rspack/tests/cases/cjs-tree-shaking/non-root-this/index.js @@ -0,0 +1,15 @@ +it("should not rewrite this nested in functions", () => { + const f = require("./function.js").fff; + expect(f.test1).toBe(true); + expect(f.test2).toBe(true); +}); +it("should not rewrite this nested in class", () => { + const f = require("./class.js").fff; + expect(f.test1).toBe(true); + expect(f.test2).toBe(true); +}); +it("should not rewrite this nested in arrow functions", () => { + const f = require("./arrow.js").fff; + expect(f.test1).toBe(true); + expect(f.test2).toBe(true); +}); diff --git a/packages/rspack/tests/cases/cjs-tree-shaking/sub-properties/exports.js b/packages/rspack/tests/cases/cjs-tree-shaking/sub-properties/exports.js new file mode 100644 index 00000000000..51eb76adbdb --- /dev/null +++ b/packages/rspack/tests/cases/cjs-tree-shaking/sub-properties/exports.js @@ -0,0 +1,16 @@ +exports.aaa = function () { + return 1; +}; +exports.aaa.bbb = function () { + return 2; +}; +Object.defineProperty(exports, "ccc", { + value: function () { + return 3; + } +}); +Object.defineProperty(exports.ccc, "ddd", { + value: function () { + return 4; + } +}); diff --git a/packages/rspack/tests/cases/cjs-tree-shaking/sub-properties/index.js b/packages/rspack/tests/cases/cjs-tree-shaking/sub-properties/index.js new file mode 100644 index 00000000000..25cff1bec19 --- /dev/null +++ b/packages/rspack/tests/cases/cjs-tree-shaking/sub-properties/index.js @@ -0,0 +1,21 @@ +it("should handle sub properties of module exports correctly", () => { + const mod = require("./module-exports.js"); + expect(mod.aaa()).toBe(1); + expect(mod.aaa.bbb()).toBe(2); + expect(mod.ccc()).toBe(3); + expect(mod.ccc.ddd()).toBe(4); +}); +it("should handle sub properties of this correctly", () => { + const mod = require("./this.js"); + expect(mod.aaa()).toBe(1); + expect(mod.aaa.bbb()).toBe(2); + expect(mod.ccc()).toBe(3); + expect(mod.ccc.ddd()).toBe(4); +}); +it("should handle sub properties of exports correctly", () => { + const mod = require("./exports.js"); + expect(mod.aaa()).toBe(1); + expect(mod.aaa.bbb()).toBe(2); + expect(mod.ccc()).toBe(3); + expect(mod.ccc.ddd()).toBe(4); +}); diff --git a/packages/rspack/tests/cases/cjs-tree-shaking/sub-properties/module-exports.js b/packages/rspack/tests/cases/cjs-tree-shaking/sub-properties/module-exports.js new file mode 100644 index 00000000000..f1316bb8ad9 --- /dev/null +++ b/packages/rspack/tests/cases/cjs-tree-shaking/sub-properties/module-exports.js @@ -0,0 +1,16 @@ +module.exports.aaa = function () { + return 1; +}; +module.exports.aaa.bbb = function () { + return 2; +}; +Object.defineProperty(module.exports, "ccc", { + value: function () { + return 3; + } +}); +Object.defineProperty(module.exports.ccc, "ddd", { + value: function () { + return 4; + } +}); diff --git a/packages/rspack/tests/cases/cjs-tree-shaking/sub-properties/this.js b/packages/rspack/tests/cases/cjs-tree-shaking/sub-properties/this.js new file mode 100644 index 00000000000..f1b86f5fc0e --- /dev/null +++ b/packages/rspack/tests/cases/cjs-tree-shaking/sub-properties/this.js @@ -0,0 +1,16 @@ +this.aaa = function () { + return 1; +}; +this.aaa.bbb = function () { + return 2; +}; +Object.defineProperty(this, "ccc", { + value: function () { + return 3; + } +}); +Object.defineProperty(this.ccc, "ddd", { + value: function () { + return 4; + } +}); diff --git a/webpack-test/cases/cjs-tree-shaking/bailouts/test.filter.js b/webpack-test/cases/cjs-tree-shaking/bailouts/test.filter.js index b4de6f09ef2..a1f8b7c2263 100644 --- a/webpack-test/cases/cjs-tree-shaking/bailouts/test.filter.js +++ b/webpack-test/cases/cjs-tree-shaking/bailouts/test.filter.js @@ -1,5 +1,4 @@ const { FilteredStatus } = require("../../../lib/util/filterUtil") -module.exports = () => {return [FilteredStatus.PARTIAL_PASS, "https://github.com/web-infra-dev/rspack/issues/4313, https://github.com/web-infra-dev/rspack/issues/4324"]} +module.exports = () => { return [FilteredStatus.PARTIAL_PASS, "https://github.com/web-infra-dev/rspack/issues/4313"] } - \ No newline at end of file diff --git a/webpack-test/cases/cjs-tree-shaking/exports/test.filter.js b/webpack-test/cases/cjs-tree-shaking/exports/test.filter.js deleted file mode 100644 index c6f9d8b823e..00000000000 --- a/webpack-test/cases/cjs-tree-shaking/exports/test.filter.js +++ /dev/null @@ -1,5 +0,0 @@ -const { FilteredStatus } = require("../../../lib/util/filterUtil") - -module.exports = () => {return [FilteredStatus.PARTIAL_PASS, "https://github.com/web-infra-dev/rspack/issues/4324"]} - - \ No newline at end of file diff --git a/webpack-test/cases/cjs-tree-shaking/object-define-property-replace/test.filter.js b/webpack-test/cases/cjs-tree-shaking/object-define-property-replace/test.filter.js deleted file mode 100644 index 05c60700f91..00000000000 --- a/webpack-test/cases/cjs-tree-shaking/object-define-property-replace/test.filter.js +++ /dev/null @@ -1,4 +0,0 @@ - -module.exports = () => {return "https://github.com/web-infra-dev/rspack/issues/4324"} - - \ No newline at end of file From 51916fce408a49d0f8c9fa780d670da096e1f04b Mon Sep 17 00:00:00 2001 From: LingyuCoder <--global> Date: Tue, 21 Nov 2023 15:02:16 +0800 Subject: [PATCH 2/2] fix: top level --- .../dependency/common_js_export_scanner.rs | 48 +++++++++++++++++-- .../cjs-tree-shaking/top-level-this/block.js | 10 ++++ .../cjs-tree-shaking/top-level-this/index.js | 10 ++++ .../cjs-tree-shaking/top-level-this/root.js | 4 ++ 4 files changed, 68 insertions(+), 4 deletions(-) create mode 100644 packages/rspack/tests/cases/cjs-tree-shaking/top-level-this/block.js create mode 100644 packages/rspack/tests/cases/cjs-tree-shaking/top-level-this/index.js create mode 100644 packages/rspack/tests/cases/cjs-tree-shaking/top-level-this/root.js diff --git a/crates/rspack_plugin_javascript/src/visitors/dependency/common_js_export_scanner.rs b/crates/rspack_plugin_javascript/src/visitors/dependency/common_js_export_scanner.rs index ae897ffd511..ad3cbf936db 100644 --- a/crates/rspack_plugin_javascript/src/visitors/dependency/common_js_export_scanner.rs +++ b/crates/rspack_plugin_javascript/src/visitors/dependency/common_js_export_scanner.rs @@ -6,15 +6,19 @@ use swc_core::{ common::{Spanned, SyntaxContext}, ecma::{ ast::{ - AssignExpr, CallExpr, Callee, Expr, ExprOrSpread, Ident, Lit, MemberExpr, ModuleItem, - ObjectLit, Pat, PatOrExpr, Program, Prop, PropName, PropOrSpread, Stmt, UnaryOp, + ArrowExpr, AssignExpr, CallExpr, Callee, ClassMember, Expr, ExprOrSpread, FnDecl, FnExpr, + Ident, Lit, MemberExpr, ModuleItem, ObjectLit, Pat, PatOrExpr, Program, Prop, PropName, + PropOrSpread, Stmt, UnaryOp, }, visit::{noop_visit_type, Visit, VisitWith}, }, }; use super::{expr_matcher, is_require_call_expr}; -use crate::dependency::{CommonJsExportsDependency, ExportsBase, ModuleDecoratorDependency}; +use crate::{ + dependency::{CommonJsExportsDependency, ExportsBase, ModuleDecoratorDependency}, + ClassExt, +}; pub struct CommonJsExportDependencyScanner<'a> { dependencies: &'a mut Vec, @@ -27,6 +31,7 @@ pub struct CommonJsExportDependencyScanner<'a> { enter_call: u32, stmt_level: u32, last_stmt_is_expr_stmt: bool, + is_top_level: bool, } impl<'a> CommonJsExportDependencyScanner<'a> { @@ -49,6 +54,7 @@ impl<'a> CommonJsExportDependencyScanner<'a> { enter_call: 0, stmt_level: 0, last_stmt_is_expr_stmt: false, + is_top_level: true, } } } @@ -257,6 +263,40 @@ impl Visit for CommonJsExportDependencyScanner<'_> { call_expr.visit_children_with(self); self.enter_call -= 1; } + + fn visit_class_member(&mut self, node: &ClassMember) { + if let Some(key) = node.class_key() + && key.is_computed() + { + key.visit_with(self); + } + + let top_level = self.is_top_level; + self.is_top_level = false; + node.visit_children_with(self); + self.is_top_level = top_level; + } + + fn visit_fn_decl(&mut self, node: &FnDecl) { + let top_level = self.is_top_level; + self.is_top_level = false; + node.visit_children_with(self); + self.is_top_level = top_level; + } + + fn visit_fn_expr(&mut self, node: &FnExpr) { + let top_level = self.is_top_level; + self.is_top_level = false; + node.visit_children_with(self); + self.is_top_level = top_level; + } + + fn visit_arrow_expr(&mut self, node: &ArrowExpr) { + let top_level = self.is_top_level; + self.is_top_level = false; + node.visit_children_with(self); + self.is_top_level = top_level; + } } impl<'a> CommonJsExportDependencyScanner<'a> { @@ -302,7 +342,7 @@ impl<'a> CommonJsExportDependencyScanner<'a> { } fn is_this_expr(&self, expr: &Expr) -> bool { - matches!(expr, Expr::This(_) if self.enter_call == 0 && self.stmt_level == 1) + matches!(expr, Expr::This(_) if self.is_top_level) } fn check_namespace(&mut self, top_level: bool, value_expr: Option<&Expr>) { diff --git a/packages/rspack/tests/cases/cjs-tree-shaking/top-level-this/block.js b/packages/rspack/tests/cases/cjs-tree-shaking/top-level-this/block.js new file mode 100644 index 00000000000..10285239b3d --- /dev/null +++ b/packages/rspack/tests/cases/cjs-tree-shaking/top-level-this/block.js @@ -0,0 +1,10 @@ +if (this.aaa !== 1) { + this.aaa = 1; +} + +while (true) { + Object.defineProperty(this, "bbb", { + value: 2 + }); + break; +} diff --git a/packages/rspack/tests/cases/cjs-tree-shaking/top-level-this/index.js b/packages/rspack/tests/cases/cjs-tree-shaking/top-level-this/index.js new file mode 100644 index 00000000000..567abbea2d5 --- /dev/null +++ b/packages/rspack/tests/cases/cjs-tree-shaking/top-level-this/index.js @@ -0,0 +1,10 @@ +it("should handle top level this in the root scope", () => { + const mod = require("./root.js"); + expect(mod.aaa).toBe(1); + expect(mod.bbb).toBe(2); +}); +it("should handle top level this in block scope", () => { + const mod = require("./block.js"); + expect(mod.aaa).toBe(1); + expect(mod.bbb).toBe(2); +}); diff --git a/packages/rspack/tests/cases/cjs-tree-shaking/top-level-this/root.js b/packages/rspack/tests/cases/cjs-tree-shaking/top-level-this/root.js new file mode 100644 index 00000000000..a457d8f784f --- /dev/null +++ b/packages/rspack/tests/cases/cjs-tree-shaking/top-level-this/root.js @@ -0,0 +1,4 @@ +this.aaa = 1; +Object.defineProperty(this, "bbb", { + value: 2 +});