Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(resolution): WIP: Improve dependency resolution performance (comparison of #5970) #5982

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hulkish
Copy link
Contributor

@hulkish hulkish commented Jun 13, 2018

This is meant as a WIP pr to compare with #5970

Instead of continuing to re-resolve a package in the dependency tree that has already been resolved,
add a request to the existing references requests array. This speeds up very deep/complex dep trees
as seen in workspaces where the workspace modules all depend on each other. Previously the building
of the tree was leading to out of memory errors or just never finishing.

fixes yarnpkg#5726 yarnpkg#5628
…ixes logic error when a dep is referenced as both a normal and an optional dependency.
@@ -279,9 +276,11 @@ export default class Config {
});

this.registries[key] = registry;
this.registryFolders.push(registry.folder);
if (this.registryFolders.indexOf(registry.folder) === -1) {
Copy link
Contributor Author

@hulkish hulkish Jun 13, 2018

Choose a reason for hiding this comment

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

this fixes an issue i was seeing where value of registryFolders was ['node_modules', 'node_modules'] - which I think was causing double lookups for ignorePatterns.

@hulkish hulkish changed the title fix(resolution): Improve dependency resolution performance (comparison of #5970) fix(resolution): WIP: Improve dependency resolution performance (comparison of #5970) Jun 13, 2018
@hulkish hulkish force-pushed the improve-resolution-performance branch from 302fd5a to ac8e244 Compare June 13, 2018 15:20
@hulkish
Copy link
Contributor Author

hulkish commented Jun 13, 2018

@rally25rs another thing I'm noticing here... For workspace packages - there should not be a fetch request for these. It should just prefer the local link.

@sibelius
Copy link

@arcanis any change to get this or the related pull request reviewed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants