From fb73eb1e9dca3e93cc7efcf5c2244e0068733843 Mon Sep 17 00:00:00 2001 From: Yoshiya Hinosawa Date: Fri, 20 Oct 2023 13:02:08 +0900 Subject: [PATCH] feat(unstable): allow bare specifier for builtin node module (#20728) closes #20566 --- Cargo.lock | 43 +++------ cli/Cargo.toml | 12 +-- cli/args/flags.rs | 15 +++ cli/args/mod.rs | 9 ++ cli/errors.rs | 7 +- cli/factory.rs | 3 + cli/graph_util.rs | 16 +++- cli/lsp/diagnostics.rs | 21 +++++ cli/lsp/documents.rs | 91 ++++++++++++++++--- cli/module_loader.rs | 2 +- cli/resolver.rs | 26 +++++- cli/tests/integration/lsp_tests.rs | 89 ++++++++++++++++++ cli/tests/integration/run_tests.rs | 37 ++++++++ .../run/node_prefix_missing/config.json | 1 + .../run/node_prefix_missing/import_map.json | 1 + .../main.ts.out_feature_enabled | 2 + cli/tools/vendor/test.rs | 1 + 17 files changed, 319 insertions(+), 57 deletions(-) create mode 100644 cli/tests/testdata/run/node_prefix_missing/config.json create mode 100644 cli/tests/testdata/run/node_prefix_missing/import_map.json create mode 100644 cli/tests/testdata/run/node_prefix_missing/main.ts.out_feature_enabled diff --git a/Cargo.lock b/Cargo.lock index 6b740fc663..e21121aada 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -995,7 +995,7 @@ dependencies = [ "deno_lockfile", "deno_npm", "deno_runtime", - "deno_semver 0.5.1", + "deno_semver", "deno_task_shell", "dissimilar", "dprint-plugin-json", @@ -1180,12 +1180,11 @@ dependencies = [ [[package]] name = "deno_config" -version = "0.3.1" +version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ed5999e360fec39bbfee5d85bac82c5f557ed93a58660bc255026a90796138c6" +checksum = "fed704bc09eb4f88b26b16c75f87795d1ea1e658e26867fe684cef91c55543f1" dependencies = [ "anyhow", - "deno_semver 0.4.0", "indexmap 2.0.0", "jsonc-parser", "log", @@ -1266,9 +1265,9 @@ dependencies = [ [[package]] name = "deno_doc" -version = "0.67.0" +version = "0.68.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2480971d683babc07eea6cdb37d1214675c25d084b0c819e2e52898634b044ce" +checksum = "b7f5649a3e8c8da3e73d27eff6306c522f1bb0c54e75fad5530f3a4746eea81f" dependencies = [ "cfg-if 1.0.0", "deno_ast", @@ -1284,9 +1283,9 @@ dependencies = [ [[package]] name = "deno_emit" -version = "0.28.0" +version = "0.29.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ebc68365e2e5ce6dd11506a1a17aac6a10ea7787e084c45690f70c46a6662fd8" +checksum = "ac5e585450dbafc39a2161bc9c8f861409cec08c014c4f82c62fdbce5ae335c2" dependencies = [ "anyhow", "base64 0.13.1", @@ -1351,16 +1350,17 @@ dependencies = [ [[package]] name = "deno_graph" -version = "0.55.0" +version = "0.56.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f09c93dac12402a37be3ee24e0b6a691ddc5fdef13831b375b6c0950efc89e40" +checksum = "f02ddb53a0d3201895566ef3c1c5861c62d06df3b0fac6a7dcf826dd081eec27" dependencies = [ "anyhow", "async-trait", "data-url", "deno_ast", - "deno_semver 0.5.1", + "deno_semver", "futures", + "import_map", "indexmap 2.0.0", "monch", "once_cell", @@ -1584,7 +1584,7 @@ dependencies = [ "anyhow", "async-trait", "deno_lockfile", - "deno_semver 0.5.1", + "deno_semver", "futures", "log", "monch", @@ -1667,19 +1667,6 @@ dependencies = [ "winres", ] -[[package]] -name = "deno_semver" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6f739a9d90c47e2af7e2fcbae0976360f3fb5292f7288a084d035ed44d12a288" -dependencies = [ - "monch", - "once_cell", - "serde", - "thiserror", - "url", -] - [[package]] name = "deno_semver" version = "0.5.1" @@ -2242,16 +2229,16 @@ dependencies = [ [[package]] name = "eszip" -version = "0.53.0" +version = "0.54.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cf1763e61b99be49c961817d2211fbcef23b04159a60895b208e4608beb20ee0" +checksum = "4032916b1567b2588723ea4d1aa47537ae0f580f64a0aba5dedc8bf13e20c6e0" dependencies = [ "anyhow", "base64 0.21.4", "deno_ast", "deno_graph", "deno_npm", - "deno_semver 0.5.1", + "deno_semver", "futures", "hashlink", "serde", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 4061456dcc..8da9599d80 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -47,18 +47,18 @@ winres.workspace = true [dependencies] deno_ast = { workspace = true, features = ["bundler", "cjs", "codegen", "dep_graph", "module_specifier", "proposal", "react", "sourcemap", "transforms", "typescript", "view", "visit"] } deno_cache_dir = "=0.6.0" -deno_config = "=0.3.1" +deno_config = "=0.4.0" deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] } -deno_doc = "=0.67.0" -deno_emit = "=0.28.0" -deno_graph = "=0.55.0" +deno_doc = "=0.68.0" +deno_emit = "=0.29.0" +deno_graph = "=0.56.3" deno_lint = { version = "=0.51.0", features = ["docs"] } deno_lockfile.workspace = true deno_npm = "0.15.2" deno_runtime = { workspace = true, features = ["dont_create_runtime_snapshot", "exclude_runtime_main_js", "include_js_files_for_snapshotting"] } deno_semver = "0.5.1" deno_task_shell = "=0.13.2" -eszip = "=0.53.0" +eszip = "=0.54.0" napi_sym.workspace = true async-trait.workspace = true @@ -68,7 +68,7 @@ bincode = "=1.3.3" bytes.workspace = true cache_control.workspace = true chrono.workspace = true -clap = { version = "=4.3.3", features = ["string"] } +clap = { version = "=4.3.3", features = ["env", "string"] } clap_complete = "=4.3.1" clap_complete_fig = "=4.3.1" console_static_text.workspace = true diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 66981b239f..138d77359b 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -1,5 +1,6 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. +use clap::builder::FalseyValueParser; use clap::value_parser; use clap::Arg; use clap::ArgAction; @@ -405,6 +406,7 @@ pub struct Flags { pub reload: bool, pub seed: Option, pub unstable: bool, + pub unstable_bare_node_builtlins: bool, pub unsafely_ignore_certificate_errors: Option>, pub v8_flags: Vec, } @@ -801,6 +803,10 @@ pub fn flags_from_vec(args: Vec) -> clap::error::Result { flags.unstable = true; } + if matches.get_flag("unstable-bare-node-builtins") { + flags.unstable_bare_node_builtlins = true; + } + if matches.get_flag("quiet") { flags.log_level = Some(Level::Error); } else if let Some(log_level) = matches.get_one::("log-level") { @@ -896,6 +902,15 @@ fn clap_root() -> Command { .action(ArgAction::SetTrue) .global(true), ) + .arg( + Arg::new("unstable-bare-node-builtins") + .long("unstable-bare-node-builtins") + .help("Enable unstable bare node builtins feature") + .env("DENO_UNSTABLE_BARE_NODE_BUILTINS") + .value_parser(FalseyValueParser::new()) + .action(ArgAction::SetTrue) + .global(true), + ) .arg( Arg::new("log-level") .short('L') diff --git a/cli/args/mod.rs b/cli/args/mod.rs index d91affed10..a95c4d840f 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -1216,6 +1216,15 @@ impl CliOptions { self.flags.unstable } + pub fn unstable_bare_node_builtlins(&self) -> bool { + self.flags.unstable_bare_node_builtlins + || self + .maybe_config_file() + .as_ref() + .map(|c| c.json.unstable.contains(&"bare-node-builtins".to_string())) + .unwrap_or(false) + } + pub fn v8_flags(&self) -> &Vec { &self.flags.v8_flags } diff --git a/cli/errors.rs b/cli/errors.rs index f867ebbccd..8c240c1e64 100644 --- a/cli/errors.rs +++ b/cli/errors.rs @@ -11,6 +11,7 @@ use deno_ast::Diagnostic; use deno_core::error::AnyError; +use deno_graph::source::ResolveError; use deno_graph::ModuleError; use deno_graph::ModuleGraphError; use deno_graph::ResolutionError; @@ -45,7 +46,11 @@ fn get_module_graph_error_class(err: &ModuleGraphError) -> &'static str { fn get_resolution_error_class(err: &ResolutionError) -> &'static str { match err { ResolutionError::ResolverError { error, .. } => { - get_error_class_name(error.as_ref()) + use ResolveError::*; + match error.as_ref() { + Specifier(_) => "TypeError", + Other(e) => get_error_class_name(e), + } } _ => "TypeError", } diff --git a/cli/factory.rs b/cli/factory.rs index 2841482f80..0c887b720a 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -378,6 +378,9 @@ impl CliFactory { .to_maybe_jsx_import_source_config()?, maybe_import_map: self.maybe_import_map().await?.clone(), maybe_vendor_dir: self.options.vendor_dir_path(), + bare_node_builtins_enabled: self + .options + .unstable_bare_node_builtlins(), }, ))) }) diff --git a/cli/graph_util.rs b/cli/graph_util.rs index b90581a145..a7c230e176 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -24,6 +24,7 @@ use deno_core::parking_lot::Mutex; use deno_core::parking_lot::RwLock; use deno_core::ModuleSpecifier; use deno_graph::source::Loader; +use deno_graph::source::ResolveError; use deno_graph::GraphKind; use deno_graph::Module; use deno_graph::ModuleError; @@ -487,10 +488,14 @@ fn get_resolution_error_bare_specifier( { Some(specifier.as_str()) } else if let ResolutionError::ResolverError { error, .. } = error { - if let Some(ImportMapError::UnmappedBareSpecifier(specifier, _)) = - error.downcast_ref::() - { - Some(specifier.as_str()) + if let ResolveError::Other(error) = (*error).as_ref() { + if let Some(ImportMapError::UnmappedBareSpecifier(specifier, _)) = + error.downcast_ref::() + { + Some(specifier.as_str()) + } else { + None + } } else { None } @@ -679,6 +684,7 @@ mod test { use std::sync::Arc; use deno_ast::ModuleSpecifier; + use deno_graph::source::ResolveError; use deno_graph::Position; use deno_graph::Range; use deno_graph::ResolutionError; @@ -696,7 +702,7 @@ mod test { let specifier = ModuleSpecifier::parse("file:///file.ts").unwrap(); let err = import_map.resolve(input, &specifier).err().unwrap(); let err = ResolutionError::ResolverError { - error: Arc::new(err.into()), + error: Arc::new(ResolveError::Other(err.into())), specifier: input.to_string(), range: Range { specifier, diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index cb368b1f25..37427d878d 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -970,6 +970,8 @@ pub enum DenoDiagnostic { ResolutionError(deno_graph::ResolutionError), /// Invalid `node:` specifier. InvalidNodeSpecifier(ModuleSpecifier), + /// Bare specifier is used for `node:` specifier + BareNodeSpecifier(String), } impl DenoDiagnostic { @@ -1003,6 +1005,7 @@ impl DenoDiagnostic { } } Self::InvalidNodeSpecifier(_) => "resolver-error", + Self::BareNodeSpecifier(_) => "import-node-prefix-missing", } } @@ -1174,6 +1177,7 @@ impl DenoDiagnostic { .map(|specifier| json!({ "specifier": specifier })) ), Self::InvalidNodeSpecifier(specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Unknown Node built-in module: {}", specifier.path()), None), + Self::BareNodeSpecifier(specifier) => (lsp::DiagnosticSeverity::WARNING, format!("\"{}\" is resolved to \"node:{}\". If you want to use a built-in Node module, add a \"node:\" prefix.", specifier, specifier), Some(json!({ "specifier": specifier }))), }; lsp::Diagnostic { range: *range, @@ -1189,6 +1193,7 @@ impl DenoDiagnostic { fn diagnose_resolution( snapshot: &language_server::StateSnapshot, + dependency_key: &str, resolution: &Resolution, is_dynamic: bool, maybe_assert_type: Option<&str>, @@ -1253,6 +1258,20 @@ fn diagnose_resolution( if !deno_node::is_builtin_node_module(module_name) { diagnostics .push(DenoDiagnostic::InvalidNodeSpecifier(specifier.clone())); + } else if module_name == dependency_key { + let mut is_mapped = false; + if let Some(import_map) = &snapshot.maybe_import_map { + if let Resolution::Ok(resolved) = &resolution { + if import_map.resolve(module_name, &resolved.specifier).is_ok() { + is_mapped = true; + } + } + } + // show diagnostics for bare node specifiers that aren't mapped by import map + if !is_mapped { + diagnostics + .push(DenoDiagnostic::BareNodeSpecifier(module_name.to_string())); + } } else if let Some(npm_resolver) = snapshot .npm .as_ref() @@ -1329,6 +1348,7 @@ fn diagnose_dependency( diagnostics.extend( diagnose_resolution( snapshot, + dependency_key, if dependency.maybe_code.is_none() { &dependency.maybe_type } else { @@ -1362,6 +1382,7 @@ fn diagnose_dependency( diagnostics.extend( diagnose_resolution( snapshot, + dependency_key, &dependency.maybe_type, dependency.is_dynamic, dependency.maybe_attribute_type.as_deref(), diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index f97d6d6186..674540281c 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -365,6 +365,7 @@ impl Document { maybe_headers: Option>, text_info: SourceTextInfo, resolver: &dyn deno_graph::source::Resolver, + npm_resolver: &dyn deno_graph::source::NpmResolver, ) -> Self { // we only ever do `Document::new` on on disk resources that are supposed to // be diagnosable, unlike `Document::open`, so it is safe to unconditionally @@ -374,6 +375,7 @@ impl Document { text_info.clone(), maybe_headers.as_ref(), resolver, + npm_resolver, ); let dependencies = Arc::new(DocumentDependencies::from_maybe_module(&maybe_module)); @@ -396,6 +398,7 @@ impl Document { fn maybe_with_new_resolver( &self, resolver: &dyn deno_graph::source::Resolver, + npm_resolver: &dyn deno_graph::source::NpmResolver, ) -> Option { let parsed_source_result = match &self.0.maybe_parsed_source { Some(parsed_source_result) => parsed_source_result.clone(), @@ -406,6 +409,7 @@ impl Document { &parsed_source_result, self.0.maybe_headers.as_ref(), resolver, + npm_resolver, )); let dependencies = Arc::new(DocumentDependencies::from_maybe_module(&maybe_module)); @@ -433,6 +437,7 @@ impl Document { content: Arc, cache: &Arc, resolver: &dyn deno_graph::source::Resolver, + npm_resolver: &dyn deno_graph::source::NpmResolver, ) -> Self { let maybe_headers = language_id.as_headers(); let text_info = SourceTextInfo::new(content); @@ -442,6 +447,7 @@ impl Document { text_info.clone(), maybe_headers, resolver, + npm_resolver, ) } else { (None, None) @@ -470,6 +476,7 @@ impl Document { version: i32, changes: Vec, resolver: &dyn deno_graph::source::Resolver, + npm_resolver: &dyn deno_graph::source::NpmResolver, ) -> Result { let mut content = self.0.text_info.text_str().to_string(); let mut line_index = self.0.line_index.clone(); @@ -505,6 +512,7 @@ impl Document { text_info.clone(), maybe_headers, resolver, + npm_resolver, ) } else { (None, None) @@ -809,6 +817,7 @@ impl FileSystemDocuments { cache: &Arc, resolver: &dyn deno_graph::source::Resolver, specifier: &ModuleSpecifier, + npm_resolver: &dyn deno_graph::source::NpmResolver, ) -> Option { let fs_version = if specifier.scheme() == "data" { Some("1".to_string()) @@ -818,7 +827,7 @@ impl FileSystemDocuments { let file_system_doc = self.docs.get(specifier); if file_system_doc.map(|d| d.fs_version().to_string()) != fs_version { // attempt to update the file on the file system - self.refresh_document(cache, resolver, specifier) + self.refresh_document(cache, resolver, specifier, npm_resolver) } else { file_system_doc.cloned() } @@ -831,6 +840,7 @@ impl FileSystemDocuments { cache: &Arc, resolver: &dyn deno_graph::source::Resolver, specifier: &ModuleSpecifier, + npm_resolver: &dyn deno_graph::source::NpmResolver, ) -> Option { let doc = if specifier.scheme() == "file" { let path = specifier_to_file_path(specifier).ok()?; @@ -845,6 +855,7 @@ impl FileSystemDocuments { None, SourceTextInfo::from_string(content), resolver, + npm_resolver, ) } else if specifier.scheme() == "data" { let (source, _) = get_source_from_data_url(specifier).ok()?; @@ -854,6 +865,7 @@ impl FileSystemDocuments { None, SourceTextInfo::from_string(source), resolver, + npm_resolver, ) } else { let fs_version = calculate_fs_version(cache, specifier)?; @@ -870,6 +882,7 @@ impl FileSystemDocuments { maybe_headers, SourceTextInfo::from_string(content), resolver, + npm_resolver, ) }; self.dirty = true; @@ -948,6 +961,7 @@ impl Documents { maybe_jsx_import_source_config: None, maybe_import_map: None, maybe_vendor_dir: None, + bare_node_builtins_enabled: false, }, )), npm_specifier_reqs: Default::default(), @@ -976,6 +990,7 @@ impl Documents { content: Arc, ) -> Document { let resolver = self.get_resolver(); + let npm_resolver = self.get_npm_resolver(); let document = Document::open( specifier.clone(), version, @@ -983,6 +998,7 @@ impl Documents { content, &self.cache, resolver, + npm_resolver, ); let mut file_system_docs = self.file_system_docs.lock(); file_system_docs.docs.remove(&specifier); @@ -1015,7 +1031,12 @@ impl Documents { )) })?; self.dirty = true; - let doc = doc.with_change(version, changes, self.get_resolver())?; + let doc = doc.with_change( + version, + changes, + self.get_resolver(), + self.get_npm_resolver(), + )?; self.open_docs.insert(doc.specifier().clone(), doc.clone()); Ok(doc) } @@ -1125,7 +1146,12 @@ impl Documents { Some(document.clone()) } else { let mut file_system_docs = self.file_system_docs.lock(); - file_system_docs.get(&self.cache, self.get_resolver(), &specifier) + file_system_docs.get( + &self.cache, + self.get_resolver(), + &specifier, + self.get_npm_resolver(), + ) } } @@ -1344,6 +1370,15 @@ impl Documents { .maybe_config_file .and_then(|c| c.vendor_dir_path()) .as_ref(), + bare_node_builtins_enabled: options + .maybe_config_file + .map(|config| { + config + .json + .unstable + .contains(&"bare-node-builtins".to_string()) + }) + .unwrap_or(false), }, )); self.imports = Arc::new( @@ -1353,8 +1388,12 @@ impl Documents { imports .into_iter() .map(|(referrer, imports)| { - let graph_import = - GraphImport::new(&referrer, imports, Some(self.get_resolver())); + let graph_import = GraphImport::new( + &referrer, + imports, + Some(self.get_resolver()), + Some(self.get_npm_resolver()), + ); (referrer, graph_import) }) .collect() @@ -1384,8 +1423,10 @@ impl Documents { document_preload_limit: usize, ) { let resolver = self.resolver.as_graph_resolver(); + let npm_resolver = self.resolver.as_graph_npm_resolver(); for doc in self.open_docs.values_mut() { - if let Some(new_doc) = doc.maybe_with_new_resolver(resolver) { + if let Some(new_doc) = doc.maybe_with_new_resolver(resolver, npm_resolver) + { *doc = new_doc; } } @@ -1411,11 +1452,18 @@ impl Documents { if !open_docs.contains_key(&specifier) && !fs_docs.docs.contains_key(&specifier) { - fs_docs.refresh_document(&self.cache, resolver, &specifier); + fs_docs.refresh_document( + &self.cache, + resolver, + &specifier, + npm_resolver, + ); } else { // update the existing entry to have the new resolver if let Some(doc) = fs_docs.docs.get_mut(&specifier) { - if let Some(new_doc) = doc.maybe_with_new_resolver(resolver) { + if let Some(new_doc) = + doc.maybe_with_new_resolver(resolver, npm_resolver) + { *doc = new_doc; } } @@ -1436,7 +1484,9 @@ impl Documents { // since we hit the limit, just update everything to use the new resolver for uri in not_found_docs { if let Some(doc) = fs_docs.docs.get_mut(&uri) { - if let Some(new_doc) = doc.maybe_with_new_resolver(resolver) { + if let Some(new_doc) = + doc.maybe_with_new_resolver(resolver, npm_resolver) + { *doc = new_doc; } } @@ -1455,7 +1505,9 @@ impl Documents { // just update to use the new resolver for doc in fs_docs.docs.values_mut() { - if let Some(new_doc) = doc.maybe_with_new_resolver(resolver) { + if let Some(new_doc) = + doc.maybe_with_new_resolver(resolver, npm_resolver) + { *doc = new_doc; } } @@ -1528,11 +1580,12 @@ impl Documents { } let resolver = self.get_resolver(); + let npm_resolver = self.get_npm_resolver(); while let Some(specifier) = doc_analyzer.pending_specifiers.pop_front() { if let Some(doc) = self.open_docs.get(&specifier) { doc_analyzer.analyze_doc(&specifier, doc); } else if let Some(doc) = - file_system_docs.get(&self.cache, resolver, &specifier) + file_system_docs.get(&self.cache, resolver, &specifier, npm_resolver) { doc_analyzer.analyze_doc(&specifier, &doc); } @@ -1563,6 +1616,10 @@ impl Documents { self.resolver.as_graph_resolver() } + fn get_npm_resolver(&self) -> &dyn deno_graph::source::NpmResolver { + self.resolver.as_graph_npm_resolver() + } + fn resolve_dependency( &self, specifier: &ModuleSpecifier, @@ -1698,10 +1755,16 @@ fn parse_and_analyze_module( text_info: SourceTextInfo, maybe_headers: Option<&HashMap>, resolver: &dyn deno_graph::source::Resolver, + npm_resolver: &dyn deno_graph::source::NpmResolver, ) -> (Option, Option) { let parsed_source_result = parse_source(specifier, text_info, maybe_headers); - let module_result = - analyze_module(specifier, &parsed_source_result, maybe_headers, resolver); + let module_result = analyze_module( + specifier, + &parsed_source_result, + maybe_headers, + resolver, + npm_resolver, + ); (Some(parsed_source_result), Some(module_result)) } @@ -1725,6 +1788,7 @@ fn analyze_module( parsed_source_result: &ParsedSourceResult, maybe_headers: Option<&HashMap>, resolver: &dyn deno_graph::source::Resolver, + npm_resolver: &dyn deno_graph::source::NpmResolver, ) -> ModuleResult { match parsed_source_result { Ok(parsed_source) => Ok(deno_graph::parse_module_from_ast( @@ -1732,6 +1796,7 @@ fn analyze_module( maybe_headers, parsed_source, Some(resolver), + Some(npm_resolver), )), Err(err) => Err(deno_graph::ModuleGraphError::ModuleError( deno_graph::ModuleError::ParseErr(specifier.clone(), err.clone()), diff --git a/cli/module_loader.rs b/cli/module_loader.rs index 9f3a7d236d..84e8b6122c 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -570,7 +570,7 @@ impl ModuleLoader for CliModuleLoader { } } - resolution + resolution.map_err(|err| err.into()) } fn load( diff --git a/cli/resolver.rs b/cli/resolver.rs index ebb808c6bc..7d17177b6f 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -1,7 +1,6 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. use deno_core::anyhow::anyhow; -use deno_core::anyhow::bail; use deno_core::error::AnyError; use deno_core::futures::future; use deno_core::futures::future::LocalBoxFuture; @@ -9,6 +8,7 @@ use deno_core::futures::FutureExt; use deno_core::ModuleSpecifier; use deno_graph::source::NpmPackageReqResolution; use deno_graph::source::NpmResolver; +use deno_graph::source::ResolveError; use deno_graph::source::Resolver; use deno_graph::source::UnknownBuiltInNodeModuleError; use deno_graph::source::DEFAULT_JSX_IMPORT_SOURCE_MODULE; @@ -104,12 +104,14 @@ pub struct CliGraphResolver { maybe_vendor_specifier: Option, npm_resolver: Option>, found_package_json_dep_flag: Arc, + bare_node_builtins_enabled: bool, } pub struct CliGraphResolverOptions<'a> { pub maybe_jsx_import_source_config: Option, pub maybe_import_map: Option>, pub maybe_vendor_dir: Option<&'a PathBuf>, + pub bare_node_builtins_enabled: bool, } impl CliGraphResolver { @@ -135,6 +137,7 @@ impl CliGraphResolver { .and_then(|v| ModuleSpecifier::from_directory_path(v).ok()), npm_resolver, found_package_json_dep_flag: Default::default(), + bare_node_builtins_enabled: options.bare_node_builtins_enabled, } } @@ -167,7 +170,7 @@ impl Resolver for CliGraphResolver { &self, specifier: &str, referrer: &ModuleSpecifier, - ) -> Result { + ) -> Result { let result = match self .mapped_specifier_resolver .resolve(specifier, referrer)? @@ -190,7 +193,7 @@ impl Resolver for CliGraphResolver { if let Some(vendor_specifier) = &self.maybe_vendor_specifier { if let Ok(specifier) = &result { if specifier.as_str().starts_with(vendor_specifier.as_str()) { - bail!("Importing from the vendor directory is not permitted. Use a remote specifier instead or disable vendoring."); + return Err(ResolveError::Other(anyhow!("Importing from the vendor directory is not permitted. Use a remote specifier instead or disable vendoring."))); } } } @@ -238,6 +241,19 @@ impl NpmResolver for CliGraphResolver { } } + fn on_resolve_bare_builtin_node_module( + &self, + module_name: &str, + range: &deno_graph::Range, + ) { + let deno_graph::Range { + start, specifier, .. + } = range; + let line = start.line + 1; + let column = start.character + 1; + log::warn!("Warning: Resolving \"{module_name}\" as \"node:{module_name}\" at {specifier}:{line}:{column}. If you want to use a built-in Node module, add a \"node:\" prefix.") + } + fn load_and_cache_npm_package_info( &self, package_name: &str, @@ -276,6 +292,10 @@ impl NpmResolver for CliGraphResolver { )), } } + + fn enables_bare_builtin_node_module(&self) -> bool { + self.bare_node_builtins_enabled + } } #[cfg(test)] diff --git a/cli/tests/integration/lsp_tests.rs b/cli/tests/integration/lsp_tests.rs index 5441649784..85e55ee81d 100644 --- a/cli/tests/integration/lsp_tests.rs +++ b/cli/tests/integration/lsp_tests.rs @@ -10419,3 +10419,92 @@ fn lsp_vendor_dir() { client.shutdown(); } + +#[test] +fn lsp_import_unstable_bare_node_builtins_auto_discovered() { + let context = TestContextBuilder::new().use_temp_cwd().build(); + let temp_dir = context.temp_dir(); + + let contents = r#"import path from "path";"#; + temp_dir.write("main.ts", contents); + temp_dir.write("deno.json", r#"{ "unstable": ["bare-node-builtins"] }"#); + let main_script = temp_dir.uri().join("main.ts").unwrap(); + + let mut client = context.new_lsp_command().capture_stderr().build(); + client.initialize_default(); + let diagnostics = client.did_open(json!({ + "textDocument": { + "uri": main_script, + "languageId": "typescript", + "version": 1, + "text": contents, + } + })); + + let diagnostics = diagnostics + .messages_with_file_and_source(main_script.as_ref(), "deno") + .diagnostics + .into_iter() + .filter(|d| { + d.code + == Some(lsp::NumberOrString::String( + "import-node-prefix-missing".to_string(), + )) + }) + .collect::>(); + + // get the quick fixes + let res = client.write_request( + "textDocument/codeAction", + json!({ + "textDocument": { + "uri": main_script + }, + "range": { + "start": { "line": 0, "character": 16 }, + "end": { "line": 0, "character": 18 }, + }, + "context": { + "diagnostics": json!(diagnostics), + "only": ["quickfix"] + } + }), + ); + assert_eq!( + res, + json!([{ + "title": "Update specifier to node:path", + "kind": "quickfix", + "diagnostics": [ + { + "range": { + "start": { "line": 0, "character": 17 }, + "end": { "line": 0, "character": 23 } + }, + "severity": 2, + "code": "import-node-prefix-missing", + "source": "deno", + "message": "\"path\" is resolved to \"node:path\". If you want to use a built-in Node module, add a \"node:\" prefix.", + "data": { + "specifier": "path" + }, + } + ], + "edit": { + "changes": { + main_script: [ + { + "range": { + "start": { "line": 0, "character": 17 }, + "end": { "line": 0, "character": 23 } + }, + "newText": "\"node:path\"" + } + ] + } + } + }]) + ); + + client.shutdown(); +} diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs index 1dff1f6d6a..f709252892 100644 --- a/cli/tests/integration/run_tests.rs +++ b/cli/tests/integration/run_tests.rs @@ -4583,6 +4583,43 @@ itest!(node_prefix_missing { 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/main.ts.out_feature_enabled", + 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/main.ts.out_feature_enabled", + 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/main.ts.out_feature_enabled", + 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/main.ts.out_feature_enabled", + 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/cli/tests/testdata/run/node_prefix_missing/config.json b/cli/tests/testdata/run/node_prefix_missing/config.json new file mode 100644 index 0000000000..67480c3d48 --- /dev/null +++ b/cli/tests/testdata/run/node_prefix_missing/config.json @@ -0,0 +1 @@ +{ "unstable": ["bare-node-builtins"] } diff --git a/cli/tests/testdata/run/node_prefix_missing/import_map.json b/cli/tests/testdata/run/node_prefix_missing/import_map.json new file mode 100644 index 0000000000..3add7d009c --- /dev/null +++ b/cli/tests/testdata/run/node_prefix_missing/import_map.json @@ -0,0 +1 @@ +{ "imports": {} } diff --git a/cli/tests/testdata/run/node_prefix_missing/main.ts.out_feature_enabled b/cli/tests/testdata/run/node_prefix_missing/main.ts.out_feature_enabled new file mode 100644 index 0000000000..0d8c1e93a6 --- /dev/null +++ b/cli/tests/testdata/run/node_prefix_missing/main.ts.out_feature_enabled @@ -0,0 +1,2 @@ +[WILDCARD]Warning: Resolving "fs" as "node:fs" at file:///[WILDCARD]/cli/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/cli/tools/vendor/test.rs b/cli/tools/vendor/test.rs index e13b8579b1..810daf4fb4 100644 --- a/cli/tools/vendor/test.rs +++ b/cli/tools/vendor/test.rs @@ -300,6 +300,7 @@ fn build_resolver( maybe_jsx_import_source_config, maybe_import_map: original_import_map.map(Arc::new), maybe_vendor_dir: None, + bare_node_builtins_enabled: false, }, ) }