summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNathan Whitaker <17734409+nathanwhit@users.noreply.github.com>2024-10-31 10:02:31 -0700
committerGitHub <noreply@github.com>2024-10-31 10:02:31 -0700
commit6d44952d4daaaab8ed9ec82212d2256c058eb05d (patch)
treefb46c0acd6d04a896a1424592e7ef703a573db01
parent951103fc8de952f32386121d3f0e4803fe267ea4 (diff)
fix(ext/node): resolve exports even if parent module filename isn't present (#26553)
Fixes https://github.com/denoland/deno/issues/26505 I'm not exactly sure how this case comes about (I tried to write tests for it but couldn't manage to reproduce it), but what happens is the parent filename ends up null, and we bail out of resolving the specifier in package exports. I've checked, and in node the parent filename is also null (so that's not a bug on our part), but node continues to resolve even in that case. So this PR should match node's behavior more closely than we currently do.
-rw-r--r--ext/node/ops/require.rs8
-rw-r--r--ext/node/polyfills/01_require.js6
-rw-r--r--tests/registry/npm/@denotest/cjs-multiple-exports/1.0.0/package.json8
-rw-r--r--tests/registry/npm/@denotest/cjs-multiple-exports/1.0.0/src/add.js3
-rw-r--r--tests/registry/npm/@denotest/cjs-multiple-exports/1.0.0/src/index.js3
-rw-r--r--tests/specs/node/require_export_from_parent_with_no_filename/__test__.jsonc13
-rw-r--r--tests/specs/node/require_export_from_parent_with_no_filename/main.cjs16
-rw-r--r--tests/specs/node/require_export_from_parent_with_no_filename/package.json5
8 files changed, 55 insertions, 7 deletions
diff --git a/ext/node/ops/require.rs b/ext/node/ops/require.rs
index 43867053b..7524fb43c 100644
--- a/ext/node/ops/require.rs
+++ b/ext/node/ops/require.rs
@@ -529,12 +529,16 @@ where
return Ok(None);
};
- let referrer = Url::from_file_path(parent_path).unwrap();
+ let referrer = if parent_path.is_empty() {
+ None
+ } else {
+ Some(Url::from_file_path(parent_path).unwrap())
+ };
let r = node_resolver.package_exports_resolve(
&pkg.path,
&format!(".{expansion}"),
exports,
- Some(&referrer),
+ referrer.as_ref(),
NodeModuleKind::Cjs,
REQUIRE_CONDITIONS,
NodeResolutionMode::Execution,
diff --git a/ext/node/polyfills/01_require.js b/ext/node/polyfills/01_require.js
index 5b0980c31..7935903a8 100644
--- a/ext/node/polyfills/01_require.js
+++ b/ext/node/polyfills/01_require.js
@@ -523,17 +523,13 @@ function resolveExports(
return;
}
- if (!parentPath) {
- return false;
- }
-
return op_require_resolve_exports(
usesLocalNodeModulesDir,
modulesPath,
request,
name,
expansion,
- parentPath,
+ parentPath ?? "",
) ?? false;
}
diff --git a/tests/registry/npm/@denotest/cjs-multiple-exports/1.0.0/package.json b/tests/registry/npm/@denotest/cjs-multiple-exports/1.0.0/package.json
new file mode 100644
index 000000000..43f07a235
--- /dev/null
+++ b/tests/registry/npm/@denotest/cjs-multiple-exports/1.0.0/package.json
@@ -0,0 +1,8 @@
+{
+ "name": "@denotest/cjs-multiple-exports",
+ "version": "1.0.0",
+ "exports": {
+ ".": "./src/index.js",
+ "./add": "./src/add.js"
+ }
+} \ No newline at end of file
diff --git a/tests/registry/npm/@denotest/cjs-multiple-exports/1.0.0/src/add.js b/tests/registry/npm/@denotest/cjs-multiple-exports/1.0.0/src/add.js
new file mode 100644
index 000000000..42c8a7c60
--- /dev/null
+++ b/tests/registry/npm/@denotest/cjs-multiple-exports/1.0.0/src/add.js
@@ -0,0 +1,3 @@
+module.exports = function add(a, b) {
+ return a + b;
+}; \ No newline at end of file
diff --git a/tests/registry/npm/@denotest/cjs-multiple-exports/1.0.0/src/index.js b/tests/registry/npm/@denotest/cjs-multiple-exports/1.0.0/src/index.js
new file mode 100644
index 000000000..432ed652e
--- /dev/null
+++ b/tests/registry/npm/@denotest/cjs-multiple-exports/1.0.0/src/index.js
@@ -0,0 +1,3 @@
+module.exports = {
+ hello: "world"
+}; \ No newline at end of file
diff --git a/tests/specs/node/require_export_from_parent_with_no_filename/__test__.jsonc b/tests/specs/node/require_export_from_parent_with_no_filename/__test__.jsonc
new file mode 100644
index 000000000..a3de08e46
--- /dev/null
+++ b/tests/specs/node/require_export_from_parent_with_no_filename/__test__.jsonc
@@ -0,0 +1,13 @@
+{
+ "tempDir": true,
+ "steps": [
+ {
+ "args": "install",
+ "output": "[WILDCARD]"
+ },
+ {
+ "args": "run -A main.cjs",
+ "output": "3\n"
+ }
+ ]
+}
diff --git a/tests/specs/node/require_export_from_parent_with_no_filename/main.cjs b/tests/specs/node/require_export_from_parent_with_no_filename/main.cjs
new file mode 100644
index 000000000..3335ae1bf
--- /dev/null
+++ b/tests/specs/node/require_export_from_parent_with_no_filename/main.cjs
@@ -0,0 +1,16 @@
+const path = require("node:path");
+const Module = require("node:module");
+function requireFromString(code, filename) {
+ const paths = Module._nodeModulePaths((0, path.dirname)(filename));
+ const m = new Module(filename, module.parent);
+ m.paths = paths;
+ m._compile(code, filename);
+ return m.exports;
+}
+
+const code = `
+const add = require("@denotest/cjs-multiple-exports/add");
+
+console.log(add(1, 2));
+`;
+requireFromString(code, "fake.js");
diff --git a/tests/specs/node/require_export_from_parent_with_no_filename/package.json b/tests/specs/node/require_export_from_parent_with_no_filename/package.json
new file mode 100644
index 000000000..9cd643895
--- /dev/null
+++ b/tests/specs/node/require_export_from_parent_with_no_filename/package.json
@@ -0,0 +1,5 @@
+{
+ "dependencies": {
+ "@denotest/cjs-multiple-exports": "1.0.0"
+ }
+}