fix(resolve): stop lockfile from clobbering dependencies (#247)

Fixes: #241

Cargo.toml

@@ -83,6 +83,7 @@ directories = "4.0.1"
 dunce = "1.0.3"
 flate2 = "1.0.25"
 futures = "0.3.26"
+indexmap = "1.9.3"
 indicatif = "0.17.3"
 io_tee = "0.1.1"
 http-cache-reqwest = "0.6.0"

crates/nassun/src/client.rs

@@ -220,10 +220,16 @@ impl Nassun {
         Self::new().resolve(spec.as_ref()).await?.entries().await
     }
 
-    /// Resolve a spec (e.g. `foo@^1.2.3`, `github:foo/bar`, etc), to a
+    /// Resolve a string spec (e.g. `foo@^1.2.3`, `github:foo/bar`, etc), to a
     /// [`Package`] that can be used for further operations.
     pub async fn resolve(&self, spec: impl AsRef<str>) -> Result<Package> {
         let spec = spec.as_ref().parse()?;
+        self.resolve_spec(spec).await
+    }
+
+    /// Resolve a spec (e.g. `foo@^1.2.3`, `github:foo/bar`, etc), to a
+    /// [`Package`] that can be used for further operations.
+    pub async fn resolve_spec(&self, spec: PackageSpec) -> Result<Package> {
         let fetcher = self.pick_fetcher(&spec);
         let name = fetcher.name(&spec, &self.resolver.base_dir).await?;
         self.resolver

crates/node-maintainer/Cargo.toml

@@ -19,6 +19,7 @@ oro-package-spec = { version = "=0.3.19", path = "../oro-package-spec" }
 async-std = { workspace = true }
 colored = { workspace = true }
 futures = { workspace = true }
+indexmap = { workspace = true }
 kdl = { workspace = true }
 miette = { workspace = true }
 node-semver = { workspace = true }

crates/node-maintainer/src/error.rs

@@ -51,12 +51,12 @@ pub enum NodeMaintainerError {
     /// Missing package node name.
     #[error("Missing package name:\n{0:#?}")]
     #[diagnostic(code(node_maintainer::npm::missing_name))]
-    NpmLockMissingName(NpmPackageLockEntry),
+    NpmLockMissingName(Box<NpmPackageLockEntry>),
 
     /// Failed to parse an integrity value while loading NPM lockfile.
     #[error("Failed to parse an integrity value while loading lockfile node:\n{0:#?}")]
     #[diagnostic(code(node_maintainer::npm::integrity_parse_error))]
-    NpmLockfileIntegrityParseError(NpmPackageLockEntry, #[source] ssri::Error),
+    NpmLockfileIntegrityParseError(Box<NpmPackageLockEntry>, #[source] ssri::Error),
 
     /// Unsupported NPM Package Lock version.
     #[error("Unsupported NPM Package Lock version: {0}")]

crates/node-maintainer/src/graph.rs

@@ -1,10 +1,11 @@
 use std::{
-    collections::{BTreeMap, VecDeque},
+    collections::VecDeque,
     ffi::OsStr,
     ops::{Index, IndexMut},
     path::Path,
 };
 
+use indexmap::IndexMap;
 use kdl::KdlDocument;
 use nassun::{package::Package, PackageResolution, PackageSpec};
 use oro_common::CorgiManifest;
@@ -34,31 +35,65 @@ pub struct Node {
     pub(crate) idx: NodeIndex,
     /// Resolved [`Package`] for this Node.
     pub(crate) package: Package,
-    /// Resolved [`CorgiManifest`] for this Node.
-    pub(crate) manifest: CorgiManifest,
     /// Quick index back to this Node's [`Graph`]'s root Node.
     pub(crate) root: NodeIndex,
     /// Name-indexed map of outgoing [`crate::Edge`]s from this Node.
-    pub(crate) dependencies: BTreeMap<UniCase<String>, EdgeIndex>,
+    pub(crate) dependencies: IndexMap<UniCase<String>, EdgeIndex>,
+    /// Map of dependencies to their requirements.
+    pub(crate) dependency_reqs: IndexMap<UniCase<String>, (PackageSpec, DepType)>,
     /// Parent, if any, of this Node in the logical filesystem hierarchy.
     pub(crate) parent: Option<NodeIndex>,
     /// Children of this node in the logical filesystem hierarchy. These are
     /// not necessarily dependencies, and this Node's dependencies may not all
     /// be in this HashMap.
-    pub(crate) children: BTreeMap<UniCase<String>, NodeIndex>,
+    pub(crate) children: IndexMap<UniCase<String>, NodeIndex>,
 }
 
 impl Node {
-    pub(crate) fn new(package: Package, manifest: CorgiManifest) -> Self {
-        Self {
+    pub(crate) fn new(
+        package: Package,
+        manifest: CorgiManifest,
+        is_root: bool,
+    ) -> Result<Self, NodeMaintainerError> {
+        let deps = manifest
+            .dependencies
+            .iter()
+            .map(|x| (x, DepType::Prod))
+            .chain(
+                manifest
+                    .optional_dependencies
+                    .iter()
+                    .map(|x| (x, DepType::Opt)),
+                // TODO: Place these properly.
+                // )
+                // .chain(
+                //     manifest
+                //         .peer_dependencies
+                //         .iter()
+                //         .map(|x| (x, DepType::Peer)),
+            );
+
+        let deps: Box<dyn Iterator<Item = ((&String, &String), DepType)> + Send> = if is_root {
+            Box::new(deps.chain(manifest.dev_dependencies.iter().map(|x| (x, DepType::Dev))))
+        } else {
+            Box::new(deps)
+        };
+        let mut dependency_reqs = IndexMap::new();
+        for ((name, spec), dep_type) in deps {
+            dependency_reqs.insert(
+                UniCase::new(name.clone()),
+                (format!("{name}@{spec}").parse()?, dep_type),
+            );
+        }
+        Ok(Self {
             package,
-            manifest,
             idx: NodeIndex::new(0),
             root: NodeIndex::new(0),
             parent: None,
-            children: BTreeMap::new(),
-            dependencies: BTreeMap::new(),
-        }
+            children: IndexMap::new(),
+            dependencies: IndexMap::new(),
+            dependency_reqs,
+        })
     }
 
     /// This Node's depth in the logical filesystem hierarchy.
@@ -158,7 +193,7 @@ impl Graph {
                     node,
                 ))
             })
-            .collect::<Result<BTreeMap<_, _>, NodeMaintainerError>>()?;
+            .collect::<Result<IndexMap<_, _>, NodeMaintainerError>>()?;
         Ok(Lockfile {
             version: 1,
             root,
@@ -243,8 +278,8 @@ impl Graph {
     pub(crate) fn node_path(&self, node_idx: NodeIndex) -> VecDeque<UniCase<String>> {
         let node = &self.inner[node_idx];
         let mut path = VecDeque::new();
-        path.push_front(UniCase::new(node.package.name().to_owned()));
         if node_idx != self.root {
+            path.push_front(UniCase::new(node.package.name().to_owned()));
             let mut parent = node.parent;
             while let Some(parent_idx) = parent {
                 if parent_idx == self.root {
@@ -324,10 +359,10 @@ impl Graph {
             None
         };
 
-        let mut prod_deps = BTreeMap::new();
-        let mut dev_deps = BTreeMap::new();
-        let mut peer_deps = BTreeMap::new();
-        let mut opt_deps = BTreeMap::new();
+        let mut prod_deps = IndexMap::new();
+        let mut dev_deps = IndexMap::new();
+        let mut peer_deps = IndexMap::new();
+        let mut opt_deps = IndexMap::new();
         let dependencies = node.dependencies.iter().map(|(name, edge_idx)| {
             let edge = &self.inner[*edge_idx];
             (name, &edge.requested, &edge.dep_type)

crates/node-maintainer/src/lockfile.rs

@@ -1,5 +1,4 @@
-use std::collections::BTreeMap;
-
+use indexmap::IndexMap;
 use kdl::{KdlDocument, KdlNode};
 use nassun::{client::Nassun, package::Package, PackageResolution};
 use node_semver::Version;
@@ -16,7 +15,7 @@ use crate::{error::NodeMaintainerError, graph::DepType, IntoKdl};
 pub struct Lockfile {
     pub(crate) version: u64,
     pub(crate) root: LockfileNode,
-    pub(crate) packages: BTreeMap<UniCase<String>, LockfileNode>,
+    pub(crate) packages: IndexMap<UniCase<String>, LockfileNode>,
 }
 
 impl Lockfile {
@@ -28,7 +27,7 @@ impl Lockfile {
         &self.root
     }
 
-    pub fn packages(&self) -> &BTreeMap<UniCase<String>, LockfileNode> {
+    pub fn packages(&self) -> &IndexMap<UniCase<String>, LockfileNode> {
         &self.packages
     }
 
@@ -68,7 +67,7 @@ impl Lockfile {
                         .join("/node_modules/");
                     Ok((UniCase::from(path_str), node))
                 })
-                .collect::<Result<BTreeMap<UniCase<String>, LockfileNode>, NodeMaintainerError>>(
+                .collect::<Result<IndexMap<UniCase<String>, LockfileNode>, NodeMaintainerError>>(
                 )?;
             Ok(Lockfile {
                 version: kdl
@@ -107,7 +106,7 @@ impl Lockfile {
                         .join("/node_modules/");
                     Ok((UniCase::from(path_str), node))
                 })
-                .collect::<Result<BTreeMap<UniCase<String>, LockfileNode>, NodeMaintainerError>>(
+                .collect::<Result<IndexMap<UniCase<String>, LockfileNode>, NodeMaintainerError>>(
                 )?;
             Ok(Lockfile {
                 version: npm
@@ -137,10 +136,10 @@ pub struct LockfileNode {
     pub resolved: Option<String>,
     pub version: Option<Version>,
     pub integrity: Option<Integrity>,
-    pub dependencies: BTreeMap<String, String>,
-    pub dev_dependencies: BTreeMap<String, String>,
-    pub peer_dependencies: BTreeMap<String, String>,
-    pub optional_dependencies: BTreeMap<String, String>,
+    pub dependencies: IndexMap<String, String>,
+    pub dev_dependencies: IndexMap<String, String>,
+    pub peer_dependencies: IndexMap<String, String>,
+    pub optional_dependencies: IndexMap<String, String>,
 }
 
 impl From<LockfileNode> for CorgiManifest {
@@ -234,11 +233,14 @@ impl LockfileNode {
                 )
             })
             .collect::<Vec<_>>();
-        let name = path
-            .last()
-            .cloned()
-            // TODO: add a miette span here
-            .ok_or_else(|| NodeMaintainerError::KdlLockMissingName(node.clone()))?;
+        let name = if is_root {
+            UniCase::new("".into())
+        } else {
+            path.last()
+                .cloned()
+                // TODO: add a miette span here
+                .ok_or_else(|| NodeMaintainerError::KdlLockMissingName(node.clone()))?
+        };
         let integrity = children
             .get_arg("integrity")
             .and_then(|i| i.as_string())
@@ -275,15 +277,15 @@ impl LockfileNode {
     fn from_kdl_deps(
         children: &KdlDocument,
         dep_type: &DepType,
-    ) -> Result<BTreeMap<String, String>, NodeMaintainerError> {
+    ) -> Result<IndexMap<String, String>, NodeMaintainerError> {
         use DepType::*;
         let type_name = match dep_type {
             Prod => "dependencies",
             Dev => "dev-dependencies",
             Peer => "peer-dependencies",
             Opt => "optional-dependencies",
         };
-        let mut deps = BTreeMap::new();
+        let mut deps = IndexMap::new();
         if let Some(node) = children.get(type_name) {
             if let Some(children) = node.children() {
                 for dep in children.nodes() {
@@ -350,7 +352,7 @@ impl LockfileNode {
         kdl_node
     }
 
-    fn to_kdl_deps(&self, dep_type: &DepType, deps: &BTreeMap<String, String>) -> KdlNode {
+    fn to_kdl_deps(&self, dep_type: &DepType, deps: &IndexMap<String, String>) -> KdlNode {
         use DepType::*;
         let type_name = match dep_type {
             Prod => "dependencies",
@@ -385,13 +387,15 @@ impl LockfileNode {
             .clone()
             .map(UniCase::new)
             .or_else(|| path.last().cloned())
-            .ok_or_else(|| NodeMaintainerError::NpmLockMissingName(npm.clone()))?;
+            .ok_or_else(|| NodeMaintainerError::NpmLockMissingName(Box::new(npm.clone())))?;
         let integrity = npm
             .integrity
             .as_ref()
             .map(|i| i.parse())
             .transpose()
-            .map_err(|e| NodeMaintainerError::NpmLockfileIntegrityParseError(npm.clone(), e))?;
+            .map_err(|e| {
+                NodeMaintainerError::NpmLockfileIntegrityParseError(Box::new(npm.clone()), e)
+            })?;
         let version = npm
             .version
             .as_ref()
@@ -420,7 +424,7 @@ pub struct NpmPackageLock {
     #[serde(default)]
     pub requires: bool,
     #[serde(default)]
-    pub packages: BTreeMap<String, NpmPackageLockEntry>,
+    pub packages: IndexMap<String, NpmPackageLockEntry>,
 }
 
 #[derive(Debug, Clone, Deserialize, Serialize)]
@@ -435,11 +439,11 @@ pub struct NpmPackageLockEntry {
     #[serde(default)]
     pub integrity: Option<String>,
     #[serde(default)]
-    pub dependencies: BTreeMap<String, String>,
+    pub dependencies: IndexMap<String, String>,
     #[serde(default)]
-    pub dev_dependencies: BTreeMap<String, String>,
+    pub dev_dependencies: IndexMap<String, String>,
     #[serde(default)]
-    pub optional_dependencies: BTreeMap<String, String>,
+    pub optional_dependencies: IndexMap<String, String>,
     #[serde(default)]
-    pub peer_dependencies: BTreeMap<String, String>,
+    pub peer_dependencies: IndexMap<String, String>,
 }

crates/node-maintainer/src/maintainer.rs

@@ -290,7 +290,10 @@ impl NodeMaintainerOptions {
             on_resolution_added: self.on_resolution_added,
             on_resolve_progress: self.on_resolve_progress,
         };
-        let node = resolver.graph.inner.add_node(Node::new(root_pkg, root));
+        let node = resolver
+            .graph
+            .inner
+            .add_node(Node::new(root_pkg, root, true)?);
         resolver.graph[node].root = node;
         let (graph, _actual_tree) = resolver.run_resolver(lockfile).await?;
         #[cfg(not(target_arch = "wasm32"))]
@@ -335,7 +338,10 @@ impl NodeMaintainerOptions {
             on_resolve_progress: self.on_resolve_progress,
         };
         let corgi = root_pkg.corgi_metadata().await?.manifest;
-        let node = resolver.graph.inner.add_node(Node::new(root_pkg, corgi));
+        let node = resolver
+            .graph
+            .inner
+            .add_node(Node::new(root_pkg, corgi, true)?);
         resolver.graph[node].root = node;
         let (graph, _actual_tree) = resolver.run_resolver(lockfile).await?;
         #[cfg(not(target_arch = "wasm32"))]