Skip to content

Commit bafbe72

Browse files
authored
fix(bundle): use node-style CJS interop for the Deno platform (#34533)
`deno bundle` (with the default `--platform=deno`) miscompiled imports of dual CJS/ESM packages whose ESM entry default-imports a CJS entry that sets `module.exports.__esModule = true`. `tslib`'s `modules/index.js` is the canonical example. The resulting bundle threw at module-init time with `TypeError: Cannot destructure property '__extends' of 'import_tslib.default' as it is undefined`, even though the same code runs fine under both `node` and `deno run`. Fixes #34524.
1 parent 862d283 commit bafbe72

10 files changed

Lines changed: 94 additions & 2 deletions

File tree

cli/tools/bundle/mod.rs

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -687,6 +687,33 @@ var __require = __deno_internal_createRequire(import.meta.url);
687687
}
688688
}
689689

690+
// Force esbuild's CommonJS-to-ESM interop helper (`__toESM`) into "node mode"
691+
// for the Deno platform.
692+
//
693+
// esbuild emits a node-mode interop call (`__toESM(require_x(), 1)`) only when
694+
// it can tell the importing module is *explicitly* ESM (a `.mjs`/`.mts` file or
695+
// a file under a `"type": "module"` package.json). It learns this while walking
696+
// package.json during its own resolve step. In `deno bundle` resolution and
697+
// loading are handled by our plugin, and esbuild's plugin protocol has no way
698+
// to communicate a module's type, so esbuild falls back to browser-mode interop
699+
// which respects the `__esModule` marker and does not synthesize `.default`.
700+
//
701+
// For packages whose ESM wrapper default-imports a CJS entry that sets
702+
// `module.exports.__esModule = true` (e.g. tslib's `modules/index.js`), that
703+
// means `import_x.default` ends up undefined and module init throws. Deno's own
704+
// runtime always uses node-style interop here, so the bundle should too. We
705+
// rewrite the helper's `<isNodeMode> || !mod || !mod.__esModule` condition to
706+
// always pick the node-mode branch. See denoland/deno#34524.
707+
//
708+
// The variable names differ between minified and non-minified output, but the
709+
// `.__esModule` access and surrounding shape are stable, so a single regex
710+
// covers both.
711+
fn force_node_cjs_interop(contents: &str) -> String {
712+
let re =
713+
lazy_regex::regex!(r#"\w+\s*\|\|\s*!\w+\s*\|\|\s*!\w+\.__esModule\s*\?"#);
714+
re.replace(contents, "1 ?").into_owned()
715+
}
716+
690717
fn format_location(
691718
location: &esbuild_client::protocol::Location,
692719
current_dir: &Path,
@@ -2024,7 +2051,7 @@ pub fn maybe_process_contents(
20242051
if is_js {
20252052
let string = str::from_utf8(&file.contents)?;
20262053
let string = if should_replace_require_shim {
2027-
replace_require_shim(string, minified)
2054+
force_node_cjs_interop(&replace_require_shim(string, minified))
20282055
} else {
20292056
string.to_string()
20302057
};
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
{
2+
// Regression test for https://github.com/denoland/deno/issues/34524
3+
//
4+
// A dual CJS/ESM package whose ESM entry default-imports a CJS entry that
5+
// sets `module.exports.__esModule = true` (the shape tslib ships). The
6+
// bundle must use node-style CJS interop so the default import is the whole
7+
// `module.exports`, matching how `deno run` executes the same files.
8+
"tempDir": true,
9+
"tests": {
10+
"bundle": {
11+
"steps": [
12+
{
13+
"args": "bundle --quiet -o out.js entry.mjs",
14+
"output": "[WILDCARD]"
15+
},
16+
{
17+
"args": "run -A out.js",
18+
"output": "out.out"
19+
}
20+
]
21+
},
22+
"bundle_minified": {
23+
"steps": [
24+
{
25+
"args": "bundle --quiet --minify -o out.min.js entry.mjs",
26+
"output": "[WILDCARD]"
27+
},
28+
{
29+
"args": "run -A out.min.js",
30+
"output": "out.out"
31+
}
32+
]
33+
}
34+
}
35+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import * as pkg from "dual-pkg";
2+
console.log("foo:", typeof pkg.foo);
3+
console.log("bar:", typeof pkg.bar);

tests/specs/bundle/cjs_esm_default_interop/node_modules/dual-pkg/cjs.js

Lines changed: 8 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/specs/bundle/cjs_esm_default_interop/node_modules/dual-pkg/modules/index.js

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/specs/bundle/cjs_esm_default_interop/node_modules/dual-pkg/modules/package.json

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/specs/bundle/cjs_esm_default_interop/node_modules/dual-pkg/package.json

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
foo: function
2+
bar: function
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{}

tests/specs/bundle/dir_import_package_json/bundle.out

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ var __toESM = (mod, isNodeMode, target) => (target = mod != null ? __create(__ge
2020
// file that has been converted to a CommonJS file using a Babel-
2121
// compatible transform (i.e. "__esModule" has not been set), then set
2222
// "default" to the CommonJS "module.exports" for node compatibility.
23-
isNodeMode || !mod || !mod.__esModule ? __defProp(target, "default", { value: mod, enumerable: true }) : target,
23+
1 ? __defProp(target, "default", { value: mod, enumerable: true }) : target,
2424
mod
2525
));
2626

0 commit comments

Comments
 (0)