mirror/pnpm
1
0
Fork 0
mirror of https://github.com/pnpm/pnpm.git synced 2026-07-19 12:12:34 -04:00
Code Issues Packages Projects Releases Wiki Activity

refactor(add): drop the vestigial Fn() -> DependencyGroupList generic from Add (#13107)

Browse Source 
refactor(add): drop the vestigial Fn() -> DependencyGroupList generic from Add

list_dependency_groups existed only because the dependency-group list
used to be consumed more than once. pnpm/pnpm#12995 collapsed that to a
single collect-and-reuse, so the Fn() -> DependencyGroupList closure
indirection no longer buys anything: take dependency_groups directly
instead.

Threads the same simplification through add_package/add_packages and
their five call sites (add, global, dlx, runtime,
self_update/install_pnpm), each of which drops a trivial
closure/std::iter::once wrapper for a plain value or array literal.

Pure internal refactor: no user-visible behavior change, so no
changeset and no TypeScript counterpart.

Closes pnpm/pnpm#13088
This commit is contained in:
Parman Mohammadalizadeh
2026-07-18 09:36:54 +02:00
committed by GitHub
parent f8b08ea63f
commit ca5e04c673
8 changed files with 55 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
pnpm/crates/cli/src/cli_args/add.rs
View File

@@ -203,14 +203,14 @@ impl AddArgs {
let pinned_version =
PinnedVersion::from_save_options(self.save_exact, self.save_prefix.as_deref());
add_packages::<Reporter, _, _>(
add_packages::<Reporter, _>(
state,
&self.package_names,
pinned_version,
save_catalog_name,
self.lockfile_only,
supported_architectures,
|| self.dependency_options.dependency_groups(),
self.dependency_options.dependency_groups(),
)
.await
}
@@ -254,46 +254,44 @@ impl AddArgs {
/// points `state` at a cache directory (via a [`Config`] whose `modules_dir`
/// is anchored there) and saves to `dependencies` so the package's bin lands
/// in `<cacheDir>/node_modules/.bin`.
pub(crate) async fn add_package<Reporter, ListDependencyGroups, DependencyGroupList>(
pub(crate) async fn add_package<Reporter, DependencyGroupList>(
state: State,
package_name: &str,
pinned_version: PinnedVersion,
save_catalog_name: Option<String>,
lockfile_only: bool,
supported_architectures: Option<pacquet_package_is_installable::SupportedArchitectures>,
list_dependency_groups: ListDependencyGroups,
dependency_groups: DependencyGroupList,
) -> miette::Result<()>
where
Reporter: self::Reporter + 'static,
ListDependencyGroups: Fn() -> DependencyGroupList,
DependencyGroupList: IntoIterator<Item = DependencyGroup>,
{
let package_names = [package_name.to_string()];
Box::pin(add_packages::<Reporter, _, _>(
Box::pin(add_packages::<Reporter, _>(
state,
&package_names,
pinned_version,
save_catalog_name,
lockfile_only,
supported_architectures,
list_dependency_groups,
dependency_groups,
))
.await
}
/// Add packages to `state`'s manifest and install them in one operation.
pub(crate) async fn add_packages<Reporter, ListDependencyGroups, DependencyGroupList>(
pub(crate) async fn add_packages<Reporter, DependencyGroupList>(
mut state: State,
package_names: &[String],
pinned_version: PinnedVersion,
save_catalog_name: Option<String>,
lockfile_only: bool,
supported_architectures: Option<pacquet_package_is_installable::SupportedArchitectures>,
list_dependency_groups: ListDependencyGroups,
dependency_groups: DependencyGroupList,
) -> miette::Result<()>
where
Reporter: self::Reporter + 'static,
ListDependencyGroups: Fn() -> DependencyGroupList,
DependencyGroupList: IntoIterator<Item = DependencyGroup>,
{
// TODO: if a package already exists in another dependency group, don't remove the existing entry.
@@ -312,7 +310,7 @@ where
manifest,
lockfile,
lockfile_path: lockfile_path.as_deref(),
list_dependency_groups,
dependency_groups,
package_names,
pinned_version,
save_catalog_name,

4
pnpm/crates/cli/src/cli_args/dlx.rs
View File

@@ -306,7 +306,7 @@ async fn install_into_cache<Reporter: self::Reporter + 'static>(
for pkg in pkgs {
let state = State::init(manifest_path.clone(), config, false)
.wrap_err("initialize the dlx install state")?;
add_package::<Reporter, _, _>(
add_package::<Reporter, _>(
state,
pkg,
// dlx records the default caret range; the spec is throwaway.
@@ -316,7 +316,7 @@ async fn install_into_cache<Reporter: self::Reporter + 'static>(
// dlx must download to run the bin, so never lockfile-only.
false,
config.supported_architectures.clone(),
|| std::iter::once(DependencyGroup::Prod),
[DependencyGroup::Prod],
)
.await?;
}

4
pnpm/crates/cli/src/cli_args/global.rs
View File

@@ -359,14 +359,14 @@ async fn run_group_install<Reporter: self::Reporter + 'static>(
.collect::<miette::Result<Vec<_>>>()?;
let state = State::init(manifest_path, config, false)
.wrap_err("initialize the global install state")?;
add_packages::<Reporter, _, _>(
add_packages::<Reporter, _>(
state,
&selectors,
pinned_version,
None,
false,
config.supported_architectures.clone(),
|| std::iter::once(DependencyGroup::Prod),
[DependencyGroup::Prod],
)
.await?;

4
pnpm/crates/cli/src/cli_args/runtime.rs
View File

@@ -77,14 +77,14 @@ impl RuntimeArgs {
/// `pnpm add <name>@runtime:<version>` in the project directory.
pub async fn run<Reporter: self::Reporter + 'static>(self, state: State) -> miette::Result<()> {
let request = self.set_request()?;
add_package::<Reporter, _, _>(
add_package::<Reporter, _>(
state,
&request.package_name,
PinnedVersion::Major,
None,
false,
None,
|| std::iter::once(request.dependency_group),
[request.dependency_group],
)
.await
}

28
pnpm/crates/cli/src/cli_args/runtime/tests.rs
View File

@@ -1,6 +1,10 @@
use pacquet_config::Config;
use pacquet_package_manifest::DependencyGroup;
use pacquet_reporter::SilentReporter;
use tempfile::tempdir;
use super::{RuntimeArgs, RuntimeError};
use crate::State;
fn args(params: &[&str]) -> RuntimeArgs {
RuntimeArgs {
@@ -93,6 +97,30 @@ fn set_request_accepts_every_supported_runtime() {
}
}
/// `run` (the local, non-`-g` path) must actually reach the shared
/// `add_package` pipeline with the request `set_request` built, not just
/// build the request and stop. `offline` makes the node resolver fail
/// fast (no network) as soon as it tries to resolve `node@runtime:22`,
/// which only happens once `run` has handed the request off.
#[tokio::test]
async fn run_hands_the_set_request_off_to_add_package() {
let dir = tempdir().expect("temp dir");
let mut config = Config::new();
config.store_dir = dir.path().join("pacquet-store").into();
config.modules_dir = dir.path().join("node_modules");
config.virtual_store_dir = config.modules_dir.join(".pacquet");
config.offline = true;
let config = config.leak();
let state = State::init(dir.path().join("package.json"), config, false).expect("init state");
let err = args(&["set", "node", "22"])
.run::<SilentReporter>(state)
.await
.expect_err("offline resolution must fail, not silently succeed");
let err = format!("{err:?}");
assert!(err.contains("Offline"), "expected the offline Node.js resolver error, got: {err}");
}
#[test]
fn global_install_builds_the_same_runtime_selector() {
// `--global` routes through `run_global`, which reuses `set_request`

4
pnpm/crates/cli/src/cli_args/self_update/install_pnpm.rs
View File

@@ -288,14 +288,14 @@ pub(crate) async fn run_install<Reporter: self::Reporter + 'static>(
let manifest_path = install_dir.join("package.json");
let state = State::init(manifest_path, config, false)
.wrap_err("initialize the self-update install state")?;
add_package::<Reporter, _, _>(
add_package::<Reporter, _>(
state,
&format!("{package_name}@{version}"),
PinnedVersion::Patch,
None,
false,
config.supported_architectures.clone(),
|| std::iter::once(DependencyGroup::Prod),
[DependencyGroup::Prod],
)
.await
}

13
pnpm/crates/package-manager/src/add.rs
View File

@@ -30,9 +30,8 @@ use pacquet_tarball::MemCache;
use pacquet_workspace_manifest_writer::{UpdateWorkspaceManifestError, update_workspace_manifest};
#[must_use]
pub struct Add<'a, ListDependencyGroups, DependencyGroupList>
pub struct Add<'a, DependencyGroupList>
where
ListDependencyGroups: Fn() -> DependencyGroupList,
DependencyGroupList: IntoIterator<Item = DependencyGroup>,
{
pub tarball_mem_cache: std::sync::Arc<MemCache>,
@@ -43,7 +42,7 @@ where
pub manifest: &'a mut PackageManifest,
pub lockfile: Option<&'a Lockfile>,
pub lockfile_path: Option<&'a std::path::Path>,
pub list_dependency_groups: ListDependencyGroups,
pub dependency_groups: DependencyGroupList,
/// Package selectors, each of which may carry an `@<version>` suffix.
pub package_names: &'a [String],
/// How the freshly-resolved version is pinned into the manifest range,
@@ -127,9 +126,8 @@ pub enum AddError {
MinimumReleaseAgeExclude(#[error(source)] pacquet_config::version_policy::VersionPolicyError),
}
impl<ListDependencyGroups, DependencyGroupList> Add<'_, ListDependencyGroups, DependencyGroupList>
impl<DependencyGroupList> Add<'_, DependencyGroupList>
where
ListDependencyGroups: Fn() -> DependencyGroupList,
DependencyGroupList: IntoIterator<Item = DependencyGroup>,
{
pub async fn run<Reporter: self::Reporter + 'static>(self) -> Result<(), AddError> {
@@ -141,7 +139,7 @@ where
manifest,
lockfile,
lockfile_path,
list_dependency_groups,
dependency_groups,
package_names,
pinned_version,
save_catalog_name,
@@ -167,8 +165,7 @@ where
.map_err(AddError::InvalidCatalogsConfiguration)?;
let prefix =
workspace_dir_opt.as_deref().unwrap_or(&manifest_dir).to_string_lossy().into_owned();
let dependency_groups: Vec<DependencyGroup> =
list_dependency_groups().into_iter().collect();
let dependency_groups: Vec<DependencyGroup> = dependency_groups.into_iter().collect();
let latest_picker = tokio::sync::OnceCell::new();
let meta_cache = std::sync::Arc::new(InMemoryPackageMetaCache::default());
let fetch_locker = shared_packument_fetch_locker();

10
pnpm/crates/package-manager/src/add/tests.rs
View File

@@ -78,7 +78,7 @@ async fn add_routes_scoped_packages_to_configured_scoped_registry() {
manifest: &mut manifest,
lockfile: None,
lockfile_path: None,
list_dependency_groups: || [DependencyGroup::Prod],
dependency_groups: [DependencyGroup::Prod],
package_names: &package_names,
pinned_version: PinnedVersion::Patch,
save_catalog_name: None,
@@ -201,7 +201,7 @@ async fn add_resolves_package_selectors_concurrently_and_reports_in_selector_ord
manifest: &mut manifest,
lockfile: None,
lockfile_path: None,
list_dependency_groups: || [DependencyGroup::Prod],
dependency_groups: [DependencyGroup::Prod],
package_names: &package_names,
pinned_version: PinnedVersion::Patch,
save_catalog_name: None,
@@ -320,7 +320,7 @@ async fn add_reuses_shared_packument_state_for_every_selector_path() {
manifest: &mut manifest,
lockfile: None,
lockfile_path: None,
list_dependency_groups: || [DependencyGroup::Prod],
dependency_groups: [DependencyGroup::Prod],
package_names: &package_names,
pinned_version: PinnedVersion::Patch,
save_catalog_name: None,
@@ -395,7 +395,7 @@ async fn add_reports_resolution_errors_in_selector_order() {
manifest: &mut manifest,
lockfile: None,
lockfile_path: None,
list_dependency_groups: || [DependencyGroup::Prod],
dependency_groups: [DependencyGroup::Prod],
package_names: &package_names,
pinned_version: PinnedVersion::Patch,
save_catalog_name: None,
@@ -470,7 +470,7 @@ async fn add_does_not_wait_for_a_slower_later_resolution_after_an_error() {
manifest: &mut manifest,
lockfile: None,
lockfile_path: None,
list_dependency_groups: || [DependencyGroup::Prod],
dependency_groups: [DependencyGroup::Prod],
package_names: &package_names,
pinned_version: PinnedVersion::Patch,
save_catalog_name: None,