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

Delay visibility propagation until deps are known #1739

Merged
merged 1 commit into from
Nov 11, 2016

Conversation

jkrems
Copy link
Contributor

@jkrems jkrems commented Nov 8, 2016

Summary

Right now visibility is set before the dependencies are added to the package reference. This means that no visibility information propagates for other requests for the same package. Example:

  1. is-glob@^2.0.0 is encountered. Visibility for the request is "env hidden"
  2. Dependencies of is-glob@^2.0.0 are walked and inherit the visibility from their parent
  3. is-glob@^2.0.1 is encountered. Visibility for the request is "used"
  4. Because there's already something compatible in flight, it's merged into the existing ref
  5. addVisibility is called - but at this point the dependencies are not yet resolved
  6. The dependencies are added - but none of them will ever learn that (one of) their parent was "used"

Test plan

I couldn't figure out a good unit test for this. What I did for manual testing in an empty directory:

npm init --yes
yarn add --dev chokidar@1.6.1
yarn add liftoff@2.3.0
rm -rf node_modules && ../../tools/yarn/bin/yarn.js --prod && node -e "require('is-glob')"

Fixes #761

@jkrems
Copy link
Contributor Author

jkrems commented Nov 8, 2016

Also tried this out against a non-trivial internal app that previously broke. Seems to work with this change.

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.

yarn install --production doesn't install correct dependencies
2 participants