From 0da01c0ca6b537f74be32126e567bdfc2c73ed16 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 26 Jun 2024 17:24:10 -0400 Subject: [PATCH] refactor: move PackageJson to deno_config (#24348) --- Cargo.lock | 41 ++-- Cargo.toml | 3 +- cli/Cargo.toml | 2 +- cli/args/mod.rs | 18 +- cli/args/package_json.rs | 231 +----------------- cli/factory.rs | 1 - cli/lsp/config.rs | 27 +-- cli/lsp/language_server.rs | 2 +- cli/lsp/resolver.rs | 12 +- cli/npm/byonm.rs | 152 +++++++----- cli/resolver.rs | 11 +- cli/standalone/binary.rs | 4 +- cli/tools/lint/mod.rs | 2 +- cli/tools/registry/pm.rs | 6 +- cli/tools/registry/unfurl.rs | 15 +- cli/worker.rs | 2 +- ext/fs/Cargo.toml | 3 +- ext/fs/interface.rs | 29 +++ ext/fs/lib.rs | 1 + ext/node/Cargo.toml | 2 + ext/node/analyze.rs | 22 +- ext/node/lib.rs | 4 +- ext/node/ops/require.rs | 68 +++--- ext/node/package_json.rs | 300 ++++------------------- ext/node/polyfills/01_require.js | 2 +- ext/node/resolution.rs | 402 +++++++++++++++++-------------- tests/integration/lsp_tests.rs | 2 +- tests/integration/npm_tests.rs | 16 +- 28 files changed, 509 insertions(+), 871 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a45108aca5..b77943b882 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1106,7 +1106,7 @@ dependencies = [ "glibc_version", "glob", "ignore", - "import_map 0.20.0", + "import_map", "indexmap", "jsonc-parser", "junction", @@ -1265,19 +1265,21 @@ dependencies = [ [[package]] name = "deno_config" -version = "0.16.4" +version = "0.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3d21c7b688ff6cb411895a93bf1d6734ed654c3a7eb9b502f96098f6659df0c5" +checksum = "01b0852c0dd8594926d51a5dae80cd1679f87f79a7c02415e60625d6ee2a99ba" dependencies = [ "anyhow", + "deno_semver", "glob", - "import_map 0.19.0", + "import_map", "indexmap", "jsonc-parser", "log", "percent-encoding", "serde", "serde_json", + "thiserror", "url", ] @@ -1385,7 +1387,7 @@ dependencies = [ "futures", "handlebars", "html-escape", - "import_map 0.20.0", + "import_map", "indexmap", "lazy_static", "regex", @@ -1407,7 +1409,7 @@ dependencies = [ "deno_graph", "escape8259", "futures", - "import_map 0.20.0", + "import_map", "parking_lot 0.11.2", "url", ] @@ -1453,6 +1455,7 @@ version = "0.68.0" dependencies = [ "async-trait", "base32", + "deno_config", "deno_core", "deno_io", "deno_permissions", @@ -1480,7 +1483,7 @@ dependencies = [ "deno_unsync", "encoding_rs", "futures", - "import_map 0.20.0", + "import_map", "indexmap", "log", "monch", @@ -1668,9 +1671,11 @@ dependencies = [ "cbc", "const-oid", "data-encoding", + "deno_config", "deno_core", "deno_fetch", "deno_fs", + "deno_io", "deno_media_type", "deno_net", "deno_permissions", @@ -3473,20 +3478,6 @@ dependencies = [ "png", ] -[[package]] -name = "import_map" -version = "0.19.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "696717335b077e26921a60be7b7bdc15d1246074f1ac79d9e8560792535f7d07" -dependencies = [ - "indexmap", - "log", - "percent-encoding", - "serde", - "serde_json", - "url", -] - [[package]] name = "import_map" version = "0.20.0" @@ -6763,18 +6754,18 @@ dependencies = [ [[package]] name = "thiserror" -version = "1.0.59" +version = "1.0.61" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f0126ad08bff79f29fc3ae6a55cc72352056dfff61e3ff8bb7129476d44b23aa" +checksum = "c546c80d6be4bc6a00c0f01730c08df82eaa7a7a61f11d656526506112cc1709" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.59" +version = "1.0.61" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d1cd413b5d558b4c5bf3680e324a6fa5014e7b7c067a51e69dbdf47eb7148b66" +checksum = "46c3384250002a6d5af4d114f2845d37b57521033f30d5c3f46c4d70e1197533" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index cbc04d80d5..85a052d089 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -101,6 +101,7 @@ console_static_text = "=0.8.1" data-encoding = "2.3.3" data-url = "=0.3.0" deno_cache_dir = "=0.10.0" +deno_config = { version = "=0.17.0", default-features = false } dlopen2 = "0.6.1" ecb = "=0.1.2" elliptic-curve = { version = "0.13.4", features = ["alloc", "arithmetic", "ecdh", "std", "pem"] } @@ -170,7 +171,7 @@ spki = "0.7.2" tar = "=0.4.40" tempfile = "3.4.0" termcolor = "1.1.3" -thiserror = "1.0.40" +thiserror = "1.0.61" tokio = { version = "1.36.0", features = ["full"] } tokio-metrics = { version = "0.3.0", features = ["rt"] } tokio-util = "0.7.4" diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 4a01f2305c..9b318a22c9 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -65,7 +65,7 @@ winres.workspace = true [dependencies] deno_ast = { workspace = true, features = ["bundler", "cjs", "codegen", "proposal", "react", "sourcemap", "transforms", "typescript", "view", "visit"] } deno_cache_dir = { workspace = true } -deno_config = "=0.16.4" +deno_config = { workspace = true, features = ["deno_json", "package_json"] } deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] } deno_doc = { version = "=0.140.0", features = ["html", "syntect"] } deno_emit = "=0.43.0" diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 7422498355..ccf00425bd 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -8,9 +8,9 @@ mod lockfile; pub mod package_json; pub use self::import_map::resolve_import_map; -use self::package_json::PackageJsonDeps; use ::import_map::ImportMap; use deno_ast::SourceMapOption; +use deno_config::package_json::PackageJsonDeps; use deno_core::resolve_url_or_path; use deno_graph::GraphKind; use deno_npm::npm_rc::NpmRc; @@ -537,7 +537,7 @@ fn discover_package_json( flags: &Flags, maybe_stop_at: Option, current_dir: &Path, -) -> Result, AnyError> { +) -> Result>, AnyError> { // TODO(bartlomieju): discover for all subcommands, but print warnings that // `package.json` is ignored in bundle/compile/etc. @@ -798,7 +798,7 @@ pub struct CliOptions { maybe_node_modules_folder: Option, maybe_vendor_folder: Option, maybe_config_file: Option, - maybe_package_json: Option, + maybe_package_json: Option>, npmrc: Arc, maybe_lockfile: Option>>, overrides: CliOptionOverrides, @@ -813,7 +813,7 @@ impl CliOptions { initial_cwd: PathBuf, maybe_config_file: Option, maybe_lockfile: Option>>, - maybe_package_json: Option, + maybe_package_json: Option>, npmrc: Arc, force_global_cache: bool, ) -> Result { @@ -839,7 +839,7 @@ impl CliOptions { &initial_cwd, &flags, maybe_config_file.as_ref(), - maybe_package_json.as_ref(), + maybe_package_json.as_deref(), ) .with_context(|| "Resolving node_modules folder.")?; let maybe_vendor_folder = if force_global_cache { @@ -949,7 +949,7 @@ impl CliOptions { let maybe_lock_file = lockfile::discover( &flags, maybe_config_file.as_ref(), - maybe_package_json.as_ref(), + maybe_package_json.as_deref(), )?; Self::new( flags, @@ -1395,8 +1395,8 @@ impl CliOptions { &self.maybe_workspace_config } - pub fn maybe_package_json(&self) -> &Option { - &self.maybe_package_json + pub fn maybe_package_json(&self) -> Option<&Arc> { + self.maybe_package_json.as_ref() } pub fn npmrc(&self) -> &Arc { @@ -1414,7 +1414,7 @@ impl CliOptions { self .maybe_package_json() .as_ref() - .map(package_json::get_local_package_json_version_reqs) + .map(|p| p.resolve_local_package_json_version_reqs()) } } diff --git a/cli/args/package_json.rs b/cli/args/package_json.rs index b1ff39abc7..b6ccb33a4d 100644 --- a/cli/args/package_json.rs +++ b/cli/args/package_json.rs @@ -2,27 +2,15 @@ use std::path::Path; use std::path::PathBuf; +use std::sync::Arc; +use deno_config::package_json::PackageJsonDeps; use deno_core::anyhow::bail; use deno_core::error::AnyError; -use deno_npm::registry::parse_dep_entry_name_and_raw_version; +use deno_runtime::deno_fs::RealFs; +use deno_runtime::deno_node::load_pkg_json; use deno_runtime::deno_node::PackageJson; -use deno_semver::npm::NpmVersionReqParseError; use deno_semver::package::PackageReq; -use deno_semver::VersionReq; -use indexmap::IndexMap; -use thiserror::Error; - -#[derive(Debug, Error, Clone)] -pub enum PackageJsonDepValueParseError { - #[error(transparent)] - VersionReq(#[from] NpmVersionReqParseError), - #[error("Not implemented scheme '{scheme}'")] - Unsupported { scheme: String }, -} - -pub type PackageJsonDeps = - IndexMap>; #[derive(Debug, Default)] pub struct PackageJsonDepsProvider(Option); @@ -51,79 +39,21 @@ impl PackageJsonDepsProvider { } } -/// 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, -) -> PackageJsonDeps { - fn parse_entry( - key: &str, - value: &str, - ) -> Result { - if value.starts_with("workspace:") - || value.starts_with("file:") - || value.starts_with("git:") - || value.starts_with("http:") - || value.starts_with("https:") - { - return Err(PackageJsonDepValueParseError::Unsupported { - scheme: value.split(':').next().unwrap().to_string(), - }); - } - let (name, version_req) = parse_dep_entry_name_and_raw_version(key, value); - let result = VersionReq::parse_from_npm(version_req); - match result { - Ok(version_req) => Ok(PackageReq { - name: name.to_string(), - version_req, - }), - Err(err) => Err(PackageJsonDepValueParseError::VersionReq(err)), - } - } - - fn insert_deps( - deps: Option<&IndexMap>, - result: &mut PackageJsonDeps, - ) { - if let Some(deps) = deps { - for (key, value) in deps { - result - .entry(key.to_string()) - .or_insert_with(|| parse_entry(key, value)); - } - } - } - - let deps = package_json.dependencies.as_ref(); - let dev_deps = package_json.dev_dependencies.as_ref(); - let mut result = IndexMap::new(); - - // favors the deps over dev_deps - insert_deps(deps, &mut result); - insert_deps(dev_deps, &mut result); - - 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> { +) -> 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 => { + let package_json = match load_pkg_json(&RealFs, &path) { + Ok(Some(package_json)) => package_json, + Ok(None) => { if let Some(stop_at) = maybe_stop_at.as_ref() { if ancestor == stop_at { break; @@ -138,7 +68,6 @@ pub fn discover_from( ), }; - let package_json = PackageJson::load_from_string(path.clone(), source)?; log::debug!("package.json file found at '{}'", path.display()); return Ok(Some(package_json)); } @@ -146,147 +75,3 @@ pub fn discover_from( log::debug!("No package.json file found"); Ok(None) } - -#[cfg(test)] -mod test { - use pretty_assertions::assert_eq; - use std::path::PathBuf; - - use super::*; - - fn get_local_package_json_version_reqs_for_tests( - package_json: &PackageJson, - ) -> IndexMap> { - get_local_package_json_version_reqs(package_json) - .into_iter() - .map(|(k, v)| { - ( - k, - match v { - Ok(v) => Ok(v), - Err(err) => Err(err.to_string()), - }, - ) - }) - .collect::>() - } - - #[test] - fn test_get_local_package_json_version_reqs() { - let mut package_json = PackageJson::empty(PathBuf::from("/package.json")); - package_json.dependencies = Some(IndexMap::from([ - ("test".to_string(), "^1.2".to_string()), - ("other".to_string(), "npm:package@~1.3".to_string()), - ])); - package_json.dev_dependencies = Some(IndexMap::from([ - ("package_b".to_string(), "~2.2".to_string()), - // should be ignored - ("other".to_string(), "^3.2".to_string()), - ])); - let deps = get_local_package_json_version_reqs_for_tests(&package_json); - assert_eq!( - deps, - IndexMap::from([ - ( - "test".to_string(), - Ok(PackageReq::from_str("test@^1.2").unwrap()) - ), - ( - "other".to_string(), - Ok(PackageReq::from_str("package@~1.3").unwrap()) - ), - ( - "package_b".to_string(), - Ok(PackageReq::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(IndexMap::from([( - "test".to_string(), - "%*(#$%()".to_string(), - )])); - let map = get_local_package_json_version_reqs_for_tests(&package_json); - assert_eq!( - map, - IndexMap::from([( - "test".to_string(), - Err( - concat!( - "Invalid npm version requirement. Unexpected character.\n", - " %*(#$%()\n", - " ~" - ) - .to_string() - ) - )]) - ); - } - - #[test] - fn test_get_local_package_json_version_reqs_range() { - let mut package_json = PackageJson::empty(PathBuf::from("/package.json")); - package_json.dependencies = Some(IndexMap::from([( - "test".to_string(), - "1.x - 1.3".to_string(), - )])); - let map = get_local_package_json_version_reqs_for_tests(&package_json); - assert_eq!( - map, - IndexMap::from([( - "test".to_string(), - Ok(PackageReq { - name: "test".to_string(), - version_req: VersionReq::parse_from_npm("1.x - 1.3").unwrap() - }) - )]) - ); - } - - #[test] - fn test_get_local_package_json_version_reqs_skips_certain_specifiers() { - let mut package_json = PackageJson::empty(PathBuf::from("/package.json")); - package_json.dependencies = Some(IndexMap::from([ - ("test".to_string(), "1".to_string()), - ("work-test".to_string(), "workspace:1.1.1".to_string()), - ("file-test".to_string(), "file:something".to_string()), - ("git-test".to_string(), "git:something".to_string()), - ("http-test".to_string(), "http://something".to_string()), - ("https-test".to_string(), "https://something".to_string()), - ])); - let result = get_local_package_json_version_reqs_for_tests(&package_json); - assert_eq!( - result, - IndexMap::from([ - ( - "file-test".to_string(), - Err("Not implemented scheme 'file'".to_string()), - ), - ( - "git-test".to_string(), - Err("Not implemented scheme 'git'".to_string()), - ), - ( - "http-test".to_string(), - Err("Not implemented scheme 'http'".to_string()), - ), - ( - "https-test".to_string(), - Err("Not implemented scheme 'https'".to_string()), - ), - ( - "test".to_string(), - Ok(PackageReq::from_str("test@1").unwrap()) - ), - ( - "work-test".to_string(), - Err("Not implemented scheme 'workspace'".to_string()), - ) - ]) - ); - } -} diff --git a/cli/factory.rs b/cli/factory.rs index ddd63e0795..c2a5425c0c 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -314,7 +314,6 @@ impl CliFactory { // any package.jsons that are in different folders options .maybe_package_json() - .as_ref() .map(|package_json| { package_json.path.parent() != lockfile.lock().filename.parent() }) diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index da5af0c2df..4f96d45a4d 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -1520,25 +1520,20 @@ impl ConfigData { }) }); - let is_workspace_root = config_file + let workspace = config_file .as_ref() - .is_some_and(|c| !c.json.workspaces.is_empty()); - let workspace_members = if is_workspace_root { + .and_then(|c| c.json.workspace.as_ref().map(|w| (c, w))); + let is_workspace_root = workspace.is_some(); + let workspace_members = if let Some((config, workspace)) = workspace { Arc::new( - config_file - .as_ref() - .map(|c| { - c.json - .workspaces - .iter() - .flat_map(|p| { - let dir_specifier = c.specifier.join(p).ok()?; - let dir_path = specifier_to_file_path(&dir_specifier).ok()?; - Url::from_directory_path(normalize_path(dir_path)).ok() - }) - .collect() + workspace + .iter() + .flat_map(|p| { + let dir_specifier = config.specifier.join(p).ok()?; + let dir_path = specifier_to_file_path(&dir_specifier).ok()?; + Url::from_directory_path(normalize_path(dir_path)).ok() }) - .unwrap_or_default(), + .collect(), ) } else if let Some(workspace_data) = workspace_root { workspace_data.workspace_members.clone() diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index be1b27cdaa..a921584c2d 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -3496,7 +3496,7 @@ impl Inner { self.initial_cwd.clone(), config_data.and_then(|d| d.config_file.as_deref().cloned()), config_data.and_then(|d| d.lockfile.clone()), - config_data.and_then(|d| d.package_json.as_deref().cloned()), + config_data.and_then(|d| d.package_json.clone()), config_data .and_then(|d| d.npmrc.clone()) .unwrap_or_else(create_default_npmrc), diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs index c4c66f1144..45d44032fe 100644 --- a/cli/lsp/resolver.rs +++ b/cli/lsp/resolver.rs @@ -48,7 +48,6 @@ use std::collections::BTreeMap; use std::collections::BTreeSet; use std::collections::HashMap; use std::collections::HashSet; -use std::rc::Rc; use std::sync::Arc; use super::cache::LspCache; @@ -359,15 +358,12 @@ impl LspResolver { pub fn get_closest_package_json( &self, referrer: &ModuleSpecifier, - ) -> Result>, AnyError> { + ) -> Result>, AnyError> { let resolver = self.get_scope_resolver(Some(referrer)); let Some(node_resolver) = resolver.node_resolver.as_ref() else { return Ok(None); }; - node_resolver.get_closest_package_json( - referrer, - &mut deno_runtime::deno_node::AllowAllNodePermissions, - ) + node_resolver.get_closest_package_json(referrer) } pub fn resolve_redirects( @@ -462,7 +458,7 @@ async fn create_npm_resolver( config_data .and_then(|d| d.package_json.as_ref()) .map(|package_json| { - package_json::get_local_package_json_version_reqs(package_json) + package_json.resolve_local_package_json_version_reqs() }), )), npmrc: config_data @@ -506,7 +502,7 @@ fn create_graph_resolver( config_data .and_then(|d| d.package_json.as_ref()) .map(|package_json| { - package_json::get_local_package_json_version_reqs(package_json) + package_json.resolve_local_package_json_version_reqs() }), )), maybe_jsx_import_source_config: config_file diff --git a/cli/npm/byonm.rs b/cli/npm/byonm.rs index ec3fb00e94..0d4b9d4d46 100644 --- a/cli/npm/byonm.rs +++ b/cli/npm/byonm.rs @@ -3,7 +3,6 @@ use std::borrow::Cow; use std::path::Path; use std::path::PathBuf; -use std::rc::Rc; use std::sync::Arc; use deno_ast::ModuleSpecifier; @@ -11,12 +10,12 @@ use deno_core::anyhow::bail; use deno_core::error::AnyError; use deno_core::serde_json; use deno_runtime::deno_fs::FileSystem; +use deno_runtime::deno_node::load_pkg_json; use deno_runtime::deno_node::NodePermissions; use deno_runtime::deno_node::NpmResolver; use deno_runtime::deno_node::PackageJson; use deno_semver::package::PackageReq; -use crate::args::package_json::get_local_package_json_version_reqs; use crate::args::NpmProcessState; use crate::args::NpmProcessStateKind; use crate::util::fs::canonicalize_path_maybe_not_exists_with_fs; @@ -51,13 +50,13 @@ impl ByonmCliNpmResolver { &self, dep_name: &str, referrer: &ModuleSpecifier, - ) -> Option> { + ) -> Option> { let referrer_path = referrer.to_file_path().ok()?; let mut current_folder = referrer_path.parent()?; loop { let pkg_json_path = current_folder.join("package.json"); - if let Ok(pkg_json) = - PackageJson::load_skip_read_permission(self.fs.as_ref(), pkg_json_path) + if let Ok(Some(pkg_json)) = + load_pkg_json(self.fs.as_ref(), &pkg_json_path) { if let Some(deps) = &pkg_json.dependencies { if deps.contains_key(dep_name) { @@ -78,6 +77,70 @@ impl ByonmCliNpmResolver { } } } + + fn resolve_pkg_json_and_alias_for_req( + &self, + req: &PackageReq, + referrer: &ModuleSpecifier, + ) -> Result<(Arc, String), AnyError> { + fn resolve_alias_from_pkg_json( + req: &PackageReq, + pkg_json: &PackageJson, + ) -> Option { + let deps = pkg_json.resolve_local_package_json_version_reqs(); + for (key, value) in deps { + if let Ok(value) = value { + if value.name == req.name + && value.version_req.intersects(&req.version_req) + { + return Some(key); + } + } + } + None + } + + // attempt to resolve the npm specifier from the referrer's package.json, + if let Ok(file_path) = specifier_to_file_path(referrer) { + let mut current_path = file_path.as_path(); + while let Some(dir_path) = current_path.parent() { + let package_json_path = dir_path.join("package.json"); + if let Some(pkg_json) = + load_pkg_json(self.fs.as_ref(), &package_json_path)? + { + if let Some(alias) = + resolve_alias_from_pkg_json(req, pkg_json.as_ref()) + { + return Ok((pkg_json, alias)); + } + } + current_path = dir_path; + } + } + + // otherwise, fall fallback to the project's package.json + let root_pkg_json_path = self + .root_node_modules_dir + .parent() + .unwrap() + .join("package.json"); + if let Some(pkg_json) = + load_pkg_json(self.fs.as_ref(), &root_pkg_json_path)? + { + if let Some(alias) = resolve_alias_from_pkg_json(req, pkg_json.as_ref()) { + return Ok((pkg_json, alias)); + } + } + + bail!( + concat!( + "Could not find a matching package for 'npm:{}' in a package.json file. ", + "You must specify this as a package.json dependency when the ", + "node_modules folder is not managed by Deno.", + ), + req, + ); + } } impl NpmResolver for ByonmCliNpmResolver { @@ -181,68 +244,29 @@ impl CliNpmResolver for ByonmCliNpmResolver { req: &PackageReq, referrer: &ModuleSpecifier, ) -> Result { - fn resolve_from_package_json( - req: &PackageReq, - fs: &dyn FileSystem, - path: PathBuf, - ) -> Result { - let package_json = PackageJson::load_skip_read_permission(fs, path)?; - let deps = get_local_package_json_version_reqs(&package_json); - for (key, value) in deps { - if let Ok(value) = value { - if value.name == req.name - && value.version_req.intersects(&req.version_req) - { - let package_path = package_json - .path - .parent() - .unwrap() - .join("node_modules") - .join(key); - return Ok(canonicalize_path_maybe_not_exists_with_fs( - &package_path, - fs, - )?); - } - } - } - bail!( - concat!( - "Could not find a matching package for 'npm:{}' in '{}'. ", - "You must specify this as a package.json dependency when the ", - "node_modules folder is not managed by Deno.", - ), - req, - package_json.path.display() - ); - } - - // attempt to resolve the npm specifier from the referrer's package.json, - // but otherwise fallback to the project's package.json - if let Ok(file_path) = specifier_to_file_path(referrer) { - let mut current_path = file_path.as_path(); - while let Some(dir_path) = current_path.parent() { - let package_json_path = dir_path.join("package.json"); - if self.fs.exists_sync(&package_json_path) { - return resolve_from_package_json( - req, - self.fs.as_ref(), - package_json_path, - ); - } - current_path = dir_path; + // resolve the pkg json and alias + let (pkg_json, alias) = + self.resolve_pkg_json_and_alias_for_req(req, referrer)?; + // now try node resolution + for ancestor in pkg_json.path.parent().unwrap().ancestors() { + let node_modules_folder = ancestor.join("node_modules"); + let sub_dir = join_package_name(&node_modules_folder, &alias); + if self.fs.is_dir_sync(&sub_dir) { + return Ok(canonicalize_path_maybe_not_exists_with_fs( + &sub_dir, + self.fs.as_ref(), + )?); } } - resolve_from_package_json( - req, - self.fs.as_ref(), - self - .root_node_modules_dir - .parent() - .unwrap() - .join("package.json"), - ) + bail!( + concat!( + "Could not find \"{}\" in a node_modules folder. ", + "Deno expects the node_modules/ directory to be up to date. ", + "Did you forget to run `npm install`?" + ), + alias, + ); } fn check_state_hash(&self) -> Option { diff --git a/cli/resolver.rs b/cli/resolver.rs index 87a82dda03..2117f250b6 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -4,6 +4,7 @@ use async_trait::async_trait; use dashmap::DashMap; use dashmap::DashSet; use deno_ast::MediaType; +use deno_config::package_json::PackageJsonDeps; use deno_core::anyhow::anyhow; use deno_core::anyhow::Context; use deno_core::error::AnyError; @@ -21,7 +22,6 @@ use deno_runtime::deno_fs; use deno_runtime::deno_fs::FileSystem; use deno_runtime::deno_node::is_builtin_node_module; use deno_runtime::deno_node::parse_npm_pkg_name; -use deno_runtime::deno_node::NodePermissions; use deno_runtime::deno_node::NodeResolution; use deno_runtime::deno_node::NodeResolutionMode; use deno_runtime::deno_node::NodeResolver; @@ -34,10 +34,8 @@ use import_map::ImportMap; use std::borrow::Cow; use std::path::Path; use std::path::PathBuf; -use std::rc::Rc; use std::sync::Arc; -use crate::args::package_json::PackageJsonDeps; use crate::args::JsxImportSourceConfig; use crate::args::PackageJsonDepsProvider; use crate::args::DENO_DISABLE_PEDANTIC_NODE_WARNINGS; @@ -95,11 +93,8 @@ impl CliNodeResolver { pub fn get_closest_package_json( &self, referrer: &ModuleSpecifier, - permissions: &mut dyn NodePermissions, - ) -> Result>, AnyError> { - self - .node_resolver - .get_closest_package_json(referrer, permissions) + ) -> Result>, AnyError> { + self.node_resolver.get_closest_package_json(referrer) } pub fn resolve_if_in_npm_package( diff --git a/cli/standalone/binary.rs b/cli/standalone/binary.rs index 8df52e3eb4..98af5fa771 100644 --- a/cli/standalone/binary.rs +++ b/cli/standalone/binary.rs @@ -15,6 +15,8 @@ use std::path::PathBuf; use std::process::Command; use deno_ast::ModuleSpecifier; +use deno_config::package_json::PackageJsonDepValueParseError; +use deno_config::package_json::PackageJsonDeps; use deno_core::anyhow::bail; use deno_core::anyhow::Context; use deno_core::error::AnyError; @@ -31,8 +33,6 @@ use log::Level; use serde::Deserialize; use serde::Serialize; -use crate::args::package_json::PackageJsonDepValueParseError; -use crate::args::package_json::PackageJsonDeps; use crate::args::CaData; use crate::args::CliOptions; use crate::args::CompileFlags; diff --git a/cli/tools/lint/mod.rs b/cli/tools/lint/mod.rs index 712b95e340..0d9868cf2b 100644 --- a/cli/tools/lint/mod.rs +++ b/cli/tools/lint/mod.rs @@ -911,7 +911,7 @@ pub fn get_configured_rules( ) -> ConfiguredRules { const NO_SLOW_TYPES_NAME: &str = "no-slow-types"; let implicit_no_slow_types = maybe_config_file - .map(|c| c.is_package() || !c.json.workspaces.is_empty()) + .map(|c| c.is_package() || c.json.workspace.is_some()) .unwrap_or(false); let no_slow_types = implicit_no_slow_types && !rules diff --git a/cli/tools/registry/pm.rs b/cli/tools/registry/pm.rs index 40493c6bf8..4fdc025505 100644 --- a/cli/tools/registry/pm.rs +++ b/cli/tools/registry/pm.rs @@ -50,7 +50,7 @@ impl DenoConfigFormat { enum DenoOrPackageJson { Deno(deno_config::ConfigFile, DenoConfigFormat), - Npm(deno_node::PackageJson, Option), + Npm(Arc, Option), } impl DenoOrPackageJson { @@ -306,8 +306,8 @@ pub async fn add(flags: Flags, add_flags: AddFlags) -> Result<(), AnyError> { .await .context("Failed to update configuration file")?; - // TODO(bartlomieju): we should now cache the imports from the deno.json. - + // clear the previously cached package.json from memory before reloading it + deno_node::PackageJsonThreadLocalCache::clear(); // make a new CliFactory to pick up the updated config file let cli_factory = CliFactory::from_flags(flags)?; // cache deps diff --git a/cli/tools/registry/unfurl.rs b/cli/tools/registry/unfurl.rs index bc3272835f..36bff64bbb 100644 --- a/cli/tools/registry/unfurl.rs +++ b/cli/tools/registry/unfurl.rs @@ -305,7 +305,6 @@ fn to_range( mod tests { use std::sync::Arc; - use crate::args::package_json::get_local_package_json_version_reqs; use crate::args::PackageJsonDepsProvider; use super::*; @@ -316,7 +315,6 @@ mod tests { use deno_runtime::deno_fs::RealFs; use deno_runtime::deno_node::PackageJson; use import_map::ImportMapWithDiagnostics; - use indexmap::IndexMap; use pretty_assertions::assert_eq; use test_util::testdata_path; @@ -349,13 +347,18 @@ mod tests { }); let ImportMapWithDiagnostics { import_map, .. } = import_map::parse_from_value(deno_json_url, value).unwrap(); - let mut package_json = PackageJson::empty(cwd.join("package.json")); - package_json.dependencies = - Some(IndexMap::from([("chalk".to_string(), "5".to_string())])); + let package_json = PackageJson::load_from_value( + cwd.join("package.json"), + json!({ + "dependencies": { + "chalk": 5 + } + }), + ); let mapped_resolver = MappedSpecifierResolver::new( Some(Arc::new(import_map)), Arc::new(PackageJsonDepsProvider::new(Some( - get_local_package_json_version_reqs(&package_json), + package_json.resolve_local_package_json_version_reqs(), ))), ); diff --git a/cli/worker.rs b/cli/worker.rs index f332fc6bb7..420d92c023 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -6,6 +6,7 @@ use std::rc::Rc; use std::sync::Arc; use deno_ast::ModuleSpecifier; +use deno_config::package_json::PackageJsonDeps; use deno_core::anyhow::bail; use deno_core::error::AnyError; use deno_core::futures::FutureExt; @@ -46,7 +47,6 @@ use deno_semver::package::PackageReqReference; use deno_terminal::colors; use tokio::select; -use crate::args::package_json::PackageJsonDeps; use crate::args::write_lockfile_if_has_changes; use crate::args::DenoSubcommand; use crate::args::StorageKeyResolver; diff --git a/ext/fs/Cargo.toml b/ext/fs/Cargo.toml index 5248f3d870..ecdebdf1b6 100644 --- a/ext/fs/Cargo.toml +++ b/ext/fs/Cargo.toml @@ -14,11 +14,12 @@ description = "Ops for interacting with the file system" path = "lib.rs" [features] -sync_fs = [] +sync_fs = ["deno_config/sync"] [dependencies] async-trait.workspace = true base32.workspace = true +deno_config = { workspace = true, default-features = false } deno_core.workspace = true deno_io.workspace = true deno_permissions.workspace = true diff --git a/ext/fs/interface.rs b/ext/fs/interface.rs index 833f362bf9..6e3c359bbc 100644 --- a/ext/fs/interface.rs +++ b/ext/fs/interface.rs @@ -304,6 +304,35 @@ pub trait FileSystem: std::fmt::Debug + MaybeSend + MaybeSync { } } +pub struct DenoConfigFsAdapter<'a>(&'a dyn FileSystem); + +impl<'a> DenoConfigFsAdapter<'a> { + pub fn new(fs: &'a dyn FileSystem) -> Self { + Self(fs) + } +} + +impl<'a> deno_config::fs::DenoConfigFs for DenoConfigFsAdapter<'a> { + fn read_to_string(&self, path: &Path) -> Result { + use deno_io::fs::FsError; + use std::io::ErrorKind; + self + .0 + .read_text_file_lossy_sync(path, None) + .map_err(|err| match err { + FsError::Io(io) => io, + FsError::FileBusy => std::io::Error::new(ErrorKind::Other, "file busy"), + FsError::NotSupported => { + std::io::Error::new(ErrorKind::Other, "not supported") + } + FsError::PermissionDenied(name) => std::io::Error::new( + ErrorKind::PermissionDenied, + format!("requires {}", name), + ), + }) + } +} + // Like String::from_utf8_lossy but operates on owned values #[inline(always)] fn string_from_utf8_lossy(buf: Vec) -> String { diff --git a/ext/fs/lib.rs b/ext/fs/lib.rs index 2dce04b325..a60408f9bd 100644 --- a/ext/fs/lib.rs +++ b/ext/fs/lib.rs @@ -9,6 +9,7 @@ pub mod sync; pub use crate::in_memory_fs::InMemoryFs; pub use crate::interface::AccessCheckCb; pub use crate::interface::AccessCheckFn; +pub use crate::interface::DenoConfigFsAdapter; pub use crate::interface::FileSystem; pub use crate::interface::FileSystemRc; pub use crate::interface::FsDirEntry; diff --git a/ext/node/Cargo.toml b/ext/node/Cargo.toml index 8b5895bc7b..4253647924 100644 --- a/ext/node/Cargo.toml +++ b/ext/node/Cargo.toml @@ -23,9 +23,11 @@ bytes.workspace = true cbc.workspace = true const-oid = "0.9.5" data-encoding.workspace = true +deno_config = { workspace = true, default-features = false, features = ["package_json"] } deno_core.workspace = true deno_fetch.workspace = true deno_fs.workspace = true +deno_io.workspace = true deno_media_type.workspace = true deno_net.workspace = true deno_permissions.workspace = true diff --git a/ext/node/analyze.rs b/ext/node/analyze.rs index d80108733a..0a4ff8dac5 100644 --- a/ext/node/analyze.rs +++ b/ext/node/analyze.rs @@ -16,13 +16,12 @@ use once_cell::sync::Lazy; use deno_core::error::AnyError; +use crate::package_json::load_pkg_json; use crate::path::to_file_specifier; use crate::resolution::NodeResolverRc; -use crate::AllowAllNodePermissions; use crate::NodeModuleKind; use crate::NodeResolutionMode; use crate::NpmResolverRc; -use crate::PackageJson; use crate::PathClean; #[derive(Debug, Clone)] @@ -312,13 +311,8 @@ impl NodeCodeTranslator { )?; let package_json_path = module_dir.join("package.json"); - let package_json = PackageJson::load( - &*self.fs, - &*self.npm_resolver, - &mut AllowAllNodePermissions, - package_json_path.clone(), - )?; - if package_json.exists { + let maybe_package_json = load_pkg_json(&*self.fs, &package_json_path)?; + if let Some(package_json) = maybe_package_json { if let Some(exports) = &package_json.exports { return self.node_resolver.package_exports_resolve( &package_json_path, @@ -337,13 +331,9 @@ impl NodeCodeTranslator { if self.fs.is_dir_sync(&d) { // subdir might have a package.json that specifies the entrypoint let package_json_path = d.join("package.json"); - let package_json = PackageJson::load( - &*self.fs, - &*self.npm_resolver, - &mut AllowAllNodePermissions, - package_json_path, - )?; - if package_json.exists { + let maybe_package_json = + load_pkg_json(&*self.fs, &package_json_path)?; + if let Some(package_json) = maybe_package_json { if let Some(main) = package_json.main(NodeModuleKind::Cjs) { return Ok(to_file_specifier(&d.join(main).clean())); } diff --git a/ext/node/lib.rs b/ext/node/lib.rs index d0a8e24d52..ff979a8ba5 100644 --- a/ext/node/lib.rs +++ b/ext/node/lib.rs @@ -32,6 +32,7 @@ mod path; mod polyfill; mod resolution; +pub use deno_config::package_json::PackageJson; pub use ops::ipc::ChildPipeFd; pub use ops::ipc::IpcJsonStreamResource; use ops::vm; @@ -39,7 +40,8 @@ pub use ops::vm::create_v8_context; pub use ops::vm::init_global_template; pub use ops::vm::ContextInitMode; pub use ops::vm::VM_CONTEXT_INDEX; -pub use package_json::PackageJson; +pub use package_json::load_pkg_json; +pub use package_json::PackageJsonThreadLocalCache; pub use path::PathClean; pub use polyfill::is_builtin_node_module; pub use polyfill::SUPPORTED_BUILTIN_NODE_MODULES; diff --git a/ext/node/ops/require.rs b/ext/node/ops/require.rs index 3fde6c31ad..3702a90471 100644 --- a/ext/node/ops/require.rs +++ b/ext/node/ops/require.rs @@ -17,7 +17,6 @@ use std::rc::Rc; use crate::resolution; use crate::resolution::NodeResolverRc; -use crate::AllowAllNodePermissions; use crate::NodeModuleKind; use crate::NodePermissions; use crate::NodeResolutionMode; @@ -390,7 +389,6 @@ where let pkg = node_resolver .get_closest_package_json( &Url::from_file_path(parent_path.unwrap()).unwrap(), - &mut AllowAllNodePermissions, ) .ok() .flatten(); @@ -497,30 +495,30 @@ where original } }; - let pkg = node_resolver.load_package_json( - &mut AllowAllNodePermissions, - PathBuf::from(&pkg_path).join("package.json"), - )?; + let Some(pkg) = node_resolver + .load_package_json(&PathBuf::from(&pkg_path).join("package.json"))? + else { + return Ok(None); + }; + let Some(exports) = &pkg.exports else { + return Ok(None); + }; - if let Some(exports) = &pkg.exports { - let referrer = Url::from_file_path(parent_path).unwrap(); - let r = node_resolver.package_exports_resolve( - &pkg.path, - &format!(".{expansion}"), - exports, - &referrer, - NodeModuleKind::Cjs, - resolution::REQUIRE_CONDITIONS, - NodeResolutionMode::Execution, - )?; - Ok(Some(if r.scheme() == "file" { - url_to_file_path_string(&r)? - } else { - r.to_string() - })) + let referrer = Url::from_file_path(parent_path).unwrap(); + let r = node_resolver.package_exports_resolve( + &pkg.path, + &format!(".{expansion}"), + exports, + &referrer, + NodeModuleKind::Cjs, + resolution::REQUIRE_CONDITIONS, + NodeResolutionMode::Execution, + )?; + Ok(Some(if r.scheme() == "file" { + url_to_file_path_string(&r)? } else { - Ok(None) - } + r.to_string() + })) } #[op2] @@ -537,12 +535,8 @@ where PathBuf::from(&filename).parent().unwrap(), )?; let node_resolver = state.borrow::().clone(); - let permissions = state.borrow_mut::

(); node_resolver - .get_closest_package_json( - &Url::from_file_path(filename).unwrap(), - permissions, - ) + .get_closest_package_json(&Url::from_file_path(filename).unwrap()) .map(|maybe_pkg| maybe_pkg.map(|pkg| (*pkg).clone())) } @@ -556,12 +550,16 @@ where P: NodePermissions + 'static, { let node_resolver = state.borrow::().clone(); - let permissions = state.borrow_mut::

