From 560ad0331bf99a2564f53201cd086ff902901bfe Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 9 Sep 2024 17:35:41 -0400 Subject: [PATCH] fix(node/byonm): do not accidentally resolve bare node built-ins (#25543) This was accidentally enabled in byonm, but it requires the `--unstable-bare-node-builtins` flag. Closes #25358 --- cli/resolver.rs | 9 ++- tests/integration/run_tests.rs | 58 ------------------- .../run/node_builtin_modules/__test__.jsonc | 12 ++++ .../run/node_builtin_modules/mod.js | 0 .../run/node_builtin_modules/mod.js.out | 0 .../run/node_builtin_modules/mod.ts | 0 .../run/node_builtin_modules/mod.ts.out | 0 .../run/node_prefix_missing/__test__.jsonc | 35 +++++++++++ .../run/node_prefix_missing/byonm/has.out | 1 + .../run/node_prefix_missing/byonm/has.ts | 3 + .../run/node_prefix_missing/byonm/missing.out | 3 + .../run/node_prefix_missing/byonm/missing.ts} | 0 .../node_prefix_missing/byonm/package.json | 2 + .../specs/run/node_prefix_missing/config.json | 3 + .../node_prefix_missing/feature_enabled.out | 2 + tests/specs/run/node_prefix_missing/main.ts | 3 + .../run/node_prefix_missing/main.ts.out | 0 .../run/node_prefix_missing/config.json | 1 - .../node_prefix_missing/feature_enabled.out | 2 - .../run/node_prefix_missing/import_map.json | 1 - tools/lint.js | 2 +- 21 files changed, 73 insertions(+), 64 deletions(-) create mode 100644 tests/specs/run/node_builtin_modules/__test__.jsonc rename tests/{testdata => specs}/run/node_builtin_modules/mod.js (100%) rename tests/{testdata => specs}/run/node_builtin_modules/mod.js.out (100%) rename tests/{testdata => specs}/run/node_builtin_modules/mod.ts (100%) rename tests/{testdata => specs}/run/node_builtin_modules/mod.ts.out (100%) create mode 100644 tests/specs/run/node_prefix_missing/__test__.jsonc create mode 100644 tests/specs/run/node_prefix_missing/byonm/has.out create mode 100644 tests/specs/run/node_prefix_missing/byonm/has.ts create mode 100644 tests/specs/run/node_prefix_missing/byonm/missing.out rename tests/{testdata/run/node_prefix_missing/main.ts => specs/run/node_prefix_missing/byonm/missing.ts} (100%) create mode 100644 tests/specs/run/node_prefix_missing/byonm/package.json create mode 100644 tests/specs/run/node_prefix_missing/config.json create mode 100644 tests/specs/run/node_prefix_missing/feature_enabled.out create mode 100644 tests/specs/run/node_prefix_missing/main.ts rename tests/{testdata => specs}/run/node_prefix_missing/main.ts.out (100%) delete mode 100644 tests/testdata/run/node_prefix_missing/config.json delete mode 100644 tests/testdata/run/node_prefix_missing/feature_enabled.out delete mode 100644 tests/testdata/run/node_prefix_missing/import_map.json diff --git a/cli/resolver.rs b/cli/resolver.rs index 5b657b89533d33..07cb6931df5960 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -707,7 +707,14 @@ impl Resolver for CliGraphResolver { .resolve_if_for_npm_pkg(raw_specifier, referrer, to_node_mode(mode)) .map_err(ResolveError::Other)?; if let Some(res) = maybe_resolution { - return Ok(res.into_url()); + match res { + NodeResolution::Esm(url) | NodeResolution::CommonJs(url) => { + return Ok(url) + } + NodeResolution::BuiltIn(_) => { + // don't resolve bare specifiers for built-in modules via node resolution + } + } } } diff --git a/tests/integration/run_tests.rs b/tests/integration/run_tests.rs index 301283e85bc141..ef688517f9c1ad 100644 --- a/tests/integration/run_tests.rs +++ b/tests/integration/run_tests.rs @@ -4605,64 +4605,6 @@ fn permission_prompt_escapes_ansi_codes_and_control_chars() { } } -itest!(node_builtin_modules_ts { - args: "run --quiet --allow-read run/node_builtin_modules/mod.ts hello there", - output: "run/node_builtin_modules/mod.ts.out", - envs: env_vars_for_npm_tests(), - exit_code: 0, -}); - -itest!(node_builtin_modules_js { - args: "run --quiet --allow-read run/node_builtin_modules/mod.js hello there", - output: "run/node_builtin_modules/mod.js.out", - envs: env_vars_for_npm_tests(), - exit_code: 0, -}); - -itest!(node_prefix_missing { - args: "run --quiet run/node_prefix_missing/main.ts", - output: "run/node_prefix_missing/main.ts.out", - envs: env_vars_for_npm_tests(), - exit_code: 1, -}); - -itest!(node_prefix_missing_unstable_bare_node_builtins_enbaled { - args: "run --unstable-bare-node-builtins run/node_prefix_missing/main.ts", - output: "run/node_prefix_missing/feature_enabled.out", - envs: env_vars_for_npm_tests(), - exit_code: 0, -}); - -itest!( - node_prefix_missing_unstable_bare_node_builtins_enbaled_by_env { - args: "run run/node_prefix_missing/main.ts", - output: "run/node_prefix_missing/feature_enabled.out", - envs: [ - env_vars_for_npm_tests(), - vec![( - "DENO_UNSTABLE_BARE_NODE_BUILTINS".to_string(), - "1".to_string() - )] - ] - .concat(), - exit_code: 0, - } -); - -itest!(node_prefix_missing_unstable_bare_node_builtins_enbaled_by_config { - args: "run --config=run/node_prefix_missing/config.json run/node_prefix_missing/main.ts", - output: "run/node_prefix_missing/feature_enabled.out", - envs: env_vars_for_npm_tests(), - exit_code: 0, -}); - -itest!(node_prefix_missing_unstable_bare_node_builtins_enbaled_with_import_map { - args: "run --unstable-bare-node-builtins --import-map run/node_prefix_missing/import_map.json run/node_prefix_missing/main.ts", - output: "run/node_prefix_missing/feature_enabled.out", - envs: env_vars_for_npm_tests(), - exit_code: 0, -}); - itest!(dynamic_import_syntax_error { args: "run -A run/dynamic_import_syntax_error.js", output: "run/dynamic_import_syntax_error.js.out", diff --git a/tests/specs/run/node_builtin_modules/__test__.jsonc b/tests/specs/run/node_builtin_modules/__test__.jsonc new file mode 100644 index 00000000000000..f3d637ac61ed2c --- /dev/null +++ b/tests/specs/run/node_builtin_modules/__test__.jsonc @@ -0,0 +1,12 @@ +{ + "tests": { + "ts": { + "args": "run --quiet --allow-read mod.ts hello there", + "output": "mod.ts.out" + }, + "js": { + "args": "run --quiet --allow-read mod.js hello there", + "output": "mod.js.out" + } + } +} diff --git a/tests/testdata/run/node_builtin_modules/mod.js b/tests/specs/run/node_builtin_modules/mod.js similarity index 100% rename from tests/testdata/run/node_builtin_modules/mod.js rename to tests/specs/run/node_builtin_modules/mod.js diff --git a/tests/testdata/run/node_builtin_modules/mod.js.out b/tests/specs/run/node_builtin_modules/mod.js.out similarity index 100% rename from tests/testdata/run/node_builtin_modules/mod.js.out rename to tests/specs/run/node_builtin_modules/mod.js.out diff --git a/tests/testdata/run/node_builtin_modules/mod.ts b/tests/specs/run/node_builtin_modules/mod.ts similarity index 100% rename from tests/testdata/run/node_builtin_modules/mod.ts rename to tests/specs/run/node_builtin_modules/mod.ts diff --git a/tests/testdata/run/node_builtin_modules/mod.ts.out b/tests/specs/run/node_builtin_modules/mod.ts.out similarity index 100% rename from tests/testdata/run/node_builtin_modules/mod.ts.out rename to tests/specs/run/node_builtin_modules/mod.ts.out diff --git a/tests/specs/run/node_prefix_missing/__test__.jsonc b/tests/specs/run/node_prefix_missing/__test__.jsonc new file mode 100644 index 00000000000000..305020ed971f43 --- /dev/null +++ b/tests/specs/run/node_prefix_missing/__test__.jsonc @@ -0,0 +1,35 @@ +{ + "tests": { + "basic": { + "args": "run --quiet main.ts", + "output": "main.ts.out", + "exitCode": 1 + }, + "unstable_bare_node_builtins_enabled": { + "args": "run --unstable-bare-node-builtins main.ts", + "output": "feature_enabled.out" + }, + "unstable_bare_node_builtins_enbaled_by_env": { + "args": "run main.ts", + "envs": { + "DENO_UNSTABLE_BARE_NODE_BUILTINS": "1" + }, + "output": "feature_enabled.out" + }, + "enabled_by_config": { + "args": "run --config=config.json main.ts", + "output": "feature_enabled.out" + }, + "byonm_missing": { + "cwd": "byonm", + "exitCode": 1, + "args": "run missing.ts", + "output": "byonm/missing.out" + }, + "byonm_has": { + "cwd": "byonm", + "args": "run has.ts", + "output": "byonm/has.out" + } + } +} diff --git a/tests/specs/run/node_prefix_missing/byonm/has.out b/tests/specs/run/node_prefix_missing/byonm/has.out new file mode 100644 index 00000000000000..e223dcff5848fb --- /dev/null +++ b/tests/specs/run/node_prefix_missing/byonm/has.out @@ -0,0 +1 @@ +[Function: writeFile] diff --git a/tests/specs/run/node_prefix_missing/byonm/has.ts b/tests/specs/run/node_prefix_missing/byonm/has.ts new file mode 100644 index 00000000000000..a52c1e3f0d4f13 --- /dev/null +++ b/tests/specs/run/node_prefix_missing/byonm/has.ts @@ -0,0 +1,3 @@ +import fs from "node:fs"; + +console.log(fs.writeFile); diff --git a/tests/specs/run/node_prefix_missing/byonm/missing.out b/tests/specs/run/node_prefix_missing/byonm/missing.out new file mode 100644 index 00000000000000..4129e79a519512 --- /dev/null +++ b/tests/specs/run/node_prefix_missing/byonm/missing.out @@ -0,0 +1,3 @@ +error: Relative import path "fs" not prefixed with / or ./ or ../ + hint: If you want to use a built-in Node module, add a "node:" prefix (ex. "node:fs"). + at file:///[WILDLINE]/missing.ts:1:16 diff --git a/tests/testdata/run/node_prefix_missing/main.ts b/tests/specs/run/node_prefix_missing/byonm/missing.ts similarity index 100% rename from tests/testdata/run/node_prefix_missing/main.ts rename to tests/specs/run/node_prefix_missing/byonm/missing.ts diff --git a/tests/specs/run/node_prefix_missing/byonm/package.json b/tests/specs/run/node_prefix_missing/byonm/package.json new file mode 100644 index 00000000000000..2c63c0851048d8 --- /dev/null +++ b/tests/specs/run/node_prefix_missing/byonm/package.json @@ -0,0 +1,2 @@ +{ +} diff --git a/tests/specs/run/node_prefix_missing/config.json b/tests/specs/run/node_prefix_missing/config.json new file mode 100644 index 00000000000000..72f40aaf362ee0 --- /dev/null +++ b/tests/specs/run/node_prefix_missing/config.json @@ -0,0 +1,3 @@ +{ + "unstable": ["bare-node-builtins"] +} diff --git a/tests/specs/run/node_prefix_missing/feature_enabled.out b/tests/specs/run/node_prefix_missing/feature_enabled.out new file mode 100644 index 00000000000000..3eff4bc32443cd --- /dev/null +++ b/tests/specs/run/node_prefix_missing/feature_enabled.out @@ -0,0 +1,2 @@ +[WILDCARD]Warning Resolving "fs" as "node:fs" at file:///[WILDCARD]/main.ts:1:16. If you want to use a built-in Node module, add a "node:" prefix. +[Function: writeFile] diff --git a/tests/specs/run/node_prefix_missing/main.ts b/tests/specs/run/node_prefix_missing/main.ts new file mode 100644 index 00000000000000..c5c1885a2f3281 --- /dev/null +++ b/tests/specs/run/node_prefix_missing/main.ts @@ -0,0 +1,3 @@ +import fs from "fs"; + +console.log(fs.writeFile); diff --git a/tests/testdata/run/node_prefix_missing/main.ts.out b/tests/specs/run/node_prefix_missing/main.ts.out similarity index 100% rename from tests/testdata/run/node_prefix_missing/main.ts.out rename to tests/specs/run/node_prefix_missing/main.ts.out diff --git a/tests/testdata/run/node_prefix_missing/config.json b/tests/testdata/run/node_prefix_missing/config.json deleted file mode 100644 index 67480c3d48655a..00000000000000 --- a/tests/testdata/run/node_prefix_missing/config.json +++ /dev/null @@ -1 +0,0 @@ -{ "unstable": ["bare-node-builtins"] } diff --git a/tests/testdata/run/node_prefix_missing/feature_enabled.out b/tests/testdata/run/node_prefix_missing/feature_enabled.out deleted file mode 100644 index c577fa92cb8f73..00000000000000 --- a/tests/testdata/run/node_prefix_missing/feature_enabled.out +++ /dev/null @@ -1,2 +0,0 @@ -[WILDCARD]Warning Resolving "fs" as "node:fs" at file:///[WILDCARD]/tests/testdata/run/node_prefix_missing/main.ts:1:16. If you want to use a built-in Node module, add a "node:" prefix. -[Function: writeFile] diff --git a/tests/testdata/run/node_prefix_missing/import_map.json b/tests/testdata/run/node_prefix_missing/import_map.json deleted file mode 100644 index 3add7d009c3c02..00000000000000 --- a/tests/testdata/run/node_prefix_missing/import_map.json +++ /dev/null @@ -1 +0,0 @@ -{ "imports": {} } diff --git a/tools/lint.js b/tools/lint.js index 74b2c28d310ce7..db4b657af79bc8 100755 --- a/tools/lint.js +++ b/tools/lint.js @@ -220,7 +220,7 @@ async function ensureNoNewITests() { "pm_tests.rs": 0, "publish_tests.rs": 0, "repl_tests.rs": 0, - "run_tests.rs": 347, + "run_tests.rs": 340, "shared_library_tests.rs": 0, "task_tests.rs": 30, "test_tests.rs": 74,