From 145a0c0797a77a0d9d9b0c7d6d99246bdcc937f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Donny/=EA=B0=95=EB=8F=99=EC=9C=A4?= Date: Wed, 3 Jan 2024 00:59:09 +0900 Subject: [PATCH] fix: Fix wrong cjs detection of `auto-cjs` pass (#60118) ### What? Make `auto-cjs` pass consider shadowing of `module`, so it can ignore `module.exports = foo` in nested scopes. ### Why? It's problematic for many tasks ### How? Closes PACK-2074 --- .../next-swc/crates/core/src/auto_cjs/mod.rs | 75 +++++++++++++++---- .../core/tests/full/auto-cjs/1/output.js | 8 -- .../core/tests/full/auto-cjs/2/output.js | 13 ---- .../core/tests/full/auto-cjs/3/output.js | 4 - .../{full => loader}/auto-cjs/1/input.js | 0 .../core/tests/loader/auto-cjs/1/output.js | 12 +++ .../{full => loader}/auto-cjs/2/input.js | 0 .../core/tests/loader/auto-cjs/2/output.js | 14 ++++ .../{full => loader}/auto-cjs/3/input.js | 0 .../core/tests/loader/auto-cjs/3/output.js | 6 ++ .../loader/auto-cjs/pack-2074/1/input.js | 5 ++ .../loader/auto-cjs/pack-2074/1/output.js | 4 + .../loader/auto-cjs/pack-2074/2/input.js | 5 ++ .../loader/auto-cjs/pack-2074/2/output.js | 4 + 14 files changed, 112 insertions(+), 38 deletions(-) delete mode 100644 packages/next-swc/crates/core/tests/full/auto-cjs/1/output.js delete mode 100644 packages/next-swc/crates/core/tests/full/auto-cjs/2/output.js delete mode 100644 packages/next-swc/crates/core/tests/full/auto-cjs/3/output.js rename packages/next-swc/crates/core/tests/{full => loader}/auto-cjs/1/input.js (100%) create mode 100644 packages/next-swc/crates/core/tests/loader/auto-cjs/1/output.js rename packages/next-swc/crates/core/tests/{full => loader}/auto-cjs/2/input.js (100%) create mode 100644 packages/next-swc/crates/core/tests/loader/auto-cjs/2/output.js rename packages/next-swc/crates/core/tests/{full => loader}/auto-cjs/3/input.js (100%) create mode 100644 packages/next-swc/crates/core/tests/loader/auto-cjs/3/output.js create mode 100644 packages/next-swc/crates/core/tests/loader/auto-cjs/pack-2074/1/input.js create mode 100644 packages/next-swc/crates/core/tests/loader/auto-cjs/pack-2074/1/output.js create mode 100644 packages/next-swc/crates/core/tests/loader/auto-cjs/pack-2074/2/input.js create mode 100644 packages/next-swc/crates/core/tests/loader/auto-cjs/pack-2074/2/output.js diff --git a/packages/next-swc/crates/core/src/auto_cjs/mod.rs b/packages/next-swc/crates/core/src/auto_cjs/mod.rs index 4ccf5f9644..316736f866 100644 --- a/packages/next-swc/crates/core/src/auto_cjs/mod.rs +++ b/packages/next-swc/crates/core/src/auto_cjs/mod.rs @@ -14,24 +14,32 @@ struct CjsFinder { found: bool, } +impl CjsFinder { + /// If the given pattern contains `module` as a parameter, we don't need to + /// recurse into it because `module` is shadowed. + fn contains_module_param<'a, I>(&self, mut iter: I) -> bool + where + I: Iterator, + { + iter.any(|p| { + if let Pat::Ident(i) = p { + &*i.id.sym == "module" + } else { + false + } + }) + } +} + /// This visitor implementation supports typescript, because the api of `swc` /// does not support changing configuration based on content of the file. impl Visit for CjsFinder { - fn visit_member_expr(&mut self, e: &MemberExpr) { - if let Expr::Ident(obj) = &*e.obj { - if let MemberProp::Ident(prop) = &e.prop { - // Detect `module.exports` and `exports.__esModule` - if (&*obj.sym == "module" && &*prop.sym == "exports") - || (&*obj.sym == "exports" && &*prop.sym == "__esModule") - { - self.found = true; - return; - } - } + fn visit_arrow_expr(&mut self, n: &ArrowExpr) { + if self.contains_module_param(n.params.iter()) { + return; } - e.obj.visit_with(self); - e.prop.visit_with(self); + n.visit_children_with(self); } // Detect `Object.defineProperty(exports, "__esModule", ...)` @@ -65,4 +73,45 @@ impl Visit for CjsFinder { e.callee.visit_with(self); } + + fn visit_class_method(&mut self, n: &ClassMethod) { + if self.contains_module_param(n.function.params.iter().map(|v| &v.pat)) { + return; + } + + n.visit_children_with(self); + } + + fn visit_function(&mut self, n: &Function) { + if self.contains_module_param(n.params.iter().map(|v| &v.pat)) { + return; + } + + n.visit_children_with(self); + } + + fn visit_member_expr(&mut self, e: &MemberExpr) { + if let Expr::Ident(obj) = &*e.obj { + if let MemberProp::Ident(prop) = &e.prop { + // Detect `module.exports` and `exports.__esModule` + if (&*obj.sym == "module" && &*prop.sym == "exports") + || (&*obj.sym == "exports" && &*prop.sym == "__esModule") + { + self.found = true; + return; + } + } + } + + e.obj.visit_with(self); + e.prop.visit_with(self); + } + + fn visit_method_prop(&mut self, n: &MethodProp) { + if self.contains_module_param(n.function.params.iter().map(|v| &v.pat)) { + return; + } + + n.visit_children_with(self); + } } diff --git a/packages/next-swc/crates/core/tests/full/auto-cjs/1/output.js b/packages/next-swc/crates/core/tests/full/auto-cjs/1/output.js deleted file mode 100644 index 0457bf600c..0000000000 --- a/packages/next-swc/crates/core/tests/full/auto-cjs/1/output.js +++ /dev/null @@ -1,8 +0,0 @@ -"use strict"; -Object.defineProperty(exports, "__esModule", { - value: !0 -}); -var e, o = (e = require("esm")) && e.__esModule ? e : { - default: e -}; -console.log(o.default.foo), module.exports = o.default; diff --git a/packages/next-swc/crates/core/tests/full/auto-cjs/2/output.js b/packages/next-swc/crates/core/tests/full/auto-cjs/2/output.js deleted file mode 100644 index 116e09cb5d..0000000000 --- a/packages/next-swc/crates/core/tests/full/auto-cjs/2/output.js +++ /dev/null @@ -1,13 +0,0 @@ -"use strict"; -Object.defineProperty(exports, "__esModule", { - value: !0 -}), Object.defineProperty(exports, "default", { - enumerable: !0, - get: function() { - return e; - } -}); -var e = 1; -Object.defineProperty(exports, "__esModule", { - value: !0 -}); diff --git a/packages/next-swc/crates/core/tests/full/auto-cjs/3/output.js b/packages/next-swc/crates/core/tests/full/auto-cjs/3/output.js deleted file mode 100644 index 7e5d970ede..0000000000 --- a/packages/next-swc/crates/core/tests/full/auto-cjs/3/output.js +++ /dev/null @@ -1,4 +0,0 @@ -export default 1; -console.log("__esModule"), Object.defineProperty({}, "__esModule", { - value: !0 -}), Object.defineProperty(); diff --git a/packages/next-swc/crates/core/tests/full/auto-cjs/1/input.js b/packages/next-swc/crates/core/tests/loader/auto-cjs/1/input.js similarity index 100% rename from packages/next-swc/crates/core/tests/full/auto-cjs/1/input.js rename to packages/next-swc/crates/core/tests/loader/auto-cjs/1/input.js diff --git a/packages/next-swc/crates/core/tests/loader/auto-cjs/1/output.js b/packages/next-swc/crates/core/tests/loader/auto-cjs/1/output.js new file mode 100644 index 0000000000..3cfb0a9c7b --- /dev/null +++ b/packages/next-swc/crates/core/tests/loader/auto-cjs/1/output.js @@ -0,0 +1,12 @@ +"use strict"; +Object.defineProperty(exports, "__esModule", { + value: true +}); +var _esm = /*#__PURE__*/ _interop_require_default(require("esm")); +function _interop_require_default(obj) { + return obj && obj.__esModule ? obj : { + default: obj + }; +} +console.log(_esm.default.foo); +module.exports = _esm.default; diff --git a/packages/next-swc/crates/core/tests/full/auto-cjs/2/input.js b/packages/next-swc/crates/core/tests/loader/auto-cjs/2/input.js similarity index 100% rename from packages/next-swc/crates/core/tests/full/auto-cjs/2/input.js rename to packages/next-swc/crates/core/tests/loader/auto-cjs/2/input.js diff --git a/packages/next-swc/crates/core/tests/loader/auto-cjs/2/output.js b/packages/next-swc/crates/core/tests/loader/auto-cjs/2/output.js new file mode 100644 index 0000000000..e5e5cec564 --- /dev/null +++ b/packages/next-swc/crates/core/tests/loader/auto-cjs/2/output.js @@ -0,0 +1,14 @@ +"use strict"; +Object.defineProperty(exports, "__esModule", { + value: true +}); +Object.defineProperty(exports, "default", { + enumerable: true, + get: function() { + return _default; + } +}); +var _default = 1; +Object.defineProperty(exports, "__esModule", { + value: true +}); diff --git a/packages/next-swc/crates/core/tests/full/auto-cjs/3/input.js b/packages/next-swc/crates/core/tests/loader/auto-cjs/3/input.js similarity index 100% rename from packages/next-swc/crates/core/tests/full/auto-cjs/3/input.js rename to packages/next-swc/crates/core/tests/loader/auto-cjs/3/input.js diff --git a/packages/next-swc/crates/core/tests/loader/auto-cjs/3/output.js b/packages/next-swc/crates/core/tests/loader/auto-cjs/3/output.js new file mode 100644 index 0000000000..3e5a204d3e --- /dev/null +++ b/packages/next-swc/crates/core/tests/loader/auto-cjs/3/output.js @@ -0,0 +1,6 @@ +export default 1; +console.log("__esModule"); +Object.defineProperty({}, "__esModule", { + value: true +}); +Object.defineProperty(); diff --git a/packages/next-swc/crates/core/tests/loader/auto-cjs/pack-2074/1/input.js b/packages/next-swc/crates/core/tests/loader/auto-cjs/pack-2074/1/input.js new file mode 100644 index 0000000000..ac315b3df8 --- /dev/null +++ b/packages/next-swc/crates/core/tests/loader/auto-cjs/pack-2074/1/input.js @@ -0,0 +1,5 @@ +export default (module) => { + module.exports = {} +} + +export const value = 'mixed-syntax-esm' diff --git a/packages/next-swc/crates/core/tests/loader/auto-cjs/pack-2074/1/output.js b/packages/next-swc/crates/core/tests/loader/auto-cjs/pack-2074/1/output.js new file mode 100644 index 0000000000..4e639dd2cd --- /dev/null +++ b/packages/next-swc/crates/core/tests/loader/auto-cjs/pack-2074/1/output.js @@ -0,0 +1,4 @@ +export default function(module) { + module.exports = {}; +}; +export var value = "mixed-syntax-esm"; diff --git a/packages/next-swc/crates/core/tests/loader/auto-cjs/pack-2074/2/input.js b/packages/next-swc/crates/core/tests/loader/auto-cjs/pack-2074/2/input.js new file mode 100644 index 0000000000..439d96626e --- /dev/null +++ b/packages/next-swc/crates/core/tests/loader/auto-cjs/pack-2074/2/input.js @@ -0,0 +1,5 @@ +function foo(module) { + module.exports = 'this is just normal assignment of scope variable' +} + +export const value = 'mixed-syntax-esm' diff --git a/packages/next-swc/crates/core/tests/loader/auto-cjs/pack-2074/2/output.js b/packages/next-swc/crates/core/tests/loader/auto-cjs/pack-2074/2/output.js new file mode 100644 index 0000000000..d4b63d0ff8 --- /dev/null +++ b/packages/next-swc/crates/core/tests/loader/auto-cjs/pack-2074/2/output.js @@ -0,0 +1,4 @@ +function foo(module) { + module.exports = "this is just normal assignment of scope variable"; +} +export var value = "mixed-syntax-esm";