(); let package_json_path = PathBuf::from(package_json_path); + if package_json_path.file_name() != Some("package.json".as_ref()) { + // permissions: do not allow reading a non-package.json file + return None; + } node_resolver - .load_package_json(permissions, package_json_path) - .map(|pkg| (*pkg).clone()) + .load_package_json(&package_json_path) .ok() + .flatten() + .map(|pkg| (*pkg).clone()) } #[op2] @@ -577,10 +575,8 @@ where let referrer_path = PathBuf::from(&referrer_filename); ensure_read_permission::

(state, &referrer_path)?; let node_resolver = state.borrow::(); - let Some(pkg) = node_resolver.get_closest_package_json_from_path( - &referrer_path, - &mut AllowAllNodePermissions, - )? + let Some(pkg) = + node_resolver.get_closest_package_json_from_path(&referrer_path)? else { return Ok(None); }; diff --git a/ext/node/package_json.rs b/ext/node/package_json.rs index a19a2d64d8..8a88fe8f10 100644 --- a/ext/node/package_json.rs +++ b/ext/node/package_json.rs @@ -1,272 +1,58 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -use crate::NodeModuleKind; -use crate::NodePermissions; - -use super::NpmResolver; - -use deno_core::anyhow; -use deno_core::anyhow::bail; -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 deno_config::package_json::PackageJson; +use deno_config::package_json::PackageJsonLoadError; +use deno_config::package_json::PackageJsonRc; +use deno_fs::DenoConfigFsAdapter; use std::cell::RefCell; use std::collections::HashMap; use std::io::ErrorKind; +use std::path::Path; use std::path::PathBuf; -use std::rc::Rc; +// use a thread local cache so that workers have their own distinct cache thread_local! { - static CACHE: RefCell>> = RefCell::new(HashMap::new()); + static CACHE: RefCell> = RefCell::new(HashMap::new()); } -#[derive(Clone, Debug, Serialize)] -pub struct PackageJson { - pub exists: bool, - pub exports: Option>, - pub imports: Option>, - pub bin: Option, - main: Option, // use .main(...) - module: Option, // use .main(...) - pub name: Option, - pub version: Option, - pub path: PathBuf, - pub typ: String, - pub types: Option, - pub dependencies: Option>, - pub dev_dependencies: Option>, - pub scripts: Option>, +pub struct PackageJsonThreadLocalCache; + +impl PackageJsonThreadLocalCache { + pub fn clear() { + CACHE.with(|cache| cache.borrow_mut().clear()); + } } -impl PackageJson { - pub fn empty(path: PathBuf) -> PackageJson { - PackageJson { - exists: false, - exports: None, - imports: None, - bin: None, - main: None, - module: None, - name: None, - version: None, - path, - typ: "none".to_string(), - types: None, - dependencies: None, - dev_dependencies: None, - scripts: None, +impl deno_config::package_json::PackageJsonCache + for PackageJsonThreadLocalCache +{ + fn get(&self, path: &Path) -> Option { + CACHE.with(|cache| cache.borrow().get(path).cloned()) + } + + fn set(&self, path: PathBuf, package_json: PackageJsonRc) { + CACHE.with(|cache| cache.borrow_mut().insert(path, package_json)); + } +} + +/// Helper to load a package.json file using the thread local cache +/// in deno_node. +pub fn load_pkg_json( + fs: &dyn deno_fs::FileSystem, + path: &Path, +) -> Result, PackageJsonLoadError> { + let result = PackageJson::load_from_path( + path, + &DenoConfigFsAdapter::new(fs), + Some(&PackageJsonThreadLocalCache), + ); + match result { + Ok(pkg_json) => Ok(Some(pkg_json)), + Err(PackageJsonLoadError::Io { source, .. }) + if source.kind() == ErrorKind::NotFound => + { + Ok(None) } - } - - pub fn load( - fs: &dyn deno_fs::FileSystem, - resolver: &dyn NpmResolver, - permissions: &mut dyn NodePermissions, - path: PathBuf, - ) -> Result, AnyError> { - resolver.ensure_read_permission(permissions, &path)?; - Self::load_skip_read_permission(fs, path) - } - - pub fn load_skip_read_permission( - fs: &dyn deno_fs::FileSystem, - path: PathBuf, - ) -> Result, AnyError> { - assert!(path.is_absolute()); - - if CACHE.with(|cache| cache.borrow().contains_key(&path)) { - return Ok(CACHE.with(|cache| cache.borrow()[&path].clone())); - } - - let source = match fs.read_text_file_lossy_sync(&path, None) { - Ok(source) => source, - Err(err) if err.kind() == ErrorKind::NotFound => { - return Ok(Rc::new(PackageJson::empty(path))); - } - Err(err) => bail!( - "Error loading package.json at {}. {:#}", - path.display(), - AnyError::from(err), - ), - }; - - let package_json = Rc::new(Self::load_from_string(path, source)?); - CACHE.with(|cache| { - cache - .borrow_mut() - .insert(package_json.path.clone(), package_json.clone()); - }); - Ok(package_json) - } - - pub fn load_from_string( - path: PathBuf, - source: String, - ) -> Result { - if source.trim().is_empty() { - return Ok(PackageJson::empty(path)); - } - - let package_json: Value = serde_json::from_str(&source).map_err(|err| { - anyhow::anyhow!( - "malformed package.json: {}\n at {}", - err, - path.display() - ) - })?; - Self::load_from_value(path, package_json) - } - - pub fn load_from_value( - path: PathBuf, - package_json: serde_json::Value, - ) -> Result { - let imports_val = package_json.get("imports"); - let main_val = package_json.get("main"); - let module_val = package_json.get("module"); - let name_val = package_json.get("name"); - let version_val = package_json.get("version"); - let type_val = package_json.get("type"); - let bin = package_json.get("bin").map(ToOwned::to_owned); - let exports = package_json.get("exports").and_then(|exports| { - Some(if is_conditional_exports_main_sugar(exports) { - let mut map = Map::new(); - map.insert(".".to_string(), exports.to_owned()); - map - } else { - exports.as_object()?.to_owned() - }) - }); - - let imports = imports_val - .and_then(|imp| imp.as_object()) - .map(|imp| imp.to_owned()); - let main = main_val.and_then(|s| s.as_str()).map(|s| s.to_string()); - let name = name_val.and_then(|s| s.as_str()).map(|s| s.to_string()); - 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: IndexMap = - 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: IndexMap = - serde_json::from_value(d.to_owned()).unwrap(); - Some(deps) - } else { - None - } - }); - - let scripts: Option> = package_json - .get("scripts") - .and_then(|d| serde_json::from_value(d.to_owned()).ok()); - - // Ignore unknown types for forwards compatibility - let typ = if let Some(t) = type_val { - if let Some(t) = t.as_str() { - if t != "module" && t != "commonjs" { - "none".to_string() - } else { - t.to_string() - } - } else { - "none".to_string() - } - } else { - "none".to_string() - }; - - // for typescript, it looks for "typings" first, then "types" - let types = package_json - .get("typings") - .or_else(|| package_json.get("types")) - .and_then(|t| t.as_str().map(|s| s.to_string())); - - let package_json = PackageJson { - exists: true, - path, - main, - name, - version, - module, - typ, - types, - exports, - imports, - bin, - dependencies, - dev_dependencies, - scripts, - }; - - Ok(package_json) - } - - pub fn main(&self, referrer_kind: NodeModuleKind) -> Option<&str> { - let main = if referrer_kind == NodeModuleKind::Esm && self.typ == "module" { - self.module.as_ref().or(self.main.as_ref()) - } else { - self.main.as_ref() - }; - main.map(|m| m.trim()).filter(|m| !m.is_empty()) - } - - pub fn specifier(&self) -> ModuleSpecifier { - ModuleSpecifier::from_file_path(&self.path).unwrap() - } -} - -fn is_conditional_exports_main_sugar(exports: &Value) -> bool { - if exports.is_string() || exports.is_array() { - return true; - } - - if exports.is_null() || !exports.is_object() { - return false; - } - - let exports_obj = exports.as_object().unwrap(); - let mut is_conditional_sugar = false; - let mut i = 0; - for key in exports_obj.keys() { - let cur_is_conditional_sugar = key.is_empty() || !key.starts_with('.'); - if i == 0 { - is_conditional_sugar = cur_is_conditional_sugar; - i += 1; - } else if is_conditional_sugar != cur_is_conditional_sugar { - panic!("\"exports\" cannot contains some keys starting with \'.\' and some not. - The exports object must either be an object of package subpath keys - or an object of main entry condition name keys only.") - } - } - - is_conditional_sugar -} - -#[cfg(test)] -mod test { - use super::*; - - #[test] - fn null_exports_should_not_crash() { - let package_json = PackageJson::load_from_string( - PathBuf::from("/package.json"), - r#"{ "exports": null }"#.to_string(), - ) - .unwrap(); - - assert!(package_json.exports.is_none()); + Err(err) => Err(err), } } diff --git a/ext/node/polyfills/01_require.js b/ext/node/polyfills/01_require.js index bdd91e0b44..da598fe290 100644 --- a/ext/node/polyfills/01_require.js +++ b/ext/node/polyfills/01_require.js @@ -1055,7 +1055,7 @@ Module._extensions[".js"] = function (module, filename) { if (StringPrototypeEndsWith(filename, ".js")) { const pkg = op_require_read_closest_package_json(filename); - if (pkg && pkg.exists && pkg.typ === "module") { + if (pkg && pkg.typ === "module") { throw createRequireEsmError( filename, moduleParentCache.get(module)?.filename, diff --git a/ext/node/resolution.rs b/ext/node/resolution.rs index 8047ac4ecb..1b9a200127 100644 --- a/ext/node/resolution.rs +++ b/ext/node/resolution.rs @@ -4,10 +4,9 @@ use std::borrow::Cow; use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; -use std::rc::Rc; +use deno_config::package_json::PackageJsonRc; use deno_core::anyhow::bail; -use deno_core::anyhow::Context; use deno_core::error::generic_error; use deno_core::error::AnyError; use deno_core::serde_json::Map; @@ -21,8 +20,6 @@ use crate::errors; use crate::is_builtin_node_module; use crate::path::to_file_specifier; use crate::polyfill::get_module_name_from_builtin_node_module_specifier; -use crate::AllowAllNodePermissions; -use crate::NodePermissions; use crate::NpmResolverRc; use crate::PackageJson; use crate::PathClean; @@ -30,11 +27,7 @@ use crate::PathClean; pub static DEFAULT_CONDITIONS: &[&str] = &["deno", "node", "import"]; pub static REQUIRE_CONDITIONS: &[&str] = &["require", "node"]; -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -pub enum NodeModuleKind { - Esm, - Cjs, -} +pub type NodeModuleKind = deno_config::package_json::NodeModuleKind; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum NodeResolutionMode { @@ -236,8 +229,7 @@ impl NodeResolver { Some(resolved_specifier) } } else if specifier.starts_with('#') { - let pkg_config = self - .get_closest_package_json(referrer, &mut AllowAllNodePermissions)?; + let pkg_config = self.get_closest_package_json(referrer)?; Some(self.package_imports_resolve( specifier, referrer, @@ -325,31 +317,18 @@ impl NodeResolver { referrer: &ModuleSpecifier, mode: NodeResolutionMode, ) -> Result, AnyError> { - let package_json_path = package_dir.join("package.json"); - let package_json = self.load_package_json( - &mut AllowAllNodePermissions, - package_json_path.clone(), - )?; let node_module_kind = NodeModuleKind::Esm; let package_subpath = package_subpath .map(|s| format!("./{s}")) .unwrap_or_else(|| ".".to_string()); - let maybe_resolved_url = self - .resolve_package_subpath( - &package_json, - &package_subpath, - referrer, - node_module_kind, - DEFAULT_CONDITIONS, - mode, - ) - .with_context(|| { - format!( - "Failed resolving package subpath '{}' for '{}'", - package_subpath, - package_json.path.display() - ) - })?; + let maybe_resolved_url = self.resolve_package_dir_subpath( + package_dir, + &package_subpath, + referrer, + node_module_kind, + DEFAULT_CONDITIONS, + mode, + )?; let resolved_url = match maybe_resolved_url { Some(resolved_path) => resolved_path, None => return Ok(None), @@ -383,10 +362,9 @@ impl NodeResolver { package_folder: &Path, ) -> Result, AnyError> { let package_json_path = package_folder.join("package.json"); - let package_json = self.load_package_json( - &mut AllowAllNodePermissions, - package_json_path.clone(), - )?; + let Some(package_json) = self.load_package_json(&package_json_path)? else { + return Ok(Vec::new()); + }; Ok(match &package_json.bin { Some(Value::String(_)) => { @@ -408,10 +386,12 @@ impl NodeResolver { sub_path: Option<&str>, ) -> Result { let package_json_path = package_folder.join("package.json"); - let package_json = self.load_package_json( - &mut AllowAllNodePermissions, - package_json_path.clone(), - )?; + let Some(package_json) = self.load_package_json(&package_json_path)? else { + bail!( + "Failed resolving binary export. '{}' did not exist", + package_json_path.display(), + ) + }; let bin_entry = resolve_bin_entry_value(&package_json, sub_path)?; let url = to_file_specifier(&package_folder.join(bin_entry)); @@ -429,8 +409,7 @@ impl NodeResolver { if url_str.starts_with("http") || url_str.ends_with(".json") { Ok(NodeResolution::Esm(url)) } else if url_str.ends_with(".js") || url_str.ends_with(".d.ts") { - let maybe_package_config = - self.get_closest_package_json(&url, &mut AllowAllNodePermissions)?; + let maybe_package_config = self.get_closest_package_json(&url)?; match maybe_package_config { Some(c) if c.typ == "module" => Ok(NodeResolution::Esm(url)), Some(_) => Ok(NodeResolution::CommonJs(url)), @@ -515,24 +494,19 @@ impl NodeResolver { return Ok(Some(to_file_specifier(&path))); } if self.fs.is_dir_sync(&path) { - let package_json_path = path.join("package.json"); - if let Ok(pkg_json) = - self.load_package_json(&mut AllowAllNodePermissions, package_json_path) - { - let maybe_resolution = self.resolve_package_subpath( - &pkg_json, - /* sub path */ ".", - referrer, - referrer_kind, - match referrer_kind { - NodeModuleKind::Esm => DEFAULT_CONDITIONS, - NodeModuleKind::Cjs => REQUIRE_CONDITIONS, - }, - NodeResolutionMode::Types, - )?; - if let Some(resolution) = maybe_resolution { - return Ok(Some(resolution)); - } + let maybe_resolution = self.resolve_package_dir_subpath( + &path, + /* sub path */ ".", + referrer, + referrer_kind, + match referrer_kind { + NodeModuleKind::Esm => DEFAULT_CONDITIONS, + NodeModuleKind::Cjs => REQUIRE_CONDITIONS, + }, + NodeResolutionMode::Types, + )?; + if let Some(resolution) = maybe_resolution { + return Ok(Some(resolution)); } let index_path = path.join("index.js"); if let Some(path) = probe_extensions( @@ -572,19 +546,59 @@ impl NodeResolver { let mut package_json_path = None; if let Some(pkg_json) = &referrer_pkg_json { - if pkg_json.exists { - package_json_path = Some(pkg_json.path.clone()); - if let Some(imports) = &pkg_json.imports { - if imports.contains_key(name) && !name.contains('*') { - let target = imports.get(name).unwrap(); + package_json_path = Some(pkg_json.path.clone()); + if let Some(imports) = &pkg_json.imports { + if imports.contains_key(name) && !name.contains('*') { + let target = imports.get(name).unwrap(); + let maybe_resolved = self.resolve_package_target( + package_json_path.as_ref().unwrap(), + target, + "", + name, + referrer, + referrer_kind, + false, + true, + conditions, + mode, + )?; + if let Some(resolved) = maybe_resolved { + return Ok(resolved); + } + } else { + let mut best_match = ""; + let mut best_match_subpath = None; + for key in imports.keys() { + let pattern_index = key.find('*'); + if let Some(pattern_index) = pattern_index { + let key_sub = &key[0..=pattern_index]; + if name.starts_with(key_sub) { + let pattern_trailer = &key[pattern_index + 1..]; + if name.len() > key.len() + && name.ends_with(&pattern_trailer) + && pattern_key_compare(best_match, key) == 1 + && key.rfind('*') == Some(pattern_index) + { + best_match = key; + best_match_subpath = Some( + name[pattern_index..=(name.len() - pattern_trailer.len())] + .to_string(), + ); + } + } + } + } + + if !best_match.is_empty() { + let target = imports.get(best_match).unwrap(); let maybe_resolved = self.resolve_package_target( package_json_path.as_ref().unwrap(), target, - "", - name, + &best_match_subpath.unwrap(), + best_match, referrer, referrer_kind, - false, + true, true, conditions, mode, @@ -592,49 +606,6 @@ impl NodeResolver { if let Some(resolved) = maybe_resolved { return Ok(resolved); } - } else { - let mut best_match = ""; - let mut best_match_subpath = None; - for key in imports.keys() { - let pattern_index = key.find('*'); - if let Some(pattern_index) = pattern_index { - let key_sub = &key[0..=pattern_index]; - if name.starts_with(key_sub) { - let pattern_trailer = &key[pattern_index + 1..]; - if name.len() > key.len() - && name.ends_with(&pattern_trailer) - && pattern_key_compare(best_match, key) == 1 - && key.rfind('*') == Some(pattern_index) - { - best_match = key; - best_match_subpath = Some( - name - [pattern_index..=(name.len() - pattern_trailer.len())] - .to_string(), - ); - } - } - } - } - - if !best_match.is_empty() { - let target = imports.get(best_match).unwrap(); - let maybe_resolved = self.resolve_package_target( - package_json_path.as_ref().unwrap(), - target, - &best_match_subpath.unwrap(), - best_match, - referrer, - referrer_kind, - true, - true, - conditions, - mode, - )?; - if let Some(resolved) = maybe_resolved { - return Ok(resolved); - } - } } } } @@ -1013,15 +984,11 @@ impl NodeResolver { let (package_name, package_subpath, _is_scoped) = parse_npm_pkg_name(specifier, referrer)?; - let Some(package_config) = - self.get_closest_package_json(referrer, &mut AllowAllNodePermissions)? - else { + let Some(package_config) = self.get_closest_package_json(referrer)? else { return Ok(None); }; // ResolveSelf - if package_config.exists - && package_config.name.as_ref() == Some(&package_name) - { + if package_config.name.as_ref() == Some(&package_name) { if let Some(exports) = &package_config.exports { return self .package_exports_resolve( @@ -1037,30 +1004,14 @@ impl NodeResolver { } } - let result = self.resolve_package_subpath_for_package( + self.resolve_package_subpath_for_package( &package_name, &package_subpath, referrer, referrer_kind, conditions, mode, - ); - if mode.is_types() && !matches!(result, Ok(Some(_))) { - // try to resolve with the @types/node package - let package_name = types_package_name(&package_name); - if let Ok(Some(result)) = self.resolve_package_subpath_for_package( - &package_name, - &package_subpath, - referrer, - referrer_kind, - conditions, - mode, - ) { - return Ok(Some(result)); - } - } - - result + ) } #[allow(clippy::too_many_arguments)] @@ -1072,11 +1023,45 @@ impl NodeResolver { referrer_kind: NodeModuleKind, conditions: &[&str], mode: NodeResolutionMode, + ) -> Result, AnyError> { + let result = self.resolve_package_subpath_for_package_inner( + package_name, + package_subpath, + referrer, + referrer_kind, + conditions, + mode, + ); + if mode.is_types() && !matches!(result, Ok(Some(_))) { + // try to resolve with the @types package + let package_name = types_package_name(package_name); + if let Ok(Some(result)) = self.resolve_package_subpath_for_package_inner( + &package_name, + package_subpath, + referrer, + referrer_kind, + conditions, + mode, + ) { + return Ok(Some(result)); + } + } + result + } + + #[allow(clippy::too_many_arguments)] + fn resolve_package_subpath_for_package_inner( + &self, + package_name: &str, + package_subpath: &str, + referrer: &ModuleSpecifier, + referrer_kind: NodeModuleKind, + conditions: &[&str], + mode: NodeResolutionMode, ) -> Result, AnyError> { let package_dir_path = self .npm_resolver .resolve_package_folder_from_package(package_name, referrer)?; - let package_json_path = package_dir_path.join("package.json"); // todo: error with this instead when can't find package // Err(errors::err_module_not_found( @@ -1092,10 +1077,8 @@ impl NodeResolver { // )) // Package match. - let package_json = self - .load_package_json(&mut AllowAllNodePermissions, package_json_path)?; - self.resolve_package_subpath( - &package_json, + self.resolve_package_dir_subpath( + &package_dir_path, package_subpath, referrer, referrer_kind, @@ -1104,6 +1087,36 @@ impl NodeResolver { ) } + #[allow(clippy::too_many_arguments)] + fn resolve_package_dir_subpath( + &self, + package_dir_path: &Path, + package_subpath: &str, + referrer: &ModuleSpecifier, + referrer_kind: NodeModuleKind, + conditions: &[&str], + mode: NodeResolutionMode, + ) -> Result, AnyError> { + let package_json_path = package_dir_path.join("package.json"); + match self.load_package_json(&package_json_path)? { + Some(pkg_json) => self.resolve_package_subpath( + &pkg_json, + package_subpath, + referrer, + referrer_kind, + conditions, + mode, + ), + None => self.resolve_package_subpath_no_pkg_json( + package_dir_path, + package_subpath, + referrer, + referrer_kind, + mode, + ), + } + } + #[allow(clippy::too_many_arguments)] fn resolve_package_subpath( &self, @@ -1139,6 +1152,7 @@ impl NodeResolver { } } } + if package_subpath == "." { return self.legacy_main_resolve( package_json, @@ -1148,7 +1162,25 @@ impl NodeResolver { ); } - let file_path = package_json.path.parent().unwrap().join(package_subpath); + self.resolve_subpath_exact( + package_json.path.parent().unwrap(), + package_subpath, + referrer, + referrer_kind, + mode, + ) + } + + fn resolve_subpath_exact( + &self, + directory: &Path, + package_subpath: &str, + referrer: &ModuleSpecifier, + referrer_kind: NodeModuleKind, + mode: NodeResolutionMode, + ) -> Result, AnyError> { + assert_ne!(package_subpath, "."); + let file_path = directory.join(package_subpath); if mode.is_types() { self.path_to_declaration_url(file_path, referrer, referrer_kind) } else { @@ -1156,49 +1188,54 @@ impl NodeResolver { } } + fn resolve_package_subpath_no_pkg_json( + &self, + directory: &Path, + package_subpath: &str, + referrer: &ModuleSpecifier, + referrer_kind: NodeModuleKind, + mode: NodeResolutionMode, + ) -> Result, AnyError> { + if package_subpath == "." { + self.legacy_index_resolve(directory, referrer_kind, mode) + } else { + self.resolve_subpath_exact( + directory, + package_subpath, + referrer, + referrer_kind, + mode, + ) + } + } + pub fn get_closest_package_json( &self, url: &ModuleSpecifier, - permissions: &mut dyn NodePermissions, - ) -> Result>, AnyError> { + ) -> Result, AnyError> { let Ok(file_path) = url.to_file_path() else { return Ok(None); }; - self.get_closest_package_json_from_path(&file_path, permissions) + self.get_closest_package_json_from_path(&file_path) } pub fn get_closest_package_json_from_path( &self, file_path: &Path, - permissions: &mut dyn NodePermissions, - ) -> Result>, AnyError> { - let Some(package_json_path) = - self.get_closest_package_json_path(file_path)? - else { - return Ok(None); - }; - self - .load_package_json(permissions, package_json_path) - .map(Some) - } - - fn get_closest_package_json_path( - &self, - file_path: &Path, - ) -> Result, AnyError> { + ) -> Result, AnyError> { let current_dir = deno_core::strip_unc_prefix( self.fs.realpath_sync(file_path.parent().unwrap())?, ); let mut current_dir = current_dir.as_path(); let package_json_path = current_dir.join("package.json"); - if self.fs.exists_sync(&package_json_path) { - return Ok(Some(package_json_path)); + if let Some(pkg_json) = self.load_package_json(&package_json_path)? { + return Ok(Some(pkg_json)); } while let Some(parent) = current_dir.parent() { current_dir = parent; let package_json_path = current_dir.join("package.json"); - if self.fs.exists_sync(&package_json_path) { - return Ok(Some(package_json_path)); + if let Some(pkg_json) = self.load_package_json(&package_json_path)? { + return Ok(Some(pkg_json)); } } @@ -1207,15 +1244,12 @@ impl NodeResolver { pub(super) fn load_package_json( &self, - permissions: &mut dyn NodePermissions, - package_json_path: PathBuf, - ) -> Result, AnyError> { - PackageJson::load( - &*self.fs, - &*self.npm_resolver, - permissions, - package_json_path, - ) + package_json_path: &Path, + ) -> Result< + Option, + deno_config::package_json::PackageJsonLoadError, + > { + crate::package_json::load_pkg_json(&*self.fs, package_json_path) } pub(super) fn legacy_main_resolve( @@ -1284,6 +1318,19 @@ impl NodeResolver { } } + self.legacy_index_resolve( + package_json.path.parent().unwrap(), + referrer_kind, + mode, + ) + } + + fn legacy_index_resolve( + &self, + directory: &Path, + referrer_kind: NodeModuleKind, + mode: NodeResolutionMode, + ) -> Result, AnyError> { let index_file_names = if mode.is_types() { // todo(dsherret): investigate exactly how typescript does this match referrer_kind { @@ -1294,12 +1341,7 @@ impl NodeResolver { vec!["index.js"] }; for index_file_name in index_file_names { - let guess = package_json - .path - .parent() - .unwrap() - .join(index_file_name) - .clean(); + let guess = directory.join(index_file_name).clean(); if self.fs.is_file_sync(&guess) { // TODO(bartlomieju): emitLegacyIndexDeprecation() return Ok(Some(to_file_specifier(&guess))); @@ -1651,7 +1693,7 @@ mod tests { use super::*; fn build_package_json(json: Value) -> PackageJson { - PackageJson::load_from_value(PathBuf::from("/package.json"), json).unwrap() + PackageJson::load_from_value(PathBuf::from("/package.json"), json) } #[test] diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index 0659bb4392..c8089a503a 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -13006,7 +13006,7 @@ fn lsp_deno_future_env_byonm() { "severity": 1, "code": "resolver-error", "source": "deno", - "message": format!("Could not find a matching package for 'npm:chalk' in '{}'. You must specify this as a package.json dependency when the node_modules folder is not managed by Deno.", temp_dir.path().join("package.json")), + "message": "Could not find a matching package for 'npm:chalk' in a package.json file. You must specify this as a package.json dependency when the node_modules folder is not managed by Deno.", }, ]) ); diff --git a/tests/integration/npm_tests.rs b/tests/integration/npm_tests.rs index 1d60d9e353..871f09d156 100644 --- a/tests/integration/npm_tests.rs +++ b/tests/integration/npm_tests.rs @@ -2255,7 +2255,7 @@ console.log(getKind()); .args("run --allow-read chalk.ts") .run(); output.assert_matches_text( - r#"error: Could not find a matching package for 'npm:chalk@5' in '[WILDCARD]package.json'. You must specify this as a package.json dependency when the node_modules folder is not managed by Deno. + r#"error: Could not find a matching package for 'npm:chalk@5' in a package.json file. You must specify this as a package.json dependency when the node_modules folder is not managed by Deno. at file:///[WILDCARD]chalk.ts:1:19 "#); output.assert_exit_code(1); @@ -2340,7 +2340,7 @@ console.log(getKind()); .args("run --allow-read chalk.ts") .run(); output.assert_matches_text( - r#"error: Could not find a matching package for 'npm:chalk@5' in '[WILDCARD]package.json'. You must specify this as a package.json dependency when the node_modules folder is not managed by Deno. + r#"error: Could not find a matching package for 'npm:chalk@5' in a package.json file. You must specify this as a package.json dependency when the node_modules folder is not managed by Deno. at file:///[WILDCARD]chalk.ts:1:19 "#); output.assert_exit_code(1); @@ -2545,7 +2545,7 @@ fn byonm_package_npm_specifier_not_installed_and_invalid_subpath() { // no npm install has been run, so this should give an informative error let output = test_context.new_command().args("run main.ts").run(); output.assert_matches_text( - r#"error: Could not find '[WILDCARD]package.json'. Deno expects the node_modules/ directory to be up to date. Did you forget to run `npm install`? + r#"error: Could not find "chalk" in a node_modules folder. Deno expects the node_modules/ directory to be up to date. Did you forget to run `npm install`? at file:///[WILDCARD]/main.ts:1:19 "#, ); @@ -2561,8 +2561,8 @@ fn byonm_package_npm_specifier_not_installed_and_invalid_subpath() { let output = test_context.new_command().args("run main.ts").run(); output.assert_matches_text( - r#"error: Failed resolving package subpath './test' for '[WILDCARD]package.json' - at file:///[WILDCARD]/main.ts:1:8 + r#"error: [ERR_PACKAGE_PATH_NOT_EXPORTED] Package subpath './test' is not defined by "exports" in '[WILDLINE]package.json' imported from '[WILDLINE]main.ts' + at file:///[WILDLINE]/main.ts:1:8 "#, ); output.assert_exit_code(1); @@ -2589,7 +2589,7 @@ fn future_byonm_package_npm_specifier_not_installed_and_invalid_subpath() { // no npm install has been run, so this should give an informative error let output = test_context.new_command().args("run main.ts").run(); output.assert_matches_text( - r#"error: Could not find '[WILDCARD]package.json'. Deno expects the node_modules/ directory to be up to date. Did you forget to run `npm install`? + r#"error: Could not find "chalk" in a node_modules folder. Deno expects the node_modules/ directory to be up to date. Did you forget to run `npm install`? at file:///[WILDCARD]/main.ts:1:19 "#, ); @@ -2605,8 +2605,8 @@ fn future_byonm_package_npm_specifier_not_installed_and_invalid_subpath() { let output = test_context.new_command().args("run main.ts").run(); output.assert_matches_text( - r#"error: Failed resolving package subpath './test' for '[WILDCARD]package.json' - at file:///[WILDCARD]/main.ts:1:8 + r#"error: [ERR_PACKAGE_PATH_NOT_EXPORTED] Package subpath './test' is not defined by "exports" in '[WILDLINE]package.json' imported from '[WILDLINE]main.ts' + at file:///[WILDLINE]/main.ts:1:8 "#, ); output.assert_exit_code(1);