Skip to content

fix(npm): prioritize packageManager field and simplify lock file dete…#302916

Open
flycran wants to merge 2 commits intomicrosoft:mainfrom
flycran:fix/npm-preferred-pm-detection-v2
Open

fix(npm): prioritize packageManager field and simplify lock file dete…#302916
flycran wants to merge 2 commits intomicrosoft:mainfrom
flycran:fix/npm-preferred-pm-detection-v2

Conversation

@flycran
Copy link

@flycran flycran commented Mar 18, 2026

Summary

The findPreferredPM function in the npm extension previously ignored the
packageManager field in package.json, which is the most authoritative
way to declare the intended package manager per the Node.js Corepack spec.

Additionally, the previous logic relied on find-yarn-workspace-root, which
detects the presence of a workspaces field in package.json and incorrectly
treats it as evidence of Yarn usage — regardless of the actual package manager
in use. This caused unexpected behavior reported in #170101 and #159465.

This PR refactors the detection logic with the following changes:

  • Read packageManager first: If package.json declares a
    packageManager field (e.g. "packageManager": "pnpm@8.6.0"), it is
    used as the authoritative source for the package manager name.
  • Unified lock file table: Replace four separate per-PM detection
    functions with a single LOCKFILE_CANDIDATES table, iterating in
    priority order and returning early as soon as two distinct lock files
    are detected.
  • Fix multipleLockFilesDetected semantics: Previously, whichPM
    results could incorrectly inflate the lock file count. Now
    multipleLockFilesDetected only reflects actual lock file conflicts,
    consistent with the warning message shown to the user in tasks.ts.
  • Remove unused dependency: find-yarn-workspace-root is no longer
    used and can be safely removed from package.json.

Testing

  • Compiled the extension via npx gulp compile-extension:npm — no errors.
  • Opened a workspace and ran npm scripts with different packageManager
    values in package.json (e.g. "pnpm@9.0.0", "yarn@4.0.0"), confirmed
    the correct package manager was invoked each time.
  • Added a workspaces field to package.json and verified it no longer
    incorrectly influences package manager detection.
  • Placed different lock files in the workspace root, reloaded the window,
    ran npm scripts, and confirmed the correct package manager was selected
    based on the detected lock file.

@meganrogge
Copy link
Collaborator

Parent traversal should not happen for generic lockfile detection here. This change now uses findUp for npm/yarn/bun too, which can pick lockfiles from unrelated ancestor folders and mis-detect the package manager.

Please restrict detection to the package root (or at least stop at workspace root), and only keep parent traversal where there is a PM-specific reason.

@meganrogge
Copy link
Collaborator

multipleLockFilesDetected currently counts lockfile hits, not distinct package managers. Since LOCKFILE_CANDIDATES has duplicate PM entries (pnpm-lock.yaml + shrinkwrap.yaml, bun.lock + bun.lockb), two lockfiles for the same PM can incorrectly trigger the multi-PM warning. Please dedupe by PM name before deciding multipleLockFilesDetected.”

@flycran
Copy link
Author

flycran commented Mar 20, 2026

@meganrogge The parent traversal via findUp is intentional, designed to align with how package managers themselves resolve workspace roots. In npm/pnpm/yarn/bun workspace projects, lock files (package-lock.json, pnpm-lock.yaml, yarn.lock, bun.lock/bun.lockb) only exist at the workspace root — never inside individual package subdirectories. Bun, for example, also automatically traverses up to find the nearest package.json when invoked in a subdirectory. Disabling parent traversal would cause lock file detection to fail for monorepo sub-packages across all these package managers, falling back to whichPM, which diverges from the actual tool behavior.

Additionally, the upward traversal of findUp for lock files follows exactly the same mechanism as npm run resolving package.json — both walk up the directory tree level by level until the target file is found or the filesystem root is reached. Using the VS Code workspace root as a stopAt boundary is conceptually unreliable: the VS Code workspace root is not equivalent to the package manager's workspace root, and determining the package manager's workspace root itself requires traversing up to find a package.json with a workspaces field — using a static boundary to constrain that traversal is self-contradictory.

I've created an online demo to verify the behavior described above: Edit in CodeSandbox

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants