Skip to content

Commit

Permalink
fix: Fix wrong cjs detection of auto-cjs pass (vercel#60118)
Browse files Browse the repository at this point in the history
### 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
  • Loading branch information
kdy1 committed Jan 2, 2024
1 parent 5f6cd8f commit 145a0c0
Show file tree
Hide file tree
Showing 14 changed files with 112 additions and 38 deletions.
75 changes: 62 additions & 13 deletions packages/next-swc/crates/core/src/auto_cjs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Item = &'a Pat>,
{
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", ...)`
Expand Down Expand Up @@ -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);
}
}
8 changes: 0 additions & 8 deletions packages/next-swc/crates/core/tests/full/auto-cjs/1/output.js

This file was deleted.

13 changes: 0 additions & 13 deletions packages/next-swc/crates/core/tests/full/auto-cjs/2/output.js

This file was deleted.

4 changes: 0 additions & 4 deletions packages/next-swc/crates/core/tests/full/auto-cjs/3/output.js

This file was deleted.

12 changes: 12 additions & 0 deletions packages/next-swc/crates/core/tests/loader/auto-cjs/1/output.js
Original file line number Diff line number Diff line change
@@ -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;
14 changes: 14 additions & 0 deletions packages/next-swc/crates/core/tests/loader/auto-cjs/2/output.js
Original file line number Diff line number Diff line change
@@ -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
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export default 1;
console.log("__esModule");
Object.defineProperty({}, "__esModule", {
value: true
});
Object.defineProperty();
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default (module) => {
module.exports = {}
}

export const value = 'mixed-syntax-esm'
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export default function(module) {
module.exports = {};
};
export var value = "mixed-syntax-esm";
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
function foo(module) {
module.exports = 'this is just normal assignment of scope variable'
}

export const value = 'mixed-syntax-esm'
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
function foo(module) {
module.exports = "this is just normal assignment of scope variable";
}
export var value = "mixed-syntax-esm";

0 comments on commit 145a0c0

Please sign in to comment.