Skip to content

Commit

Permalink
fix(node/byonm): do not accidentally resolve bare node built-ins (#25543
Browse files Browse the repository at this point in the history
)

This was accidentally enabled in byonm, but it requires the
`--unstable-bare-node-builtins` flag.

Closes #25358
  • Loading branch information
dsherret authored Sep 9, 2024
1 parent aadcf33 commit 560ad03
Show file tree
Hide file tree
Showing 21 changed files with 73 additions and 64 deletions.
9 changes: 8 additions & 1 deletion cli/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
}

Expand Down
58 changes: 0 additions & 58 deletions tests/integration/run_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
12 changes: 12 additions & 0 deletions tests/specs/run/node_builtin_modules/__test__.jsonc
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
File renamed without changes.
File renamed without changes.
35 changes: 35 additions & 0 deletions tests/specs/run/node_prefix_missing/__test__.jsonc
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
1 change: 1 addition & 0 deletions tests/specs/run/node_prefix_missing/byonm/has.out
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[Function: writeFile]
3 changes: 3 additions & 0 deletions tests/specs/run/node_prefix_missing/byonm/has.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import fs from "node:fs";

console.log(fs.writeFile);
3 changes: 3 additions & 0 deletions tests/specs/run/node_prefix_missing/byonm/missing.out
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions tests/specs/run/node_prefix_missing/byonm/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}
3 changes: 3 additions & 0 deletions tests/specs/run/node_prefix_missing/config.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"unstable": ["bare-node-builtins"]
}
2 changes: 2 additions & 0 deletions tests/specs/run/node_prefix_missing/feature_enabled.out
Original file line number Diff line number Diff line change
@@ -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]
3 changes: 3 additions & 0 deletions tests/specs/run/node_prefix_missing/main.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import fs from "fs";

console.log(fs.writeFile);
1 change: 0 additions & 1 deletion tests/testdata/run/node_prefix_missing/config.json

This file was deleted.

2 changes: 0 additions & 2 deletions tests/testdata/run/node_prefix_missing/feature_enabled.out

This file was deleted.

1 change: 0 additions & 1 deletion tests/testdata/run/node_prefix_missing/import_map.json

This file was deleted.

2 changes: 1 addition & 1 deletion tools/lint.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 560ad03

Please sign in to comment.