fix(node): Include "node" condition during CJS re-export analysis (#2…

…5785) Fixes #25777. We were missing the "node" condition, so we were resolving to the wrong conditional export, causing our analysis to be incorrect.
denoland · Sep 21, 2024 · 9be8dce · 9be8dce
1 parent 4b02210
commit 9be8dce
Show file tree

Hide file tree

Showing 13 changed files with 67 additions and 1 deletion.
diff --git a/ext/node_resolver/analyze.rs b/ext/node_resolver/analyze.rs
@@ -196,7 +196,7 @@ impl<TCjsCodeAnalyzer: CjsCodeAnalyzer, TNodeResolverEnv: NodeResolverEnv>
             &referrer,
             // FIXME(bartlomieju): check if these conditions are okay, probably
             // should be `deno-require`, because `deno` is already used in `esm_resolver.rs`
-            &["deno", "require", "default"],
+            &["deno", "node", "require", "default"],
             NodeResolutionMode::Execution,
           );
           let reexport_specifier = match result {

diff --git a/tests/registry/npm/@denotest/conditional-exports-node/1.0.0/bad.js b/tests/registry/npm/@denotest/conditional-exports-node/1.0.0/bad.js
@@ -0,0 +1,3 @@
+module.exports = {
+  hello: "bad"
+};
diff --git a/tests/registry/npm/@denotest/conditional-exports-node/1.0.0/good.cjs b/tests/registry/npm/@denotest/conditional-exports-node/1.0.0/good.cjs
@@ -0,0 +1,3 @@
+module.exports = {
+    hello: "from node"
+};
diff --git a/tests/registry/npm/@denotest/conditional-exports-node/1.0.0/index.js b/tests/registry/npm/@denotest/conditional-exports-node/1.0.0/index.js
@@ -0,0 +1,3 @@
+export default {
+  hello: "default export"
+}
diff --git a/tests/registry/npm/@denotest/conditional-exports-node/1.0.0/package.json b/tests/registry/npm/@denotest/conditional-exports-node/1.0.0/package.json
@@ -0,0 +1,13 @@
+{
+  "name": "@denotest/conditional-exports",
+  "version": "1.0.0",
+  "type": "module",
+  "exports": {
+    ".": {
+      "node": "./good.cjs",
+      "require": "./bad.js",
+      "default": "./index.js"
+    },
+    "./*": "./*"
+  }
+}
diff --git a/tests/registry/npm/@denotest/mjs-reexport-cjs/1.0.0/dist/package.cjs.js b/tests/registry/npm/@denotest/mjs-reexport-cjs/1.0.0/dist/package.cjs.js
@@ -0,0 +1,7 @@
+Object.defineProperty(exports, '__esModule', { value: true });
+
+const pkg = require("@denotest/conditional-exports-node");
+
+Object.keys(pkg).forEach(function (k) {
+  if (k !== 'default' && !Object.prototype.hasOwnProperty.call(exports, k)) exports[k] = pkg[k];
+});
diff --git a/tests/registry/npm/@denotest/mjs-reexport-cjs/1.0.0/index.js b/tests/registry/npm/@denotest/mjs-reexport-cjs/1.0.0/index.js
@@ -0,0 +1 @@
+module.exports = require('./dist/package.cjs.js')
diff --git a/tests/registry/npm/@denotest/mjs-reexport-cjs/1.0.0/index.mjs b/tests/registry/npm/@denotest/mjs-reexport-cjs/1.0.0/index.mjs
@@ -0,0 +1 @@
+export * from "./index.js";
diff --git a/tests/registry/npm/@denotest/mjs-reexport-cjs/1.0.0/package.json b/tests/registry/npm/@denotest/mjs-reexport-cjs/1.0.0/package.json
@@ -0,0 +1,11 @@
+{
+  "name": "@denotest/mjs-reexport-cjs",
+  "version": "1.0.0",
+  "dependencies": {
+    "@denotest/conditional-exports-node": "*"
+  },
+  "exports": {
+    "node": "./index.mjs",
+    "default": "./index.js"
+  }
+}
diff --git a/tests/specs/node/cjs_reexport_node_condition/__test__.jsonc b/tests/specs/node/cjs_reexport_node_condition/__test__.jsonc
@@ -0,0 +1,13 @@
+{
+  "tempDir": true,
+  "steps": [
+    {
+      "args": "install",
+      "output": "[WILDCARD]"
+    },
+    {
+      "args": "run -A ./main.ts",
+      "output": "from node\n"
+    }
+  ]
+}
diff --git a/tests/specs/node/cjs_reexport_node_condition/deno.json b/tests/specs/node/cjs_reexport_node_condition/deno.json
@@ -0,0 +1,3 @@
+{
+  "nodeModulesDir": "manual"
+}
diff --git a/tests/specs/node/cjs_reexport_node_condition/main.ts b/tests/specs/node/cjs_reexport_node_condition/main.ts
@@ -0,0 +1,3 @@
+import { hello } from "@denotest/mjs-reexport-cjs";
+
+console.log(hello);
diff --git a/tests/specs/node/cjs_reexport_node_condition/package.json b/tests/specs/node/cjs_reexport_node_condition/package.json
@@ -0,0 +1,5 @@
+{
+  "dependencies": {
+    "@denotest/mjs-reexport-cjs": "*"
+  }
+}