Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

fix: deps of linked deps in package-lock #229

Merged

Conversation

ruyadorno
Copy link
Contributor

When having optional dependencies of a dependency of
a top-level link dependency, e.g:

root -> LINK(a)
a -> (b)
b -> OPTIONAL(c)

* c marked as extraneous

Any optional dependency (and all its nested nodes) were marked
as extraneous in package-lock due to calcDepFlags missing
properly following the target of Link nodes.

This changeset fixes it by properly following the result of the
treeverse.visit method that will have already followed any
link targets at that point.

Fixes: npm/cli#2505

References

@ruyadorno
Copy link
Contributor Author

@isaacs fixup commit fixes snapshots in a few load-actual and shrinkwrap tests which pruned some "peer": true lines.

I assume these now missing lines are due to calcDepFlags properly traversing the tree and unsetFlag(to, 'peer') in some places it wasn't reaching before.

Would be good to double check that it all makes sense 👍

@@ -11,7 +11,7 @@ const calcDepFlags = (tree, resetRoot = true) => {
tree,
visit: node => calcDepFlagsStep(node),
filter: node => node,
getChildren: node => [...node.edgesOut.values()].map(edge => edge.to),
getChildren: (node, tree) => [...tree.edgesOut.values()].map(edge => edge.to),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the optimally unconfusing way to do it, but it tickles me that the whole diff could've been:

- getChildren: node => ....
+ getChildren: (_,node) => ....

When having optional dependencies of a dependency of
a top-level link dependency, e.g:

    root -> LINK(a)
    a -> (b)
    b -> OPTIONAL(c)

    * c marked as extraneous

Any optional dependency (and all its nested nodes) were marked
as extraneous in package-lock due to `calcDepFlags` missing
properly following the target of Link nodes.

This changeset fixes it by properly following the result of the
`treeverse.visit` method that will have already followed any
link targets at that point.

Fixes: npm/cli#2505

PR-URL: npm#229
Credit: @ruyadorno
Close: npm#229
Reviewed-by: @isaacs
@isaacs isaacs force-pushed the fix-package-lock-optional-deps-of-linked-deps branch from 1f4f74d to c2666af Compare February 12, 2021 16:54
@isaacs isaacs merged commit c2666af into npm:main Feb 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] package-lock issues with workspaces
2 participants