Skip to content

Commit

Permalink
fix(pnpm): support new pnpm9 default link-workspace-packages (#7865)
Browse files Browse the repository at this point in the history
### Description

With [PNPM 9](https://github.com/pnpm/pnpm/releases/tag/v9.0.0-alpha.0),
[link-workspace-packages]() is now defaulting to `false` meaning that
unless the `workspace` protocol is explicitly used, packages from the
registry will be preferred over workspace packages.

It's odd to have 3 variants of the PNPM package manager, but that is the
current state of the world. Unsure of why we changed from a generic
interface to a enum in the Go->Rust port, but we'll probably want to go
back to that world to reduce match statement complexity.

### Testing Instructions

Unit tests for package manager detection. Quick spot check in a
`create-turbo` repo with workspace deps updates so they don't use
`workspace` protocol.




Closes TURBO-2733
  • Loading branch information
chris-olszewski authored Mar 29, 2024
1 parent f41618a commit 941bc0b
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 16 deletions.
2 changes: 2 additions & 0 deletions crates/turborepo-lib/src/daemon/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ pub(crate) mod proto {
PackageManager::Berry => Self::Berry,
PackageManager::Pnpm => Self::Pnpm,
PackageManager::Pnpm6 => Self::Pnpm6,
PackageManager::Pnpm9 => Self::Pnpm9,
PackageManager::Bun => Self::Bun,
}
}
Expand All @@ -123,6 +124,7 @@ pub(crate) mod proto {
turborepo_repository::package_manager::PackageManager::Berry => Self::Berry,
turborepo_repository::package_manager::PackageManager::Pnpm => Self::Pnpm,
turborepo_repository::package_manager::PackageManager::Pnpm6 => Self::Pnpm6,
turborepo_repository::package_manager::PackageManager::Pnpm9 => Self::Pnpm9,
turborepo_repository::package_manager::PackageManager::Bun => Self::Bun,
}
}
Expand Down
1 change: 1 addition & 0 deletions crates/turborepo-lib/src/daemon/proto/turbod.proto
Original file line number Diff line number Diff line change
Expand Up @@ -110,4 +110,5 @@ enum PackageManager {
Pnpm6 = 3;
Yarn = 4;
Bun = 5;
Pnpm9 = 6;
}
5 changes: 4 additions & 1 deletion crates/turborepo-repository/src/package_graph/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ impl<'a, T: PackageDiscovery> BuildState<'a, ResolvedWorkspaces, T> {
self.repo_root,
&entry.package_json_path,
&self.workspaces,
package_manager,
npmrc.as_ref(),
entry.package_json.all_dependencies(),
),
Expand Down Expand Up @@ -554,6 +555,7 @@ impl Dependencies {
repo_root: &AbsoluteSystemPath,
workspace_json_path: &AnchoredSystemPathBuf,
workspaces: &HashMap<PackageName, PackageInfo>,
package_manager: PackageManager,
npmrc: Option<&NpmRc>,
dependencies: I,
) -> Self {
Expand All @@ -563,7 +565,8 @@ impl Dependencies {
.expect("package.json path should have parent");
let mut internal = HashSet::new();
let mut external = BTreeMap::new();
let splitter = DependencySplitter::new(repo_root, workspace_dir, workspaces, npmrc);
let splitter =
DependencySplitter::new(repo_root, workspace_dir, workspaces, package_manager, npmrc);
for (name, version) in dependencies.into_iter() {
if let Some(workspace) = splitter.is_internal(name, version) {
internal.insert(workspace);
Expand Down
6 changes: 3 additions & 3 deletions crates/turborepo-repository/src/package_graph/dep_splitter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use turbopath::{
};

use super::{npmrc::NpmRc, PackageInfo, PackageName};
use crate::package_manager::PackageManager;

pub struct DependencySplitter<'a> {
repo_root: &'a AbsoluteSystemPath,
Expand All @@ -19,17 +20,16 @@ impl<'a> DependencySplitter<'a> {
repo_root: &'a AbsoluteSystemPath,
workspace_dir: &'a AbsoluteSystemPath,
workspaces: &'a HashMap<PackageName, PackageInfo>,
package_manager: PackageManager,
npmrc: Option<&'a NpmRc>,
) -> Self {
Self {
repo_root,
workspace_dir,
workspaces,
// TODO: default needs to depend on package manager as pnpm 9 changes the default to
// false
link_workspace_packages: npmrc
.and_then(|npmrc| npmrc.link_workspace_packages)
.unwrap_or(true),
.unwrap_or(!matches!(package_manager, PackageManager::Pnpm9)),
}
}

Expand Down
26 changes: 14 additions & 12 deletions crates/turborepo-repository/src/package_manager/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ impl From<Workspaces> for Vec<String> {
pub enum PackageManager {
Berry,
Npm,
Pnpm9,
Pnpm,
Pnpm6,
Yarn,
Expand All @@ -76,13 +77,12 @@ pub enum PackageManager {

impl Display for PackageManager {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
// Do not change these without also changing `GetPackageManager` in
// packagemanager.go
match self {
PackageManager::Berry => write!(f, "berry"),
PackageManager::Npm => write!(f, "npm"),
PackageManager::Pnpm => write!(f, "pnpm"),
PackageManager::Pnpm6 => write!(f, "pnpm6"),
PackageManager::Pnpm9 => write!(f, "pnpm9"),
PackageManager::Yarn => write!(f, "yarn"),
PackageManager::Bun => write!(f, "bun"),
}
Expand Down Expand Up @@ -216,7 +216,7 @@ impl Display for NoPackageManager {
impl Display for MissingWorkspaceError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let err = match self.package_manager {
PackageManager::Pnpm | PackageManager::Pnpm6 => {
PackageManager::Pnpm | PackageManager::Pnpm6 | PackageManager::Pnpm9 => {
"pnpm-workspace.yaml: no packages found. Turborepo requires pnpm workspaces and \
thus packages to be defined in the root pnpm-workspace.yaml"
}
Expand Down Expand Up @@ -308,7 +308,7 @@ impl PackageManager {
pub fn command(&self) -> &'static str {
match self {
PackageManager::Npm => "npm",
PackageManager::Pnpm | PackageManager::Pnpm6 => "pnpm",
PackageManager::Pnpm | PackageManager::Pnpm6 | PackageManager::Pnpm9 => "pnpm",
PackageManager::Yarn | PackageManager::Berry => "yarn",
PackageManager::Bun => "bun",
}
Expand All @@ -335,7 +335,7 @@ impl PackageManager {

pub fn get_default_exclusions(&self) -> impl Iterator<Item = String> {
let ignores = match self {
PackageManager::Pnpm | PackageManager::Pnpm6 => {
PackageManager::Pnpm | PackageManager::Pnpm6 | PackageManager::Pnpm9 => {
["**/node_modules/**", "**/bower_components/**"].as_slice()
}
PackageManager::Npm => ["**/node_modules/**"].as_slice(),
Expand All @@ -351,7 +351,7 @@ impl PackageManager {
root_path: &AbsoluteSystemPath,
) -> Result<(Vec<String>, Vec<String>), Error> {
let globs = match self {
PackageManager::Pnpm | PackageManager::Pnpm6 => {
PackageManager::Pnpm | PackageManager::Pnpm6 | PackageManager::Pnpm9 => {
// Make sure to convert this to a missing workspace error
// so we can catch it in the case of single package mode.
let source = self.workspace_glob_source(root_path);
Expand Down Expand Up @@ -490,14 +490,16 @@ impl PackageManager {
match self {
PackageManager::Npm => npm::LOCKFILE,
PackageManager::Bun => bun::LOCKFILE,
PackageManager::Pnpm | PackageManager::Pnpm6 => pnpm::LOCKFILE,
PackageManager::Pnpm | PackageManager::Pnpm6 | PackageManager::Pnpm9 => pnpm::LOCKFILE,
PackageManager::Yarn | PackageManager::Berry => yarn::LOCKFILE,
}
}

pub fn workspace_configuration_path(&self) -> Option<&'static str> {
match self {
PackageManager::Pnpm | PackageManager::Pnpm6 => Some("pnpm-workspace.yaml"),
PackageManager::Pnpm | PackageManager::Pnpm6 | PackageManager::Pnpm9 => {
Some("pnpm-workspace.yaml")
}
PackageManager::Npm
| PackageManager::Berry
| PackageManager::Yarn
Expand Down Expand Up @@ -533,7 +535,7 @@ impl PackageManager {
) -> Result<Box<dyn Lockfile>, Error> {
Ok(match self {
PackageManager::Npm => Box::new(turborepo_lockfiles::NpmLockfile::load(contents)?),
PackageManager::Pnpm | PackageManager::Pnpm6 => {
PackageManager::Pnpm | PackageManager::Pnpm6 | PackageManager::Pnpm9 => {
Box::new(turborepo_lockfiles::PnpmLockfile::from_bytes(contents)?)
}
PackageManager::Yarn => {
Expand Down Expand Up @@ -562,7 +564,7 @@ impl PackageManager {
) -> PackageJson {
match self {
PackageManager::Berry => yarn::prune_patches(package_json, patches),
PackageManager::Pnpm6 | PackageManager::Pnpm => {
PackageManager::Pnpm9 | PackageManager::Pnpm6 | PackageManager::Pnpm => {
pnpm::prune_patches(package_json, patches)
}
PackageManager::Yarn | PackageManager::Npm | PackageManager::Bun => {
Expand All @@ -589,7 +591,7 @@ impl PackageManager {
}
}
PackageManager::Npm | PackageManager::Pnpm6 => Some("--"),
PackageManager::Pnpm | PackageManager::Berry => None,
PackageManager::Pnpm | PackageManager::Pnpm9 | PackageManager::Berry => None,
}
}
}
Expand Down Expand Up @@ -687,7 +689,7 @@ mod tests {
PackageManager::Berry => &["**/node_modules", "**/.git", "**/.yarn"],
PackageManager::Bun => &["**/node_modules", "**/.git"],
PackageManager::Yarn => &["apps/*/node_modules/**", "packages/*/node_modules/**"],
PackageManager::Pnpm | PackageManager::Pnpm6 => &[
PackageManager::Pnpm | PackageManager::Pnpm6 | PackageManager::Pnpm9 => &[
"**/node_modules/**",
"**/bower_components/**",
"packages/skip",
Expand Down
17 changes: 17 additions & 0 deletions crates/turborepo-repository/src/package_manager/pnpm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,11 @@ impl<'a> PnpmDetector<'a> {

pub fn detect_pnpm6_or_pnpm(version: &Version) -> Result<PackageManager, Error> {
let pnpm6_constraint: Range = "<7.0.0".parse()?;
let pnpm9_constraint: Range = ">=9.0.0-alpha.0".parse()?;
if pnpm6_constraint.satisfies(version) {
Ok(PackageManager::Pnpm6)
} else if pnpm9_constraint.satisfies(version) {
Ok(PackageManager::Pnpm9)
} else {
Ok(PackageManager::Pnpm)
}
Expand Down Expand Up @@ -95,6 +98,7 @@ mod test {
use std::collections::BTreeMap;

use serde_json::json;
use test_case::test_case;
use turbopath::RelativeUnixPathBuf;

use super::*;
Expand Down Expand Up @@ -127,4 +131,17 @@ mod test {
.as_ref()
);
}

#[test_case("6.0.0", PackageManager::Pnpm6)]
#[test_case("7.0.0", PackageManager::Pnpm)]
#[test_case("8.0.0", PackageManager::Pnpm)]
#[test_case("9.0.0", PackageManager::Pnpm9)]
#[test_case("9.0.0-alpha.0", PackageManager::Pnpm9)]
fn test_version_detection(version: &str, expected: PackageManager) {
let version = Version::parse(version).unwrap();
assert_eq!(
PnpmDetector::detect_pnpm6_or_pnpm(&version).unwrap(),
expected
);
}
}

0 comments on commit 941bc0b

Please sign in to comment.