Skip to content

Commit

Permalink
Fix peer warning message requester (#6376)
Browse files Browse the repository at this point in the history
## What's the problem this PR addresses?

When an incompatible peer warning occurs, the warning message shown
during install simply uses the first peer requester found but that may
not have any relation to the unsatisfied request.

See #6205 (comment)
(partial fix)

## How did you fix it?

Actually find the first unsatisfied requester, prioritizing root
requesters (i.e. direct dependencies).

If an unsatisfied root requester is found, the warning message looks
similar to 4.2.2


![image](https://github.com/yarnpkg/berry/assets/41266433/08a3b91e-ed03-4002-9f60-e982a8d7189b)

```  
➤ YN0000: ┌ Post-resolution validation
➤ YN0060: │ react is listed by your project with version 18.3.1 (pfa021), which doesn't satisfy what reflux and other dependencies request (but they have non-overlapping ranges!).
➤ 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.
➤ YN0000: └ Completed
```

If all root requesters are satisfied, then an unsatisfied requester is
found and reported with one of the corresponding root requesters


![image](https://github.com/yarnpkg/berry/assets/41266433/753f7b94-2151-47b0-ac04-69621de8e355)

```
➤ YN0000: ┌ Post-resolution validation
➤ YN0060: │ react is listed by your project with version 18.3.1 (pfa021), which doesn't satisfy what reflux (via @local/test-reflux-dependent) and other dependencies request 
(but they have non-overlapping ranges!).
➤ 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.
➤ YN0000: └ Completed
```

One concern I had was that it will make the message longer, but we are
already way past the common terminal width so I don't think that's a big
issue.

## 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 <[email protected]>
  • Loading branch information
clemyan and arcanis authored Jul 29, 2024
1 parent 9daddcf commit 2a47867
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 5 deletions.
34 changes: 34 additions & 0 deletions .yarn/versions/db8fe3a1.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
releases:
"@yarnpkg/cli": patch
"@yarnpkg/core": patch

declined:
- "@yarnpkg/plugin-compat"
- "@yarnpkg/plugin-constraints"
- "@yarnpkg/plugin-dlx"
- "@yarnpkg/plugin-essentials"
- "@yarnpkg/plugin-exec"
- "@yarnpkg/plugin-file"
- "@yarnpkg/plugin-git"
- "@yarnpkg/plugin-github"
- "@yarnpkg/plugin-http"
- "@yarnpkg/plugin-init"
- "@yarnpkg/plugin-interactive-tools"
- "@yarnpkg/plugin-link"
- "@yarnpkg/plugin-nm"
- "@yarnpkg/plugin-npm"
- "@yarnpkg/plugin-npm-cli"
- "@yarnpkg/plugin-pack"
- "@yarnpkg/plugin-patch"
- "@yarnpkg/plugin-pnp"
- "@yarnpkg/plugin-pnpm"
- "@yarnpkg/plugin-stage"
- "@yarnpkg/plugin-typescript"
- "@yarnpkg/plugin-version"
- "@yarnpkg/plugin-workspace-tools"
- "@yarnpkg/builder"
- "@yarnpkg/doctor"
- "@yarnpkg/extensions"
- "@yarnpkg/nm"
- "@yarnpkg/pnpify"
- "@yarnpkg/sdks"
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,24 @@ describe(`Features`, () => {
);

test(
`it should report collapsed a peer dependency warning when a set of mismatched peerDependency requirements is detected`,
`it should report collapsed a peer dependency warning when a direct peerDependency request is mismatched`,
makeTemporaryEnv(
{
dependencies: {
[`mismatched-peer-deps-lvl1`]: `1.0.0`,
[`no-deps`]: `1.1.0`,
},
},
async ({path, run, source}) => {
const {stdout} = await run(`install`);

expect(stdout).toMatch(/no-deps is listed by your project with version 1\.1\.0 \(p[a-f0-9]{5}\), which doesn't satisfy what mismatched-peer-deps-lvl1 and other dependencies request \(1\.0\.0\)/);
},
),
);

test(
`it should report collapsed a peer dependency warning with corresponding root when a transitive peerDependency request is mismatched`,
makeTemporaryEnv(
{
dependencies: {
Expand All @@ -40,7 +57,7 @@ describe(`Features`, () => {
async ({path, run, source}) => {
const {stdout} = await run(`install`);

expect(stdout).toMatch(/no-deps is listed by your project with version 1\.1\.0 \(p[a-f0-9]{5}\), which doesn't satisfy what mismatched-peer-deps-lvl0 and other dependencies request \(1\.0\.0\)/);
expect(stdout).toMatch(/no-deps is listed by your project with version 1\.1\.0 \(p[a-f0-9]{5}\), which doesn't satisfy what mismatched-peer-deps-lvl[12] \(via mismatched-peer-deps-lvl0\) and other dependencies request \(1\.0\.0\)/);
},
),
);
Expand Down
33 changes: 30 additions & 3 deletions packages/yarnpkg-core/sources/Project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2678,6 +2678,27 @@ function applyVirtualResolutionMutations({
}
}

function * allPeerRequestsWithRoot(root: PeerRequestNode | PeerRequirementNode) {
const roots = new Map<PeerRequestNode, PeerRequestNode>();

if (`children` in root) {
roots.set(root, root);
} else {
for (const request of root.requests.values()) {
roots.set(request, request);
}
}

for (const [request, root] of roots) {
yield {request, root};
for (const child of request.children.values()) {
if (!roots.has(child)) {
roots.set(child, root);
}
}
}
}

function emitPeerDependencyWarnings(project: Project, report: Report) {
const incompatibleWarnings: Array<string> = [];
const missingWarnings: Array<string> = [];
Expand All @@ -2701,6 +2722,14 @@ function emitPeerDependencyWarnings(project: Project, report: Report) {
if (typeof peerPackage === `undefined`)
throw new Error(`Assertion failed: Expected the package to be registered`);

const requester = miscUtils.mapAndFind(allPeerRequestsWithRoot(warning.node), ({request, root}) => {
if (!semverUtils.satisfiesWithPrereleases(peerPackage.version ?? `0.0.0`, request.descriptor.range))
return request === root
? structUtils.prettyIdent(project.configuration, request.requester)
: `${structUtils.prettyIdent(project.configuration, request.requester)} (via ${structUtils.prettyIdent(project.configuration, root.requester)})`;

return miscUtils.mapAndFind.skip;
});
const otherPackages = [...structUtils.allPeerRequests(warning.node)].length > 1
? `and other dependencies request`
: `requests`;
Expand All @@ -2715,9 +2744,7 @@ function emitPeerDependencyWarnings(project: Project, report: Report) {
structUtils.prettyReference(project.configuration, peerPackage.version ?? `0.0.0`)
} (${
formatUtils.pretty(project.configuration, warning.hash, formatUtils.Type.CODE)
}), which doesn't satisfy what ${
structUtils.prettyIdent(project.configuration, warning.node.requests.values().next().value.requester)
} ${otherPackages} (${rangeDescription}).`);
}), which doesn't satisfy what ${requester} ${otherPackages} (${rangeDescription}).`);
}

if (warning.type === PeerWarningType.NodeNotProvided) {
Expand Down

0 comments on commit 2a47867

Please sign in to comment.