Skip to content

Commit ed9cdb1

Browse files
authored
fix(cli): resolve package manager defaults centrally (#2748)
After #2742, `vp migrate` fails in npm projects with only a package-lock because the internal `bundled` marker reaches package-manager version resolution, which treats it as a semver range. This broke the decoders Ecosystem CI job. Package-manager detection now returns `default` for every manager inferred from lockfiles or configuration, leaving version policy to the shared resolver. It resolves npm's default from the prepared PATH and pnpm/yarn/bun defaults from the registry's latest release. Explicit versions and explicit `latest` requests keep their existing behavior. 🤖 Generated with Codex
1 parent d6dcc4b commit ed9cdb1

6 files changed

Lines changed: 109 additions & 49 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
const assert = require('node:assert/strict');
2+
const { dirname, resolve } = require('node:path');
3+
4+
const bundled = require(resolve(
5+
dirname(process.execPath),
6+
process.platform === 'win32' ? 'node_modules/npm/package.json' : '../lib/node_modules/npm/package.json',
7+
)).version;
8+
const pkg = require('./package.json');
9+
assert.equal(pkg.devEngines.packageManager.name, 'npm');
10+
assert.equal(pkg.devEngines.packageManager.version, bundled);
11+
console.log('Migration pins the bundled npm version');

crates/vp_cli_snapshots/tests/cli_snapshots/fixtures/migration_bundled_npm/package-lock.json

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"name": "migration-bundled-npm",
3+
"private": true,
4+
"devDependencies": {
5+
"vite": "^7.0.0"
6+
}
7+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
[[case]]
2+
name = "migration_bundled_npm"
3+
vp = "global"
4+
steps = [
5+
{ argv = ["vp", "migrate", "--no-interactive", "--no-hooks", "--no-agent"], comment = "An npm lockfile without a package-manager pin migrates using Node's bundled npm version" },
6+
{ argv = ["node", "assert-npm-version.cjs"], comment = "Migration persists a concrete npm version matching the selected Node runtime" },
7+
]
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# migration_bundled_npm
2+
3+
## `vp migrate --no-interactive --no-hooks --no-agent`
4+
5+
An npm lockfile without a package-manager pin migrates using Node's bundled npm version
6+
7+
```
8+
VITE+ - The Unified Toolchain for the Web
9+
10+
◇ Migrated . to Vite+ <version>
11+
• Node <version> npm <version>
12+
• 1 config update applied
13+
```
14+
15+
## `node assert-npm-version.cjs`
16+
17+
Migration persists a concrete npm version matching the selected Node runtime
18+
19+
```
20+
Migration pins the bundled npm version
21+
```

crates/vp_pm_cli/src/package_manager.rs

Lines changed: 58 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -238,28 +238,7 @@ impl PackageManagerBuilder {
238238
PackageManagerSource::LockfileOrConfig | PackageManagerSource::Default
239239
)
240240
{
241-
// Version gates must describe the npm on PATH, not the latest registry release.
242-
let npm = vp_command::resolve_bin("npm", None, &self.cwd)?;
243-
let output = tokio::process::Command::new(npm.as_path())
244-
.arg("--version")
245-
.current_dir(&self.cwd)
246-
// User preloads can print to stdout; only the actual command should run them.
247-
.env_remove("NODE_OPTIONS")
248-
.output()
249-
.await?;
250-
if !output.status.success() {
251-
return Err(io::Error::other("failed to read npm version").into());
252-
}
253-
let version = Version::parse(String::from_utf8_lossy(&output.stdout).trim())?;
254-
let bin_prefix = npm
255-
.parent()
256-
.ok_or_else(|| Error::CannotFindBinaryPath("npm".into()))?
257-
.to_absolute_path_buf();
258-
return Ok(PackageManager {
259-
client: package_manager_type,
260-
version: version.to_string().into(),
261-
bin_prefix,
262-
});
241+
return resolve_npm_from_path(&self.cwd).await;
263242
}
264243

265244
// only download the package manager if it's not already downloaded
@@ -290,6 +269,31 @@ impl PackageManagerBuilder {
290269
}
291270
}
292271

