From 1ff525e25b6ca833893c03f720a1298adffb37db Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 4 Oct 2023 23:05:12 -0400 Subject: refactor(node): combine node resolution code for resolving a package subpath from external code (#20791) We had two methods that did the same functionality. --- cli/args/package_json.rs | 36 ++++++++++++++++++------------------ cli/lsp/documents.rs | 5 ++--- 2 files changed, 20 insertions(+), 21 deletions(-) (limited to 'cli') diff --git a/cli/args/package_json.rs b/cli/args/package_json.rs index 8ee30214e..4dc449d57 100644 --- a/cli/args/package_json.rs +++ b/cli/args/package_json.rs @@ -1,7 +1,5 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. -use std::collections::BTreeMap; -use std::collections::HashMap; use std::path::Path; use std::path::PathBuf; @@ -13,6 +11,7 @@ use deno_runtime::deno_node::PackageJson; use deno_semver::package::PackageReq; use deno_semver::VersionReq; use deno_semver::VersionReqSpecifierParseError; +use indexmap::IndexMap; use thiserror::Error; #[derive(Debug, Error, Clone)] @@ -26,7 +25,7 @@ pub enum PackageJsonDepValueParseError { } pub type PackageJsonDeps = - BTreeMap>; + IndexMap>; #[derive(Debug, Default)] pub struct PackageJsonDepsProvider(Option); @@ -92,24 +91,25 @@ pub fn get_local_package_json_version_reqs( } fn insert_deps( - deps: Option<&HashMap>, + deps: Option<&IndexMap>, result: &mut PackageJsonDeps, ) { if let Some(deps) = deps { for (key, value) in deps { - result.insert(key.to_string(), parse_entry(key, value)); + 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 = BTreeMap::new(); + let mut result = IndexMap::new(); - // insert the dev dependencies first so the dependencies will - // take priority and overwrite any collisions - insert_deps(dev_deps, &mut result); + // favors the deps over dev_deps insert_deps(deps, &mut result); + insert_deps(dev_deps, &mut result); result } @@ -182,7 +182,7 @@ mod test { fn get_local_package_json_version_reqs_for_tests( package_json: &PackageJson, - ) -> BTreeMap> { + ) -> IndexMap> { get_local_package_json_version_reqs(package_json) .into_iter() .map(|(k, v)| { @@ -194,17 +194,17 @@ mod test { }, ) }) - .collect::>() + .collect::>() } #[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([ + 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(HashMap::from([ + 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()), @@ -212,7 +212,7 @@ mod test { let deps = get_local_package_json_version_reqs_for_tests(&package_json); assert_eq!( deps, - BTreeMap::from([ + IndexMap::from([ ( "test".to_string(), Ok(PackageReq::from_str("test@^1.2").unwrap()) @@ -232,14 +232,14 @@ mod test { #[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([( + 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, - BTreeMap::from([( + IndexMap::from([( "test".to_string(), Err( concat!( @@ -256,7 +256,7 @@ mod test { #[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(HashMap::from([ + 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()), @@ -267,7 +267,7 @@ mod test { let result = get_local_package_json_version_reqs_for_tests(&package_json); assert_eq!( result, - BTreeMap::from([ + IndexMap::from([ ( "file-test".to_string(), Err("Not implemented scheme 'file'".to_string()), diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 92c2e0a3b..3a0a2ed9f 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -47,7 +47,6 @@ use indexmap::IndexMap; use lsp::Url; use once_cell::sync::Lazy; use package_json::PackageJsonDepsProvider; -use std::collections::BTreeMap; use std::collections::HashMap; use std::collections::HashSet; use std::collections::VecDeque; @@ -1267,8 +1266,8 @@ impl Documents { if let Some(package_json_deps) = &maybe_package_json_deps { // We need to ensure the hashing is deterministic so explicitly type // this in order to catch if the type of package_json_deps ever changes - // from a sorted/deterministic BTreeMap to something else. - let package_json_deps: &BTreeMap<_, _> = *package_json_deps; + // from a sorted/deterministic IndexMap to something else. + let package_json_deps: &IndexMap<_, _> = *package_json_deps; for (key, value) in package_json_deps { hasher.write_hashable(key); match value { -- cgit v1.2.3