fix: rewrite dependency type system

Previously, a naive dependency type was used. This
had 2 fields in a dependency graph node:
resolved_ty and is_peer. These fields were
incorrect, since the dependency can be dependended
on as multiple types. Now, the dependency type is
stored in the dependencies fields. This change
makes old lockfiles incompatible since the old
format had data loss when it comes to dependency
types.

Fixes #33.

CHANGELOG.md

@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 - Download engines in install step rather than lazily by @daimond113
+- Rewrite dependency type system to solve multiple issues by @daimond113
 
 ### Performance
 - Remove unnecessary `Arc`s from codebase by @daimond113

src/cli/commands/publish.rs

@@ -172,7 +172,10 @@ impl PublishCommand {
 
 								Ok::<_, anyhow::Error>(
 									target.build_files().is_none()
-										&& !matches!(node.resolved_ty, DependencyType::Dev),
+										&& node
+											.direct
+											.as_ref()
+											.is_some_and(|(_, _, ty)| *ty != DependencyType::Dev),
 								)
 							}
 						})

src/cli/install.rs

@@ -53,7 +53,7 @@ impl DownloadAndLinkHooks for InstallHooks {
 		let aliases = graph
 			.iter()
 			.flat_map(|(_, node)| node.node.dependencies.iter())
-			.filter_map(|(id, alias)| binary_packages.contains(id).then_some(alias.as_str()))
+			.filter_map(|(id, (alias, _))| binary_packages.contains(id).then_some(alias.as_str()))
 			.chain(
 				graph
 					.iter()
@@ -202,6 +202,8 @@ pub async fn install(
 				.await
 				.context("failed to build dependency graph")?;
 
+			check_peers_satisfied(&graph);
+
 			let mut tasks = graph
 				.iter()
 				.filter_map(|(id, node)| {
@@ -537,3 +539,58 @@ pub fn print_package_diff(prefix: &str, old_graph: &DependencyGraph, new_graph:
 		println!();
 	}
 }
+
+pub fn check_peers_satisfied(graph: &DependencyGraph) {
+	for (id, node) in graph {
+		let Some((alias, _, _)) = &node.direct else {
+			continue;
+		};
+
+		let mut queue = node
+			.dependencies
+			.iter()
+			.map(|(dep_id, (dep_alias, dep_ty))| (vec![(id, alias)], (dep_id, dep_alias), *dep_ty))
+			.collect::<Vec<_>>();
+
+		while let Some((path, (dep_id, dep_alias), dep_ty)) = queue.pop() {
+			if dep_ty == DependencyType::Peer {
+				let mut iter = path
+					.iter()
+					.map(|(id, _)| id)
+					.rev()
+					// skip our parent since we're always going to be descendants of it
+					.skip(1)
+					.take(2);
+
+				let satisfied = if iter.len() > 0 {
+					iter.any(|id| graph[id].dependencies.contains_key(dep_id))
+				} else {
+					graph.get(dep_id).is_some_and(|node| node.direct.is_some())
+				};
+
+				if !satisfied {
+					eprintln!(
+						"{WARN_PREFIX}: peer dependency {}>{dep_alias} is not satisfied",
+						path.iter()
+							.map(|(_, alias)| alias.as_str())
+							.collect::<Vec<_>>()
+							.join(">"),
+					);
+				}
+			}
+
+			queue.extend(graph[dep_id].dependencies.iter().map(
+				|(inner_dep_id, (inner_dep_alias, inner_dep_ty))| {
+					(
+						path.iter()
+							.copied()
+							.chain(std::iter::once((dep_id, dep_alias)))
+							.collect(),
+						(inner_dep_id, inner_dep_alias),
+						*inner_dep_ty,
+					)
+				},
+			));
+		}
+	}
+}

src/download_and_link.rs

@@ -268,7 +268,9 @@ impl Project {
 				let mut queue = graph
 					.iter()
 					.filter(|(_, node)| {
-						node.direct.is_some() && install_dependencies_mode.fits(node.resolved_ty)
+						node.direct
+							.as_ref()
+							.is_some_and(|(_, _, ty)| install_dependencies_mode.fits(*ty))
 					})
 					.collect::<VecDeque<_>>();
 

src/graph.rs

@@ -25,12 +25,7 @@ pub struct DependencyGraphNode {
 	pub direct: Option<(Alias, DependencySpecifiers, DependencyType)>,
 	/// The dependencies of the package
 	#[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
-	pub dependencies: BTreeMap<PackageId, Alias>,
+	pub dependencies: BTreeMap<PackageId, (Alias, DependencyType)>,
-	/// The resolved (transformed, for example Peer -> Standard) type of the dependency
-	pub resolved_ty: DependencyType,
-	/// Whether the resolved type should be Peer if this isn't depended on
-	#[serde(default, skip_serializing_if = "std::ops::Not::not")]
-	pub is_peer: bool,
 	/// The package reference
 	pub pkg_ref: PackageRefs,
 }

src/linking/mod.rs

@@ -307,7 +307,7 @@ impl Project {
 						(container_folder, base_folder)
 					};
 
-					for (dep_id, dep_alias) in &node.node.dependencies {
+					for (dep_id, (dep_alias, _)) in &node.node.dependencies {
 						let dep_id = dep_id.clone();
 						let dep_alias = dep_alias.clone();
 						let dep_node = graph.get(&dep_id).cloned();

src/lockfile.rs

@@ -3,15 +3,15 @@ use crate::{
 	graph::DependencyGraph,
 	manifest::{overrides::OverrideKey, target::TargetKind},
 	names::PackageName,
-	source::{ids::PackageId, specifiers::DependencySpecifiers},
+	source::specifiers::DependencySpecifiers,
 };
 use relative_path::RelativePathBuf;
 use semver::Version;
 use serde::{Deserialize, Serialize};
-use std::{collections::BTreeMap, fmt::Debug};
+use std::collections::BTreeMap;
 
 /// The current format of the lockfile
-pub const CURRENT_FORMAT: usize = 1;
+pub const CURRENT_FORMAT: usize = 2;
 
 /// A lockfile
 #[derive(Serialize, Deserialize, Debug, Clone)]
@@ -47,120 +47,12 @@ pub fn parse_lockfile(lockfile: &str) -> Result<Lockfile, errors::ParseLockfileE
 	let format = format.format;
 
 	match format {
-		0 => {
-			let this = match toml::from_str(lockfile) {
-				Ok(lockfile) => return Ok(lockfile),
-				Err(e) => match toml::from_str::<v0::Lockfile>(lockfile) {
-					Ok(this) => this,
-					Err(_) => return Err(errors::ParseLockfileError::De(e)),
-				},
-			};
-
-			Ok(Lockfile {
-				name: this.name,
-				version: this.version,
-				target: this.target,
-				overrides: this.overrides,
-				workspace: this.workspace,
-				graph: this
-					.graph
-					.into_iter()
-					.flat_map(|(name, versions)| {
-						versions.into_iter().map(move |(version, node)| {
-							(
-								PackageId(name.clone(), version),
-								crate::graph::DependencyGraphNode {
-									direct: node.node.direct,
-									dependencies: node
-										.node
-										.dependencies
-										.into_iter()
-										.map(|(name, (version, alias))| {
-											(PackageId(name, version), alias)
-										})
-										.collect(),
-									resolved_ty: node.node.resolved_ty,
-									is_peer: node.node.is_peer,
-									pkg_ref: node.node.pkg_ref,
-								},
-							)
-						})
-					})
-					.collect(),
-			})
-		}
 		CURRENT_FORMAT => toml::de::from_str(lockfile).map_err(Into::into),
+		format if format < CURRENT_FORMAT => Err(errors::ParseLockfileError::TooOld(format)),
 		format => Err(errors::ParseLockfileError::TooNew(format)),
 	}
 }
 
-/// Lockfile v0
-pub mod v0 {
-	use crate::{
-		manifest::{
-			overrides::OverrideKey,
-			target::{Target, TargetKind},
-			Alias, DependencyType,
-		},
-		names::{PackageName, PackageNames},
-		source::{ids::VersionId, refs::PackageRefs, specifiers::DependencySpecifiers},
-	};
-	use relative_path::RelativePathBuf;
-	use semver::Version;
-	use serde::{Deserialize, Serialize};
-	use std::collections::BTreeMap;
-
-	/// A dependency graph node
-	#[derive(Serialize, Deserialize, Debug, Clone)]
-	pub struct DependencyGraphNode {
-		/// The alias, specifier, and original (as in the manifest) type for the dependency, if it is a direct dependency (i.e. used by the current project)
-		#[serde(default, skip_serializing_if = "Option::is_none")]
-		pub direct: Option<(Alias, DependencySpecifiers, DependencyType)>,
-		/// The dependencies of the package
-		#[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
-		pub dependencies: BTreeMap<PackageNames, (VersionId, Alias)>,
-		/// The resolved (transformed, for example Peer -> Standard) type of the dependency
-		pub resolved_ty: DependencyType,
-		/// Whether the resolved type should be Peer if this isn't depended on
-		#[serde(default, skip_serializing_if = "std::ops::Not::not")]
-		pub is_peer: bool,
-		/// The package reference
-		pub pkg_ref: PackageRefs,
-	}
-
-	/// A downloaded dependency graph node, i.e. a `DependencyGraphNode` with a `Target`
-	#[derive(Serialize, Deserialize, Debug, Clone)]
-	pub struct DownloadedDependencyGraphNode {
-		/// The target of the package
-		pub target: Target,
-		/// The node
-		#[serde(flatten)]
-		pub node: DependencyGraphNode,
-	}
-
-	/// A lockfile
-	#[derive(Serialize, Deserialize, Debug, Clone)]
-	pub struct Lockfile {
-		/// The name of the package
-		pub name: PackageName,
-		/// The version of the package
-		pub version: Version,
-		/// The target of the package
-		pub target: TargetKind,
-		/// The overrides of the package
-		#[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
-		pub overrides: BTreeMap<OverrideKey, DependencySpecifiers>,
-
-		/// The workspace members
-		#[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
-		pub workspace: BTreeMap<PackageName, BTreeMap<TargetKind, RelativePathBuf>>,
-
-		/// The graph of dependencies
-		#[serde(default, skip_serializing_if = "BTreeMap::is_empty")]
-		pub graph: BTreeMap<PackageNames, BTreeMap<VersionId, DownloadedDependencyGraphNode>>,
-	}
-}
-
 /// Errors that can occur when working with lockfiles
 pub mod errors {
 	use thiserror::Error;
@@ -173,6 +65,10 @@ pub mod errors {
 		#[error("lockfile format {} is too new. newest supported format: {}", .0, super::CURRENT_FORMAT)]
 		TooNew(usize),
 
+		/// The lockfile format is too old
+		#[error("lockfile format {} is too old. manual deletion is required. current format: {}", .0, super::CURRENT_FORMAT)]
+		TooOld(usize),
+
 		/// Deserializing the lockfile failed
 		#[error("deserializing the lockfile failed")]
 		De(#[from] toml::de::Error),

src/resolver.rs

@@ -114,7 +114,9 @@ impl Project {
 				let mut queue = node
 					.dependencies
 					.iter()
-					.map(|(id, dep_alias)| (id, vec![alias.to_string(), dep_alias.to_string()]))
+					.map(|(id, (dep_alias, _))| {
+						(id, vec![alias.to_string(), dep_alias.to_string()])
+					})
 					.collect::<VecDeque<_>>();
 
 				while let Some((dep_id, path)) = queue.pop_front() {
@@ -135,7 +137,7 @@ impl Project {
 						dep_node
 							.dependencies
 							.iter()
-							.map(|(id, alias)| {
+							.map(|(id, (alias, _))| {
 								(
 									id,
 									path.iter()
@@ -270,19 +272,12 @@ impl Project {
 					)));
 				};
 
-				let resolved_ty = if (is_published_package || depth == 0) && ty == DependencyType::Peer
-				{
-					DependencyType::Standard
-				} else {
-					ty
-				};
-
 				if let Some(dependant_id) = dependant {
 					graph
 						.get_mut(&dependant_id)
 						.expect("dependant package not found in graph")
 						.dependencies
-						.insert(package_id.clone(), alias.clone());
+						.insert(package_id.clone(), (alias.clone(), ty));
 				}
 
 				let pkg_ref = &resolved[package_id.version_id()];
@@ -296,14 +291,6 @@ impl Project {
                         );
 					}
 
-					if already_resolved.resolved_ty == DependencyType::Peer {
-						already_resolved.resolved_ty = resolved_ty;
-					}
-
-					if ty == DependencyType::Peer && depth == 0 {
-						already_resolved.is_peer = true;
-					}
-
 					if already_resolved.direct.is_none() && depth == 0 {
 						already_resolved.direct = Some((alias.clone(), specifier.clone(), ty));
 					}
@@ -315,12 +302,6 @@ impl Project {
 					direct: (depth == 0).then(|| (alias.clone(), specifier.clone(), ty)),
 					pkg_ref: pkg_ref.clone(),
 					dependencies: Default::default(),
-					resolved_ty,
-					is_peer: if depth == 0 {
-						false
-					} else {
-						ty == DependencyType::Peer
-					},
 				};
 				insert_node(
 					&mut graph,
@@ -388,16 +369,6 @@ impl Project {
 				.await?;
 		}
 
-		for (id, node) in &mut graph {
-			if node.is_peer && node.direct.is_none() {
-				node.resolved_ty = DependencyType::Peer;
-			}
-
-			if node.resolved_ty == DependencyType::Peer {
-				tracing::warn!("peer dependency {id} was not resolved");
-			}
-		}
-
 		Ok(graph)
 	}
 }