272+
// Version gates and migration must use the npm on PATH, not the latest registry release.
273+
async fn resolve_npm_from_path(cwd: &AbsolutePath) -> Result<PackageManager, Error> {
274+
let npm = vp_command::resolve_bin("npm", None, cwd)?;
275+
let output = tokio::process::Command::new(npm.as_path())
276+
.arg("--version")
277+
.current_dir(cwd)
278+
// User preloads can print to stdout; only the actual command should run them.
279+
.env_remove("NODE_OPTIONS")
280+
.output()
281+
.await?;
282+
if !output.status.success() {
283+
return Err(io::Error::other("failed to read npm version").into());
284+
}
285+
let version = Version::parse(String::from_utf8_lossy(&output.stdout).trim())?;
286+
let bin_prefix = npm
287+
.parent()
288+
.ok_or_else(|| Error::CannotFindBinaryPath("npm".into()))?
289+
.to_absolute_path_buf();
290+
Ok(PackageManager {
291+
client: PackageManagerType::Npm,
292+
version: version.to_string().into(),
293+
bin_prefix,
294+
})
295+
}
296+
293297
impl PackageManager {
294298
pub fn builder(cwd: impl AsRef<AbsolutePath>) -> PackageManagerBuilder {
295299
PackageManagerBuilder::new(cwd)
@@ -346,7 +350,7 @@ impl PackageManager {
346350
/// from the workspace root.
347351
///
348352
/// The returned version is exact when detected from the `packageManager` field,
349-
/// `"bundled"` for unpinned npm, `"latest"` for other lockfile/config/default selections, and may be a
353+
/// `"default"` when inferred from lockfiles/config files/default, and may be a
350354
/// semver range (or `"*"` for an absent version) when detected from
351355
/// `devEngines.packageManager` (see rfcs/dev-engines.md).
352356
pub fn get_package_manager_type_and_version(
@@ -378,33 +382,33 @@ pub fn get_package_manager_type_and_version(
378382
));
379383
}
380384

381-
let version = Str::from("latest");
385+
let version = Str::from("default");
382386
let source = PackageManagerSource::LockfileOrConfig;
383-
// if pnpm-workspace.yaml exists, use pnpm@latest
387+
// if pnpm-workspace.yaml exists, select pnpm
384388
if matches!(workspace_root.workspace_file, WorkspaceFile::PnpmWorkspaceYaml(_)) {
385389
return Ok((PackageManagerType::Pnpm, version, None, source));
386390
}
387391

388-
// if pnpm-lock.yaml exists, use pnpm@latest
392+
// if pnpm-lock.yaml exists, select pnpm
389393
let pnpm_lock_yaml_path = workspace_root.path.join("pnpm-lock.yaml");
390394
if is_exists_file(&pnpm_lock_yaml_path)? {
391395
return Ok((PackageManagerType::Pnpm, version, None, source));
392396
}
393397

394-
// if yarn.lock or .yarnrc.yml exists, use yarn@latest
398+
// if yarn.lock or .yarnrc.yml exists, select yarn
395399
let yarn_lock_path = workspace_root.path.join("yarn.lock");
396400
let yarnrc_yml_path = workspace_root.path.join(".yarnrc.yml");
397401
if is_exists_file(&yarn_lock_path)? || is_exists_file(&yarnrc_yml_path)? {
398402
return Ok((PackageManagerType::Yarn, version, None, source));
399403
}
400404

401-
// A package-lock.json selects npm without requiring a separate installation.
405+
// if package-lock.json exists, select npm
402406
let package_lock_json_path = workspace_root.path.join("package-lock.json");
403407
if is_exists_file(&package_lock_json_path)? {
404-
return Ok((PackageManagerType::Npm, "bundled".into(), None, source));
408+
return Ok((PackageManagerType::Npm, version, None, source));
405409
}
406410

407-
// if bun.lock (text format) or bun.lockb (binary format) exists, use bun@latest
411+
// if bun.lock (text format) or bun.lockb (binary format) exists, select bun
408412
let bun_lock_path = workspace_root.path.join("bun.lock");
409413
if is_exists_file(&bun_lock_path)? {
410414
return Ok((PackageManagerType::Bun, version, None, source));
@@ -414,33 +418,32 @@ pub fn get_package_manager_type_and_version(
414418
return Ok((PackageManagerType::Bun, version, None, source));
415419
}
416420

417-
// if .pnpmfile.cjs exists, use pnpm@latest
421+
// if .pnpmfile.cjs exists, select pnpm
418422
let pnpmfile_cjs_path = workspace_root.path.join(".pnpmfile.cjs");
419423
if is_exists_file(&pnpmfile_cjs_path)? {
420424
return Ok((PackageManagerType::Pnpm, version, None, source));
421425
}
422-
// if legacy pnpmfile.cjs exists, use pnpm@latest
426+
// if legacy pnpmfile.cjs exists, select pnpm
423427
// https://newreleases.io/project/npm/pnpm/release/6.0.0
424428
let legacy_pnpmfile_cjs_path = workspace_root.path.join("pnpmfile.cjs");
425429
if is_exists_file(&legacy_pnpmfile_cjs_path)? {
426430
return Ok((PackageManagerType::Pnpm, version, None, source));
427431
}
428432

429-
// if bunfig.toml exists, use bun@latest
433+
// if bunfig.toml exists, select bun
430434
let bunfig_toml_path = workspace_root.path.join("bunfig.toml");
431435
if is_exists_file(&bunfig_toml_path)? {
432436
return Ok((PackageManagerType::Bun, version, None, source));
433437
}
434438

435-
// if yarn.config.cjs exists, use yarn@latest (yarn 2.0+)
439+
// if yarn.config.cjs exists, select yarn (yarn 2.0+)
436440
let yarn_config_cjs_path = workspace_root.path.join("yarn.config.cjs");
437441
if is_exists_file(&yarn_config_cjs_path)? {
438442
return Ok((PackageManagerType::Yarn, version, None, source));
439443
}
440444

441445
// if default is specified, use it
442446
if let Some(default) = default {
443-
let version = if default == PackageManagerType::Npm { "bundled".into() } else { version };
444447
return Ok((default, version, None, PackageManagerSource::Default));
445448
}
446449

@@ -975,17 +978,23 @@ async fn get_latest_version(package_manager_type: PackageManagerType) -> Result<
975978
}
976979
}
977980

978-
/// Resolve an exact, range, or `latest` package-manager version without downloading it.
981+
/// Resolve an exact, range, `latest`, or manager-specific `default` version without downloading it.
979982
pub async fn resolve_package_manager_version(
980983
package_manager_type: PackageManagerType,
981984
version: &str,
982985
) -> Result<Str, Error> {
983-
if version == "latest" {
984-
get_latest_version(package_manager_type).await
985-
} else if Version::parse(version).is_ok() {
986-
Ok(version.into())
987-
} else {
988-
resolve_package_manager_range(package_manager_type, version).await
986+
match version {
987+
"default" => match package_manager_type {
988+
PackageManagerType::Npm => {
989+
Ok(resolve_npm_from_path(&vt_path::current_dir()?).await?.version)
990+
}
991+
PackageManagerType::Pnpm | PackageManagerType::Yarn | PackageManagerType::Bun => {
992+
get_latest_version(package_manager_type).await
993+
}
994+
},
995+
"latest" => get_latest_version(package_manager_type).await,
996+
_ if Version::parse(version).is_ok() => Ok(version.into()),
997+
_ => resolve_package_manager_range(package_manager_type, version).await,
989998
}
990999
}
9911000

@@ -3174,7 +3183,7 @@ mod tests {
31743183

31753184
// onFail: ignore continues down the detection chain to the lockfile
31763185
assert_eq!(pm_type, PackageManagerType::Pnpm);
3177-
assert_eq!(version, "latest");
3186+
assert_eq!(version, "default");
31783187
assert_eq!(source, PackageManagerSource::LockfileOrConfig);
31793188
}
31803189

@@ -3196,7 +3205,7 @@ mod tests {
31963205

31973206
// an empty array imposes nothing: detection falls through to the lockfile
31983207
assert_eq!(pm_type, PackageManagerType::Pnpm);
3199-
assert_eq!(version, "latest");
3208+
assert_eq!(version, "default");
32003209
assert_eq!(source, PackageManagerSource::LockfileOrConfig);
32013210
}
32023211

@@ -3245,7 +3254,7 @@ mod tests {
32453254

32463255
// onFail: warn on the last entry warns and continues down the chain
32473256
assert_eq!(pm_type, PackageManagerType::Pnpm);
3248-
assert_eq!(version, "latest");
3257+
assert_eq!(version, "default");
32493258
assert_eq!(source, PackageManagerSource::LockfileOrConfig);
32503259
}
32513260

@@ -4285,7 +4294,7 @@ mod tests {
42854294
PackageManagerType::Npm,
42864295
"package-lock.json should take precedence over pnpmfile.cjs and yarn.config.cjs"
42874296
);
4288-
assert_eq!(version, "bundled");
4297+
assert_eq!(version, "default");
42894298
assert_eq!(hash, None);
42904299
assert_eq!(source, PackageManagerSource::LockfileOrConfig);
42914300
}
@@ -4342,7 +4351,7 @@ mod tests {
43424351
let (pm_type, version, hash, _) =
43434352
get_package_manager_type_and_version(&workspace_root, None).expect("Should detect bun");
43444353
assert_eq!(pm_type, PackageManagerType::Bun);
4345-
assert_eq!(version.as_str(), "latest");
4354+
assert_eq!(version.as_str(), "default");
43464355
assert!(hash.is_none());
43474356
}
43484357

@@ -4362,7 +4371,7 @@ mod tests {
43624371
let (pm_type, version, hash, _) =
43634372
get_package_manager_type_and_version(&workspace_root, None).expect("Should detect bun");
43644373
assert_eq!(pm_type, PackageManagerType::Bun);
4365-
assert_eq!(version.as_str(), "latest");
4374+
assert_eq!(version.as_str(), "default");
43664375
assert!(hash.is_none());
43674376
}
43684377

@@ -4382,7 +4391,7 @@ mod tests {
43824391
let (pm_type, version, hash, _) =
43834392
get_package_manager_type_and_version(&workspace_root, None).expect("Should detect bun");
43844393
assert_eq!(pm_type, PackageManagerType::Bun);
4385-
assert_eq!(version.as_str(), "latest");
4394+
assert_eq!(version.as_str(), "default");
43864395
assert!(hash.is_none());
43874396
}
43884397

0 commit comments

Comments
 (0)