diff --git a/cli/args/config_file.rs b/cli/args/config_file.rs index f7fd855796..3ba88c6805 100644 --- a/cli/args/config_file.rs +++ b/cli/args/config_file.rs @@ -2,7 +2,6 @@ use crate::args::ConfigFlag; use crate::args::Flags; -use crate::args::TaskFlags; use crate::util::fs::canonicalize_path; use crate::util::path::specifier_parent; use crate::util::path::specifier_to_file_path; @@ -508,18 +507,6 @@ impl ConfigFile { return Ok(Some(cf)); } } - // attempt to resolve the config file from the task subcommand's - // `--cwd` when specified - if let crate::args::DenoSubcommand::Task(TaskFlags { - cwd: Some(path), - .. - }) = &flags.subcommand - { - let task_cwd = canonicalize_path(&PathBuf::from(path))?; - if let Some(path) = Self::discover_from(&task_cwd, &mut checked)? { - return Ok(Some(path)); - } - }; // From CWD walk up to root looking for deno.json or deno.jsonc Self::discover_from(cwd, &mut checked) } else { diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 825bf96d03..35da62fc87 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -19,6 +19,8 @@ use std::num::NonZeroUsize; use std::path::PathBuf; use std::str::FromStr; +use crate::util::fs::canonicalize_path; + use super::flags_allow_net; static LONG_VERSION: Lazy = Lazy::new(|| { @@ -499,6 +501,16 @@ impl Flags { Some(vec![]) } } + Task(TaskFlags { + cwd: Some(path), .. + }) => { + // attempt to resolve the config file from the task subcommand's + // `--cwd` when specified + match canonicalize_path(&PathBuf::from(path)) { + Ok(path) => Some(vec![path]), + Err(_) => Some(vec![]), + } + } _ => Some(vec![]), } } @@ -533,7 +545,8 @@ impl Flags { .to_file_path() .ok() } - Task(TaskFlags { cwd: None, .. }) => std::env::current_dir().ok(), + Task(_) | Check(_) | Coverage(_) | Cache(_) | Info(_) | Eval(_) + | Test(_) | Bench(_) => std::env::current_dir().ok(), _ => None, } } diff --git a/cli/args/mod.rs b/cli/args/mod.rs index aa1781bd8b..fe6883f2c7 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -391,49 +391,13 @@ fn discover_package_json( flags: &Flags, maybe_stop_at: Option, ) -> Result, AnyError> { - fn discover_from( - start: &Path, - maybe_stop_at: Option, - ) -> Result, AnyError> { - const PACKAGE_JSON_NAME: &str = "package.json"; - - // note: ancestors() includes the `start` path - for ancestor in start.ancestors() { - let path = ancestor.join(PACKAGE_JSON_NAME); - - let source = match std::fs::read_to_string(&path) { - Ok(source) => source, - Err(err) if err.kind() == std::io::ErrorKind::NotFound => { - if let Some(stop_at) = maybe_stop_at.as_ref() { - if ancestor == stop_at { - break; - } - } - continue; - } - Err(err) => bail!( - "Error loading package.json at {}. {:#}", - path.display(), - err - ), - }; - - let package_json = PackageJson::load_from_string(path.clone(), source)?; - log::debug!("package.json file found at '{}'", path.display()); - return Ok(Some(package_json)); - } - // No config file found. - log::debug!("No package.json file found"); - Ok(None) - } - // TODO(bartlomieju): discover for all subcommands, but print warnings that // `package.json` is ignored in bundle/compile/etc. if let Some(package_json_dir) = flags.package_json_search_dir() { let package_json_dir = canonicalize_path_maybe_not_exists(&package_json_dir)?; - return discover_from(&package_json_dir, maybe_stop_at); + return package_json::discover_from(&package_json_dir, maybe_stop_at); } log::debug!("No package.json file found"); diff --git a/cli/args/package_json.rs b/cli/args/package_json.rs index 76d353c5ec..73bcfb5824 100644 --- a/cli/args/package_json.rs +++ b/cli/args/package_json.rs @@ -1,6 +1,8 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. use std::collections::HashMap; +use std::path::Path; +use std::path::PathBuf; use deno_core::anyhow::anyhow; use deno_core::anyhow::bail; @@ -83,6 +85,44 @@ pub fn get_local_package_json_version_reqs( Ok(result) } +/// Attempts to discover the package.json file, maybe stopping when it +/// reaches the specified `maybe_stop_at` directory. +pub fn discover_from( + start: &Path, + maybe_stop_at: Option, +) -> Result, AnyError> { + const PACKAGE_JSON_NAME: &str = "package.json"; + + // note: ancestors() includes the `start` path + for ancestor in start.ancestors() { + let path = ancestor.join(PACKAGE_JSON_NAME); + + let source = match std::fs::read_to_string(&path) { + Ok(source) => source, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => { + if let Some(stop_at) = maybe_stop_at.as_ref() { + if ancestor == stop_at { + break; + } + } + continue; + } + Err(err) => bail!( + "Error loading package.json at {}. {:#}", + path.display(), + err + ), + }; + + let package_json = PackageJson::load_from_string(path.clone(), source)?; + log::debug!("package.json file found at '{}'", path.display()); + return Ok(Some(package_json)); + } + + log::debug!("No package.json file found"); + Ok(None) +} + #[cfg(test)] mod test { use pretty_assertions::assert_eq; diff --git a/cli/cache/mod.rs b/cli/cache/mod.rs index 6c50c4c8bc..90f88530fa 100644 --- a/cli/cache/mod.rs +++ b/cli/cache/mod.rs @@ -2,6 +2,7 @@ use crate::errors::get_error_class_name; use crate::file_fetcher::FileFetcher; +use crate::util::fs::canonicalize_path; use deno_core::futures; use deno_core::futures::FutureExt; @@ -101,12 +102,23 @@ impl Loader for FetchCacher { is_dynamic: bool, ) -> LoadFuture { if let Some(node_modules_url) = self.maybe_local_node_modules_url.as_ref() { - if specifier.as_str().starts_with(node_modules_url.as_str()) { - return Box::pin(futures::future::ready(Ok(Some( - LoadResponse::External { - specifier: specifier.clone(), - }, - )))); + // The specifier might be in a completely different symlinked tree than + // what the resolved node_modules_url is in (ex. `/my-project-1/node_modules` + // symlinked to `/my-project-2/node_modules`), so first check if the path + // is in a node_modules dir to avoid needlessly canonicalizing, then compare + // against the canonicalized specifier. + if specifier.path().contains("/node_modules/") { + let specifier = specifier + .to_file_path() + .ok() + .and_then(|path| canonicalize_path(&path).ok()) + .and_then(|path| ModuleSpecifier::from_file_path(path).ok()) + .unwrap_or_else(|| specifier.clone()); + if specifier.as_str().starts_with(node_modules_url.as_str()) { + return Box::pin(futures::future::ready(Ok(Some( + LoadResponse::External { specifier }, + )))); + } } } diff --git a/cli/graph_util.rs b/cli/graph_util.rs index 148ab1cee6..27c5f1e0ab 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -156,6 +156,9 @@ pub async fn create_graph_and_maybe_check( ); let maybe_imports = ps.options.to_maybe_imports()?; let maybe_package_json_deps = ps.options.maybe_package_json_deps()?; + ps.npm_resolver + .add_package_json_deps(maybe_package_json_deps.as_ref()) + .await?; let cli_resolver = CliGraphResolver::new( ps.options.to_maybe_jsx_import_source_config(), ps.maybe_import_map.clone(), diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index fa8b62306c..98c3b715a6 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -5,6 +5,7 @@ use super::text::LineIndex; use super::tsc; use super::tsc::AssetDocument; +use crate::args::package_json; use crate::args::ConfigFile; use crate::args::JsxImportSourceConfig; use crate::cache::CachedUrlMetadata; @@ -13,6 +14,7 @@ use crate::cache::HttpCache; use crate::file_fetcher::get_source_from_bytes; use crate::file_fetcher::map_content_type; use crate::file_fetcher::SUPPORTED_SCHEMES; +use crate::lsp::logging::lsp_log; use crate::node; use crate::node::node_resolve_npm_reference; use crate::node::NodeResolution; @@ -37,9 +39,11 @@ use deno_graph::npm::NpmPackageReqReference; use deno_graph::GraphImport; use deno_graph::Resolution; use deno_runtime::deno_node::NodeResolutionMode; +use deno_runtime::deno_node::PackageJson; use deno_runtime::permissions::PermissionsContainer; use indexmap::IndexMap; use once_cell::sync::Lazy; +use std::collections::BTreeMap; use std::collections::HashMap; use std::collections::HashSet; use std::collections::VecDeque; @@ -818,8 +822,10 @@ pub struct Documents { /// A resolver that takes into account currently loaded import map and JSX /// settings. resolver: CliGraphResolver, - /// The npm package requirements. - npm_reqs: Arc>, + /// The npm package requirements found in a package.json file. + npm_package_json_reqs: Arc>, + /// The npm package requirements found in npm specifiers. + npm_specifier_reqs: Arc>, /// Gets if any document had a node: specifier such that a @types/node package /// should be injected. has_injected_types_node_package: bool, @@ -838,7 +844,8 @@ impl Documents { resolver_config_hash: 0, imports: Default::default(), resolver: CliGraphResolver::default(), - npm_reqs: Default::default(), + npm_package_json_reqs: Default::default(), + npm_specifier_reqs: Default::default(), has_injected_types_node_package: false, specifier_resolver: Arc::new(SpecifierResolver::new(location)), } @@ -970,9 +977,15 @@ impl Documents { } /// Returns a collection of npm package requirements. - pub fn npm_package_reqs(&mut self) -> HashSet { + pub fn npm_package_reqs(&mut self) -> Vec { self.calculate_dependents_if_dirty(); - (*self.npm_reqs).clone() + let mut reqs = Vec::with_capacity( + self.npm_package_json_reqs.len() + self.npm_specifier_reqs.len(), + ); + // resolve the package.json reqs first, then the npm specifiers + reqs.extend(self.npm_package_json_reqs.iter().cloned()); + reqs.extend(self.npm_specifier_reqs.iter().cloned()); + reqs } /// Returns if a @types/node package was injected into the npm @@ -1150,12 +1163,14 @@ impl Documents { &mut self, maybe_import_map: Option>, maybe_config_file: Option<&ConfigFile>, + maybe_package_json: Option<&PackageJson>, npm_registry_api: NpmRegistryApi, npm_resolution: NpmResolution, ) { fn calculate_resolver_config_hash( maybe_import_map: Option<&import_map::ImportMap>, maybe_jsx_config: Option<&JsxImportSourceConfig>, + maybe_package_json_deps: Option<&HashMap>, ) -> u64 { let mut hasher = FastInsecureHasher::default(); if let Some(import_map) = maybe_import_map { @@ -1165,23 +1180,46 @@ impl Documents { if let Some(jsx_config) = maybe_jsx_config { hasher.write_hashable(&jsx_config); } + if let Some(deps) = maybe_package_json_deps { + let deps = deps.iter().collect::>(); + hasher.write_hashable(&deps); + } hasher.finish() } + let maybe_package_json_deps = maybe_package_json.and_then(|package_json| { + match package_json::get_local_package_json_version_reqs(package_json) { + Ok(deps) => Some(deps), + Err(err) => { + lsp_log!("Error parsing package.json deps: {err:#}"); + None + } + } + }); let maybe_jsx_config = maybe_config_file.and_then(|cf| cf.to_maybe_jsx_import_source_config()); let new_resolver_config_hash = calculate_resolver_config_hash( maybe_import_map.as_deref(), maybe_jsx_config.as_ref(), + maybe_package_json_deps.as_ref(), ); - // TODO(bartlomieju): handle package.json dependencies here + self.npm_package_json_reqs = Arc::new({ + match &maybe_package_json_deps { + Some(deps) => { + let mut reqs = deps.values().cloned().collect::>(); + reqs.sort(); + reqs + } + None => Vec::new(), + } + }); self.resolver = CliGraphResolver::new( maybe_jsx_config, maybe_import_map, false, npm_registry_api, npm_resolution, - None, + maybe_package_json_deps, ); self.imports = Arc::new( if let Some(Ok(imports)) = @@ -1306,7 +1344,11 @@ impl Documents { } self.dependents_map = Arc::new(doc_analyzer.dependents_map); - self.npm_reqs = Arc::new(npm_reqs); + self.npm_specifier_reqs = Arc::new({ + let mut reqs = npm_reqs.into_iter().collect::>(); + reqs.sort(); + reqs + }); self.dirty = false; file_system_docs.dirty = false; } @@ -1589,6 +1631,7 @@ console.log(b, "hello deno"); documents.update_config( Some(Arc::new(import_map)), None, + None, npm_registry_api.clone(), npm_resolution.clone(), ); @@ -1627,6 +1670,7 @@ console.log(b, "hello deno"); documents.update_config( Some(Arc::new(import_map)), None, + None, npm_registry_api, npm_resolution, ); diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 808a98a2cc..dfbb9c2e17 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -9,12 +9,14 @@ use deno_core::serde_json; use deno_core::serde_json::json; use deno_core::serde_json::Value; use deno_core::ModuleSpecifier; +use deno_runtime::deno_node::PackageJson; use deno_runtime::deno_web::BlobStore; use import_map::ImportMap; use log::error; use log::warn; use serde_json::from_value; use std::collections::HashMap; +use std::collections::HashSet; use std::env; use std::fmt::Write as _; use std::path::PathBuf; @@ -58,6 +60,7 @@ use super::tsc::AssetsSnapshot; use super::tsc::TsServer; use super::urls; use crate::args::get_root_cert_store; +use crate::args::package_json; use crate::args::resolve_import_map_from_specifier; use crate::args::CaData; use crate::args::CacheSetting; @@ -130,6 +133,8 @@ pub struct Inner { maybe_import_map: Option>, /// The URL for the import map which is used to determine relative imports. maybe_import_map_uri: Option, + /// An optional package.json configuration file. + maybe_package_json: Option, /// Configuration for formatter which has been taken from specified config file. fmt_options: FmtOptions, /// An optional configuration for linter which has been taken from specified config file. @@ -376,6 +381,7 @@ impl Inner { maybe_config_file: None, maybe_import_map: None, maybe_import_map_uri: None, + maybe_package_json: None, fmt_options: Default::default(), lint_options: Default::default(), maybe_testing_server: None, @@ -456,8 +462,6 @@ impl Inner { Ok(navigation_tree) } - /// Returns a tuple with parsed `ConfigFile` and `Url` pointing to that file. - /// If there's no config file specified in settings returns `None`. fn get_config_file(&self) -> Result, AnyError> { let workspace_settings = self.config.get_workspace_settings(); let maybe_config = workspace_settings.config; @@ -501,6 +505,28 @@ impl Inner { } } + fn get_package_json( + &self, + maybe_config_file: Option<&ConfigFile>, + ) -> Result, AnyError> { + // It is possible that root_uri is not set, for example when having a single + // file open and not a workspace. In those situations we can't + // automatically discover the configuration + if let Some(root_uri) = &self.config.root_uri { + let root_path = specifier_to_file_path(root_uri)?; + let maybe_package_json = package_json::discover_from( + &root_path, + maybe_config_file.and_then(|f| f.specifier.to_file_path().ok()), + )?; + Ok(maybe_package_json.map(|c| { + lsp_log!(" Auto-resolved package.json: \"{}\"", c.specifier()); + c + })) + } else { + Ok(None) + } + } + fn is_diagnosable(&self, specifier: &ModuleSpecifier) -> bool { if specifier.scheme() == "asset" { matches!( @@ -785,6 +811,7 @@ impl Inner { fn update_config_file(&mut self) -> Result<(), AnyError> { self.maybe_config_file = None; + self.maybe_package_json = None; self.fmt_options = Default::default(); self.lint_options = Default::default(); @@ -814,6 +841,15 @@ impl Inner { Ok(()) } + /// Updates the package.json. Always ensure this is done after updating + /// the configuration file as the resolution of this depends on that. + fn update_package_json(&mut self) -> Result<(), AnyError> { + self.maybe_package_json = None; + self.maybe_package_json = + self.get_package_json(self.maybe_config_file.as_ref())?; + Ok(()) + } + async fn update_tsconfig(&mut self) -> Result<(), AnyError> { let mark = self.performance.mark("update_tsconfig", None::<()>); let mut tsconfig = TsConfig::new(json!({ @@ -923,6 +959,9 @@ impl Inner { if let Err(err) = self.update_config_file() { self.client.show_message(MessageType::WARNING, err).await; } + if let Err(err) = self.update_package_json() { + self.client.show_message(MessageType::WARNING, err).await; + } if let Err(err) = self.update_tsconfig().await { self.client.show_message(MessageType::WARNING, err).await; } @@ -950,6 +989,7 @@ impl Inner { self.documents.update_config( self.maybe_import_map.clone(), self.maybe_config_file.as_ref(), + self.maybe_package_json.as_ref(), self.npm_resolver.api().clone(), self.npm_resolver.resolution().clone(), ); @@ -1129,6 +1169,9 @@ impl Inner { if let Err(err) = self.update_config_file() { self.client.show_message(MessageType::WARNING, err).await; } + if let Err(err) = self.update_package_json() { + self.client.show_message(MessageType::WARNING, err).await; + } if let Err(err) = self.update_import_map().await { self.client.show_message(MessageType::WARNING, err).await; } @@ -1139,6 +1182,7 @@ impl Inner { self.documents.update_config( self.maybe_import_map.clone(), self.maybe_config_file.as_ref(), + self.maybe_package_json.as_ref(), self.npm_resolver.api().clone(), self.npm_resolver.resolution().clone(), ); @@ -1155,7 +1199,7 @@ impl Inner { .performance .mark("did_change_watched_files", Some(¶ms)); let mut touched = false; - let changes: Vec = params + let changes: HashSet = params .changes .iter() .map(|f| self.url_map.normalize_url(&f.uri)) @@ -1163,7 +1207,7 @@ impl Inner { // if the current tsconfig has changed, we need to reload it if let Some(config_file) = &self.maybe_config_file { - if changes.iter().any(|uri| config_file.specifier == *uri) { + if changes.contains(&config_file.specifier) { if let Err(err) = self.update_config_file() { self.client.show_message(MessageType::WARNING, err).await; } @@ -1173,10 +1217,18 @@ impl Inner { touched = true; } } + if let Some(package_json) = &self.maybe_package_json { + if changes.contains(&package_json.specifier()) { + if let Err(err) = self.update_package_json() { + self.client.show_message(MessageType::WARNING, err).await; + } + touched = true; + } + } // if the current import map, or config file has changed, we need to reload // reload the import map if let Some(import_map_uri) = &self.maybe_import_map_uri { - if changes.iter().any(|uri| import_map_uri == uri) || touched { + if changes.contains(import_map_uri) || touched { if let Err(err) = self.update_import_map().await { self.client.show_message(MessageType::WARNING, err).await; } @@ -1187,6 +1239,7 @@ impl Inner { self.documents.update_config( self.maybe_import_map.clone(), self.maybe_config_file.as_ref(), + self.maybe_package_json.as_ref(), self.npm_resolver.api().clone(), self.npm_resolver.resolution().clone(), ); diff --git a/cli/npm/cache.rs b/cli/npm/cache.rs index 9f58bcd0d9..1bc2f84872 100644 --- a/cli/npm/cache.rs +++ b/cli/npm/cache.rs @@ -179,6 +179,10 @@ impl ReadonlyNpmCache { Self::new(dir.npm_folder_path()) } + pub fn root_dir_url(&self) -> &Url { + &self.root_dir_url + } + pub fn package_folder_for_id( &self, folder_id: &NpmPackageCacheFolderId, @@ -345,6 +349,10 @@ impl NpmCache { &self.cache_setting } + pub fn root_dir_url(&self) -> &Url { + self.readonly.root_dir_url() + } + /// Checks if the cache should be used for the provided name and version. /// NOTE: Subsequent calls for the same package will always return `true` /// to ensure a package is only downloaded once per run of the CLI. This diff --git a/cli/npm/resolution/mod.rs b/cli/npm/resolution/mod.rs index 8584958b5e..53284d7127 100644 --- a/cli/npm/resolution/mod.rs +++ b/cli/npm/resolution/mod.rs @@ -292,17 +292,18 @@ impl NpmResolution { pub async fn set_package_reqs( &self, - package_reqs: HashSet, + package_reqs: Vec, ) -> Result<(), AnyError> { let inner = &self.0; // only allow one thread in here at a time let _permit = inner.update_semaphore.acquire().await?; let snapshot = inner.snapshot.read().clone(); + let reqs_set = package_reqs.iter().collect::>(); let has_removed_package = !snapshot .package_reqs .keys() - .all(|req| package_reqs.contains(req)); + .all(|req| reqs_set.contains(req)); // if any packages were removed, we need to completely recreate the npm resolution snapshot let snapshot = if has_removed_package { NpmResolutionSnapshot::default() @@ -311,7 +312,7 @@ impl NpmResolution { }; let snapshot = add_package_reqs_to_snapshot( &inner.api, - package_reqs.into_iter().collect(), + package_reqs, snapshot, self.0.maybe_lockfile.clone(), ) diff --git a/cli/npm/resolvers/common.rs b/cli/npm/resolvers/common.rs index 8c1ecd8924..3f1c2a7047 100644 --- a/cli/npm/resolvers/common.rs +++ b/cli/npm/resolvers/common.rs @@ -20,6 +20,9 @@ use crate::npm::NpmResolutionPackage; /// Part of the resolution that interacts with the file system. #[async_trait] pub trait NpmPackageFsResolver: Send + Sync { + /// Specifier for the root directory. + fn root_dir_url(&self) -> &Url; + fn resolve_package_folder_from_deno_module( &self, id: &NpmPackageId, diff --git a/cli/npm/resolvers/global.rs b/cli/npm/resolvers/global.rs index 1d4d14ac88..87ad92675a 100644 --- a/cli/npm/resolvers/global.rs +++ b/cli/npm/resolvers/global.rs @@ -68,6 +68,10 @@ impl GlobalNpmPackageResolver { #[async_trait] impl NpmPackageFsResolver for GlobalNpmPackageResolver { + fn root_dir_url(&self) -> &Url { + self.cache.root_dir_url() + } + fn resolve_package_folder_from_deno_module( &self, id: &NpmPackageId, diff --git a/cli/npm/resolvers/local.rs b/cli/npm/resolvers/local.rs index ba395d1b63..bf5b8529c4 100644 --- a/cli/npm/resolvers/local.rs +++ b/cli/npm/resolvers/local.rs @@ -44,7 +44,7 @@ pub struct LocalNpmPackageResolver { resolution: NpmResolution, registry_url: Url, root_node_modules_path: PathBuf, - root_node_modules_specifier: ModuleSpecifier, + root_node_modules_url: Url, } impl LocalNpmPackageResolver { @@ -58,10 +58,8 @@ impl LocalNpmPackageResolver { cache, resolution, registry_url, - root_node_modules_specifier: ModuleSpecifier::from_directory_path( - &node_modules_folder, - ) - .unwrap(), + root_node_modules_url: Url::from_directory_path(&node_modules_folder) + .unwrap(), root_node_modules_path: node_modules_folder, } } @@ -92,8 +90,7 @@ impl LocalNpmPackageResolver { &self, specifier: &ModuleSpecifier, ) -> Option { - let relative_url = - self.root_node_modules_specifier.make_relative(specifier)?; + let relative_url = self.root_node_modules_url.make_relative(specifier)?; if relative_url.starts_with("../") { return None; } @@ -126,6 +123,10 @@ impl LocalNpmPackageResolver { #[async_trait] impl NpmPackageFsResolver for LocalNpmPackageResolver { + fn root_dir_url(&self) -> &Url { + &self.root_node_modules_url + } + fn resolve_package_folder_from_deno_module( &self, node_id: &NpmPackageId, diff --git a/cli/npm/resolvers/mod.rs b/cli/npm/resolvers/mod.rs index 039864c5ff..583a1c955b 100644 --- a/cli/npm/resolvers/mod.rs +++ b/cli/npm/resolvers/mod.rs @@ -19,7 +19,7 @@ use deno_runtime::deno_node::RequireNpmResolver; use global::GlobalNpmPackageResolver; use serde::Deserialize; use serde::Serialize; -use std::collections::HashSet; +use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; use std::sync::Arc; @@ -214,9 +214,9 @@ impl NpmPackageResolver { /// Gets if the provided specifier is in an npm package. pub fn in_npm_package(&self, specifier: &ModuleSpecifier) -> bool { - self - .resolve_package_folder_from_specifier(specifier) - .is_ok() + let root_dir_url = self.fs_resolver.root_dir_url(); + debug_assert!(root_dir_url.as_str().ends_with('/')); + specifier.as_ref().starts_with(root_dir_url.as_str()) } /// If the resolver has resolved any npm packages. @@ -224,6 +224,19 @@ impl NpmPackageResolver { self.resolution.has_packages() } + /// Adds the package reqs from a package.json if they exist. + pub async fn add_package_json_deps( + &self, + maybe_package_json_deps: Option<&HashMap>, + ) -> Result<(), AnyError> { + if let Some(deps) = maybe_package_json_deps { + let mut package_reqs = deps.values().cloned().collect::>(); + package_reqs.sort(); // deterministic resolution + self.add_package_reqs(package_reqs).await?; + } + Ok(()) + } + /// Adds package requirements to the resolver and ensures everything is setup. pub async fn add_package_reqs( &self, @@ -250,7 +263,7 @@ impl NpmPackageResolver { /// This will retrieve and resolve package information, but not cache any package files. pub async fn set_package_reqs( &self, - packages: HashSet, + packages: Vec, ) -> Result<(), AnyError> { self.resolution.set_package_reqs(packages).await } diff --git a/cli/proc_state.rs b/cli/proc_state.rs index 3f360fd9c0..52ac117707 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -237,12 +237,10 @@ impl ProcState { cli_options.resolve_inspector_server().map(Arc::new); let maybe_package_json_deps = cli_options.maybe_package_json_deps()?; - if let Some(deps) = &maybe_package_json_deps { - // resolve the package.json npm requirements ahead of time - let mut package_reqs = deps.values().cloned().collect::>(); - package_reqs.sort(); // deterministic resolution - npm_resolver.add_package_reqs(package_reqs).await?; - } + // resolve the package.json npm requirements ahead of time + npm_resolver + .add_package_json_deps(maybe_package_json_deps.as_ref()) + .await?; let resolver = Arc::new(CliGraphResolver::new( cli_options.to_maybe_jsx_import_source_config(), maybe_import_map.clone(), @@ -639,14 +637,18 @@ impl ProcState { ) -> Result { let maybe_imports = self.options.to_maybe_imports()?; + let maybe_package_json_deps = self.options.maybe_package_json_deps()?; + self + .npm_resolver + .add_package_json_deps(maybe_package_json_deps.as_ref()) + .await?; let cli_resolver = CliGraphResolver::new( self.options.to_maybe_jsx_import_source_config(), self.maybe_import_map.clone(), self.options.no_npm(), self.npm_resolver.api().clone(), self.npm_resolver.resolution().clone(), - // TODO(bartlomieju): this should use dependencies from `package.json`? - None, + maybe_package_json_deps, ); let graph_resolver = cli_resolver.as_graph_resolver(); let graph_npm_resolver = cli_resolver.as_graph_npm_resolver(); diff --git a/cli/tests/integration/bench_tests.rs b/cli/tests/integration/bench_tests.rs index 15a86ee9d0..5a010ec629 100644 --- a/cli/tests/integration/bench_tests.rs +++ b/cli/tests/integration/bench_tests.rs @@ -2,6 +2,7 @@ use deno_core::url::Url; use test_util as util; +use util::env_vars_for_npm_tests; itest!(overloads { args: "bench bench/overloads.ts", @@ -216,3 +217,13 @@ fn file_protocol() { }) .run(); } + +itest!(package_json_basic { + args: "bench", + output: "package_json/basic/main.bench.out", + envs: env_vars_for_npm_tests(), + http_server: true, + cwd: Some("package_json/basic"), + copy_temp_dir: Some("package_json/basic"), + exit_code: 0, +}); diff --git a/cli/tests/integration/cache_tests.rs b/cli/tests/integration/cache_tests.rs index ae4dc001ad..c80f7f5c85 100644 --- a/cli/tests/integration/cache_tests.rs +++ b/cli/tests/integration/cache_tests.rs @@ -1,5 +1,7 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. +use test_util::env_vars_for_npm_tests; + itest!(_036_import_map_fetch { args: "cache --quiet --reload --import-map=import_maps/import_map.json import_maps/test.ts", @@ -95,3 +97,13 @@ itest!(json_import { // should not error args: "cache --quiet cache/json_import/main.ts", }); + +itest!(package_json_basic { + args: "cache main.ts", + output: "package_json/basic/main.cache.out", + envs: env_vars_for_npm_tests(), + http_server: true, + cwd: Some("package_json/basic"), + copy_temp_dir: Some("package_json/basic"), + exit_code: 0, +}); diff --git a/cli/tests/integration/check_tests.rs b/cli/tests/integration/check_tests.rs index 66433f81da..021a536c42 100644 --- a/cli/tests/integration/check_tests.rs +++ b/cli/tests/integration/check_tests.rs @@ -3,6 +3,8 @@ use std::process::Command; use std::process::Stdio; use test_util as util; +use util::env_vars_for_npm_tests; +use util::env_vars_for_npm_tests_no_sync_download; use util::TempDir; itest!(_095_check_with_bare_import { @@ -229,3 +231,23 @@ fn ts_no_recheck_on_redirect() { assert!(std::str::from_utf8(&output.stderr).unwrap().is_empty()); } + +itest!(package_json_basic { + args: "check main.ts", + output: "package_json/basic/main.check.out", + envs: env_vars_for_npm_tests(), + http_server: true, + cwd: Some("package_json/basic"), + copy_temp_dir: Some("package_json/basic"), + exit_code: 0, +}); + +itest!(package_json_fail_check { + args: "check --quiet fail_check.ts", + output: "package_json/basic/fail_check.check.out", + envs: env_vars_for_npm_tests_no_sync_download(), + http_server: true, + cwd: Some("package_json/basic"), + copy_temp_dir: Some("package_json/basic"), + exit_code: 1, +}); diff --git a/cli/tests/integration/info_tests.rs b/cli/tests/integration/info_tests.rs index 6c75deea6b..704aaa7afc 100644 --- a/cli/tests/integration/info_tests.rs +++ b/cli/tests/integration/info_tests.rs @@ -2,6 +2,7 @@ use test_util as util; use test_util::TempDir; +use util::env_vars_for_npm_tests_no_sync_download; #[test] fn info_with_compiled_source() { @@ -127,3 +128,13 @@ itest!(with_config_override { args: "info info/with_config/test.ts --config info/with_config/deno-override.json --import-map info/with_config/import_map.json", output: "info/with_config/with_config.out", }); + +itest!(package_json_basic { + args: "info --quiet main.ts", + output: "package_json/basic/main.info.out", + envs: env_vars_for_npm_tests_no_sync_download(), + http_server: true, + cwd: Some("package_json/basic"), + copy_temp_dir: Some("package_json/basic"), + exit_code: 0, +}); diff --git a/cli/tests/integration/npm_tests.rs b/cli/tests/integration/npm_tests.rs index 54053710e9..910936ac37 100644 --- a/cli/tests/integration/npm_tests.rs +++ b/cli/tests/integration/npm_tests.rs @@ -1565,3 +1565,23 @@ itest!(create_require { envs: env_vars_for_npm_tests(), http_server: true, }); + +itest!(node_modules_import_run { + args: "run --quiet main.ts", + output: "npm/node_modules_import/main.out", + envs: env_vars_for_npm_tests(), + http_server: true, + cwd: Some("npm/node_modules_import/"), + copy_temp_dir: Some("npm/node_modules_import/"), + exit_code: 0, +}); + +itest!(node_modules_import_check { + args: "check --quiet main.ts", + output: "npm/node_modules_import/main_check.out", + envs: env_vars_for_npm_tests(), + http_server: true, + cwd: Some("npm/node_modules_import/"), + copy_temp_dir: Some("npm/node_modules_import/"), + exit_code: 1, +}); diff --git a/cli/tests/integration/test_tests.rs b/cli/tests/integration/test_tests.rs index efe50ac16a..8b318e8e14 100644 --- a/cli/tests/integration/test_tests.rs +++ b/cli/tests/integration/test_tests.rs @@ -2,6 +2,7 @@ use deno_core::url::Url; use test_util as util; +use util::env_vars_for_npm_tests; #[test] fn no_color() { @@ -452,3 +453,13 @@ itest!(parallel_output { output: "test/parallel_output.out", exit_code: 1, }); + +itest!(package_json_basic { + args: "test", + output: "package_json/basic/main.test.out", + envs: env_vars_for_npm_tests(), + http_server: true, + cwd: Some("package_json/basic"), + copy_temp_dir: Some("package_json/basic"), + exit_code: 0, +}); diff --git a/cli/tests/testdata/npm/node_modules_import/main.out b/cli/tests/testdata/npm/node_modules_import/main.out new file mode 100644 index 0000000000..51993f072d --- /dev/null +++ b/cli/tests/testdata/npm/node_modules_import/main.out @@ -0,0 +1,2 @@ +2 +2 diff --git a/cli/tests/testdata/npm/node_modules_import/main.ts b/cli/tests/testdata/npm/node_modules_import/main.ts new file mode 100644 index 0000000000..ed433ed049 --- /dev/null +++ b/cli/tests/testdata/npm/node_modules_import/main.ts @@ -0,0 +1,13 @@ +import * as myImport1 from "@denotest/esm-basic"; +import * as myImport2 from "./node_modules/@denotest/esm-basic/main.mjs"; + +myImport1.setValue(5); +myImport2.setValue(2); + +// these should both give type errors +const value1: string = myImport1.getValue(); +const value2: string = myImport2.getValue(); + +// these should both be equal because it should be mutating the same module +console.log(value1); +console.log(value2); diff --git a/cli/tests/testdata/npm/node_modules_import/main_check.out b/cli/tests/testdata/npm/node_modules_import/main_check.out new file mode 100644 index 0000000000..4442a97ba0 --- /dev/null +++ b/cli/tests/testdata/npm/node_modules_import/main_check.out @@ -0,0 +1,11 @@ +error: TS2322 [ERROR]: Type 'number' is not assignable to type 'string'. +const value1: string = myImport1.getValue(); + ~~~~~~ + at file:///[WILDCARD]/npm/node_modules_import/main.ts:8:7 + +TS2322 [ERROR]: Type 'number' is not assignable to type 'string'. +const value2: string = myImport2.getValue(); + ~~~~~~ + at file:///[WILDCARD]/npm/node_modules_import/main.ts:9:7 + +Found 2 errors. diff --git a/cli/tests/testdata/npm/node_modules_import/package.json b/cli/tests/testdata/npm/node_modules_import/package.json new file mode 100644 index 0000000000..ed77298e0c --- /dev/null +++ b/cli/tests/testdata/npm/node_modules_import/package.json @@ -0,0 +1,5 @@ +{ + "dependencies": { + "@denotest/esm-basic": "^1" + } +} diff --git a/cli/tests/testdata/npm/registry/@denotest/esm-basic/1.0.0/main.d.mts b/cli/tests/testdata/npm/registry/@denotest/esm-basic/1.0.0/main.d.mts new file mode 100644 index 0000000000..fa7814911e --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/esm-basic/1.0.0/main.d.mts @@ -0,0 +1,2 @@ +export declare function setValue(val: number): void; +export declare function getValue(): number; diff --git a/cli/tests/testdata/npm/registry/@denotest/esm-basic/1.0.0/main.mjs b/cli/tests/testdata/npm/registry/@denotest/esm-basic/1.0.0/main.mjs new file mode 100644 index 0000000000..23df4221cb --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/esm-basic/1.0.0/main.mjs @@ -0,0 +1,9 @@ +let value = 0; + +export function setValue(newValue) { + value = newValue; +} + +export function getValue() { + return value; +} diff --git a/cli/tests/testdata/npm/registry/@denotest/esm-basic/1.0.0/package.json b/cli/tests/testdata/npm/registry/@denotest/esm-basic/1.0.0/package.json new file mode 100644 index 0000000000..757ac2db90 --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/esm-basic/1.0.0/package.json @@ -0,0 +1,7 @@ +{ + "name": "@denotest/esm-basic", + "version": "1.0.0", + "type": "module", + "main": "main.mjs", + "types": "main.d.mts" +} diff --git a/cli/tests/testdata/package_json/basic/fail_check.check.out b/cli/tests/testdata/package_json/basic/fail_check.check.out new file mode 100644 index 0000000000..03997a0518 --- /dev/null +++ b/cli/tests/testdata/package_json/basic/fail_check.check.out @@ -0,0 +1,4 @@ +error: TS2322 [ERROR]: Type 'number' is not assignable to type 'string'. +const _test: string = getValue(); + ~~~~~ + at file:///[WILDCARD]/fail_check.ts:3:7 diff --git a/cli/tests/testdata/package_json/basic/fail_check.ts b/cli/tests/testdata/package_json/basic/fail_check.ts new file mode 100644 index 0000000000..ce849d92fb --- /dev/null +++ b/cli/tests/testdata/package_json/basic/fail_check.ts @@ -0,0 +1,3 @@ +import { getValue } from "./main.ts"; + +const _test: string = getValue(); diff --git a/cli/tests/testdata/package_json/basic/main.bench.out b/cli/tests/testdata/package_json/basic/main.bench.out new file mode 100644 index 0000000000..db0db21147 --- /dev/null +++ b/cli/tests/testdata/package_json/basic/main.bench.out @@ -0,0 +1,11 @@ +Download http://localhost:4545/npm/registry/@denotest/esm-basic +Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz +Check file:///[WILDCARD]/main.bench.ts +0 +cpu: [WILDCARD] +runtime: [WILDCARD] + +file:///[WILDCARD]/main.bench.ts +[WILDCARD] +-------------------------------------------------- ----------------------------- +should add [WILDCARD] diff --git a/cli/tests/testdata/package_json/basic/main.bench.ts b/cli/tests/testdata/package_json/basic/main.bench.ts new file mode 100644 index 0000000000..51bfc7bba1 --- /dev/null +++ b/cli/tests/testdata/package_json/basic/main.bench.ts @@ -0,0 +1,7 @@ +import { add } from "./main.ts"; + +Deno.bench("should add", () => { + if (add(1, 2) !== 3) { + throw new Error("Fail"); + } +}); diff --git a/cli/tests/testdata/package_json/basic/main.cache.out b/cli/tests/testdata/package_json/basic/main.cache.out new file mode 100644 index 0000000000..4be62e9eb7 --- /dev/null +++ b/cli/tests/testdata/package_json/basic/main.cache.out @@ -0,0 +1,2 @@ +Download http://localhost:4545/npm/registry/@denotest/esm-basic +Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz diff --git a/cli/tests/testdata/package_json/basic/main.check.out b/cli/tests/testdata/package_json/basic/main.check.out new file mode 100644 index 0000000000..09c3773144 --- /dev/null +++ b/cli/tests/testdata/package_json/basic/main.check.out @@ -0,0 +1,3 @@ +Download http://localhost:4545/npm/registry/@denotest/esm-basic +Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz +Check file://[WILDCARD]/main.ts diff --git a/cli/tests/testdata/package_json/basic/main.info.out b/cli/tests/testdata/package_json/basic/main.info.out new file mode 100644 index 0000000000..48c10a0bae --- /dev/null +++ b/cli/tests/testdata/package_json/basic/main.info.out @@ -0,0 +1,7 @@ +local: [WILDCARD]main.ts +type: TypeScript +dependencies: 1 unique +size: [WILDCARD] + +file://[WILDCARD]/package_json/basic/main.ts ([WILDCARD]) +└── npm:@denotest/esm-basic@1.0.0 ([WILDCARD]) diff --git a/cli/tests/testdata/package_json/basic/main.test.out b/cli/tests/testdata/package_json/basic/main.test.out new file mode 100644 index 0000000000..b04420b3bc --- /dev/null +++ b/cli/tests/testdata/package_json/basic/main.test.out @@ -0,0 +1,9 @@ +Download http://localhost:4545/npm/registry/@denotest/esm-basic +Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz +Check file://[WILDCARD]/main.test.ts +0 +running 1 test from [WILDCARD]main.test.ts +should add ... ok ([WILDCARD]) + +ok | 1 passed | 0 failed ([WILDCARD]) + diff --git a/cli/tests/testdata/package_json/basic/main.test.ts b/cli/tests/testdata/package_json/basic/main.test.ts new file mode 100644 index 0000000000..298ce1f5be --- /dev/null +++ b/cli/tests/testdata/package_json/basic/main.test.ts @@ -0,0 +1,7 @@ +import { add } from "./main.ts"; + +Deno.test("should add", () => { + if (add(1, 2) !== 3) { + throw new Error("Fail"); + } +}); diff --git a/cli/tests/testdata/package_json/basic/main.ts b/cli/tests/testdata/package_json/basic/main.ts new file mode 100644 index 0000000000..5911fe32d0 --- /dev/null +++ b/cli/tests/testdata/package_json/basic/main.ts @@ -0,0 +1,11 @@ +import * as test from "@denotest/esm-basic"; + +console.log(test.getValue()); + +export function add(a: number, b: number) { + return a + b; +} + +export function getValue() { + return test.getValue(); +} diff --git a/cli/tests/testdata/package_json/basic/package.json b/cli/tests/testdata/package_json/basic/package.json new file mode 100644 index 0000000000..54ca824d64 --- /dev/null +++ b/cli/tests/testdata/package_json/basic/package.json @@ -0,0 +1,5 @@ +{ + "dependencies": { + "@denotest/esm-basic": "*" + } +} diff --git a/cli/tsc/mod.rs b/cli/tsc/mod.rs index cc6350642c..b296bf9c66 100644 --- a/cli/tsc/mod.rs +++ b/cli/tsc/mod.rs @@ -556,7 +556,17 @@ fn op_load(state: &mut OpState, args: Value) -> Result { media_type = MediaType::Json; Some(Cow::Borrowed(&*module.source)) } - Module::External(_) | Module::Npm(_) | Module::Node(_) => None, + Module::Npm(_) | Module::Node(_) => None, + Module::External(module) => { + // means it's Deno code importing an npm module + media_type = MediaType::from(&module.specifier); + let file_path = specifier.to_file_path().unwrap(); + let code = + std::fs::read_to_string(&file_path).with_context(|| { + format!("Unable to load {}", file_path.display()) + })?; + Some(Cow::Owned(code)) + } } } else if state .maybe_npm_resolver @@ -718,7 +728,16 @@ fn resolve_graph_specifier_types( Ok(None) } } - Some(Module::External(_) | Module::Node(_)) | None => Ok(None), + Some(Module::External(module)) => { + // we currently only use "External" for when the module is in an npm package + Ok(state.maybe_npm_resolver.as_ref().map(|npm_resolver| { + NodeResolution::into_specifier_and_media_type( + node::url_to_node_resolution(module.specifier.clone(), npm_resolver) + .ok(), + ) + })) + } + Some(Module::Node(_)) | None => Ok(None), } } diff --git a/ext/node/package_json.rs b/ext/node/package_json.rs index 4fa3025bf9..b0816dd85e 100644 --- a/ext/node/package_json.rs +++ b/ext/node/package_json.rs @@ -11,6 +11,7 @@ use deno_core::error::AnyError; use deno_core::serde_json; use deno_core::serde_json::Map; use deno_core::serde_json::Value; +use deno_core::ModuleSpecifier; use indexmap::IndexMap; use serde::Serialize; use std::cell::RefCell; @@ -205,6 +206,10 @@ impl PackageJson { self.main.as_ref() } } + + pub fn specifier(&self) -> ModuleSpecifier { + ModuleSpecifier::from_file_path(&self.path).unwrap() + } } fn is_conditional_exports_main_sugar(exports: &Value) -> bool {