Skip to content

Commit db8427b

Browse files
fix(pnpm): read linkWorkspacePackages from pnpm-workspace.yaml (#10391)
### Description Fixes #10387 by adding partial support for the new configuration location added in pnpm/pnpm#9121. There will be future work to add additional configuration options that lived in `.npmrc` and `package.json` to the YAML definition. I highly suggest reviewing the first 2 commits on their own as they are prefactors. They both get a slightly smaller to slimming down the `impl PackageManager` so we can move to an interface instead of a single struct with `match`s for every package manager x version. ### Testing Instructions Added new unit tests for this behavior, also manually verified behavior change in repro provided in #10387. Note that after this PR the topological `^lint` dependency is correctly applied so the application lints to not start until `@repo/lint` completes. Before ``` [0 olszewski@macbookpro] /tmp/turbo-respect-pnpm-configs $ turbo lint turbo 2.5.2 • Packages in scope: @repo/eslint-config, @repo/typescript-config, @repo/ui, docs, web • Running lint in 5 packages • Remote caching disabled ┌ web#lint > cache miss, executing b9dab19525365b14 │ │ │ > [email protected] lint /private/tmp/turbo-respect-pnpm-configs/apps/web │ > next lint --max-warnings 0 └────> ┌ docs#lint > cache miss, executing 72e63e1338f32778 │ │ │ > [email protected] lint /private/tmp/turbo-respect-pnpm-configs/apps/docs │ > next lint --max-warnings 0 └────> ┌ @repo/ui#lint > cache miss, executing 1e7e4e418db8cf29 │ │ │ > @repo/[email protected] lint /private/tmp/turbo-respect-pnpm-configs/packages/ui │ > echo 'failing lint' && exit 1 │ │ failing lint │  ELIFECYCLE  Command failed with exit code 1. │ command finished with error: command (/private/tmp/turbo-respect-pnpm-configs/packages/ui) /User │ s/olszewski/.nvm/versions/node/v22.13.0/bin/pnpm run lint exited (1) └────> @repo/ui#lint: command (/private/tmp/turbo-respect-pnpm-configs/packages/ui) /Users/olszewski/.nvm/versions/node/v22.13.0/bin/pnpm run lint exited (1) Tasks: 0 successful, 3 total Cached: 0 cached, 3 total Time: 1.096s Failed: @repo/ui#lint ERROR run failed: command exited (1) ``` After ``` [1 olszewski@macbookpro] /tmp/turbo-respect-pnpm-configs $ turbo_dev --skip-infer lint turbo 2.5.2 • Packages in scope: @repo/eslint-config, @repo/typescript-config, @repo/ui, docs, web • Running lint in 5 packages • Remote caching disabled ┌ @repo/ui#lint > cache miss, executing 5e80cac629794173 │ │ │ > @repo/[email protected] lint /private/tmp/turbo-respect-pnpm-configs/packages/ui │ > echo 'failing lint' && exit 1 │ │ failing lint │  ELIFECYCLE  Command failed with exit code 1. │ command finished with error: command (/private/tmp/turbo-respect-pnpm-configs/packages/ui) /User │ s/olszewski/.nvm/versions/node/v22.13.0/bin/pnpm run lint exited (1) └────> @repo/ui#lint: command (/private/tmp/turbo-respect-pnpm-configs/packages/ui) /Users/olszewski/.nvm/versions/node/v22.13.0/bin/pnpm run lint exited (1) Tasks: 0 successful, 1 total Cached: 0 cached, 1 total Time: 363ms Failed: @repo/ui#lint ERROR run failed: command exited (1) ```
1 parent 10d678c commit db8427b

File tree

4 files changed

+222
-59
lines changed

4 files changed

+222
-59
lines changed

.github/workflows/lint.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ jobs:
7070
- name: Lint
7171
# Manually set TURBO_API to an empty string to override Hetzner env
7272
run: |
73-
TURBO_API= turbo run lint knip --env-mode=strict
73+
TURBO_API= turbo run lint --env-mode=strict
7474
7575
cleanup:
7676
name: Cleanup
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
use tracing::debug;
2+
use turbopath::AbsoluteSystemPath;
3+
4+
use super::yarnrc;
5+
6+
pub fn link_workspace_packages(repo_root: &AbsoluteSystemPath) -> bool {
7+
let yarnrc_config = yarnrc::YarnRc::from_file(repo_root)
8+
.inspect_err(|e| debug!("unable to read yarnrc: {e}"))
9+
.unwrap_or_default();
10+
yarnrc_config.enable_transparent_workspaces
11+
}
12+
13+
#[cfg(test)]
14+
mod test {
15+
use test_case::test_case;
16+
17+
use super::*;
18+
19+
#[test_case(None, true)]
20+
#[test_case(Some(false), false)]
21+
#[test_case(Some(true), true)]
22+
fn test_link_workspace_packages(enabled: Option<bool>, expected: bool) {
23+
let tmpdir = tempfile::tempdir().unwrap();
24+
let repo_root = AbsoluteSystemPath::from_std_path(tmpdir.path()).unwrap();
25+
if let Some(enabled) = enabled {
26+
repo_root
27+
.join_component(yarnrc::YARNRC_FILENAME)
28+
.create_with_contents(format!("enableTransparentWorkspaces: {enabled}"))
29+
.unwrap();
30+
}
31+
let actual = link_workspace_packages(repo_root);
32+
assert_eq!(actual, expected);
33+
}
34+
}

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

Lines changed: 17 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
pub mod berry;
12
pub mod bun;
23
pub mod npm;
34
pub mod npmrc;
@@ -17,15 +18,12 @@ use lazy_regex::{lazy_regex, Lazy};
1718
use miette::{Diagnostic, NamedSource, SourceSpan};
1819
use node_semver::SemverError;
1920
use npm::NpmDetector;
20-
use npmrc::NpmRc;
2121
use regex::Regex;
2222
use serde::Deserialize;
2323
use thiserror::Error;
24-
use tracing::debug;
2524
use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf, RelativeUnixPath};
2625
use turborepo_errors::Spanned;
2726
use turborepo_lockfiles::Lockfile;
28-
use yarnrc::YarnRc;
2927

