feat: require packageManager in package.json

crates/turborepo-filewatch/src/hash_watcher.rs

@@ -780,7 +780,9 @@ mod tests {
             .unwrap();
         repo_root
             .join_component("package.json")
-            .create_with_contents(r#"{"workspaces": ["packages/*"]}"#)
+            .create_with_contents(
+                r#"{"workspaces": ["packages/*"], "packageManager": "npm@10.0.0"}"#,
+            )
             .unwrap();
         repo_root
             .join_component("package-lock.json")

crates/turborepo-filewatch/src/package_watcher.rs

@@ -488,7 +488,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();
@@ -591,7 +593,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();
@@ -631,7 +635,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();
@@ -692,7 +698,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")
@@ -761,7 +767,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")
@@ -784,7 +790,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();
@@ -799,7 +805,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
@@ -810,7 +816,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);
@@ -833,7 +839,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();
@@ -851,8 +857,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
@@ -868,7 +874,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);

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(&package_json)?
             }
         };
 

crates/turborepo-repository/src/inference.rs

@@ -77,8 +77,7 @@ impl RepoState {
                     .ok()
                     .map(|package_json| {
                         // FIXME: We should save this package manager that we detected
-                        let package_manager =
-                            PackageManager::get_package_manager(path, Some(&package_json));
+                        let package_manager = PackageManager::get_package_manager(&package_json);
                         let workspace_globs = package_manager
                             .as_ref()
                             .ok()
@@ -152,7 +151,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 +189,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")

crates/turborepo-repository/src/package_manager/bun.rs

@@ -1,63 +1 @@
-use turbopath::AbsoluteSystemPath;
-
-use crate::package_manager::{Error, PackageManager};
-
 pub const LOCKFILE: &str = "bun.lockb";
-
-pub struct BunDetector<'a> {
-    repo_root: &'a AbsoluteSystemPath,
-    found: bool,
-}
-
-impl<'a> BunDetector<'a> {
-    pub fn new(repo_root: &'a AbsoluteSystemPath) -> Self {
-        Self {
-            repo_root,
-            found: false,
-        }
-    }
-}
-
-impl<'a> Iterator for BunDetector<'a> {
-    type Item = Result<PackageManager, Error>;
-
-    fn next(&mut self) -> Option<Self::Item> {
-        if self.found {
-            return None;
-        }
-
-        self.found = true;
-        let package_json = self.repo_root.join_component(LOCKFILE);
-
-        if package_json.exists() {
-            Some(Ok(PackageManager::Bun))
-        } else {
-            None
-        }
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use std::fs::File;
-
-    use anyhow::Result;
-    use tempfile::tempdir;
-    use turbopath::AbsoluteSystemPathBuf;
-
-    use super::LOCKFILE;
-    use crate::package_manager::PackageManager;
-
-    #[test]
-    fn test_detect_bun() -> Result<()> {
-        let repo_root = tempdir()?;
-        let repo_root_path = AbsoluteSystemPathBuf::try_from(repo_root.path())?;
-
-        let lockfile_path = repo_root.path().join(LOCKFILE);
-        File::create(lockfile_path)?;
-        let package_manager = PackageManager::detect_package_manager(&repo_root_path)?;
-        assert_eq!(package_manager, PackageManager::Bun);
-
-        Ok(())
-    }
-}

crates/turborepo-repository/src/package_manager/mod.rs

@@ -24,8 +24,8 @@
 
 use crate::{
     discovery,
-    package_json::PackageJson,
-    package_manager::{bun::BunDetector, npm::NpmDetector, pnpm::PnpmDetector, yarn::YarnDetector},
+    package_json::{self, PackageJson},
+    package_manager::{pnpm::PnpmDetector, yarn::YarnDetector},
 };
 
 #[derive(Debug, Deserialize)]
@@ -273,6 +273,8 @@
     #[error("globbing error: {0}")]
     Wax(Box<wax::BuildError>, #[backtrace] backtrace::Backtrace),
     #[error(transparent)]
+    PackageJson(#[from] package_json::Error),
+    #[error(transparent)]
     Other(#[from] anyhow::Error),
     #[error(transparent)]
     NoPackageManager(#[from] NoPackageManager),
@@ -303,6 +305,10 @@
 
     #[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<std::convert::Infallible> for Error {
@@ -414,57 +420,24 @@
     ///
     /// TODO: consider if this method should not need an Option, and possibly be
     /// a method on PackageJSON
-    pub fn get_package_manager(
-        repo_root: &AbsoluteSystemPath,
-        pkg: Option<&PackageJson>,
-    ) -> Result<Self, Error> {
-        // 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)
+    pub fn get_package_manager(package_json: &PackageJson) -> Result<Self, Error> {
+        Self::read_package_manager(package_json)
     }
 
     // Attempts to read the package manager from the package.json
-    fn read_package_manager(pkg: &PackageJson) -> Result<Option<Self>, Error> {
+    fn read_package_manager(pkg: &PackageJson) -> Result<Self, Error> {
         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)
-    }
-
-    fn detect_package_manager(repo_root: &AbsoluteSystemPath) -> Result<PackageManager, Error> {
-        let mut detected_package_managers = PnpmDetector::new(repo_root)
-            .chain(NpmDetector::new(repo_root))
-            .chain(YarnDetector::new(repo_root))
-            .chain(BunDetector::new(repo_root))
-            .collect::<Result<Vec<_>, Error>>()?;
-
-        match detected_package_managers.len() {
-            0 => Err(NoPackageManager.into()),
-            1 => Ok(detected_package_managers.pop().unwrap()),
-            _ => {
-                let managers = detected_package_managers
-                    .iter()
-                    .map(|mgr| mgr.to_string())
-                    .collect();
-                Err(Error::MultiplePackageManagers { managers })
-            }
+        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())),
         }
     }
 
@@ -609,10 +582,10 @@
 
 #[cfg(test)]
 mod tests {
     use std::{collections::HashSet, fs::File};
 
     use pretty_assertions::assert_eq;
     use tempfile::tempdir;
 
     use super::*;
 
@@ -805,52 +778,27 @@
             ..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));
-
-        Ok(())
-    }
-
-    #[test]
-    fn test_detect_multiple_package_managers() -> Result<(), Error> {
-        let repo_root = tempdir()?;
-        let repo_root_path = AbsoluteSystemPathBuf::try_from(repo_root.path())?;
-
-        let package_lock_json_path = repo_root.path().join(npm::LOCKFILE);
-        File::create(&package_lock_json_path)?;
-        let pnpm_lock_path = repo_root.path().join(pnpm::LOCKFILE);
-        File::create(pnpm_lock_path)?;
-
-        let error = PackageManager::detect_package_manager(&repo_root_path).unwrap_err();
-        assert_eq!(
-            error.to_string(),
-            "We detected multiple package managers in your repository: pnpm, npm. Please remove \
-             one of them."
-        );
-
-        fs::remove_file(&package_lock_json_path)?;
-
-        let package_manager = PackageManager::detect_package_manager(&repo_root_path)?;
-        assert_eq!(package_manager, PackageManager::Pnpm);
+        assert_eq!(package_manager, PackageManager::Bun);
 
         Ok(())
     }