Rework peer requirement and warning system #6205
Open
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.
What's the problem this PR addresses?
There are a number of issues with the current peer requirement and peer warning system:
Firstly, since v4, we lost the ability to list all peer requirements in the project and to explain missing peer dependency warnings, which is a huge degradation to DX.
Secondly, also since v4, transitive peer warnings are no longer displayed. The rationale given was "those are not actionable". But they are actionable because we have
packageExtensions
.More importantly, there are no indication nor discoverability when this happens. Yarn exits without even a warning when transitive peer warnings can put the project in an invalid state. Even when the user somehow knows that there are transitive peer warnings, they are impossible to investigate and fix without the proper tools.
Thirdly, as I just reported, the peer warning aggregation is aggregating unrelated peer requests, causing
yarn explain peer-requirements
to give incorrect advice. #6203Finally, the peer resolution pipeline has been refactored and added to many times over the years which make the code quite difficult to understand. This is compounded by misplaced comments and overloading terms like "peer descriptor" in the code.
Closes #5841
Closes #5977
Closes #6016
Closes #6118
Closes #6203
How did you fix it?
On the last problem, it is quite impossible to completely solve in one go, so I only took small bites and at least rewrote some of the comments to better capture the current peer pipeline. I have also unified the terminology and change variable names to use those terminology to reduce ambiguity. Maybe we should add these to the lexicon?
Let's say
A
(regular-)depends onB
, which peer-depends onC
A
also depends onC
In the scope of this single level
A@1.0.0
is known as the subject or requesteeB@1.1.0
is known as the requester. Also called the virtualized package in the code because of what the pipeline is doingC
is known as the peer ident and the descriptorC@^1.0.0
is known as the peer descriptorC@^1.2.0
is the provided descriptor. Similarly,C@1.3.0
is the provided locator/package. Provision can refer to any of those depending on context.A
's dependency.A
itself could be used if its ident were matching, or a package can be not provided at all.Based on those we can create some composite data structures:
With those concepts established, the core of this PR changes the peer requirement and peer warning system to use a tree-based structure. The nodes of the tree are either peer requests or peer requirements. A peer request's children are the requests it forwards, and a peer requirement's children are the requests it includes. Note that a tree can only ever include peer requirements and requests regarding a single peer ident.
By storing all peer requirement nodes in
Project
, other places can easily display information on peer requirements/requests/warnings by retrieving them form the tree (using the new tree-view UI, even). This allows us to reimplementyarn explain peer-requirements
andyarn explain peer-requirements <hash>
to list all requirements and explain any peer requirement (even without warnings) respectively. This also fixes #6203 because the peer requirement nodes correctly group the peer requests.To solve the discoverability problem without bringing back the clutter, I've added an additional CTA for transitive peer warnings, so all transitive peer warnings are reduced to a single line.
A user wishing to view the transitive warnings can do so with the reimplemented
yarn explain peer-requirements
command.Lastly, I don't know how much of the original peer requirement and warning system matter for BC. For now, I have recreated the data structures created by the original system, except the aggregated warnings which is the wrong abstraction as noted. I have added TODO comments to remove those code for the next major. Please advice if the original system can be safely removed in a minor.
Checklist