fix: only automatically resolve optional peers with versions that satisfy the ranges #8028
Conversation
| pkgAddresses: resolveDependenciesResult.pkgAddresses, | ||
| ...filterMissingPeers(await resolveDependenciesResult.resolvingPeers, parentPkgAliases), | ||
| } | ||
| pkgAddresses.push(...importerResolutionResult.pkgAddresses) |
There was a problem hiding this comment.
I see this happens above on line 48 too, but I'd be a bit worried about stack overflows here with large arrays. It might be safer to use a for loop since that doesn't have the same downside?
Are we guaranteed importerResolutionResult.pkgAddresses is small? I'm guessing it will be 99% of the time, but still depends on user input.
There was a problem hiding this comment.
I am not sure what you mean. How does an array concatenation cause a stackoverflow?
There was a problem hiding this comment.
Sorry, I should have cited something. The best discussion of the issue I can find is:
The problem is that all arguments are placed onto the stack one by one before calling .push, which means that this code can throw depending on how deep in the stack we are already and argument size.
I tend to avoid spreading arguments into a function if the arguments aren't a fixed size so I don't have to think about this problem.
| @@ -27,3 +27,20 @@ export function hoistPeers ( | |||
| } | |||
| return dependencies | |||
| } | |||
|
|
|||
| export function hoistOptionalPeers ( | |||
There was a problem hiding this comment.
nitpick: The name hoistOptionalPeers makes me think it'll perform a side-effect. In this case, it looks like it's just a pure function that computes a map. How about getSatisfyingOptionalPeersFromFullGraph?
close #7985