Fix missing subdeps in production install when those are present in devDependencies list - #2921
Merged
bestander merged 3 commits intoApr 7, 2017
Conversation
…t devDependencies
…e packages instead of faulty inherit logic. Fixes yarnpkg#2819
Contributor
Author
|
@SimenB requested a few extra tests in #2895 (comment), haven't come round to adding them. |
Contributor
|
Can this get backported to the freshly minted 0.22.0? 👼 |
Member
|
@SimenB, send a PR targeting 0.22-stable branch and we'll make a patch release. |
Contributor
|
@bestander done: #3065 |
bestander
pushed a commit
that referenced
this pull request
Apr 7, 2017
…evDependencies list (#2921) (#3065) * Test for install skipping subdependencies when those are named in root devDependencies * Add 'incompatible' flag to references, use that to ignore incompatible packages instead of faulty inherit logic. Fixes #2819 * Unconditionally mark packages as ignored, hoister now fully corrects transitive uses
arcanis
pushed a commit
to arcanis/yarn
that referenced
this pull request
Apr 7, 2017
…evDependencies list (yarnpkg#2921) * Test for install skipping subdependencies when those are named in root devDependencies * Add 'incompatible' flag to references, use that to ignore incompatible packages instead of faulty inherit logic. Fixes yarnpkg#2819 * Unconditionally mark packages as ignored, hoister now fully corrects transitive uses
bestander
pushed a commit
that referenced
this pull request
Apr 7, 2017
* Checks that the webpack builds are working properly (#3064) * Fix missing subdeps in production install when those are present in devDependencies list (#2921) * Test for install skipping subdependencies when those are named in root devDependencies * Add 'incompatible' flag to references, use that to ignore incompatible packages instead of faulty inherit logic. Fixes #2819 * Unconditionally mark packages as ignored, hoister now fully corrects transitive uses * replaced deprecated asserts (#3069) * fixing lint (#3070) * Fixes integrity check for --production flag (#3067) * fixed integrity check when running with --production * added test * removed unused var * Remove the dependency on the "rc" module (#3063) * Removes dependency on the "rc" module * Removers shebang-loader, not used anymore * Fixes flow errors * Fixes tests on Windows
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
3 participants
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.
Summary
This PR fixes #2819.
This bug originates in the package hoister, which, in production mode, fails to mark devDependencies that are also a sub-dependency of the normal dependencies as non-ignored.
This happens in the following scenario:
dep: A -> B
devDep: B
In the first hoisting round, A en B are both processed. Because the reference to B was marked as 'ignore' in production mode, as a root package (without a parent) it will have inheritIsIgnored set to false. Later, when propagating non-ignoredness, B will be skipped (thus keeping it as ignored), and not installed (as well as its private subdepencies).
At the moment, the 'ignore' field in the package reference has a dual purpose - signalling incompatible optional packages and ignored devDependencies. When propagating non-ignoredness, it can always be propagated to devDependencies, but never to incompatible packages.
To fix the bug, this PR adds an explicit 'incompatible' flag in the package reference. In the packagehoister, the isIgnored flag is replaced with isRequired, better signalling intent. Propagating requiredness is done for all compatible packages, and inheritIsIgnored flag is replaced by checks for compatibility.
Test plan
A test has been added that fails before the fix, and passes after it. Also, I have recreated the package.json from #2819, and verified that gulp is now installed properly.