Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Rework peer requirement and warning system (#6205)
**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. #6203 ---- Finally, 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 - Package `A` (regular-)depends on `B`, which peer-depends on `C` - Package `A` also depends on `C` ``` A@1.0.0 --> B@^1.0.0 (resolves to B@1.1.0) ==> C@^1.0.0 --> C@^1.2.0 (resolves to C@1.3.0) ``` In the scope of this single level - The package `A@1.0.0` is known as the **subject** or **requestee** - The package `B@1.1.0` is known as the **requester**. Also called the **virtualized package** in the code because of what the pipeline is doing - The ident `C` is known as the **peer ident** and the descriptor `C@^1.0.0` is known as the **peer descriptor** - The descriptor `C@^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. - Note that the provision is not necessarily `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: - A **peer request** encapsulates a requester + peer ident. - If a subject does not provide to the request itself, but instead requests a peer dependency under the same ident, that subject and the new peer request it issues are both said to **forward** the original peer request. - A **peer requirement** encapsulates a subject + peer ident + one or more peer requests. This captures the fact that a single subject that depends on multiple peer requesters can satisfy multiple peer requests at the same time - Note that peer requests and peer requirements is a many-to-many relationship. A peer request is included in multiple peer requirements if the requester is depended upon multiple times (which can happen due to deduplication). - A peer requirement is said to be a **root peer requirement** if the subject does not forward the peer requests. ---- 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 reimplement `yarn explain peer-requirements` and `yarn 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. ``` ➤ YN0000: ┌ Post-resolution validation ➤ YN0002: │ workspace-b@workspace:workspace-b doesn't provide p (p427bb), requested by y. ➤ YN0086: │ Some peer dependencies are incorrectly met by your project; run yarn explain peer-requirements <hash> for details, where <hash> is the six-letter p-prefixed code. ➤ YN0086: │ Some peer dependencies are incorrectly met by dependencies; run yarn explain peer-requirements for details. ➤ YN0000: └ Completed ``` 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** <!--- Don't worry if you miss something, chores are automatically tested. --> <!--- This checklist exists to help you remember doing the chores when you submit a PR. --> <!--- Put an `x` in all the boxes that apply. --> - [x] I have read the [Contributing Guide](https://yarnpkg.com/advanced/contributing). <!-- See https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released for more details. --> <!-- Check with `yarn version check` and fix with `yarn version check -i` --> - [x] I have set the packages that need to be released for my changes to be effective. <!-- The "Testing chores" workflow validates that your PR follows our guidelines. --> <!-- If it doesn't pass, click on it to see details as to what your PR might be missing. --> - [x] I will check that all automated PR checks pass before the PR gets reviewed. --------- Co-authored-by: Maël Nison <nison.mael@gmail.com>
- Loading branch information
