Skip to content

Comments

[rush] Fix .npmrc syncing cache bug that strips pnpm hoisting properties#5642

Merged
iclanton merged 1 commit intomicrosoft:mainfrom
iclanton:fix-npmrc-syncing
Feb 20, 2026
Merged

[rush] Fix .npmrc syncing cache bug that strips pnpm hoisting properties#5642
iclanton merged 1 commit intomicrosoft:mainfrom
iclanton:fix-npmrc-syncing

Conversation

@iclanton
Copy link
Member

@iclanton iclanton commented Feb 19, 2026

Summary

Fixes #5641.

The .npmrc file synced to temp in pnpm repos was having pnpm-specific properties (like hoist, hoist-pattern, public-hoist-pattern, shamefully-hoist) incorrectly stripped, causing packages to be hoisted with pnpm's default behavior instead of the configured behavior.

Details

The _trimNpmrcFile() function in npmrcUtilities.ts had a cache (_combinedNpmrcMap) keyed only by sourceNpmrcPath, ignoring all other parameters (filterNpmIncompatibleProperties, linesToPrepend, supportEnvVarFallbackSyntax, env).

In a pnpm repo without subspaces, both calls resolve to the same source path (common/config/rush/.npmrc), but are called with different options:

  1. ensureLocalPackageManagerAsync calls syncNpmrc with filterNpmIncompatibleProperties: true — this correctly comments out pnpm-specific hoisting properties because npm is used to install the package manager itself. This result gets cached.

  2. BaseInstallManager then calls syncNpmrc with filterNpmIncompatibleProperties: false (since pnpm should keep these properties) — but the stale cached result from step 1 is returned, with hoisting properties already stripped.

The fix removes the broken cache entirely. The I/O cost of re-reading a small .npmrc config file a few times during install is negligible, and correctness is critical.

No alternate approaches like fixing the cache key were pursued because the cache provides marginal benefit (the file is only read a small number of times during an install operation) and a correct composite cache key would need to account for many parameters including env (a large object), making it complex and fragile.

How it was tested

Ran the existing npmrcUtilities unit tests via heft test --test-path-pattern npmrcUtilities — all 17 tests pass.

Impacted documentation

None.

@iclanton iclanton merged commit ddf9e3f into microsoft:main Feb 20, 2026
6 checks passed
@github-project-automation github-project-automation bot moved this from Needs triage to Closed in Bug Triage Feb 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

[rush] Rush is filtering out pnpm options from .npmrc when copying into common/temp

2 participants