From 8e3d15b57db2285a01b171c2cf829d2ebabb2ab9 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Fri, 19 Apr 2024 08:14:43 -0700 Subject: [PATCH] feat: require packageManager in package.json --- .../src/package_watcher.rs | 30 ++++++----- crates/turborepo-repository/src/discovery.rs | 5 +- crates/turborepo-repository/src/inference.rs | 10 ++-- .../src/package_manager/mod.rs | 52 +++++++++---------- 4 files changed, 53 insertions(+), 44 deletions(-) diff --git a/crates/turborepo-filewatch/src/package_watcher.rs b/crates/turborepo-filewatch/src/package_watcher.rs index 80f8d0ff1e9f3..b365c4a350746 100644 --- a/crates/turborepo-filewatch/src/package_watcher.rs +++ b/crates/turborepo-filewatch/src/package_watcher.rs @@ -484,7 +484,9 @@ mod test { // write workspaces to root repo_root .join_component("package.json") - .create_with_contents(r#"{"workspaces":["packages/*"]}"#) + .create_with_contents( + r#"{"workspaces":["packages/*"], "packageManager": "npm@10.0.0"}"#, + ) .unwrap(); let watcher = FileSystemWatcher::new_with_default_cookie_dir(&repo_root).unwrap(); @@ -587,7 +589,9 @@ mod test { // write workspaces to root repo_root .join_component("package.json") - .create_with_contents(r#"{"workspaces":["packages/*", "packages2/*"]}"#) + .create_with_contents( + r#"{"workspaces":["packages/*", "packages2/*"], "packageManager": "npm@10.0.0"}"#, + ) .unwrap(); let watcher = FileSystemWatcher::new_with_default_cookie_dir(&repo_root).unwrap(); @@ -627,7 +631,9 @@ mod test { // update workspaces to no longer cover packages2 repo_root .join_component("package.json") - .create_with_contents(r#"{"workspaces":["packages/*"]}"#) + .create_with_contents( + r#"{"workspaces":["packages/*"], "packageManager": "npm@10.0.0"}"#, + ) .unwrap(); let mut data = package_watcher.discover_packages_blocking().await.unwrap(); @@ -688,7 +694,7 @@ mod test { let root_package_json_path = repo_root.join_component("package.json"); // Start with no workspace glob root_package_json_path - .create_with_contents(r#"{"packageManager": "pnpm@7.0"}"#) + .create_with_contents(r#"{"packageManager": "pnpm@7.0.0"}"#) .unwrap(); repo_root .join_component("pnpm-lock.yaml") @@ -757,7 +763,7 @@ mod test { let root_package_json_path = repo_root.join_component("package.json"); // Start with no workspace glob root_package_json_path - .create_with_contents(r#"{"packageManager": "npm@7.0"}"#) + .create_with_contents(r#"{"packageManager": "npm@7.0.0"}"#) .unwrap(); repo_root .join_component("package-lock.json") @@ -780,7 +786,7 @@ mod test { .unwrap_err(); root_package_json_path - .create_with_contents(r#"{"packageManager": "pnpm@7.0", "workspaces": ["foo/*"]}"#) + .create_with_contents(r#"{"packageManager": "npm@7.0.0", "workspaces": ["foo/*"]}"#) .unwrap(); let resp = package_watcher.discover_packages_blocking().await.unwrap(); @@ -795,7 +801,7 @@ mod test { // Create an invalid workspace glob root_package_json_path - .create_with_contents(r#"{"packageManager": "pnpm@7.0", "workspaces": ["foo/***"]}"#) + .create_with_contents(r#"{"packageManager": "npm@7.0.0", "workspaces": ["foo/***"]}"#) .unwrap(); // We expect an error due to invalid workspace glob @@ -806,7 +812,7 @@ mod test { // Set it back to valid, ensure we recover root_package_json_path - .create_with_contents(r#"{"packageManager": "pnpm@7.0", "workspaces": ["foo/*"]}"#) + .create_with_contents(r#"{"packageManager": "npm@7.0.0", "workspaces": ["foo/*"]}"#) .unwrap(); let resp = package_watcher.discover_packages_blocking().await.unwrap(); assert_eq!(resp.package_manager, PackageManager::Npm); @@ -829,7 +835,7 @@ mod test { let root_package_json_path = repo_root.join_component("package.json"); // Start with no workspace glob root_package_json_path - .create_with_contents(r#"{"packageManager": "pnpm@7.0"}"#) + .create_with_contents(r#"{"packageManager": "pnpm@7.0.0"}"#) .unwrap(); let pnpm_lock_file = repo_root.join_component("pnpm-lock.yaml"); pnpm_lock_file.create_with_contents("").unwrap(); @@ -847,8 +853,8 @@ mod test { let resp = package_watcher.discover_packages_blocking().await.unwrap(); assert_eq!(resp.package_manager, PackageManager::Pnpm); - pnpm_lock_file.remove_file().unwrap(); - // No more lock file, verify we're in an invalid state + workspaces_path.remove_file().unwrap(); + // No more workspaces file, verify we're in an invalid state package_watcher .discover_packages_blocking() .await @@ -864,7 +870,7 @@ mod test { // update package.json to complete the transition root_package_json_path - .create_with_contents(r#"{"packageManager": "npm@7.0", "workspaces": ["foo/*"]}"#) + .create_with_contents(r#"{"packageManager": "npm@7.0.0", "workspaces": ["foo/*"]}"#) .unwrap(); let resp = package_watcher.discover_packages_blocking().await.unwrap(); assert_eq!(resp.package_manager, PackageManager::Npm); diff --git a/crates/turborepo-repository/src/discovery.rs b/crates/turborepo-repository/src/discovery.rs index cafceb506960b..0033198b91d76 100644 --- a/crates/turborepo-repository/src/discovery.rs +++ b/crates/turborepo-repository/src/discovery.rs @@ -108,7 +108,10 @@ impl PackageDiscoveryBuilder for LocalPackageDiscoveryBuilder { let package_manager = match self.package_manager { Some(pm) => pm, None => { - PackageManager::get_package_manager(&self.repo_root, self.package_json.as_ref())? + let package_json = self.package_json.map(Ok).unwrap_or_else(|| { + PackageJson::load(&self.repo_root.join_component("package.json")) + })?; + PackageManager::get_package_manager(&self.repo_root, &package_json)? } }; diff --git a/crates/turborepo-repository/src/inference.rs b/crates/turborepo-repository/src/inference.rs index 681deeba162bf..d4dc9647a08d1 100644 --- a/crates/turborepo-repository/src/inference.rs +++ b/crates/turborepo-repository/src/inference.rs @@ -78,7 +78,7 @@ impl RepoState { .map(|package_json| { // FIXME: We should save this package manager that we detected let package_manager = - PackageManager::get_package_manager(path, Some(&package_json)); + PackageManager::get_package_manager(path, &package_json); let workspace_globs = package_manager .as_ref() .ok() @@ -152,7 +152,9 @@ mod test { let monorepo_pkg_json = monorepo_root.join_component("package.json"); monorepo_pkg_json.ensure_dir().unwrap(); monorepo_pkg_json - .create_with_contents("{\"workspaces\": [\"packages/*\"]}") + .create_with_contents( + "{\"workspaces\": [\"packages/*\"], \"packageManager\": \"npm@7.0.0\"}", + ) .unwrap(); monorepo_root .join_component("package-lock.json") @@ -188,7 +190,9 @@ mod test { .unwrap(); standalone_monorepo .join_component("package.json") - .create_with_contents("{\"workspaces\": [\"packages/*\"]}") + .create_with_contents( + "{\"workspaces\": [\"packages/*\"], \"packageManager\": \"npm@7.0.0\"}", + ) .unwrap(); standalone_monorepo .join_component("package-lock.json") diff --git a/crates/turborepo-repository/src/package_manager/mod.rs b/crates/turborepo-repository/src/package_manager/mod.rs index 1c4e7fb862c1e..a77bb3a482105 100644 --- a/crates/turborepo-repository/src/package_manager/mod.rs +++ b/crates/turborepo-repository/src/package_manager/mod.rs @@ -24,7 +24,7 @@ use which::which; use crate::{ discovery, - package_json::PackageJson, + package_json::{self, PackageJson}, package_manager::{bun::BunDetector, npm::NpmDetector, pnpm::PnpmDetector, yarn::YarnDetector}, }; @@ -264,6 +264,8 @@ pub enum Error { #[error("globbing error: {0}")] Wax(Box, #[backtrace] backtrace::Backtrace), #[error(transparent)] + PackageJson(#[from] package_json::Error), + #[error(transparent)] Other(#[from] anyhow::Error), #[error(transparent)] NoPackageManager(#[from] NoPackageManager), @@ -293,6 +295,10 @@ pub enum Error { #[error("discovering workspace: {0}")] WorkspaceDiscovery(#[from] discovery::Error), + #[error("missing packageManager field in package.json")] + MissingPackageManager, + #[error("{0} set in packageManager is not a supported package manager")] + UnsupportedPackageManager(String), } impl From for Error { @@ -406,36 +412,26 @@ impl PackageManager { /// a method on PackageJSON pub fn get_package_manager( repo_root: &AbsoluteSystemPath, - pkg: Option<&PackageJson>, + package_json: &PackageJson, ) -> Result { - // We don't surface errors for `read_package_manager` as we can fall back to - // `detect_package_manager` - if let Some(package_json) = pkg { - if let Ok(Some(package_manager)) = Self::read_package_manager(package_json) { - return Ok(package_manager); - } - } - - Self::detect_package_manager(repo_root) + Self::read_package_manager(package_json) } // Attempts to read the package manager from the package.json - fn read_package_manager(pkg: &PackageJson) -> Result, Error> { + fn read_package_manager(pkg: &PackageJson) -> Result { let Some(package_manager) = &pkg.package_manager else { - return Ok(None); + return Err(Error::MissingPackageManager); }; let (manager, version) = Self::parse_package_manager_string(package_manager)?; let version = version.parse()?; - let manager = match manager { - "npm" => Some(PackageManager::Npm), - "bun" => Some(PackageManager::Bun), - "yarn" => Some(YarnDetector::detect_berry_or_yarn(&version)?), - "pnpm" => Some(PnpmDetector::detect_pnpm6_or_pnpm(&version)?), - _ => None, - }; - - Ok(manager) + match manager { + "npm" => Ok(PackageManager::Npm), + "bun" => Ok(PackageManager::Bun), + "yarn" => Ok(YarnDetector::detect_berry_or_yarn(&version)?), + "pnpm" => Ok(PnpmDetector::detect_pnpm6_or_pnpm(&version)?), + _ => Err(Error::UnsupportedPackageManager(manager.to_owned())), + } } fn detect_package_manager(repo_root: &AbsoluteSystemPath) -> Result { @@ -794,27 +790,27 @@ mod tests { ..Default::default() }; let package_manager = PackageManager::read_package_manager(&package_json)?; - assert_eq!(package_manager, Some(PackageManager::Npm)); + assert_eq!(package_manager, PackageManager::Npm); package_json.package_manager = Some("yarn@2.0.0".to_string()); let package_manager = PackageManager::read_package_manager(&package_json)?; - assert_eq!(package_manager, Some(PackageManager::Berry)); + assert_eq!(package_manager, PackageManager::Berry); package_json.package_manager = Some("yarn@1.9.0".to_string()); let package_manager = PackageManager::read_package_manager(&package_json)?; - assert_eq!(package_manager, Some(PackageManager::Yarn)); + assert_eq!(package_manager, PackageManager::Yarn); package_json.package_manager = Some("pnpm@6.0.0".to_string()); let package_manager = PackageManager::read_package_manager(&package_json)?; - assert_eq!(package_manager, Some(PackageManager::Pnpm6)); + assert_eq!(package_manager, PackageManager::Pnpm6); package_json.package_manager = Some("pnpm@7.2.0".to_string()); let package_manager = PackageManager::read_package_manager(&package_json)?; - assert_eq!(package_manager, Some(PackageManager::Pnpm)); + assert_eq!(package_manager, PackageManager::Pnpm); package_json.package_manager = Some("bun@1.0.1".to_string()); let package_manager = PackageManager::read_package_manager(&package_json)?; - assert_eq!(package_manager, Some(PackageManager::Bun)); + assert_eq!(package_manager, PackageManager::Bun); Ok(()) }