Skip to content

Commit

Permalink
Fix SWC's auto_cjs handling (#46448)
Browse files Browse the repository at this point in the history
Closes #43732.

We currently detect if a module is CJS by look through all string tokens to see if any contains `"__esModule"`. That causes `console.log('__esModule')` being detected as CJS accidentally. This PR changes to detect these cases:
- `module.exports`
- `exports.__esModule` (e.g. `exports.__esModule = true`)
- `Object.defineProperty(exports, "__esModule", ...)` 

Although this solution isn't perfect, it's much more reliable that the current one already.

Note that this also fixes a bug for `@vercel/og` that although `satori`'s ESM module is imported, it still has the `__esModule` string inside, introduced by bundling its vendors.

## Bug

- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] [e2e](https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm build && pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)
  • Loading branch information
shuding committed Feb 27, 2023
1 parent 69fe4ee commit 16d7ffd
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 4 deletions.
37 changes: 33 additions & 4 deletions packages/next-swc/crates/core/src/auto_cjs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ 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 {
if &*obj.sym == "module" && &*prop.sym == "exports" {
// Detect `module.exports` and `exports.__esModule`
if (&*obj.sym == "module" && &*prop.sym == "exports")
|| (&*obj.sym == "exports" && &*prop.sym == "__esModule")
{
self.found = true;
return;
}
Expand All @@ -31,9 +34,35 @@ impl Visit for CjsFinder {
e.prop.visit_with(self);
}

fn visit_str(&mut self, s: &Str) {
if s.value.contains("__esModule") {
self.found = true;
// Detect `Object.defineProperty(exports, "__esModule", ...)`
// Note that `Object.defineProperty(module.exports, ...)` will be handled by
// `visit_member_expr`.
fn visit_call_expr(&mut self, e: &CallExpr) {
if let Callee::Expr(expr) = &e.callee {
if let Expr::Member(member_expr) = &**expr {
if let (Expr::Ident(obj), MemberProp::Ident(prop)) =
(&*member_expr.obj, &member_expr.prop)
{
if &*obj.sym == "Object" && &*prop.sym == "defineProperty" {
if let Some(ExprOrSpread { expr: expr0, .. }) = e.args.get(0) {
if let Expr::Ident(arg0) = &**expr0 {
if &*arg0.sym == "exports" {
if let Some(ExprOrSpread { expr: expr1, .. }) = e.args.get(1) {
if let Expr::Lit(Lit::Str(arg1)) = &**expr1 {
if &*arg1.value == "__esModule" {
self.found = true;
return;
}
}
}
}
}
}
}
}
}
}

e.callee.visit_with(self);
}
}
2 changes: 2 additions & 0 deletions packages/next-swc/crates/core/tests/full/auto-cjs/2/input.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export default 1
Object.defineProperty(exports, "__esModule", { value: true });
13 changes: 13 additions & 0 deletions packages/next-swc/crates/core/tests/full/auto-cjs/2/output.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
"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
});
4 changes: 4 additions & 0 deletions packages/next-swc/crates/core/tests/full/auto-cjs/3/input.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export default 1
console.log("__esModule")
Object.defineProperty({}, '__esModule', { value: true })
Object.defineProperty()
4 changes: 4 additions & 0 deletions packages/next-swc/crates/core/tests/full/auto-cjs/3/output.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export default 1;
console.log("__esModule"), Object.defineProperty({}, "__esModule", {
value: !0
}), Object.defineProperty();

0 comments on commit 16d7ffd

Please sign in to comment.