diff --git a/.changeset/pacquet-injected-workspace-catalogs.md b/.changeset/pacquet-injected-workspace-catalogs.md new file mode 100644 index 0000000000..f8b82823d8 --- /dev/null +++ b/.changeset/pacquet-injected-workspace-catalogs.md @@ -0,0 +1,5 @@ +--- +"pacquet": patch +--- + +Resolve `catalog:` specifiers in the dependencies of injected workspace packages (`injectWorkspacePackages: true`). Previously such a child spec bypassed catalog resolution and failed with `ERR_PNPM_SPEC_NOT_SUPPORTED_BY_ANY_RESOLVER`, matching the TypeScript CLI. diff --git a/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs b/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs index fee7c91c2b..9ff39ab679 100644 --- a/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs +++ b/pnpm/crates/resolving-deps-resolver/src/resolve_dependency_tree.rs @@ -892,6 +892,10 @@ pub struct TreeCtx { /// `time-based` publish-date cutoff in `published_by`. Built once /// per importer by [`Self::with_resolution_mode`]. subdep_opts: ResolveOptions, + /// Workspace catalogs used to resolve `catalog:` children of injected + /// workspace packages. Other transitive dependencies keep catalog + /// resolution disabled. + catalogs: Catalogs, workspace: Arc, /// Configured `patchedDependencies` (already grouped by name). /// Shared by `Arc` so the lookup table doesn't get cloned per @@ -916,6 +920,7 @@ impl TreeCtx { direct_opts: base_opts.clone(), subdep_opts: base_opts.clone(), base_opts, + catalogs: Catalogs::new(), workspace: Arc::new(WorkspaceTreeCtx::default()), patched_dependencies: None, importer_id: pacquet_lockfile::Lockfile::ROOT_IMPORTER_KEY.to_string(), @@ -932,6 +937,7 @@ impl TreeCtx { direct_opts: base_opts.clone(), subdep_opts: base_opts.clone(), base_opts, + catalogs: Catalogs::new(), workspace, patched_dependencies: None, importer_id: pacquet_lockfile::Lockfile::ROOT_IMPORTER_KEY.to_string(), @@ -964,6 +970,12 @@ impl TreeCtx { self } + #[must_use] + pub fn with_catalogs(mut self, catalogs: Catalogs) -> Self { + self.catalogs = catalogs; + self + } + /// The [`ResolveOptions`] to hand the resolver for a node at the /// given `depth`: importer-level deps (`depth == 0`) use /// [`Self::direct_opts`]; everything below uses @@ -1622,6 +1634,20 @@ where .or_insert_with(|| Arc::clone(&specs)); specs }; + let child_specs = + if result.resolved_via == "workspace" && result.id.as_str().starts_with("file:") { + Cow::Owned( + child_specs + .iter() + .map(|(name, range, optional)| { + resolve_catalog_specifier(name.clone(), range.clone(), &ctx.catalogs) + .map(|(name, range)| (name, range, *optional)) + }) + .collect::, _>>()?, + ) + } else { + Cow::Borrowed(child_specs.as_slice()) + }; // A freshly-resolved node forces its whole subtree to // re-resolve: an updated parent discards its `resolvedDependencies` // child refs. But when the parent landed back on its previously @@ -2705,21 +2731,27 @@ pub(crate) fn resolve_catalog_specifiers( specs .into_iter() .map(|(name, range, optional, injected)| { - let wanted = - CatalogWantedDependency { alias: name.clone(), bare_specifier: range.clone() }; - match resolve_from_catalog(catalogs, &wanted) { - CatalogResolutionResult::Found(found) => { - Ok((name, found.resolution.specifier, optional, injected)) - } - CatalogResolutionResult::Unused => Ok((name, range, optional, injected)), - CatalogResolutionResult::Misconfiguration(misconfig) => { - Err(ResolveDependencyTreeError::CatalogMisconfiguration(misconfig.error)) - } - } + resolve_catalog_specifier(name, range, catalogs) + .map(|(name, range)| (name, range, optional, injected)) }) .collect() } +fn resolve_catalog_specifier( + name: String, + range: String, + catalogs: &Catalogs, +) -> Result<(String, String), ResolveDependencyTreeError> { + let wanted = CatalogWantedDependency { alias: name.clone(), bare_specifier: range.clone() }; + match resolve_from_catalog(catalogs, &wanted) { + CatalogResolutionResult::Found(found) => Ok((name, found.resolution.specifier)), + CatalogResolutionResult::Unused => Ok((name, range)), + CatalogResolutionResult::Misconfiguration(misconfig) => { + Err(ResolveDependencyTreeError::CatalogMisconfiguration(misconfig.error)) + } + } +} + /// Compute the `pkgIdWithPatchHash` for a freshly-resolved package: /// /// 1. Prefix the resolver's `id` with `@` when it doesn't already diff --git a/pnpm/crates/resolving-deps-resolver/src/resolve_importer.rs b/pnpm/crates/resolving-deps-resolver/src/resolve_importer.rs index 67c9bbca46..4c2fe2762f 100644 --- a/pnpm/crates/resolving-deps-resolver/src/resolve_importer.rs +++ b/pnpm/crates/resolving-deps-resolver/src/resolve_importer.rs @@ -112,10 +112,8 @@ pub struct ResolveImporterOptions { /// `base_opts.published_by`, never this value. pub subdep_published_by: Option>, - /// Catalogs parsed from `pnpm-workspace.yaml`. Applied only to the - /// importer's direct dependencies; transitive `catalog:` entries - /// are not resolved through the catalog, keeping the catalog scope - /// importer-only. + /// Catalogs parsed from `pnpm-workspace.yaml`. Applied to importer + /// dependencies and to children of injected workspace packages. pub catalogs: Catalogs, /// When `true`, `link:` direct deps whose target lives outside @@ -344,18 +342,18 @@ impl ImporterHoistState { pnpmfile_hook: _, } = opts; - let ctx = TreeCtx::with_workspace(workspace, base_opts) - .with_importer_id(importer_id) - .with_importer_order(importer_order) - .with_patched_dependencies(patched_dependencies) - .with_resolution_mode(pick_lowest_direct, subdep_published_by); - let initial_wanted = importer_direct_wanted_specs( manifest, dependency_groups, auto_install_peers, &catalogs, )?; + let ctx = TreeCtx::with_workspace(workspace, base_opts) + .with_importer_id(importer_id) + .with_importer_order(importer_order) + .with_patched_dependencies(patched_dependencies) + .with_resolution_mode(pick_lowest_direct, subdep_published_by) + .with_catalogs(catalogs); let direct = extend_tree(&ctx, resolver, initial_wanted, importer_id).await?; let parent_pkg_aliases: HashSet = direct.iter().map(|dep| dep.alias.clone()).collect(); diff --git a/pnpm/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs b/pnpm/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs index a33c5b230f..32292e0f08 100644 --- a/pnpm/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs +++ b/pnpm/crates/resolving-deps-resolver/src/resolve_workspace/tests.rs @@ -1,7 +1,11 @@ //! `resolutionMode: time-based` cutoff tests for //! [`fn@super::resolve_workspace`]. -use std::{collections::HashMap, str::FromStr, sync::Mutex}; +use std::{ + collections::{BTreeMap, HashMap}, + str::FromStr, + sync::Mutex, +}; use chrono::{DateTime, TimeZone, Utc}; use pacquet_lockfile::{DirectoryResolution, LockfileResolution}; @@ -237,6 +241,82 @@ async fn workspace_link_results_are_cached_per_importer_project_dir() { ); } +#[tokio::test] +async fn catalogs_work_in_injected_workspace_packages() { + let (_project1_tmp, project1) = fake_manifest(serde_json::json!({ "project2": "workspace:*" })); + let (_project2_tmp, project2) = fake_manifest(serde_json::json!({ "is-positive": "catalog:" })); + let resolver = RecordingResolver { + table: HashMap::from([ + ( + ("project2".to_string(), "workspace:*".to_string()), + ResolveResult { + id: PkgResolutionId::from("file:packages/project2".to_string()), + name_ver: None, + latest: None, + published_at: None, + manifest: Some(std::sync::Arc::new(serde_json::json!({ + "name": "project2", + "version": "0.0.0", + "dependencies": { "is-positive": "catalog:" }, + }))), + resolution: LockfileResolution::Directory(DirectoryResolution { + directory: "packages/project2".to_string(), + }), + resolved_via: "workspace".to_string(), + normalized_bare_specifier: None, + alias: Some("project2".to_string()), + policy_violation: None, + }, + ), + ( + ("is-positive".to_string(), "1.0.0".to_string()), + fake_result( + "is-positive", + "1.0.0", + None, + serde_json::json!({ "name": "is-positive", "version": "1.0.0" }), + ), + ), + ]), + seen: Mutex::new(HashMap::new()), + }; + let importers = [ + WorkspaceImporter { id: "packages/project1".to_string(), manifest: &project1 }, + WorkspaceImporter { id: "packages/project2".to_string(), manifest: &project2 }, + ]; + let catalogs = BTreeMap::from([( + "default".to_string(), + BTreeMap::from([("is-positive".to_string(), "1.0.0".to_string())]), + )]); + + let result = resolve_workspace( + &resolver, + &importers, + &[DependencyGroup::Prod], + workspace_opts(false, false), + |importer| { + let mut opts = + importer_opts(std::path::PathBuf::from("/repo").join(&importer.id), None); + opts.catalogs = catalogs.clone(); + opts + }, + ) + .await + .expect("resolve catalog dependency of injected workspace package"); + + assert!(result.merged_tree.packages.contains_key("project2@file:packages/project2")); + assert!(result.merged_tree.packages.contains_key("is-positive@1.0.0")); + let children = result + .merged_tree + .children_by_id + .get("project2@file:packages/project2") + .expect("injected workspace package children"); + assert_eq!(children.len(), 1); + assert_eq!(children[0].alias, "is-positive"); + assert_eq!(children[0].pkg_id, "is-positive@1.0.0"); + assert!(!children[0].optional); +} + #[tokio::test] async fn workspace_root_direct_deps_resolve_child_importer_peers() { let (_root_tmp, root_manifest) = fake_manifest(serde_json::json!({