3028
use crate::{
3129
discovery,
@@ -34,11 +32,6 @@ use crate::{
3432
workspaces::WorkspaceGlobs,
3533
};
3634

37-
#[derive(Debug, Deserialize)]
38-
struct PnpmWorkspace {
39-
pub packages: Vec<String>,
40-
}
41-
4235
#[derive(Debug, Deserialize)]
4336
struct PackageJsonWorkspaces {
4437
workspaces: Workspaces,
@@ -264,7 +257,7 @@ impl PackageManager {
264257
pub fn get_default_exclusions(&self) -> impl Iterator<Item = String> {
265258
let ignores = match self {
266259
PackageManager::Pnpm | PackageManager::Pnpm6 | PackageManager::Pnpm9 => {
267-
["**/node_modules/**", "**/bower_components/**"].as_slice()
260+
pnpm::get_default_exclusions()
268261
}
269262
PackageManager::Npm => ["**/node_modules/**"].as_slice(),
270263
PackageManager::Bun => ["**/node_modules", "**/.git"].as_slice(),
@@ -282,15 +275,8 @@ impl PackageManager {
282275
PackageManager::Pnpm | PackageManager::Pnpm6 | PackageManager::Pnpm9 => {
283276
// Make sure to convert this to a missing workspace error
284277
// so we can catch it in the case of single package mode.
285-
let source = self.workspace_glob_source(root_path);
286-
let workspace_yaml = fs::read_to_string(source)
287-
.map_err(|_| Error::Workspace(MissingWorkspaceError::from(self.clone())))?;
288-
let pnpm_workspace: PnpmWorkspace = serde_yaml::from_str(&workspace_yaml)?;
289-
if pnpm_workspace.packages.is_empty() {
290-
return Err(MissingWorkspaceError::from(self.clone()).into());
291-
} else {
292-
pnpm_workspace.packages
293-
}
278+
pnpm::get_configured_workspace_globs(root_path)
279+
.ok_or_else(|| Error::Workspace(MissingWorkspaceError::from(self.clone())))?
294280
}
295281
PackageManager::Berry
296282
| PackageManager::Npm
@@ -452,7 +438,7 @@ impl PackageManager {
452438
pub fn workspace_configuration_path(&self) -> Option<&'static str> {
453439
match self {
454440
PackageManager::Pnpm | PackageManager::Pnpm6 | PackageManager::Pnpm9 => {
455-
Some("pnpm-workspace.yaml")
441+
Some(pnpm::WORKSPACE_CONFIGURATION_PATH)
456442
}
457443
PackageManager::Npm
458444
| PackageManager::Berry
@@ -550,21 +536,11 @@ impl PackageManager {
550536
/// be used where `false` would use a `lib` package from the registry.
551537
pub fn link_workspace_packages(&self, repo_root: &AbsoluteSystemPath) -> bool {
552538
match self {
553-
PackageManager::Berry => {
554-
let yarnrc = YarnRc::from_file(repo_root)
555-
.inspect_err(|e| debug!("unable to read yarnrc: {e}"))
556-
.unwrap_or_default();
557-
yarnrc.enable_transparent_workspaces
558-
}
539+
PackageManager::Berry => berry::link_workspace_packages(repo_root),
559540
PackageManager::Pnpm9 | PackageManager::Pnpm | PackageManager::Pnpm6 => {
560-
let npmrc = NpmRc::from_file(repo_root)
561-
.inspect_err(|e| debug!("unable to read npmrc: {e}"))
562-
.unwrap_or_default();
563-
npmrc
564-
.link_workspace_packages
565-
// The default for pnpm 9 is false if not explicitly set
566-
// All previous versions had a default of true
567-
.unwrap_or(!matches!(self, PackageManager::Pnpm9))
541+
let pnpm_version = pnpm::PnpmVersion::try_from(self)
542+
.expect("attempted to extract pnpm version from non-pnpm package manager");
543+
pnpm::link_workspace_packages(pnpm_version, repo_root)
568544
}
569545
PackageManager::Yarn | PackageManager::Bun | PackageManager::Npm => true,
570546
}
@@ -849,33 +825,16 @@ mod tests {
849825
Ok(())
850826
}
851827

852-
#[test_case(PackageManager::Npm, None, true)]
853-
#[test_case(PackageManager::Yarn, None, true)]
854-
#[test_case(PackageManager::Bun, None, true)]
855-
#[test_case(PackageManager::Pnpm6, None, true)]
856-
#[test_case(PackageManager::Pnpm, None, true)]
857-
#[test_case(PackageManager::Pnpm, Some(false), false)]
858-
#[test_case(PackageManager::Pnpm, Some(true), true)]
859-
#[test_case(PackageManager::Pnpm9, None, false)]
860-
#[test_case(PackageManager::Pnpm9, Some(true), true)]
861-
#[test_case(PackageManager::Pnpm9, Some(false), false)]
862-
#[test_case(PackageManager::Berry, None, true)]
863-
#[test_case(PackageManager::Berry, Some(false), false)]
864-
#[test_case(PackageManager::Berry, Some(true), true)]
865-
fn test_link_workspace_packages(pm: PackageManager, enabled: Option<bool>, expected: bool) {
828+
#[test_case(PackageManager::Npm)]
829+
#[test_case(PackageManager::Yarn)]
830+
#[test_case(PackageManager::Bun)]
831+
fn test_link_workspace_packages_enabled_by_default(pm: PackageManager) {
866832
let tmpdir = tempfile::tempdir().unwrap();
867833
let repo_root = AbsoluteSystemPath::from_std_path(tmpdir.path()).unwrap();
868-
if let Some(enabled) = enabled {
869-
repo_root
870-
.join_component(npmrc::NPMRC_FILENAME)
871-
.create_with_contents(format!("link-workspace-packages={enabled}"))
872-
.unwrap();
873-
repo_root
874-
.join_component(yarnrc::YARNRC_FILENAME)
875-
.create_with_contents(format!("enableTransparentWorkspaces: {enabled}"))
876-
.unwrap();
877-
}
878834
let actual = pm.link_workspace_packages(repo_root);
879-
assert_eq!(actual, expected);
835+
assert!(
836+
actual,
837+
"all package managers without a special implementation should use workspace packages"
838+
);
880839
}
881840
}

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

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,30 @@
11
use std::collections::HashSet;
22

33
use node_semver::{Range, Version};
4+
use serde::Deserialize;
5+
use tracing::debug;
46
use turbopath::{AbsoluteSystemPath, RelativeUnixPath};
57

8+
use super::npmrc;
69
use crate::{
710
package_json::PackageJson,
811
package_manager::{Error, PackageManager},
912
};
1013

1114
pub const LOCKFILE: &str = "pnpm-lock.yaml";
15+
pub const WORKSPACE_CONFIGURATION_PATH: &str = "pnpm-workspace.yaml";
16+
17+
/// A representation of the pnpm versions have different treatment by turbo.
18+
///
19+
/// Not all behaviors are gated by this enum, lockfile interpretations are
20+
/// decided by `lockfileVersion` in `pnpm-lock.yaml`. In the future, this would
21+
/// be better represented by the semver to allow better gating of behavior
22+
/// based on when it changed in pnpm.
23+
pub enum PnpmVersion {
24+
Pnpm6,
25+
Pnpm7And8,
26+
Pnpm9,
27+
}
1228

1329
pub struct PnpmDetector<'a> {
1430
found: bool,
@@ -69,6 +85,88 @@ pub(crate) fn prune_patches<R: AsRef<RelativeUnixPath>>(
6985
pruned_json
7086
}
7187

88+
pub fn link_workspace_packages(pnpm_version: PnpmVersion, repo_root: &AbsoluteSystemPath) -> bool {
89+
let npmrc_config = npmrc::NpmRc::from_file(repo_root)
90+
.inspect_err(|e| debug!("unable to read npmrc: {e}"))
91+
.unwrap_or_default();
92+
let workspace_config = matches!(pnpm_version, PnpmVersion::Pnpm9)
93+
.then(|| {
94+
PnpmWorkspace::from_file(repo_root)
95+
.inspect_err(|e| debug!("unable to read {WORKSPACE_CONFIGURATION_PATH}: {e}"))
96+
.ok()
97+
})
98+
.flatten()
99+
.and_then(|config| config.link_workspace_packages);
100+
workspace_config
101+
.or(npmrc_config.link_workspace_packages)
102+
// The default for pnpm 9 is false if not explicitly set
103+
// All previous versions had a default of true
104+
.unwrap_or(match pnpm_version {
105+
PnpmVersion::Pnpm6 | PnpmVersion::Pnpm7And8 => true,
106+
PnpmVersion::Pnpm9 => false,
107+
})
108+
}
109+
110+
pub fn get_configured_workspace_globs(repo_root: &AbsoluteSystemPath) -> Option<Vec<String>> {
111+
let pnpm_workspace = PnpmWorkspace::from_file(repo_root).ok()?;
112+
if pnpm_workspace.packages.is_empty() {
113+
None
114+
} else {
115+
Some(pnpm_workspace.packages)
116+
}
117+
}
118+
119+
pub fn get_default_exclusions() -> &'static [&'static str] {
120+
["**/node_modules/**", "**/bower_components/**"].as_slice()
121+
}
122+
123+
#[derive(Debug, Deserialize)]
124+
#[serde(rename_all = "camelCase")]
125+
struct PnpmWorkspace {
126+
pub packages: Vec<String>,
127+
link_workspace_packages: Option<bool>,
128+
}
129+
130+
impl PnpmWorkspace {
131+
pub fn from_file(repo_root: &AbsoluteSystemPath) -> Result<Self, Error> {
132+
let workspace_yaml_path = repo_root.join_component(WORKSPACE_CONFIGURATION_PATH);
133+
let workspace_yaml = workspace_yaml_path.read_to_string()?;
134+
Ok(serde_yaml::from_str(&workspace_yaml)?)
135+
}
136+
}
137+
138+
#[derive(Debug)]
139+
pub struct NotPnpmError {
140+
package_manager: PackageManager,
141+
}
142+
143+
impl std::fmt::Display for NotPnpmError {
144+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
145+
f.write_fmt(format_args!(
146+
"Package managers other than pnpm cannot have pnpm version: {:?}",
147+
self.package_manager
148+
))
149+
}
150+
}
151+
152+
impl TryFrom<&'_ PackageManager> for PnpmVersion {
153+
type Error = NotPnpmError;
154+
155+
fn try_from(value: &PackageManager) -> Result<Self, Self::Error> {
156+
match value {
157+
PackageManager::Pnpm9 => Ok(Self::Pnpm9),
158+
PackageManager::Pnpm => Ok(Self::Pnpm7And8),
159+
PackageManager::Pnpm6 => Ok(Self::Pnpm6),
160+
PackageManager::Berry
161+
| PackageManager::Yarn
162+
| PackageManager::Npm
163+
| PackageManager::Bun => Err(NotPnpmError {
164+
package_manager: value.clone(),
165+
}),
166+
}
167+
}
168+
}
169+
72170
#[cfg(test)]
73171
mod test {
74172
use std::{collections::BTreeMap, fs::File};
@@ -133,4 +231,76 @@ mod test {
133231
expected
134232
);
135233
}
234+
235+
#[test]
236+
fn test_workspace_parsing() {
237+
let config: PnpmWorkspace =
238+
serde_yaml::from_str("linkWorkspacePackages: true\npackages:\n - \"apps/*\"\n")
239+
.unwrap();
240+
assert_eq!(config.link_workspace_packages, Some(true));
241+
assert_eq!(config.packages, vec!["apps/*".to_string()]);
242+
}
243+
244+
#[test_case(PnpmVersion::Pnpm6, None, true)]
245+
#[test_case(PnpmVersion::Pnpm7And8, None, true)]
246+
#[test_case(PnpmVersion::Pnpm7And8, Some(false), false)]
247+
#[test_case(PnpmVersion::Pnpm7And8, Some(true), true)]
248+
#[test_case(PnpmVersion::Pnpm9, None, false)]
249+
#[test_case(PnpmVersion::Pnpm9, Some(true), true)]
250+
#[test_case(PnpmVersion::Pnpm9, Some(false), false)]
251+
fn test_link_workspace_packages(version: PnpmVersion, enabled: Option<bool>, expected: bool) {
252+
let tmpdir = tempfile::tempdir().unwrap();
253+
let repo_root = AbsoluteSystemPath::from_std_path(tmpdir.path()).unwrap();
254+
if let Some(enabled) = enabled {
255+
repo_root
256+
.join_component(npmrc::NPMRC_FILENAME)
257+
.create_with_contents(format!("link-workspace-packages={enabled}"))
258+
.unwrap();
259+
}
260+
let actual = link_workspace_packages(version, repo_root);
261+
assert_eq!(actual, expected);
262+
}
263+
264+
#[test_case(PnpmVersion::Pnpm6, None, true)]
265+
#[test_case(PnpmVersion::Pnpm7And8, None, true)]
266+
// Pnpm <9 doesn't use workspace config
267+
#[test_case(PnpmVersion::Pnpm7And8, Some(false), true)]
268+
#[test_case(PnpmVersion::Pnpm7And8, Some(true), true)]
269+
#[test_case(PnpmVersion::Pnpm9, None, false)]
270+
#[test_case(PnpmVersion::Pnpm9, Some(true), true)]
271+
#[test_case(PnpmVersion::Pnpm9, Some(false), false)]
272+
fn test_link_workspace_packages_via_workspace(
273+
version: PnpmVersion,
274+
enabled: Option<bool>,
275+
expected: bool,
276+
) {
277+
let tmpdir = tempfile::tempdir().unwrap();
278+
let repo_root = AbsoluteSystemPath::from_std_path(tmpdir.path()).unwrap();
279+
if let Some(enabled) = enabled {
280+
repo_root
281+
.join_component(WORKSPACE_CONFIGURATION_PATH)
282+
.create_with_contents(format!(
283+
"linkWorkspacePackages: {enabled}\npackages:\n - \"apps/*\"\n"
284+
))
285+
.unwrap();
286+
}
287+
let actual = link_workspace_packages(version, repo_root);
288+
assert_eq!(actual, expected);
289+
}
290+
291+
#[test]
292+
fn test_workspace_yaml_wins_over_npmrc() {
293+
let tmpdir = tempfile::tempdir().unwrap();
294+
let repo_root = AbsoluteSystemPath::from_std_path(tmpdir.path()).unwrap();
295+
repo_root
296+
.join_component(WORKSPACE_CONFIGURATION_PATH)
297+
.create_with_contents("linkWorkspacePackages: true\npackages:\n - \"apps/*\"\n")
298+
.unwrap();
299+
repo_root
300+
.join_component(npmrc::NPMRC_FILENAME)
301+
.create_with_contents("link-workspace-packages=false")
302+
.unwrap();
303+
let actual = link_workspace_packages(PnpmVersion::Pnpm9, repo_root);
304+
assert!(actual);
305+
}
136306
}

0 commit comments

Comments
 (0)