fix(npm): handle cjs re-exports with the same name as an export (#15626)

cli/node/mod.rs

@@ -539,6 +539,11 @@ pub fn translate_cjs_to_esm(
     maybe_syntax: None,
   })?;
   let analysis = parsed_source.analyze_cjs();
+  let root_exports = analysis
+    .exports
+    .iter()
+    .map(|s| s.as_str())
+    .collect::<HashSet<_>>();
   let mut temp_var_count = 0;
 
   let mut source = vec![
@@ -578,8 +583,9 @@ pub fn translate_cjs_to_esm(
         idx, reexport
       ));
 
-      for export in analysis.exports.iter().filter(|e| e.as_str() != "default")
-      {
+      for export in analysis.exports.iter().filter(|e| {
+        e.as_str() != "default" && !root_exports.contains(e.as_str())
+      }) {
         add_export(
           &mut source,
           export,
@@ -605,12 +611,13 @@ pub fn translate_cjs_to_esm(
   let mut had_default = false;
   for export in analysis.exports.iter() {
     if export.as_str() == "default" {
-      // todo(dsherret): we should only do this if there was a `_esModule: true` instead
-      source.push(format!(
-        "export default Deno[Deno.internal].require.bindExport(mod[\"{}\"], mod);",
-        export,
-      ));
-      had_default = true;
+      if root_exports.contains("__esModule") {
+        source.push(format!(
+          "export default Deno[Deno.internal].require.bindExport(mod[\"{}\"], mod);",
+          export,
+        ));
+        had_default = true;
+      }
     } else {
       add_export(
         &mut source,

cli/tests/integration/npm_tests.rs

@@ -54,6 +54,13 @@ itest!(cjs_local_global_decls {
   http_server: true,
 });
 
+itest!(cjs_reexport_collision {
+  args: "run --unstable -A --quiet npm/cjs_reexport_collision/main.ts",
+  output: "npm/cjs_reexport_collision/main.out",
+  envs: env_vars(),
+  http_server: true,
+});
+
 itest!(compare_globals {
   args: "run --allow-read --unstable npm/compare_globals/main.js",
   output: "npm/compare_globals/main.out",

cli/tests/testdata/npm/cjs_reexport_collision/main.out

@@ -0,0 +1 @@
+Hi.

cli/tests/testdata/npm/cjs_reexport_collision/main.ts

@@ -0,0 +1,2 @@
+import ReExportCollision from "npm:@denotest/cjs-reexport-collision";
+ReExportCollision.sayHello();

cli/tests/testdata/npm/registry/@denotest/cjs-reexport-collision/1.0.0/index.js

@@ -0,0 +1,19 @@
+"use strict";
+var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) {
+    if (k2 === undefined) k2 = k;
+    Object.defineProperty(o, k2, { enumerable: true, get: function() { return m[k]; } });
+}) : (function(o, m, k, k2) {
+    if (k2 === undefined) k2 = k;
+    o[k2] = m[k];
+}));
+var __exportStar = (this && this.__exportStar) || function(m, exports) {
+    for (var p in m) if (p !== "default" && !Object.prototype.hasOwnProperty.call(exports, p)) __createBinding(exports, m, p);
+};
+var __importDefault = (this && this.__importDefault) || function (mod) {
+    return (mod && mod.__esModule) ? mod : { "default": mod };
+};
+// collision will occur with __esModule in other_file.js
+Object.defineProperty(exports, "__esModule", { value: true });
+const other_file_1 = __importDefault(require("./other_file"));
+__exportStar(require("./other_file"), exports);
+exports.default = other_file_1.default;

cli/tests/testdata/npm/registry/@denotest/cjs-reexport-collision/1.0.0/other_file.js

@@ -0,0 +1,10 @@
+"use strict";
+class Hello {
+  sayHello() {
+    console.log("Hi.");
+  }
+}
+// conflict will be with __esModule
+Object.defineProperty(exports, "__esModule", { value: true });
+exports.hello = new Hello();
+exports.default = new Hello();

cli/tests/testdata/npm/registry/@denotest/cjs-reexport-collision/1.0.0/package.json

@@ -0,0 +1,5 @@
+{
+  "name": "@denotest/cjs-reexport-collision",
+  "version": "1.0.0",
+  "main": "./index.js"
+}