fix(arborist): materialize installed transitive optional deps under linked#9660
Closed
manzoorwanijk wants to merge 1 commit into
Closed
fix(arborist): materialize installed transitive optional deps under linked#9660manzoorwanijk wants to merge 1 commit into
manzoorwanijk wants to merge 1 commit into
Conversation
…linked actual tree
Contributor
Author
|
Already completed via #9654 |
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
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.
In continuation of our exploration of using
install-strategy=linkedin the Gutenberg monorepo, which powers the WordPress Block Editor.Under
install-strategy=linked, an installed transitive optional dependency (a non-root package'soptionalDependenciesentry, which lives as a sibling innode_modules/.store/<key>/node_modules/<dep>) was reported asUNMET OPTIONAL DEPENDENCYbynpm lsand omitted fromnpm sbom, even though it is installed and resolves at runtime. Hoisted is unaffected, and a root-declared optional dep works because it is symlinked at the top level.Why
loadActualresolves store siblings through#findMissingEdges(), which walks up ancestornode_modulesfor edges that still need a target. It skipped any edge wherenotMissingwas true. An unresolved optional edge reportsedge.missing === false(Edge.errorisnull, not'MISSING', for an optional edge with no target) andedge.to === null, so the old condition treated it as satisfied and never loaded the installed store sibling. The edge stayed unresolved, whichnpm lsrenders asUNMETandnpm sbomnever reaches.The bug surfaces only on the FS-scan path. When the hidden lockfile validates,
loadVirtualreplay masks it; it reappears whenever the hidden lockfile is absent or stale, orforceActualis set.How
#findMissingEdges()now walks any edge with no resolved target (edge.to === null), instead of only those flaggedmissing. The walk still loads a node only when the package is actually present in an ancestornode_modules, so a genuinely uninstalled optional dep stays unresolved and a present-but-wrong-version resolves toINVALID— matching node's resolution and the hoisted strategy.References
Fixes #9622