Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(repository): honour handleTransparentWorkspaces setting in Yarn/Berry #9626

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 2 additions & 15 deletions crates/turborepo-repository/src/package_graph/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ use turborepo_graph_utils as graph;
use turborepo_lockfiles::Lockfile;

use super::{
dep_splitter::DependencySplitter, npmrc::NpmRc, PackageGraph, PackageInfo, PackageName,
PackageNode,
dep_splitter::DependencySplitter, PackageGraph, PackageInfo, PackageName, PackageNode,
};
use crate::{
discovery::{
Expand Down Expand Up @@ -352,16 +351,6 @@ impl<'a, T: PackageDiscovery> BuildState<'a, ResolvedWorkspaces, T> {
&mut self,
package_manager: &PackageManager,
) -> Result<(), Error> {
let npmrc = match package_manager {
PackageManager::Pnpm | PackageManager::Pnpm6 | PackageManager::Pnpm9 => {
let npmrc_path = self.repo_root.join_component(".npmrc");
match npmrc_path.read_existing_to_string().ok().flatten() {
Some(contents) => NpmRc::from_reader(contents.as_bytes()).ok(),
None => None,
}
}
_ => None,
};
let split_deps = self
.workspaces
.iter()
Expand All @@ -374,7 +363,6 @@ impl<'a, T: PackageDiscovery> BuildState<'a, ResolvedWorkspaces, T> {
&entry.package_json_path,
&self.workspaces,
package_manager,
npmrc.as_ref(),
entry.package_json.all_dependencies(),
),
)
Expand Down Expand Up @@ -566,7 +554,6 @@ impl Dependencies {
workspace_json_path: &AnchoredSystemPathBuf,
workspaces: &HashMap<PackageName, PackageInfo>,
package_manager: &PackageManager,
npmrc: Option<&NpmRc>,
dependencies: I,
) -> Self {
let resolved_workspace_json_path = repo_root.resolve(workspace_json_path);
Expand All @@ -576,7 +563,7 @@ impl Dependencies {
let mut internal = HashSet::new();
let mut external = BTreeMap::new();
let splitter =
DependencySplitter::new(repo_root, workspace_dir, workspaces, package_manager, npmrc);
DependencySplitter::new(repo_root, workspace_dir, workspaces, package_manager);
for (name, version) in dependencies.into_iter() {
if let Some(workspace) = splitter.is_internal(name, version) {
internal.insert(workspace);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use turbopath::{
RelativeUnixPathBuf,
};

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

pub struct DependencySplitter<'a> {
Expand All @@ -21,15 +21,13 @@ impl<'a> DependencySplitter<'a> {
workspace_dir: &'a AbsoluteSystemPath,
workspaces: &'a HashMap<PackageName, PackageInfo>,
package_manager: &PackageManager,
npmrc: Option<&'a NpmRc>,
) -> Self {
let link_workspace_packages = package_manager.link_workspace_packages(repo_root);
Self {
repo_root,
workspace_dir,
workspaces,
link_workspace_packages: npmrc
.and_then(|npmrc| npmrc.link_workspace_packages)
.unwrap_or(!matches!(package_manager, PackageManager::Pnpm9)),
link_workspace_packages,
}
}

Expand Down
1 change: 0 additions & 1 deletion crates/turborepo-repository/src/package_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use crate::{

pub mod builder;
mod dep_splitter;
mod npmrc;

pub use builder::{Error, PackageGraphBuilder};

Expand Down
66 changes: 66 additions & 0 deletions crates/turborepo-repository/src/package_manager/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
mod bun;
mod npm;
mod npmrc;
mod pnpm;
mod yarn;
mod yarnrc;

use std::{
backtrace,
Expand All @@ -16,13 +18,16 @@ use lazy_regex::{lazy_regex, Lazy};
use miette::{Diagnostic, NamedSource, SourceSpan};
use node_semver::SemverError;
use npm::NpmDetector;
use npmrc::NpmRc;
use regex::Regex;
use serde::Deserialize;
use thiserror::Error;
use tracing::debug;
use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf, RelativeUnixPath};
use turborepo_errors::Spanned;
use turborepo_lockfiles::Lockfile;
use which::which;
use yarnrc::YarnRc;

use crate::{
discovery,
Expand Down Expand Up @@ -190,6 +195,8 @@ pub enum Error {
WorkspaceDiscovery(#[from] discovery::Error),
#[error("missing packageManager field in package.json")]
MissingPackageManager,
#[error(transparent)]
Yarnrc(#[from] yarnrc::Error),
}

impl From<std::convert::Infallible> for Error {
Expand Down Expand Up @@ -545,6 +552,34 @@ impl PackageManager {
PackageManager::Pnpm | PackageManager::Pnpm9 | PackageManager::Berry => None,
}
}

/// Returns whether or not the package manager will select a package in the
/// workspace as a dependency if the `workspace:` protocol isn't used.
/// For example if a package in the workspace has `"lib": "1.2.3"` and
/// there's a package in the workspace with the name of `lib` and
/// version `1.2.3` if this is true, then the local `lib` package will
/// be used where `false` would use a `lib` package from the registry.
pub fn link_workspace_packages(&self, repo_root: &AbsoluteSystemPath) -> bool {
match self {
PackageManager::Berry => {
let yarnrc = YarnRc::from_file(repo_root)
.inspect_err(|e| debug!("unable to read yarnrc: {e}"))
.unwrap_or_default();
yarnrc.enable_transparent_workspaces
}
PackageManager::Pnpm9 | PackageManager::Pnpm | PackageManager::Pnpm6 => {
let npmrc = NpmRc::from_file(repo_root)
.inspect_err(|e| debug!("unable to read npmrc: {e}"))
.unwrap_or_default();
npmrc
.link_workspace_packages
// The default for pnpm 9 is false if not explicitly set
// All previous versions had a default of true
.unwrap_or(!matches!(self, PackageManager::Pnpm9))
}
PackageManager::Yarn | PackageManager::Bun | PackageManager::Npm => true,
}
}
}

#[cfg(test)]
Expand All @@ -553,6 +588,7 @@ mod tests {

use pretty_assertions::assert_eq;
use tempfile::TempDir;
use test_case::test_case;

use super::*;

Expand Down Expand Up @@ -823,4 +859,34 @@ mod tests {
assert_eq!(nested.workspaces.as_ref(), vec!["packages/**"]);
Ok(())
}

#[test_case(PackageManager::Npm, None, true)]
#[test_case(PackageManager::Yarn, None, true)]
#[test_case(PackageManager::Bun, None, true)]
#[test_case(PackageManager::Pnpm6, None, true)]
#[test_case(PackageManager::Pnpm, None, true)]
#[test_case(PackageManager::Pnpm, Some(false), false)]
#[test_case(PackageManager::Pnpm, Some(true), true)]
#[test_case(PackageManager::Pnpm9, None, false)]
#[test_case(PackageManager::Pnpm9, Some(true), true)]
#[test_case(PackageManager::Pnpm9, Some(false), false)]
#[test_case(PackageManager::Berry, None, true)]
#[test_case(PackageManager::Berry, Some(false), false)]
#[test_case(PackageManager::Berry, Some(true), true)]
fn test_link_workspace_packages(pm: PackageManager, enabled: Option<bool>, expected: bool) {
let tmpdir = tempfile::tempdir().unwrap();
let repo_root = AbsoluteSystemPath::from_std_path(tmpdir.path()).unwrap();
if let Some(enabled) = enabled {
repo_root
.join_component(npmrc::NPMRC_FILENAME)
.create_with_contents(format!("link-workspace-packages={enabled}"))
.unwrap();
repo_root
.join_component(yarnrc::YARNRC_FILENAME)
.create_with_contents(format!("enableTransparentWorkspaces: {enabled}"))
.unwrap();
}
let actual = pm.link_workspace_packages(repo_root);
assert_eq!(actual, expected);
}
}
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
use std::io;

use ini::Ini;
use turbopath::AbsoluteSystemPath;

pub const NPMRC_FILENAME: &str = ".npmrc";

#[derive(Debug, thiserror::Error)]
pub enum Error {
#[error("encountered error reading .npmrc: {0}")]
Io(#[from] std::io::Error),
#[error("encountered error parsing .npmrc: {0}")]
Ini(#[from] ini::Error),
}

/// Representation of .npmrc used by both npm and pnpm to configure behavior
/// The representation is intentionally incomplete and is only intended to
/// contain settings that affect the package graph.
#[derive(Debug, PartialEq, Eq, Clone)]
#[derive(Debug, PartialEq, Eq, Clone, Default)]
pub struct NpmRc {
/// Used by pnpm to determine whether dependencies
/// declared without an explicit workspace protocol
Expand All @@ -20,6 +25,16 @@ pub struct NpmRc {
}

impl NpmRc {
pub fn from_file(repo_root: &AbsoluteSystemPath) -> Result<Self, Error> {
let npmrc_path = repo_root.join_component(NPMRC_FILENAME);
let npmrc = npmrc_path
.read_existing_to_string()?
.map(|contents| Self::from_reader(contents.as_bytes()))
.transpose()?
.unwrap_or_default();
Ok(npmrc)
}

pub fn from_reader(mut reader: impl io::Read) -> Result<Self, Error> {
let ini = Ini::read_from(&mut reader)?;
Ok(Self::from_ini(ini))
Expand Down
94 changes: 94 additions & 0 deletions crates/turborepo-repository/src/package_manager/yarnrc.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
use std::io;

use serde::Deserialize;
use serde_yaml;
use turbopath::AbsoluteSystemPath;

pub const YARNRC_FILENAME: &str = ".yarnrc.yml";

#[derive(Debug, thiserror::Error)]
pub enum Error {
#[error("encountered error opening yarnrc.yml: {0}")]
Io(#[from] std::io::Error),
#[error("encountered error parsing yarnrc.yml: {0}")]
SerdeYaml(#[from] serde_yaml::Error),
}

/// A yarnrc.yaml file representing settings affecting the package graph.
#[derive(Debug, PartialEq, Eq, Clone, Deserialize)]
#[serde(rename_all = "camelCase")]
pub struct YarnRc {
/// Used by Yarn(Berry) as `enableTransparentWorkspaces`.
/// When true, treats local workspaces that match a package name
/// and semver range as correct match resulting in turbo including
/// the package in the dependency graph
#[serde(default = "default_enable_transparent_workspaces")]
pub enable_transparent_workspaces: bool,
}

fn default_enable_transparent_workspaces() -> bool {
true
}

impl Default for YarnRc {
fn default() -> YarnRc {
YarnRc {
enable_transparent_workspaces: default_enable_transparent_workspaces(),
}
}
}

impl YarnRc {
pub fn from_reader(mut reader: impl io::Read) -> Result<Self, Error> {
let config: YarnRc = serde_yaml::from_reader(&mut reader)?;
Ok(config)
}

pub fn from_file(repo_root: &AbsoluteSystemPath) -> Result<Self, Error> {
let yarnrc_path = repo_root.join_component(YARNRC_FILENAME);
let yarnrc = yarnrc_path
.read_existing_to_string()?
.map(|contents| Self::from_reader(contents.as_bytes()))
.transpose()?
.unwrap_or_default();
Ok(yarnrc)
}
}

#[cfg(test)]
mod test {
use super::*;

#[test]
fn test_parse_empty_yarnrc() {
let empty = YarnRc::from_reader(b"".as_slice()).unwrap();
assert_eq!(
empty,
YarnRc {
enable_transparent_workspaces: true
}
);
}

#[test]
fn test_parses_transparent_workspaces() {
let empty = YarnRc::from_reader(b"enableTransparentWorkspaces: false".as_slice()).unwrap();
assert_eq!(
empty,
YarnRc {
enable_transparent_workspaces: false
}
);
}

#[test]
fn test_parses_additional_settings() {
let empty = YarnRc::from_reader(b"httpProxy: \"http://my-proxy.com\"".as_slice()).unwrap();
assert_eq!(
empty,
YarnRc {
enable_transparent_workspaces: true
}
);
}
}
Loading