diff options
31 files changed, 593 insertions, 179 deletions
diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 5d1affb09..2c9f4c09d 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -327,7 +327,7 @@ pub struct Flags { pub cached_only: bool, pub type_check_mode: TypeCheckMode, pub config_flag: ConfigFlag, - pub node_modules_dir: bool, + pub node_modules_dir: Option<bool>, pub coverage_dir: Option<String>, pub enable_testing_features: bool, pub ignore: Vec<PathBuf>, @@ -503,6 +503,33 @@ impl Flags { } } + /// Extract path argument for `package.json` search paths. + /// If it returns Some(path), the `package.json` should be discovered + /// from the `path` dir. + /// If it returns None, the `package.json` file shouldn't be discovered at + /// all. + pub fn package_json_arg(&self) -> Option<PathBuf> { + use DenoSubcommand::*; + + if let Run(RunFlags { script }) = &self.subcommand { + if let Ok(module_specifier) = deno_core::resolve_url_or_path(script) { + if module_specifier.scheme() == "file" { + let p = module_specifier + .to_file_path() + .unwrap() + .parent()? + .to_owned(); + return Some(p); + } else if module_specifier.scheme() == "npm" { + let p = std::env::current_dir().unwrap(); + return Some(p); + } + } + } + + None + } + pub fn has_permission(&self) -> bool { self.allow_all || self.allow_hrtime @@ -2309,7 +2336,12 @@ fn no_npm_arg<'a>() -> Arg<'a> { fn local_npm_arg<'a>() -> Arg<'a> { Arg::new("node-modules-dir") .long("node-modules-dir") - .help("Creates a local node_modules folder") + .min_values(0) + .max_values(1) + .takes_value(true) + .require_equals(true) + .possible_values(["true", "false"]) + .help("Creates a local node_modules folder. This option is implicitly true when a package.json is auto-discovered.") } fn unsafely_ignore_certificate_errors_arg<'a>() -> Arg<'a> { @@ -3247,9 +3279,7 @@ fn no_npm_arg_parse(flags: &mut Flags, matches: &clap::ArgMatches) { } fn local_npm_args_parse(flags: &mut Flags, matches: &ArgMatches) { - if matches.is_present("node-modules-dir") { - flags.node_modules_dir = true; - } + flags.node_modules_dir = optional_bool_parse(matches, "node-modules-dir"); } fn inspect_arg_validate(val: &str) -> Result<(), String> { @@ -5448,7 +5478,24 @@ mod tests { subcommand: DenoSubcommand::Run(RunFlags { script: "script.ts".to_string(), }), - node_modules_dir: true, + node_modules_dir: Some(true), + ..Flags::default() + } + ); + + let r = flags_from_vec(svec![ + "deno", + "run", + "--node-modules-dir=false", + "script.ts" + ]); + assert_eq!( + r.unwrap(), + Flags { + subcommand: DenoSubcommand::Run(RunFlags { + script: "script.ts".to_string(), + }), + node_modules_dir: Some(false), ..Flags::default() } ); diff --git a/cli/args/mod.rs b/cli/args/mod.rs index da36c7071..7cb2213e9 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -5,9 +5,13 @@ mod flags; mod flags_allow_net; mod import_map; mod lockfile; +pub mod package_json; pub use self::import_map::resolve_import_map_from_specifier; use ::import_map::ImportMap; + +use crate::npm::NpmResolutionSnapshot; +use crate::util::fs::canonicalize_path; pub use config_file::BenchConfig; pub use config_file::CompilerOptions; pub use config_file::ConfigFile; @@ -32,8 +36,11 @@ use deno_core::anyhow::Context; use deno_core::error::AnyError; use deno_core::normalize_path; use deno_core::parking_lot::Mutex; +use deno_core::serde_json; use deno_core::url::Url; +use deno_graph::npm::NpmPackageReq; use deno_runtime::colors; +use deno_runtime::deno_node::PackageJson; use deno_runtime::deno_tls::rustls; use deno_runtime::deno_tls::rustls::RootCertStore; use deno_runtime::deno_tls::rustls_native_certs::load_native_certs; @@ -41,17 +48,22 @@ use deno_runtime::deno_tls::rustls_pemfile; use deno_runtime::deno_tls::webpki_roots; use deno_runtime::inspector_server::InspectorServer; use deno_runtime::permissions::PermissionsOptions; +use once_cell::sync::Lazy; use std::collections::BTreeMap; +use std::collections::HashMap; +use std::collections::HashSet; use std::env; use std::io::BufReader; use std::io::Cursor; use std::net::SocketAddr; use std::num::NonZeroUsize; +use std::path::Path; use std::path::PathBuf; use std::sync::Arc; use crate::cache::DenoDir; use crate::file_fetcher::FileFetcher; +use crate::npm::NpmProcessState; use crate::util::fs::canonicalize_path_maybe_not_exists; use crate::version; @@ -375,6 +387,74 @@ fn resolve_lint_rules_options( } } +/// Discover `package.json` file. If `maybe_stop_at` is provided, we will stop +/// crawling up the directory tree at that path. +fn discover_package_json( + flags: &Flags, + maybe_stop_at: Option<PathBuf>, +) -> Result<Option<PackageJson>, AnyError> { + pub fn discover_from( + start: &Path, + checked: &mut HashSet<PathBuf>, + maybe_stop_at: Option<PathBuf>, + ) -> Result<Option<PackageJson>, AnyError> { + const PACKAGE_JSON_NAME: &str = "package.json"; + + for ancestor in start.ancestors() { + if checked.insert(ancestor.to_path_buf()) { + 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_arg) = flags.package_json_arg() { + return discover_from( + &package_json_arg, + &mut HashSet::new(), + maybe_stop_at, + ); + } else if let crate::args::DenoSubcommand::Task(TaskFlags { + cwd: Some(path), + .. + }) = &flags.subcommand + { + // attempt to resolve the config file from the task subcommand's + // `--cwd` when specified + let task_cwd = canonicalize_path(&PathBuf::from(path))?; + return discover_from(&task_cwd, &mut HashSet::new(), None); + } + + log::debug!("No package.json file found"); + Ok(None) +} + /// Create and populate a root cert store based on the passed options and /// environment. pub fn get_root_cert_store( @@ -459,6 +539,21 @@ pub fn get_root_cert_store( Ok(root_cert_store) } +const RESOLUTION_STATE_ENV_VAR_NAME: &str = + "DENO_DONT_USE_INTERNAL_NODE_COMPAT_STATE"; + +static IS_NPM_MAIN: Lazy<bool> = + Lazy::new(|| std::env::var(RESOLUTION_STATE_ENV_VAR_NAME).is_ok()); + +static NPM_PROCESS_STATE: Lazy<Option<NpmProcessState>> = Lazy::new(|| { + let state = std::env::var(RESOLUTION_STATE_ENV_VAR_NAME).ok()?; + let state: NpmProcessState = serde_json::from_str(&state).ok()?; + // remove the environment variable so that sub processes + // that are spawned do not also use this. + std::env::remove_var(RESOLUTION_STATE_ENV_VAR_NAME); + Some(state) +}); + /// Overrides for the options below that when set will /// use these values over the values derived from the /// CLI flags or config file. @@ -474,6 +569,7 @@ pub struct CliOptions { // application need not concern itself with, so keep these private flags: Flags, maybe_config_file: Option<ConfigFile>, + maybe_package_json: Option<PackageJson>, maybe_lockfile: Option<Arc<Mutex<Lockfile>>>, overrides: CliOptionOverrides, } @@ -483,6 +579,7 @@ impl CliOptions { flags: Flags, maybe_config_file: Option<ConfigFile>, maybe_lockfile: Option<Lockfile>, + maybe_package_json: Option<PackageJson>, ) -> Self { if let Some(insecure_allowlist) = flags.unsafely_ignore_certificate_errors.as_ref() @@ -503,6 +600,7 @@ impl CliOptions { Self { maybe_config_file, maybe_lockfile, + maybe_package_json, flags, overrides: Default::default(), } @@ -510,9 +608,30 @@ impl CliOptions { pub fn from_flags(flags: Flags) -> Result<Self, AnyError> { let maybe_config_file = ConfigFile::discover(&flags)?; + + let mut maybe_package_json = None; + if let Some(config_file) = &maybe_config_file { + let specifier = config_file.specifier.clone(); + if specifier.scheme() == "file" { + let maybe_stop_at = specifier + .to_file_path() + .unwrap() + .parent() + .map(|p| p.to_path_buf()); + + maybe_package_json = discover_package_json(&flags, maybe_stop_at)?; + } + } else { + maybe_package_json = discover_package_json(&flags, None)?; + } let maybe_lock_file = lockfile::discover(&flags, maybe_config_file.as_ref())?; - Ok(Self::new(flags, maybe_config_file, maybe_lock_file)) + Ok(Self::new( + flags, + maybe_config_file, + maybe_lock_file, + maybe_package_json, + )) } pub fn maybe_config_file_specifier(&self) -> Option<ModuleSpecifier> { @@ -576,7 +695,7 @@ impl CliOptions { }; resolve_import_map_from_specifier( &import_map_specifier, - self.get_maybe_config_file().as_ref(), + self.maybe_config_file().as_ref(), file_fetcher, ) .await @@ -586,21 +705,56 @@ impl CliOptions { .map(Some) } + fn get_npm_process_state(&self) -> Option<&NpmProcessState> { + if !self.is_npm_main() { + return None; + } + + (*NPM_PROCESS_STATE).as_ref() + } + + pub fn get_npm_resolution_snapshot(&self) -> Option<NpmResolutionSnapshot> { + if let Some(state) = self.get_npm_process_state() { + // TODO(bartlomieju): remove this clone + return Some(state.snapshot.clone()); + } + + None + } + + // If the main module should be treated as being in an npm package. + // This is triggered via a secret environment variable which is used + // for functionality like child_process.fork. Users should NOT depend + // on this functionality. + pub fn is_npm_main(&self) -> bool { + *IS_NPM_MAIN + } + /// Overrides the import map specifier to use. pub fn set_import_map_specifier(&mut self, path: Option<ModuleSpecifier>) { self.overrides.import_map_specifier = Some(path); } pub fn node_modules_dir(&self) -> bool { - self.flags.node_modules_dir + if let Some(node_modules_dir) = self.flags.node_modules_dir { + return node_modules_dir; + } + + if let Some(npm_process_state) = self.get_npm_process_state() { + return npm_process_state.local_node_modules_path.is_some(); + } + + self.maybe_package_json.is_some() } /// Resolves the path to use for a local node_modules folder. pub fn resolve_local_node_modules_folder( &self, ) -> Result<Option<PathBuf>, AnyError> { - let path = if !self.flags.node_modules_dir { + let path = if !self.node_modules_dir() { return Ok(None); + } else if let Some(state) = self.get_npm_process_state() { + return Ok(state.local_node_modules_path.as_ref().map(PathBuf::from)); } else if let Some(config_path) = self .maybe_config_file .as_ref() @@ -699,10 +853,24 @@ impl CliOptions { } } - pub fn get_maybe_config_file(&self) -> &Option<ConfigFile> { + pub fn maybe_config_file(&self) -> &Option<ConfigFile> { &self.maybe_config_file } + pub fn maybe_package_json(&self) -> &Option<PackageJson> { + &self.maybe_package_json + } + + pub fn maybe_package_json_deps( + &self, + ) -> Result<Option<HashMap<String, NpmPackageReq>>, AnyError> { + if let Some(package_json) = self.maybe_package_json() { + package_json::get_local_package_json_version_reqs(package_json).map(Some) + } else { + Ok(None) + } + } + pub fn resolve_fmt_options( &self, fmt_flags: FmtFlags, diff --git a/cli/args/package_json.rs b/cli/args/package_json.rs new file mode 100644 index 000000000..76d353c5e --- /dev/null +++ b/cli/args/package_json.rs @@ -0,0 +1,167 @@ +// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. + +use std::collections::HashMap; + +use deno_core::anyhow::anyhow; +use deno_core::anyhow::bail; +use deno_core::error::AnyError; +use deno_graph::npm::NpmPackageReq; +use deno_graph::semver::VersionReq; +use deno_runtime::deno_node::PackageJson; + +/// Gets the name and raw version constraint taking into account npm +/// package aliases. +pub fn parse_dep_entry_name_and_raw_version<'a>( + key: &'a str, + value: &'a str, +) -> Result<(&'a str, &'a str), AnyError> { + if let Some(package_and_version) = value.strip_prefix("npm:") { + if let Some((name, version)) = package_and_version.rsplit_once('@') { + Ok((name, version)) + } else { + bail!("could not find @ symbol in npm url '{}'", value); + } + } else { + Ok((key, value)) + } +} + +/// Gets an application level package.json's npm package requirements. +/// +/// Note that this function is not general purpose. It is specifically for +/// parsing the application level package.json that the user has control +/// over. This is a design limitation to allow mapping these dependency +/// entries to npm specifiers which can then be used in the resolver. +pub fn get_local_package_json_version_reqs( + package_json: &PackageJson, +) -> Result<HashMap<String, NpmPackageReq>, AnyError> { + fn insert_deps( + deps: Option<&HashMap<String, String>>, + result: &mut HashMap<String, NpmPackageReq>, + ) -> Result<(), AnyError> { + if let Some(deps) = deps { + for (key, value) in deps { + let (name, version_req) = + parse_dep_entry_name_and_raw_version(key, value)?; + + let version_req = { + let result = VersionReq::parse_from_specifier(version_req); + match result { + Ok(version_req) => version_req, + Err(e) => { + let err = anyhow!("{:#}", e).context(concat!( + "Parsing version constraints in the application-level ", + "package.json is more strict at the moment" + )); + return Err(err); + } + } + }; + result.insert( + key.to_string(), + NpmPackageReq { + name: name.to_string(), + version_req: Some(version_req), + }, + ); + } + } + Ok(()) + } + + let deps = package_json.dependencies.as_ref(); + let dev_deps = package_json.dev_dependencies.as_ref(); + let mut result = HashMap::with_capacity( + deps.map(|d| d.len()).unwrap_or(0) + dev_deps.map(|d| d.len()).unwrap_or(0), + ); + + // insert the dev dependencies first so the dependencies will + // take priority and overwrite any collisions + insert_deps(dev_deps, &mut result)?; + insert_deps(deps, &mut result)?; + + Ok(result) +} + +#[cfg(test)] +mod test { + use pretty_assertions::assert_eq; + use std::path::PathBuf; + + use super::*; + + #[test] + fn test_parse_dep_entry_name_and_raw_version() { + let cases = [ + ("test", "^1.2", Ok(("test", "^1.2"))), + ("test", "1.x - 2.6", Ok(("test", "1.x - 2.6"))), + ("test", "npm:package@^1.2", Ok(("package", "^1.2"))), + ( + "test", + "npm:package", + Err("could not find @ symbol in npm url 'npm:package'"), + ), + ]; + for (key, value, expected_result) in cases { + let result = parse_dep_entry_name_and_raw_version(key, value); + match result { + Ok(result) => assert_eq!(result, expected_result.unwrap()), + Err(err) => assert_eq!(err.to_string(), expected_result.err().unwrap()), + } + } + } + + #[test] + fn test_get_local_package_json_version_reqs() { + let mut package_json = PackageJson::empty(PathBuf::from("/package.json")); + package_json.dependencies = Some(HashMap::from([ + ("test".to_string(), "^1.2".to_string()), + ("other".to_string(), "npm:package@~1.3".to_string()), + ])); + package_json.dev_dependencies = Some(HashMap::from([ + ("package_b".to_string(), "~2.2".to_string()), + // should be ignored + ("other".to_string(), "^3.2".to_string()), + ])); + let result = get_local_package_json_version_reqs(&package_json).unwrap(); + assert_eq!( + result, + HashMap::from([ + ( + "test".to_string(), + NpmPackageReq::from_str("test@^1.2").unwrap() + ), + ( + "other".to_string(), + NpmPackageReq::from_str("package@~1.3").unwrap() + ), + ( + "package_b".to_string(), + NpmPackageReq::from_str("package_b@~2.2").unwrap() + ) + ]) + ); + } + + #[test] + fn test_get_local_package_json_version_reqs_errors_non_npm_specifier() { + let mut package_json = PackageJson::empty(PathBuf::from("/package.json")); + package_json.dependencies = Some(HashMap::from([( + "test".to_string(), + "1.x - 1.3".to_string(), + )])); + let err = get_local_package_json_version_reqs(&package_json) + .err() + .unwrap(); + assert_eq!( + format!("{err:#}"), + concat!( + "Parsing version constraints in the application-level ", + "package.json is more strict at the moment: Invalid npm specifier ", + "version requirement. Unexpected character.\n", + " - 1.3\n", + " ~" + ) + ); + } +} diff --git a/cli/graph_util.rs b/cli/graph_util.rs index 0e6b308e1..77dc3011c 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -150,9 +150,11 @@ pub async fn create_graph_and_maybe_check( PermissionsContainer::allow_all(), ); let maybe_imports = ps.options.to_maybe_imports()?; + let maybe_package_json_deps = ps.options.maybe_package_json_deps()?; let cli_resolver = CliGraphResolver::new( ps.options.to_maybe_jsx_import_source_config(), ps.maybe_import_map.clone(), + maybe_package_json_deps, ); let graph_resolver = cli_resolver.as_graph_resolver(); let analyzer = ps.parsed_source_cache.as_analyzer(); diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 8d483fc73..faabb6a2f 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -1181,7 +1181,9 @@ impl Documents { maybe_import_map.as_deref(), maybe_jsx_config.as_ref(), ); - self.resolver = CliGraphResolver::new(maybe_jsx_config, maybe_import_map); + // TODO(bartlomieju): handle package.json dependencies here + self.resolver = + CliGraphResolver::new(maybe_jsx_config, maybe_import_map, None); self.imports = Arc::new( if let Some(Ok(imports)) = maybe_config_file.map(|cf| cf.to_maybe_imports()) diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 70797eaf2..1bde85d8f 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -322,7 +322,7 @@ fn create_lsp_npm_resolver( http_client, progress_bar, ); - NpmPackageResolver::new(npm_cache, api, false, None) + NpmPackageResolver::new(npm_cache, api) } impl Inner { @@ -3018,6 +3018,8 @@ impl Inner { self.maybe_config_file.clone(), // TODO(#16510): add support for lockfile None, + // TODO(bartlomieju): handle package.json dependencies here + None, ); cli_options.set_import_map_specifier(self.maybe_import_map_uri.clone()); diff --git a/cli/npm/mod.rs b/cli/npm/mod.rs index 4be13707e..20db61081 100644 --- a/cli/npm/mod.rs +++ b/cli/npm/mod.rs @@ -16,3 +16,4 @@ pub use resolution::NpmPackageNodeId; pub use resolution::NpmResolutionPackage; pub use resolution::NpmResolutionSnapshot; pub use resolvers::NpmPackageResolver; +pub use resolvers::NpmProcessState; diff --git a/cli/npm/registry.rs b/cli/npm/registry.rs index d267d2224..a758ae7be 100644 --- a/cli/npm/registry.rs +++ b/cli/npm/registry.rs @@ -24,6 +24,7 @@ use deno_graph::semver::VersionReq; use deno_runtime::colors; use serde::Serialize; +use crate::args::package_json::parse_dep_entry_name_and_raw_version; use crate::args::CacheSetting; use crate::cache::CACHE_PERM; use crate::http_util::HttpClient; @@ -113,30 +114,19 @@ impl NpmPackageVersionInfo { &self, ) -> Result<Vec<NpmDependencyEntry>, AnyError> { fn parse_dep_entry( - entry: (&String, &String), + (key, value): (&String, &String), kind: NpmDependencyEntryKind, ) -> Result<NpmDependencyEntry, AnyError> { - let bare_specifier = entry.0.clone(); let (name, version_req) = - if let Some(package_and_version) = entry.1.strip_prefix("npm:") { - if let Some((name, version)) = package_and_version.rsplit_once('@') { - (name.to_string(), version.to_string()) - } else { - bail!("could not find @ symbol in npm url '{}'", entry.1); - } - } else { - (entry.0.clone(), entry.1.clone()) - }; + parse_dep_entry_name_and_raw_version(key, value)?; let version_req = - VersionReq::parse_from_npm(&version_req).with_context(|| { - format!( - "error parsing version requirement for dependency: {bare_specifier}@{version_req}" - ) + VersionReq::parse_from_npm(version_req).with_context(|| { + format!("error parsing version requirement for dependency: {key}@{version_req}") })?; Ok(NpmDependencyEntry { kind, - bare_specifier, - name, + bare_specifier: key.to_string(), + name: name.to_string(), version_req, peer_dep_version_req: None, }) @@ -339,7 +329,11 @@ impl RealNpmRegistryApiInner { .load_package_info_from_registry(name) .await .with_context(|| { - format!("Error getting response at {}", self.get_package_url(name)) + format!( + "Error getting response at {} for package \"{}\"", + self.get_package_url(name), + name + ) })?; } let maybe_package_info = maybe_package_info.map(Arc::new); diff --git a/cli/npm/resolution/specifier.rs b/cli/npm/resolution/specifier.rs index 36d93bca4..f8b3776a3 100644 --- a/cli/npm/resolution/specifier.rs +++ b/cli/npm/resolution/specifier.rs @@ -8,7 +8,6 @@ use std::collections::VecDeque; use deno_ast::ModuleSpecifier; use deno_graph::npm::NpmPackageReference; use deno_graph::npm::NpmPackageReq; -use deno_graph::semver::VersionReq; use deno_graph::ModuleGraph; pub struct GraphNpmInfo { @@ -182,7 +181,7 @@ pub fn resolve_graph_npm_info(graph: &ModuleGraph) -> GraphNpmInfo { let reqs = std::mem::take(&mut leaf.reqs); let mut reqs = reqs.into_iter().collect::<Vec<_>>(); - reqs.sort_by(cmp_package_req); + reqs.sort(); result.extend(reqs); let mut deps = std::mem::take(&mut leaf.dependencies) @@ -380,46 +379,6 @@ fn cmp_folder_specifiers(a: &ModuleSpecifier, b: &ModuleSpecifier) -> Ordering { } } -// Sort the package requirements alphabetically then the version -// requirement in a way that will lead to the least number of -// duplicate packages (so sort None last since it's `*`), but -// mostly to create some determinism around how these are resolved. -fn cmp_package_req(a: &NpmPackageReq, b: &NpmPackageReq) -> Ordering { - fn cmp_specifier_version_req(a: &VersionReq, b: &VersionReq) -> Ordering { - match a.tag() { - Some(a_tag) => match b.tag() { - Some(b_tag) => b_tag.cmp(a_tag), // sort descending - None => Ordering::Less, // prefer a since tag - }, - None => { - match b.tag() { - Some(_) => Ordering::Greater, // prefer b since tag - None => { - // At this point, just sort by text descending. - // We could maybe be a bit smarter here in the future. - b.to_string().cmp(&a.to_string()) - } - } - } - } - } - - match a.name.cmp(&b.name) { - Ordering::Equal => { - match &b.version_req { - Some(b_req) => { - match &a.version_req { - Some(a_req) => cmp_specifier_version_req(a_req, b_req), - None => Ordering::Greater, // prefer b, since a is * - } - } - None => Ordering::Less, // prefer a, since b is * - } - } - ordering => ordering, - } -} - #[cfg(test)] mod tests { use pretty_assertions::assert_eq; @@ -485,31 +444,6 @@ mod tests { } #[test] - fn sorting_package_reqs() { - fn cmp_req(a: &str, b: &str) -> Ordering { - let a = NpmPackageReq::from_str(a).unwrap(); - let b = NpmPackageReq::from_str(b).unwrap(); - cmp_package_req(&a, &b) - } - - // sort by name - assert_eq!(cmp_req("a", "b@1"), Ordering::Less); - assert_eq!(cmp_req("b@1", "a"), Ordering::Greater); - // prefer non-wildcard - assert_eq!(cmp_req("a", "a@1"), Ordering::Greater); - assert_eq!(cmp_req("a@1", "a"), Ordering::Less); - // prefer tag - assert_eq!(cmp_req("a@tag", "a"), Ordering::Less); - assert_eq!(cmp_req("a", "a@tag"), Ordering::Greater); - // sort tag descending - assert_eq!(cmp_req("a@latest-v1", "a@latest-v2"), Ordering::Greater); - assert_eq!(cmp_req("a@latest-v2", "a@latest-v1"), Ordering::Less); - // sort version req descending - assert_eq!(cmp_req("a@1", "a@2"), Ordering::Greater); - assert_eq!(cmp_req("a@2", "a@1"), Ordering::Less); - } - - #[test] fn test_get_folder_path_specifier() { fn get(a: &str) -> String { get_folder_path_specifier(&ModuleSpecifier::parse(a).unwrap()).to_string() diff --git a/cli/npm/resolvers/mod.rs b/cli/npm/resolvers/mod.rs index 9dda160b0..a2638a15b 100644 --- a/cli/npm/resolvers/mod.rs +++ b/cli/npm/resolvers/mod.rs @@ -17,7 +17,6 @@ use deno_runtime::deno_node::NodeResolutionMode; use deno_runtime::deno_node::PathClean; use deno_runtime::deno_node::RequireNpmResolver; use global::GlobalNpmPackageResolver; -use once_cell::sync::Lazy; use serde::Deserialize; use serde::Serialize; use std::collections::HashSet; @@ -35,37 +34,11 @@ use super::NpmPackageNodeId; use super::NpmResolutionSnapshot; use super::RealNpmRegistryApi; -const RESOLUTION_STATE_ENV_VAR_NAME: &str = - "DENO_DONT_USE_INTERNAL_NODE_COMPAT_STATE"; - -static IS_NPM_MAIN: Lazy<bool> = - Lazy::new(|| std::env::var(RESOLUTION_STATE_ENV_VAR_NAME).is_ok()); - /// State provided to the process via an environment variable. -#[derive(Debug, Serialize, Deserialize)] -struct NpmProcessState { - snapshot: NpmResolutionSnapshot, - local_node_modules_path: Option<String>, -} - -impl NpmProcessState { - pub fn was_set() -> bool { - *IS_NPM_MAIN - } - - pub fn take() -> Option<NpmProcessState> { - // initialize the lazy before we remove the env var below - if !Self::was_set() { - return None; - } - - let state = std::env::var(RESOLUTION_STATE_ENV_VAR_NAME).ok()?; - let state = serde_json::from_str(&state).ok()?; - // remove the environment variable so that sub processes - // that are spawned do not also use this. - std::env::remove_var(RESOLUTION_STATE_ENV_VAR_NAME); - Some(state) - } +#[derive(Clone, Debug, Serialize, Deserialize)] +pub struct NpmProcessState { + pub snapshot: NpmResolutionSnapshot, + pub local_node_modules_path: Option<String>, } #[derive(Clone)] @@ -89,13 +62,8 @@ impl std::fmt::Debug for NpmPackageResolver { } impl NpmPackageResolver { - pub fn new( - cache: NpmCache, - api: RealNpmRegistryApi, - no_npm: bool, - local_node_modules_path: Option<PathBuf>, - ) -> Self { - Self::new_inner(cache, api, no_npm, local_node_modules_path, None, None) + pub fn new(cache: NpmCache, api: RealNpmRegistryApi) -> Self { + Self::new_inner(cache, api, false, None, None, None) } pub async fn new_with_maybe_lockfile( @@ -103,32 +71,34 @@ impl NpmPackageResolver { api: RealNpmRegistryApi, no_npm: bool, local_node_modules_path: Option<PathBuf>, + initial_snapshot: Option<NpmResolutionSnapshot>, maybe_lockfile: Option<Arc<Mutex<Lockfile>>>, ) -> Result<Self, AnyError> { - let maybe_snapshot = if let Some(lockfile) = &maybe_lockfile { - if lockfile.lock().overwrite { - None - } else { - Some( - NpmResolutionSnapshot::from_lockfile(lockfile.clone(), &api) - .await - .with_context(|| { - format!( - "failed reading lockfile '{}'", - lockfile.lock().filename.display() - ) - })?, - ) + let mut initial_snapshot = initial_snapshot; + + if initial_snapshot.is_none() { + if let Some(lockfile) = &maybe_lockfile { + if !lockfile.lock().overwrite { + initial_snapshot = Some( + NpmResolutionSnapshot::from_lockfile(lockfile.clone(), &api) + .await + .with_context(|| { + format!( + "failed reading lockfile '{}'", + lockfile.lock().filename.display() + ) + })?, + ) + } } - } else { - None - }; + } + Ok(Self::new_inner( cache, api, no_npm, local_node_modules_path, - maybe_snapshot, + initial_snapshot, maybe_lockfile, )) } @@ -138,17 +108,9 @@ impl NpmPackageResolver { api: RealNpmRegistryApi, no_npm: bool, local_node_modules_path: Option<PathBuf>, - initial_snapshot: Option<NpmResolutionSnapshot>, + maybe_snapshot: Option<NpmResolutionSnapshot>, maybe_lockfile: Option<Arc<Mutex<Lockfile>>>, ) -> Self { - let process_npm_state = NpmProcessState::take(); - let local_node_modules_path = local_node_modules_path.or_else(|| { - process_npm_state - .as_ref() - .and_then(|s| s.local_node_modules_path.as_ref().map(PathBuf::from)) - }); - let maybe_snapshot = - initial_snapshot.or_else(|| process_npm_state.map(|s| s.snapshot)); let inner: Arc<dyn InnerNpmPackageResolver> = match &local_node_modules_path { Some(node_modules_folder) => Arc::new(LocalNpmPackageResolver::new( @@ -289,14 +251,6 @@ impl NpmPackageResolver { self.inner.set_package_reqs(packages).await } - // If the main module should be treated as being in an npm package. - // This is triggered via a secret environment variable which is used - // for functionality like child_process.fork. Users should NOT depend - // on this functionality. - pub fn is_npm_main(&self) -> bool { - NpmProcessState::was_set() - } - /// Gets the state of npm for the process. pub fn get_npm_process_state(&self) -> String { serde_json::to_string(&NpmProcessState { diff --git a/cli/proc_state.rs b/cli/proc_state.rs index f40b5d575..b4467b6be 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -216,9 +216,18 @@ impl ProcState { let maybe_inspector_server = cli_options.resolve_inspector_server().map(Arc::new); + let maybe_package_json_deps = cli_options.maybe_package_json_deps()?; + let package_json_reqs = if let Some(deps) = &maybe_package_json_deps { + let mut package_reqs = deps.values().cloned().collect::<Vec<_>>(); + package_reqs.sort(); // deterministic resolution + package_reqs + } else { + Vec::new() + }; let resolver = Arc::new(CliGraphResolver::new( cli_options.to_maybe_jsx_import_source_config(), maybe_import_map.clone(), + maybe_package_json_deps, )); let maybe_file_watcher_reporter = @@ -255,9 +264,11 @@ impl ProcState { cli_options .resolve_local_node_modules_folder() .with_context(|| "Resolving local node_modules folder.")?, + cli_options.get_npm_resolution_snapshot(), lockfile.as_ref().cloned(), ) .await?; + npm_resolver.add_package_reqs(package_json_reqs).await?; let node_analysis_cache = NodeAnalysisCache::new(Some(dir.node_analysis_db_file_path())); @@ -637,6 +648,8 @@ impl ProcState { let cli_resolver = CliGraphResolver::new( self.options.to_maybe_jsx_import_source_config(), self.maybe_import_map.clone(), + // TODO(bartlomieju): this should use dependencies from `package.json`? + None, ); let graph_resolver = cli_resolver.as_graph_resolver(); let analyzer = self.parsed_source_cache.as_analyzer(); diff --git a/cli/resolver.rs b/cli/resolver.rs index 11b2d874c..aa58549a7 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -5,15 +5,21 @@ use deno_core::ModuleSpecifier; use deno_graph::source::Resolver; use deno_graph::source::DEFAULT_JSX_IMPORT_SOURCE_MODULE; use import_map::ImportMap; +use std::collections::HashMap; use std::sync::Arc; use crate::args::JsxImportSourceConfig; +use deno_graph::npm::NpmPackageReq; /// A resolver that takes care of resolution, taking into account loaded /// import map, JSX settings. #[derive(Debug, Clone, Default)] pub struct CliGraphResolver { maybe_import_map: Option<Arc<ImportMap>>, + // TODO(bartlomieju): actually use in `resolver`, once + // deno_graph refactors and upgrades land. + #[allow(dead_code)] + maybe_package_json_deps: Option<HashMap<String, NpmPackageReq>>, maybe_default_jsx_import_source: Option<String>, maybe_jsx_import_source_module: Option<String>, } @@ -22,6 +28,7 @@ impl CliGraphResolver { pub fn new( maybe_jsx_import_source_config: Option<JsxImportSourceConfig>, maybe_import_map: Option<Arc<ImportMap>>, + maybe_package_json_deps: Option<HashMap<String, NpmPackageReq>>, ) -> Self { Self { maybe_import_map, @@ -30,6 +37,7 @@ impl CliGraphResolver { .and_then(|c| c.default_specifier.clone()), maybe_jsx_import_source_module: maybe_jsx_import_source_config .map(|c| c.module), + maybe_package_json_deps, } } @@ -55,6 +63,8 @@ impl Resolver for CliGraphResolver { specifier: &str, referrer: &ModuleSpecifier, ) -> Result<ModuleSpecifier, AnyError> { + // TODO(bartlomieju): actually use `maybe_package_json_deps` here, once + // deno_graph refactors and upgrades land. if let Some(import_map) = &self.maybe_import_map { import_map .resolve(specifier, referrer) diff --git a/cli/standalone.rs b/cli/standalone.rs index c3a74dc3b..7da246fa9 100644 --- a/cli/standalone.rs +++ b/cli/standalone.rs @@ -240,6 +240,7 @@ pub async fn run( Some(Arc::new( parse_from_json(&base, &source).unwrap().import_map, )), + None, ) }, ), diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs index 02fa5c2a0..6a436c318 100644 --- a/cli/tests/integration/run_tests.rs +++ b/cli/tests/integration/run_tests.rs @@ -2693,6 +2693,34 @@ itest!(config_not_auto_discovered_for_remote_script { http_server: true, }); +itest!(package_json_auto_discovered_for_local_script_log { + args: "run -L debug no_deno_json/main.ts", + output: "run/with_package_json/no_deno_json/main.out", + maybe_cwd: Some("run/with_package_json/"), + envs: env_vars_for_npm_tests_no_sync_download(), + http_server: true, +}); + +// In this case we shouldn't discover `package.json` file, because it's in a +// directory that is above the directory containing `deno.json` file. +itest!( + package_json_auto_discovered_for_local_script_log_with_stop { + args: "run -L debug with_stop/some/nested/dir/main.ts", + output: "run/with_package_json/with_stop/main.out", + maybe_cwd: Some("run/with_package_json/"), + envs: env_vars_for_npm_tests_no_sync_download(), + http_server: true, + } +); + +itest!(package_json_auto_discovered_for_npm_binary { + args: "run -L debug -A npm:@denotest/bin/cli-esm this is a test", + output: "run/with_package_json/npm_binary/main.out", + maybe_cwd: Some("run/with_package_json/npm_binary/"), + envs: env_vars_for_npm_tests_no_sync_download(), + http_server: true, +}); + itest!(wasm_streaming_panic_test { args: "run run/wasm_streaming_panic_test.js", output: "run/wasm_streaming_panic_test.js.out", diff --git a/cli/tests/integration/watcher_tests.rs b/cli/tests/integration/watcher_tests.rs index 99d5a5727..0c9b8c29f 100644 --- a/cli/tests/integration/watcher_tests.rs +++ b/cli/tests/integration/watcher_tests.rs @@ -1177,7 +1177,7 @@ fn run_watch_dynamic_imports() { .spawn() .unwrap(); let (mut stdout_lines, mut stderr_lines) = child_lines(&mut child); - + assert_contains!(stderr_lines.next().unwrap(), "No package.json file found"); assert_contains!(stderr_lines.next().unwrap(), "Process started"); wait_contains( diff --git a/cli/tests/testdata/npm/cached_only/main.out b/cli/tests/testdata/npm/cached_only/main.out index e902bff49..f49494839 100644 --- a/cli/tests/testdata/npm/cached_only/main.out +++ b/cli/tests/testdata/npm/cached_only/main.out @@ -1,4 +1,4 @@ -error: Error getting response at http://localhost:4545/npm/registry/chalk +error: Error getting response at http://localhost:4545/npm/registry/chalk for package "chalk" Caused by: An npm specifier not found in cache: "chalk", --cached-only is specified. diff --git a/cli/tests/testdata/run/with_package_json/.gitignore b/cli/tests/testdata/run/with_package_json/.gitignore new file mode 100644 index 000000000..40b878db5 --- /dev/null +++ b/cli/tests/testdata/run/with_package_json/.gitignore @@ -0,0 +1 @@ +node_modules/
\ No newline at end of file diff --git a/cli/tests/testdata/run/with_package_json/no_deno_json/main.out b/cli/tests/testdata/run/with_package_json/no_deno_json/main.out new file mode 100644 index 000000000..c0dab77d0 --- /dev/null +++ b/cli/tests/testdata/run/with_package_json/no_deno_json/main.out @@ -0,0 +1,3 @@ +[WILDCARD]package.json file found at '[WILDCARD]with_package_json[WILDCARD]package.json' +[WILDCARD] +ok diff --git a/cli/tests/testdata/run/with_package_json/no_deno_json/main.ts b/cli/tests/testdata/run/with_package_json/no_deno_json/main.ts new file mode 100644 index 000000000..daefa8f60 --- /dev/null +++ b/cli/tests/testdata/run/with_package_json/no_deno_json/main.ts @@ -0,0 +1,6 @@ +// TODO(bartlomieju): currently we don't support actual bare specifier +// imports; this will be done in a follow up PR. +// import express from "express"; + +// console.log(express); +console.log("ok"); diff --git a/cli/tests/testdata/run/with_package_json/no_deno_json/package.json b/cli/tests/testdata/run/with_package_json/no_deno_json/package.json new file mode 100644 index 000000000..9ee3f39a8 --- /dev/null +++ b/cli/tests/testdata/run/with_package_json/no_deno_json/package.json @@ -0,0 +1,8 @@ +{ + "dependencies": { + "@denotest/check-error": "1.0.0" + }, + "devDependencies": { + "@denotest/cjs-default-export": "1.0.0" + } +} diff --git a/cli/tests/testdata/run/with_package_json/npm_binary/main.out b/cli/tests/testdata/run/with_package_json/npm_binary/main.out new file mode 100644 index 000000000..56cdae6f9 --- /dev/null +++ b/cli/tests/testdata/run/with_package_json/npm_binary/main.out @@ -0,0 +1,6 @@ +[WILDCARD]package.json file found at '[WILDCARD]with_package_json[WILDCARD]npm_binary[WILDCARD]package.json' +[WILDCARD] +this +is +a +test diff --git a/cli/tests/testdata/run/with_package_json/npm_binary/package.json b/cli/tests/testdata/run/with_package_json/npm_binary/package.json new file mode 100644 index 000000000..9ee3f39a8 --- /dev/null +++ b/cli/tests/testdata/run/with_package_json/npm_binary/package.json @@ -0,0 +1,8 @@ +{ + "dependencies": { + "@denotest/check-error": "1.0.0" + }, + "devDependencies": { + "@denotest/cjs-default-export": "1.0.0" + } +} diff --git a/cli/tests/testdata/run/with_package_json/with_stop/main.out b/cli/tests/testdata/run/with_package_json/with_stop/main.out new file mode 100644 index 000000000..e7ef053e4 --- /dev/null +++ b/cli/tests/testdata/run/with_package_json/with_stop/main.out @@ -0,0 +1,4 @@ +[WILDCARD]Config file found at '[WILDCARD]with_package_json[WILDCARD]with_stop[WILDCARD]some[WILDCARD]nested[WILDCARD]deno.json' +[WILDCARD]No package.json file found +[WILDCARD] +ok diff --git a/cli/tests/testdata/run/with_package_json/with_stop/package.json b/cli/tests/testdata/run/with_package_json/with_stop/package.json new file mode 100644 index 000000000..9ee3f39a8 --- /dev/null +++ b/cli/tests/testdata/run/with_package_json/with_stop/package.json @@ -0,0 +1,8 @@ +{ + "dependencies": { + "@denotest/check-error": "1.0.0" + }, + "devDependencies": { + "@denotest/cjs-default-export": "1.0.0" + } +} diff --git a/cli/tests/testdata/run/with_package_json/with_stop/some/nested/deno.json b/cli/tests/testdata/run/with_package_json/with_stop/some/nested/deno.json new file mode 100644 index 000000000..36e1765d1 --- /dev/null +++ b/cli/tests/testdata/run/with_package_json/with_stop/some/nested/deno.json @@ -0,0 +1,5 @@ +{ + "tasks": { + "dev": "deno run main.ts" + } +} diff --git a/cli/tests/testdata/run/with_package_json/with_stop/some/nested/dir/main.ts b/cli/tests/testdata/run/with_package_json/with_stop/some/nested/dir/main.ts new file mode 100644 index 000000000..daefa8f60 --- /dev/null +++ b/cli/tests/testdata/run/with_package_json/with_stop/some/nested/dir/main.ts @@ -0,0 +1,6 @@ +// TODO(bartlomieju): currently we don't support actual bare specifier +// imports; this will be done in a follow up PR. +// import express from "express"; + +// console.log(express); +console.log("ok"); diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index 209eb30b7..e3536a00d 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -156,7 +156,7 @@ fn maybe_update_config_file(output_dir: &Path, ps: &ProcState) -> bool { let fmt_config = ps .options - .get_maybe_config_file() + .maybe_config_file() .as_ref() .and_then(|config| config.to_fmt_config().ok()) .unwrap_or_default() diff --git a/cli/tools/vendor/test.rs b/cli/tools/vendor/test.rs index 31df151f2..874b329da 100644 --- a/cli/tools/vendor/test.rs +++ b/cli/tools/vendor/test.rs @@ -260,8 +260,8 @@ async fn build_test_graph( mut loader: TestLoader, analyzer: &dyn deno_graph::ModuleAnalyzer, ) -> ModuleGraph { - let resolver = - original_import_map.map(|m| CliGraphResolver::new(None, Some(Arc::new(m)))); + let resolver = original_import_map + .map(|m| CliGraphResolver::new(None, Some(Arc::new(m)), None)); let mut graph = ModuleGraph::default(); graph .build( diff --git a/cli/worker.rs b/cli/worker.rs index 72126d44f..7293183bf 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -457,7 +457,7 @@ async fn create_main_worker_internal( let is_main_cjs = matches!(node_resolution, node::NodeResolution::CommonJs(_)); (node_resolution.into_url(), is_main_cjs) - } else if ps.npm_resolver.is_npm_main() { + } else if ps.options.is_npm_main() { let node_resolution = node::url_to_node_resolution(main_module, &ps.npm_resolver)?; let is_main_cjs = diff --git a/ext/node/package_json.rs b/ext/node/package_json.rs index e40448930..b05362890 100644 --- a/ext/node/package_json.rs +++ b/ext/node/package_json.rs @@ -33,6 +33,8 @@ pub struct PackageJson { pub path: PathBuf, pub typ: String, pub types: Option<String>, + pub dependencies: Option<HashMap<String, String>>, + pub dev_dependencies: Option<HashMap<String, String>>, } impl PackageJson { @@ -49,6 +51,8 @@ impl PackageJson { path, typ: "none".to_string(), types: None, + dependencies: None, + dev_dependencies: None, } } @@ -86,6 +90,13 @@ impl PackageJson { return Ok(PackageJson::empty(path)); } + Self::load_from_string(path, source) + } + + pub fn load_from_string( + path: PathBuf, + source: String, + ) -> Result<PackageJson, AnyError> { let package_json: Value = serde_json::from_str(&source) .map_err(|err| anyhow::anyhow!("malformed package.json {}", err))?; @@ -114,6 +125,25 @@ impl PackageJson { let version = version_val.and_then(|s| s.as_str()).map(|s| s.to_string()); let module = module_val.and_then(|s| s.as_str()).map(|s| s.to_string()); + let dependencies = package_json.get("dependencies").and_then(|d| { + if d.is_object() { + let deps: HashMap<String, String> = + serde_json::from_value(d.to_owned()).unwrap(); + Some(deps) + } else { + None + } + }); + let dev_dependencies = package_json.get("devDependencies").and_then(|d| { + if d.is_object() { + let deps: HashMap<String, String> = + serde_json::from_value(d.to_owned()).unwrap(); + Some(deps) + } else { + None + } + }); + // Ignore unknown types for forwards compatibility let typ = if let Some(t) = type_val { if let Some(t) = t.as_str() { @@ -147,6 +177,8 @@ impl PackageJson { exports, imports, bin, + dependencies, + dev_dependencies, }; CACHE.with(|cache| { diff --git a/test_util/src/lib.rs b/test_util/src/lib.rs index 5aac13855..555c26ffe 100644 --- a/test_util/src/lib.rs +++ b/test_util/src/lib.rs @@ -1923,6 +1923,8 @@ pub struct CheckOutputIntegrationTest<'a> { pub envs: Vec<(String, String)>, pub env_clear: bool, pub temp_cwd: bool, + // Relative to "testdata" directory + pub maybe_cwd: Option<&'a str>, } impl<'a> CheckOutputIntegrationTest<'a> { @@ -1954,9 +1956,11 @@ impl<'a> CheckOutputIntegrationTest<'a> { let deno_dir = new_deno_dir(); // keep this alive for the test let mut command = deno_cmd_with_deno_dir(&deno_dir); let cwd = if self.temp_cwd { - deno_dir.path() + deno_dir.path().to_owned() + } else if let Some(cwd_) = &self.maybe_cwd { + testdata_dir.join(cwd_) } else { - testdata_dir.as_path() + testdata_dir.clone() }; println!("deno_exe args {}", args.join(" ")); println!("deno_exe cwd {:?}", &cwd); |