Draft
Conversation
manzoorwanijk
added
the
[Type] Code Quality
github-actions
bot
added
[Package] Core data
|
Size Change: +17.4 kB (+0.25%) Total Size: 6.86 MB
ℹ️ View Unchanged
|
github-actions
bot
added
[Package] Element
manzoorwanijk
force-pushed
the
update/set-npm-install-strategy
branch
2 times, most recently
from
a57e609 to
7675dcf
Compare
github-actions
bot
removed
[Package] Element
manzoorwanijk
force-pushed
the
update/set-npm-install-strategy
branch
from
2fbf94d to
1b65b17
Compare
|
Flaky tests detected in 7050ffe. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/22296383359
|
Member
Author
The script mentioned in the description// bin/rebuild-lockfile.js
#!/usr/bin/env node
/**
* Rebuild package-lock.json with the install-strategy from .npmrc
* while preserving the resolved versions from the existing lockfile.
*
* Usage:
* git checkout trunk package-lock.json
* node bin/rebuild-lockfile.js [--dry-run]
*
* This works by:
* 1. Reading all resolved versions from the current lockfile
* 2. Temporarily pinning all caret/tilde ranges in every package.json
* 3. Temporarily adding overrides for all transitive deps in root package.json
* 4. Deleting the lockfile
* 5. Running `npm install --package-lock-only --ignore-scripts`
* 6. Restoring all original package.json files
*/
const fs = require( 'fs' );
const path = require( 'path' );
const { execSync } = require( 'child_process' );
const ROOT = path.resolve( __dirname, '..' );
const DRY_RUN = process.argv.includes( '--dry-run' );
// 1. Read the current lockfile to get resolved versions.
const lockPath = path.join( ROOT, 'package-lock.json' );
const lock = JSON.parse( fs.readFileSync( lockPath, 'utf8' ) );
// Build a map of package name -> resolved version from lockfile.
// Also track leaf package names that have multiple distinct major versions —
// these cannot be pinned with a single override (e.g. wrap-ansi v3/v5/v6/v7/v8).
// Packages with multiple minor/patch versions within the same major are fine.
const resolvedVersions = new Map();
const multiMajorPackages = new Set();
const nonNpmPackages = new Set(); // packages resolved from non-npm sources
const leafMajors = new Map(); // leaf name -> Set of major versions
for ( const [ pkgPath, meta ] of Object.entries( lock.packages || {} ) ) {
if ( ! meta.version || pkgPath === '' ) {
continue;
}
// Extract package name from the path.
// e.g. "node_modules/foo" -> "foo"
// e.g. "packages/components/node_modules/framer-motion" -> "framer-motion"
const match = pkgPath.match( /node_modules\/(.+)$/ );
if ( match ) {
const name = match[ 1 ];
// Skip alias packages (e.g. "string-width-cjs" is really
// "npm:string-width@4.2.3"). These have a `name` field in the
// lockfile that differs from the directory name.
if ( meta.name && meta.name !== name ) {
continue;
}
// Skip packages resolved from non-npm sources (Git URLs, tarballs).
// Overrides for these would fail with ETARGET.
if (
meta.resolved &&
! meta.resolved.startsWith( 'https://registry.npmjs.org/' )
) {
nonNpmPackages.add( name );
}
// Keep the first (most hoisted) version as the canonical one.
if ( ! resolvedVersions.has( name ) ) {
resolvedVersions.set( name, meta.version );
}
// Track major versions per leaf name to detect multi-major packages.
const leafName = name.includes( '/node_modules/' )
? name.split( '/node_modules/' ).pop()
: name;
const major = meta.version.split( '.' )[ 0 ];
if ( ! leafMajors.has( leafName ) ) {
leafMajors.set( leafName, new Set() );
}
leafMajors.get( leafName ).add( major );
}
}
for ( const [ leafName, majors ] of leafMajors ) {
if ( majors.size > 1 ) {
multiMajorPackages.add( leafName );
}
}
console.log( `Found ${ resolvedVersions.size } resolved packages in lockfile` );
console.log(
`Found ${ multiMajorPackages.size } multi-major packages (will skip overrides)`
);
// 2. Find all package.json files (root + workspaces).
const rootPkgPath = path.join( ROOT, 'package.json' );
const rootPkgContent = fs.readFileSync( rootPkgPath, 'utf8' );
const rootPkg = JSON.parse( rootPkgContent );
const workspaceGlobs = rootPkg.workspaces || [];
const pkgJsonPaths = [ rootPkgPath ];
for ( const wsGlob of workspaceGlobs ) {
const base = wsGlob.replace( /\/\*$/, '' );
const wsDir = path.join( ROOT, base );
if ( ! fs.existsSync( wsDir ) ) {
continue;
}
for ( const entry of fs.readdirSync( wsDir, { withFileTypes: true } ) ) {
if ( ! entry.isDirectory() ) {
continue;
}
const pkgJson = path.join( wsDir, entry.name, 'package.json' );
if ( fs.existsSync( pkgJson ) ) {
pkgJsonPaths.push( pkgJson );
}
}
}
console.log( `Found ${ pkgJsonPaths.length } package.json files` );
// 3. Pin direct dependency versions in all package.json files.
const backups = new Map();
let pinCount = 0;
// Collect root direct dependency names — overrides for these cause EOVERRIDE.
// Workspace direct deps are OK: npm just ignores the override for that
// workspace but still applies it to other consumers.
const rootDirectDeps = new Set();
for ( const pkgJsonPath of pkgJsonPaths ) {
const content = fs.readFileSync( pkgJsonPath, 'utf8' );
const pkg = JSON.parse( content );
const isRoot = pkgJsonPath === rootPkgPath;
let changed = false;
for ( const depField of [ 'dependencies', 'devDependencies' ] ) {
const deps = pkg[ depField ];
if ( ! deps ) {
continue;
}
for ( const [ name, range ] of Object.entries( deps ) ) {
if ( isRoot ) {
rootDirectDeps.add( name );
}
// Only pin semver ranges (skip file:, workspace:, npm:, url, etc.)
if ( typeof range !== 'string' || ! /^[\^~]/.test( range ) ) {
continue;
}
const resolved = resolvedVersions.get( name );
if ( resolved ) {
deps[ name ] = resolved;
changed = true;
pinCount++;
}
}
}
if ( changed ) {
backups.set( pkgJsonPath, content );
if ( ! DRY_RUN ) {
fs.writeFileSync(
pkgJsonPath,
JSON.stringify( pkg, null, '\t' ) + '\n'
);
}
}
}
console.log(
`Pinned ${ pinCount } dependency ranges across ${ backups.size } files`
);
// 4. Add overrides for transitive dependencies to root package.json.
// This pins versions that are resolved inside published packages (e.g.
// @emotion/react -> @emotion/cache) which can't be pinned via
// package.json edits alone.
const existingOverrides = rootPkg.overrides || {};
const transOverrides = { ...existingOverrides };
let overrideCount = 0;
for ( const [ name, version ] of resolvedVersions ) {
// Skip root direct dependencies — overrides for these cause EOVERRIDE.
if ( rootDirectDeps.has( name ) ) {
continue;
}
// Skip packages resolved from non-npm sources (ETARGET).
if ( nonNpmPackages.has( name ) ) {
continue;
}
// For packages with nested node_modules in their name (e.g.
// "@babel/core/node_modules/lru-cache"), extract just the leaf
// package name for the override.
const overrideName = name.includes( '/node_modules/' )
? name.split( '/node_modules/' ).pop()
: name;
// Skip if the leaf name is a root direct dependency or non-npm package.
if ( rootDirectDeps.has( overrideName ) ) {
continue;
}
if ( nonNpmPackages.has( overrideName ) ) {
continue;
}
// Skip packages with multiple distinct versions in the lockfile.
// A single override can't represent different versions for different
// consumers (e.g. wrap-ansi v6 for some, v7/v8 for others).
if ( multiMajorPackages.has( overrideName ) ) {
continue;
}
if ( transOverrides[ overrideName ] ) {
continue;
}
if ( ! existingOverrides[ overrideName ] ) {
// Prefer the hoisted (plain) version over a nested one. If the
// plain name exists in resolvedVersions, use that version instead
// of the nested entry's version (which may be a different major).
const preferredVersion =
overrideName !== name && resolvedVersions.has( overrideName )
? resolvedVersions.get( overrideName )
: version;
transOverrides[ overrideName ] = preferredVersion;
overrideCount++;
}
}
console.log( `Adding ${ overrideCount } transitive dependency overrides` );
if ( ! DRY_RUN ) {
// Re-read root package.json in case it was already modified above.
const currentRootContent = fs.readFileSync( rootPkgPath, 'utf8' );
const currentRootPkg = JSON.parse( currentRootContent );
// Back up root package.json if not already backed up.
if ( ! backups.has( rootPkgPath ) ) {
backups.set( rootPkgPath, rootPkgContent );
}
currentRootPkg.overrides = transOverrides;
fs.writeFileSync(
rootPkgPath,
JSON.stringify( currentRootPkg, null, '\t' ) + '\n'
);
}
if ( DRY_RUN ) {
console.log( 'Dry run — no changes made' );
process.exit( 0 );
}
// 5. Delete lockfile and regenerate.
console.log( 'Deleting lockfile...' );
fs.unlinkSync( lockPath );
console.log( 'Running npm install --package-lock-only --ignore-scripts ...' );
try {
execSync( 'npm install --package-lock-only --ignore-scripts', {
cwd: ROOT,
stdio: 'inherit',
timeout: 10 * 60 * 1000,
} );
} finally {
// 6. Restore original package.json files (always, even if install fails).
console.log( 'Restoring original package.json files...' );
for ( const [ filePath, original ] of backups ) {
fs.writeFileSync( filePath, original );
}
}
// 7. Patch the lockfile to restore original caret/tilde ranges.
// Running `npm install` again would re-resolve transitive deps, undoing the
// version pinning from step 4. Instead, we directly edit the lockfile entries
// to match the original package.json dependency ranges.
console.log( 'Patching lockfile with original package.json ranges...' );
const newLock = JSON.parse( fs.readFileSync( lockPath, 'utf8' ) );
for ( const [ pkgJsonPath, originalContent ] of backups ) {
const originalPkg = JSON.parse( originalContent );
const relativePath = path.relative( ROOT, path.dirname( pkgJsonPath ) );
// Root package is "" in the lockfile, workspace packages are "packages/foo".
const lockKey = relativePath === '' ? '' : relativePath;
const lockEntry = newLock.packages[ lockKey ];
if ( ! lockEntry ) {
continue;
}
for ( const depField of [ 'dependencies', 'devDependencies' ] ) {
if ( originalPkg[ depField ] && lockEntry[ depField ] ) {
for ( const [ name, range ] of Object.entries(
originalPkg[ depField ]
) ) {
if ( lockEntry[ depField ][ name ] ) {
lockEntry[ depField ][ name ] = range;
}
}
}
}
}
fs.writeFileSync( lockPath, JSON.stringify( newLock, null, '\t' ) + '\n' );
console.log(
'Done! Lockfile regenerated with nested structure and pinned versions.'
);
console.log( 'Run `npm install` to install from the new lockfile.' ); |
Member
Author
|
The GitHub actions worker seems to be running out of resources/space due to nested node_modules and thus there are CI failures, making even a stronger case for pnpm migration - #74689 |
manzoorwanijk
force-pushed
the
update/set-npm-install-strategy
branch
3 times, most recently
from
d1f975d to
06d56bd
Compare
owlstronaut
pushed a commit
to npm/cli
that referenced
this pull request
Feb 20, 2026
#8996) We're looking at using `install-strategy=linked` in the [Gutenberg monorepo](https://github.com/WordPress/gutenberg) (~200 workspace packages), which powers the WordPress Block Editor. While [testing it in a PR](WordPress/gutenberg#75213), we ran into several issues with the linked strategy that this PR fixes. ## Summary 1. Scoped workspace packages were losing their `@scope/` prefix in `node_modules` because the symlink name came from the folder basename instead of the package name. 2. Aliased packages (e.g. `"prettier": "npm:custom-prettier@3.0.3"`) in workspace projects were getting symlinked under the real package name instead of the alias, so `require('prettier')` would fail. 3. With `legacy-peer-deps = true`, peer dependencies weren't being placed alongside packages in the store, so `require()` calls for peer deps failed from within the store. 4. With `strict-peer-deps = true`, the linked strategy could crash with `Cannot read properties of undefined` when store entries or parent nodes were missing for excluded peer deps. ## Root cause `assignCommonProperties` was using a single `result.name` for both consumer-facing symlinks and store-internal paths. For workspaces, `node.name` comes from the folder basename (missing the scope). For aliases, `node.packageName` gives the real name but we need the alias for the consumer symlink. Separately, `legacy-peer-deps` tells the arborist to skip creating peer dep edges entirely, so the isolated reifier never saw them and never placed them in the store. And `strict-peer-deps` can cause nodes to be excluded from the tree while still being referenced by edges, leading to undefined lookups. ## Changes - Split proxy identity into `result.name` (consumer-facing: alias or scoped workspace name) and `result.packageName` (store-internal: real package name from `package.json`). Store paths (`getKey`, `treeHash`, `generateChild`, `processEdges`, `processDeps`) use `packageName`; consumer symlinks keep using `name`. - When `legacyPeerDeps` is enabled, resolve missing peer dep edges from the tree via `node.resolve()` so they still get symlinked in the store. - Guard against undefined `from` and `target` nodes in `processEdges`/`processDeps` to prevent crashes with `strict-peer-deps`. - Guard `idealTree.children.get(ws)` in `reify.js` since the isolated tree uses an array for `children`, not a Map. - Test fixture updates: `recursive: true` for `mkdirSync`, scoped workspace glob support. - New tests for scoped workspace packages, aliased packages in workspaces, and peer deps with `legacyPeerDeps`. ## References Fixes #6122
manzoorwanijk
added a commit
to manzoorwanijk/npm-cli
that referenced
this pull request
Feb 23, 2026
npm#8996) We're looking at using `install-strategy=linked` in the [Gutenberg monorepo](https://github.com/WordPress/gutenberg) (~200 workspace packages), which powers the WordPress Block Editor. While [testing it in a PR](WordPress/gutenberg#75213), we ran into several issues with the linked strategy that this PR fixes. ## Summary 1. Scoped workspace packages were losing their `@scope/` prefix in `node_modules` because the symlink name came from the folder basename instead of the package name. 2. Aliased packages (e.g. `"prettier": "npm:custom-prettier@3.0.3"`) in workspace projects were getting symlinked under the real package name instead of the alias, so `require('prettier')` would fail. 3. With `legacy-peer-deps = true`, peer dependencies weren't being placed alongside packages in the store, so `require()` calls for peer deps failed from within the store. 4. With `strict-peer-deps = true`, the linked strategy could crash with `Cannot read properties of undefined` when store entries or parent nodes were missing for excluded peer deps. ## Root cause `assignCommonProperties` was using a single `result.name` for both consumer-facing symlinks and store-internal paths. For workspaces, `node.name` comes from the folder basename (missing the scope). For aliases, `node.packageName` gives the real name but we need the alias for the consumer symlink. Separately, `legacy-peer-deps` tells the arborist to skip creating peer dep edges entirely, so the isolated reifier never saw them and never placed them in the store. And `strict-peer-deps` can cause nodes to be excluded from the tree while still being referenced by edges, leading to undefined lookups. ## Changes - Split proxy identity into `result.name` (consumer-facing: alias or scoped workspace name) and `result.packageName` (store-internal: real package name from `package.json`). Store paths (`getKey`, `treeHash`, `generateChild`, `processEdges`, `processDeps`) use `packageName`; consumer symlinks keep using `name`. - When `legacyPeerDeps` is enabled, resolve missing peer dep edges from the tree via `node.resolve()` so they still get symlinked in the store. - Guard against undefined `from` and `target` nodes in `processEdges`/`processDeps` to prevent crashes with `strict-peer-deps`. - Guard `idealTree.children.get(ws)` in `reify.js` since the isolated tree uses an array for `children`, not a Map. - Test fixture updates: `recursive: true` for `mkdirSync`, scoped workspace glob support. - New tests for scoped workspace packages, aliased packages in workspaces, and peer deps with `legacyPeerDeps`. ## References Fixes npm#6122
manzoorwanijk
force-pushed
the
update/set-npm-install-strategy
branch
from
06d56bd to
7050ffe
Compare
manzoorwanijk
changed the title
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
1 participant
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.
What?
Sets npm's
install-strategyto"nested"in.npmrc, so that each workspace package gets its ownnode_moduleswith its declared dependencies, rather than relying on hoisted packages at the root.Why?
Gutenberg currently uses npm's default
hoistedinstall strategy, which allows packages to implicitly depend on modules hoisted to the rootnode_modules— so-called "phantom dependencies." This makes it easy to use a package without declaring it inpackage.json, leading to breakage when dependency trees change.Switching to
nestedinstalls catches these phantom dependencies by ensuring each workspace resolves from its ownnode_modules. Note that npm's nested strategy still hoists workspace packages and common dependencies to the root when all consumers agree on the same version — it is not as strict as pnpm's isolatednode_modules, but it is a meaningful improvement over the current situation.This is intended as a temporary workaround until the full pnpm migration (#74689), for which the Make Core proposal (requested by community members) is in progress. While not as strict as pnpm, it is an improvement over the current setup:
dependencies/devDependenciesentries early.Since npm's nested strategy still hoists workspace packages and common dependencies to the root, any dependency in the root
package.jsonremains available to all workspaces as a phantom dependency. Combined with #75041 — which convertstools/andtest/directories into workspace packages and reduces root devDependencies from 81 to only essential shared tools — this becomes significantly more effective: fewer root dependencies means fewer phantom dependencies available for accidental use.How?
Why
nestedand notlinked?npm offers another strategy —
install-strategy = "linked"— which would be closer to pnpm's strict isolation. Unfortunately, it has a known bug that breaks scoped packages: it strips the scope from the directory name, turning e.g.node_modules/@wordpress/core-dataintonode_modules/core-data, which causes resolution failures across the board. Until that is fixed,nestedis the most viable option..npmrcchanges (install-strategydocs)install-strategy = "nested"— each workspace gets its ownnode_modules.strict-peer-deps = true— fail on unmet peer dependencies.Lockfile regeneration
npm has a known bug where changing the install strategy in
.npmrcdoes not restructure an existing lockfile — the old hoisted layout is preserved until the lockfile is deleted and regenerated. However, deleting and regenerating the lockfile causes version drift (dependencies resolve to their latest versions), which breaks snapshot tests and can introduce subtle behavioral changes.To work around this, the lockfile was rebuilt with the help of Claude Code, which iteratively developed a script to delete and regenerate the lockfile while preserving trunk's resolved versions. The approach uses temporary npm overrides for ~2,000 transitive dependencies to pin them during regeneration, then restores the original
package.jsonfiles and patches the lockfile to keep the original dependency ranges. Several edge cases had to be handled (alias packages, non-npm tarballs, multi-major packages, root vs. workspace override conflicts).Dependency fixes
Several workspace packages had missing or incorrect dependency declarations that were masked by hoisting. These have been fixed:
dependenciesanddevDependenciesentries across multiple packages.typeRootsto include./node_modules/@typesfor nested resolution.Patch updates
patch-packagepatch file paths use++as the separator for nestednode_modulespaths (instead of+for hoisted). Existing patches have been updated accordingly.Storybook
@emotion/reactas a rootdevDependencyso Vite can resolve@emotion/react/jsx-runtimefrom workspace packages.@storybook/addon-docsand@storybook/react-viteas rootdevDependenciesso they are findable from where thestorybookCLI is installed.Testing Instructions
node_modulesandpackage-lock.json.git checkout update/set-npm-install-strategy -- package-lock.jsonnpm install— should complete without errors.npm run test:unit -- packages/components— all snapshot tests should pass (no vendor-prefix drift).npm run build— should complete successfully.node_modules: