fix(cli): honor packageManager field and check package-lock.json last when detecting package manager#1587
Closed
claude[bot] wants to merge 3 commits into
Closed
fix(cli): honor packageManager field and check package-lock.json last when detecting package manager#1587claude[bot] wants to merge 3 commits into
claude[bot] wants to merge 3 commits into
Conversation
Package manager detection now treats the corepack packageManager field in package.json as the strongest signal of intent, checking it before any lock files. Among lock files, package-lock.json is now checked last: npm is already the ecosystem-wide fallback, so a stray package-lock.json (left by a one-off npm install) no longer shadows a deliberate pnpm-lock.yaml or yarn.lock in the same directory. The same precedence applies to workspace root detection.
Contributor
📦 Bundle Stats —
|
| Metric | Value | vs main (f6fd22c) |
|---|---|---|
| Internal (raw) | 2.2 KB | - |
| Internal (gzip) | 838 B | - |
| Bundled (raw) | 11.20 MB | - |
| Bundled (gzip) | 2.11 MB | - |
| Import time | 779ms | +3ms, +0.4% |
bin:sanity
| Metric | Value | vs main (f6fd22c) |
|---|---|---|
| Internal (raw) | 782 B | - |
| Internal (gzip) | 423 B | - |
| Bundled (raw) | 9.90 MB | - |
| Bundled (gzip) | 1.78 MB | - |
| Import time | 1.88s | -16ms, -0.9% |
🗺️ View treemap · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — @sanity/cli-core
Compared against main (f6fd22cc)
| Metric | Value | vs main (f6fd22c) |
|---|---|---|
| Internal (raw) | 114.1 KB | - |
| Internal (gzip) | 29.2 KB | - |
| Bundled (raw) | 21.76 MB | - |
| Bundled (gzip) | 3.46 MB | - |
| Import time | 690ms | -3ms, -0.5% |
🗺️ View treemap · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — @sanity/cli-build
Compared against main (f6fd22cc)
@sanity/cli-build/_internal/build
| Metric | Value | vs main (f6fd22c) |
|---|---|---|
| Internal (raw) | 113.8 KB | - |
| Internal (gzip) | 28.7 KB | - |
| Bundled (raw) | 17.76 MB | - |
| Bundled (gzip) | 3.56 MB | - |
| Import time | 1.00s | -27ms, -2.6% |
@sanity/cli-build/_internal/env
| Metric | Value | vs main (f6fd22c) |
|---|---|---|
| Internal (raw) | 1.8 KB | - |
| Internal (gzip) | 644 B | - |
| Bundled (raw) | 1.31 MB | - |
| Bundled (gzip) | 333.8 KB | - |
| Import time | 116ms | +0ms, +0.1% |
@sanity/cli-build/_internal/extract
| Metric | Value | vs main (f6fd22c) |
|---|---|---|
| Internal (raw) | 8.6 KB | - |
| Internal (gzip) | 2.7 KB | - |
| Bundled (raw) | 155.0 KB | - |
| Bundled (gzip) | 39.5 KB | - |
| Import time | 210ms | -2ms, -0.9% |
🗺️ ./_internal/env · ./_internal/extract · @sanity/cli-build:./_internal/build treemap too large to embed · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — create-sanity
Compared against main (f6fd22cc)
| Metric | Value | vs main (f6fd22c) |
|---|---|---|
| Internal (raw) | 908 B | - |
| Internal (gzip) | 483 B | - |
| Bundled (raw) | 931 B | - |
| Bundled (gzip) | 491 B | - |
| Import time | ❌ ChildProcess denied: node | - |
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
Contributor
Coverage Delta
Comparing 1 changed file against main @ Overall Coverage
|
Extends detection with two additions: the devEngines.packageManager declaration (object or array form) is honored after the corepack packageManager field and before lock files, and npm-shrinkwrap.json now maps to npm, checked last alongside package-lock.json so it cannot shadow a pnpm, yarn, or bun lock file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requested by Knut Melvær, Jonah Snider · Slack thread
Fixes AIGRO-5244
Description
Before: Running
sanity dev(or any command that installs/upgrades packages) in a pnpm project that happened to have a straypackage-lock.json— left behind by a one-offnpm installor an npx tool — silently picked npm. The auto-update rannpm install --legacy-peer-depsin a pnpm project, corrupting the install. ExplicitpackageManageranddevEngines.packageManagerdeclarations in package.json were ignored entirely.After: Explicit declarations in package.json are honored as the top-priority signal: the corepack
packageManagerfield (e.g."pnpm@9.1.2") first, thendevEngines.packageManager(object or array form). When lock files are the deciding factor and several coexist, the npm lock files (package-lock.json,npm-shrinkwrap.json) are now checked last, so the deliberatepnpm-lock.yamloryarn.lockwins. An npm lock file on its own still resolves to npm, exactly as before.How: Changes in
packages/@sanity/cli/src/util/packageManager/preferredPm.ts:detectFromPackageManagerFieldcheck runs beforedetectFromLockFileinpreferredPm, parsing thepackageManagerfield (<name>@<version>,+shasuffixes tolerated) and falling back todetectFromDevEngines, which readsdevEngines.packageManager.nameper the npm devEngines spec (first entry wins when the field is an array). Malformed values, malformed shapes, and unknown names fall through to the existing logic. The same check is applied symmetrically at the workspace root indetectFromWorkspaceRoot.detectFromLockFilenow checkspackage-lock.jsonandnpm-shrinkwrap.jsonafter yarn/pnpm/bun lock files: npm is already the ecosystem-wide fallback, so when multiple lockfiles coexist, the non-npm one is the stronger signal of intent. SincedetectFromWorkspaceRootreuses the same function, workspace-root detection gets the same fix automatically.What to review
preferredPmanddetectFromLockFileinpackages/@sanity/cli/src/util/packageManager/preferredPm.ts:packageManagerfield >devEngines.packageManager> lock files (npm ones last) > pnpm walk-up > workspace root > node_modules markerspackageManagerfield parsing (regex^(npm|pnpm|yarn|bun)@\d) and thedevEnginesshape handling (object or array, only npm/pnpm/yarn/bun names accepted), plus error handling for missing/malformed package.jsongetPackageManagerChoice, and through itsanity devauto-update,install,build) only consume the returned name, so no call-site changes are neededTesting
Updated and extended the existing tests in
packages/@sanity/cli/test/integration/util/packageManager/preferredPm.test.ts:package-lock.json+pnpm-lock.yaml→ pnpm;package-lock.json+yarn.lock→ yarn;npm-shrinkwrap.json+pnpm-lock.yaml→ pnpm;package-lock.jsonalone → npm;npm-shrinkwrap.jsonalone → npmpackageManagerfield:pnpm@9.1.2beatspackage-lock.json;yarn@4.1.0+sha224.…→ yarn;bun@1.1.0→ bun; unknown name or missing version falls through to lock files; workspace rootpackageManagerbeats the root lock filedevEngines.packageManager: detected on its own;packageManagerfield beats it when both are present; beatspackage-lock.json; array form uses the first entry; malformed shapes and unknown names fall through to lock filesprioritizes package-lock.json over yarn.lock) was inverted to match the new precedenceNotes for release
The package manager auto-detection now honors explicit
packageManageranddevEngines.packageManagerdeclarations in package.json as the top-priority signals and checks the npm lock files (package-lock.json,npm-shrinkwrap.json) last, so a stray npm lockfile no longer overrides a pnpm or yarn project's lockfile.