[pull] latest from npm:latest#223
Merged
Merged
Conversation
…ategy (#9628) In continuation of our exploration of using `install-strategy=linked` in the [Gutenberg monorepo](WordPress/gutenberg#75814), which powers the WordPress Block Editor. Under `install-strategy=linked`, if a top-level `node_modules/<dep>` symlink points to a store key that exists on disk but is the **wrong version**, re-running `npm install` does not repair it. npm reports success and leaves the dependency resolving to the wrong version. This is the state an interrupted update leaves behind: the new store key is extracted but the symlink has not yet been repointed. A symlink pointing at a **non-existent** target is already repaired on reinstall; only a wrong-but-existing target slips through, because cleanup validates the link name, not its target. ## Why For linked installs, `#buildLinkedActualForDiff` synthesizes the "actual" tree the diff compares against from the **ideal** children, never reading the real on-disk symlink target. So a link whose on-disk target is a valid-but-wrong store key looks identical to the ideal node, the diff reports no change, and the symlink is left untouched. The hoisted strategy is unaffected because it self-heals the analogous corruption. ## How In `#buildLinkedActualForDiff`, when an existing link's resolved on-disk target differs from its ideal target, skip creating a synthetic actual entry for it. With no actual entry to match, the diff treats the link as an `ADD`, and `#reifyNode` removes the old symlink and recreates it pointing at the correct store key. A new `#linkTargetMismatch` helper compares the two resolved targets; it runs only after the existing `existsSync` guards, so both paths are known to exist. This repairs both the top-level symlink and wrong transitive/sibling links inside the store, and leaves already-correct trees untouched (no spurious relinking on an idempotent reinstall). ## References Fixes #9611
) In continuation of our exploration of using `install-strategy=linked` in the [Gutenberg monorepo](WordPress/gutenberg#75814), which powers the WordPress Block Editor. Under `install-strategy=linked`, several common installs failed with `npm error invalid filterNode: outside idealTree/actualTree`, with no workaround besides dropping the linked strategy. Hoisted handled all of them. This fixes two distinct paths that both produced that error. ## Why A linked reify diffs the ideal tree against a synthesized actual wrapper (`#linkedActualForDiff`) rather than `this.actualTree`. `Diff.calculate` rejects any filter node whose root is neither the ideal nor the actual it was given, so a filter node taken from the real `this.actualTree` is "outside" the diff and throws. Two places fed it such nodes: - `--workspaces=false` and `-w <ws> --include-workspace-root` go through the `includeRootDeps` branch of `_diffTrees()`, which collected root-dep edge targets from both `this.idealTree` and `this.actualTree`. The actual-side targets are rooted at the real actual tree, not the wrapper, so they tripped the guard. The sibling `includeWorkspaces` branch already accounted for this; the root-dep branch did not. - A global install with a per-call `installStrategy: 'linked'` re-engaged the linked path even though the constructor normalizes global installs to `shallow` (the linked layout is unsupported for globals). Re-installing an already-present global package then hit the global explicit-request branch, which pushes actual-side nodes, and tripped the same guard. Suppressing the crash there was worse: the isolated reifier does not materialize the global layout and removed the package instead. ## How `_diffTrees()` now iterates only the ideal tree for root-dep filter nodes when the linked wrapper is in use, matching the existing workspace-node handling. The ideal-side nodes are sufficient to scope the diff, and the post-reify orphan sweep continues to prune deps removed from the manifest. `reify()` now honors the constructor's global-to-shallow normalization when deriving the `linked` flag, so a global install never engages the linked path regardless of a per-call `installStrategy`. Global installs fall back to shallow, which materializes and upgrades packages correctly. No change to the global explicit-request branch is needed once global is never linked. ## References Fixes #9614 Part of #9608
Restores the global 100% coverage gate on `latest`, which broke after #9626. `filterLinkedStrategyEdges` in `lib/commands/ls.js` skips dev edges on non-root packages — a guard added in #9095 to suppress false `UNMET DEPENDENCY` output in the linked strategy. #9626 fixed the root cause for store packages (they no longer load `devDependencies` as required edges), so the store-based test no longer produces a dev edge, leaving that branch unexercised. Rather than ignore the line, this adds a regression test that genuinely exercises the guard: arborist still loads dev edges for a `file:`-linked transitive package, so listing one with `--all` under the linked strategy reaches the guard at depth > 0 and confirms its devDependency is suppressed instead of reported as `UNMET DEPENDENCY`. The full test suite passes at 100%. This is the `latest` counterpart of #9636, which restored coverage on `release/v11` via an `istanbul ignore`. ## References Follows up #9626
…egy (#9639) In continuation of our exploration of using `install-strategy=linked` in the [Gutenberg monorepo](WordPress/gutenberg#75814), which powers the WordPress Block Editor. Under `install-strategy=linked`, `npm exec -w <ws> -- <bin>` ignored a workspace-local bin (provided by a sibling workspace dependency) and fell through to the registry, producing a spurious `E404`. The hoisted strategy ran the local bin correctly. ## Why For a workspace exec, the command computed the local bin directory as `resolve(this.npm.localDir, name, 'node_modules', '.bin')`, i.e. `<root>/node_modules/<name>/node_modules/.bin`. That path only resolves when the workspace is symlinked into the root `node_modules` as `<name>`, which is how the hoisted strategy lays workspaces out. The linked strategy does not hoist workspaces into the root `node_modules`; the workspace's real bin lives at `<workspace>/node_modules/.bin`. libnpmexec walks up from the given bin directory looking for `node_modules/.bin/<bin>`, so starting from the nonexistent hoisted path never reached the workspace's actual bin and the lookup fell back to the registry. ## How Base the local bin directory on the workspace's own path (`runPath`) instead of the hoisted `localDir/<name>` location. This is correct under both strategies: linked finds the bin in the workspace's `node_modules/.bin`, and hoisted still finds the root-hoisted bin because the walk-up continues from the workspace directory to the root `node_modules/.bin`. ## References Fixes #9616
…tch (#9647) In continuation of our exploration of using `install-strategy=linked` in the [Gutenberg monorepo](WordPress/gutenberg#75814), which powers the WordPress Block Editor. Switching `install-strategy` in the same project directory left behind the previous strategy's layout. Going hoisted → linked kept the stale real top-level transitive directories alongside the new `.store/` and symlinks; going linked → hoisted kept the entire `node_modules/.store/` directory. A fresh install of either strategy was already clean — only the switch was affected. ## Why Under the linked strategy the actual tree the diff compares against is synthesized from the ideal tree (`#buildLinkedActualForDiff`), so real directories left over from a prior hoisted layout are never seen, and `#cleanOrphanedTopLevelLinks` only removed symlinks. In the other direction `load-actual` ignores dot-directories, so the hoisted diff never sees `node_modules/.store` and never removes it. ## How `reify.js` now removes the leftover `.store` on a non-linked reify via `#removeStaleStoreDir`. The store lives only at the project root and is exclusively a linked artifact, so a single removal covers the project. It runs only for a full-project install — a workspace-filtered or `--workspaces=false` install is skipped, because out-of-scope workspaces may still link into the store. `#cleanOrphanedTopLevelLinks` (run only under linked) additionally removes stale real package directories — a directory containing a `package.json` that is not in the ideal tree's valid top-level set — and prunes an emptied `@scope` directory afterward. Non-package real directories and symlinks pointing outside the project are still preserved. The valid-top-level collection in `#cleanOrphanedStoreEntries` no longer skips non-link nodes, so the root's bundled dependencies — materialized as real top-level directories under linked — are recorded as valid and never swept as stale. ## References Fixes #9615 Part of #9608
Owner
|
| Status | Scan Engine |  Critical |
 High |
 Medium |
 Low |
Total (2) |
|---|---|---|---|---|---|---|
| Open Source Security | 1 | 1 | 0 | 0 | See details |
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )