Skip to content

chore(npm): point peer-dependency warning at the overrides remedy (#35196)#35205

Open
tsushanth wants to merge 2 commits into
denoland:mainfrom
tsushanth:improve-peer-dep-warning
Open

chore(npm): point peer-dependency warning at the overrides remedy (#35196)#35205
tsushanth wants to merge 2 commits into
denoland:mainfrom
tsushanth:improve-peer-dep-warning

Conversation

@tsushanth

Copy link
Copy Markdown

Closes #35196.

The current "The following peer dependency issues were found:" line lists the conflicts but says nothing about how to fix them. Reporters in #35196 and #32801 independently arrived at the same workaround — pinning the affected packages under "overrides" in package.json — but only after going through several iterations of trimming imports and finding the older issue. Putting the remedy on the line right next to the warning is the cheapest way to shortcut that.

- Warning The following peer dependency issues were found:
+ Warning The following peer dependency issues were found.
+   To resolve, pin the affected package(s) under "overrides" in your package.json (https://docs.npmjs.com/cli/v10/configuring-npm/package-json#overrides):
  └─┬ @denotest/peer-dep-specific-constraint@1.0.0
    └── peer @denotest/add@0.5: resolved to 1.0.0

The change is a single format! string in peer_dep_diagnostics_to_display_tree in libs/npm_installer/resolution.rs plus the matching .out update under tests/specs/install/peer_dep_specific_constraint/. The tree itself is untouched, so the existing same_ancestor_peer_dep_message unit test (which only asserts children counts) continues to pass.

Scope notes for the maintainer:

  • I deliberately referenced only package.json overrides, since Workspace::npm_overrides() reads root_pkg_json().overrides exclusively; there's no deno.json-side equivalent today and I'd rather not invent one in a warning message.
  • The other half of the reporter's ask — naming the importer that pulled the offending peer in — would need plumbing through UnmetPeerDepDiagnostic.ancestors back up to the entry specifier, which is a bigger change than belongs in this PR.

@deno-cla-assistant

Copy link
Copy Markdown

Deno Individual Contributor License Agreement

The following contributors need to sign the CLA before this PR can be merged:

Click here to review and sign the CLA | Re-run CLA check


This is an automated message from CLA Assistant

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.

Print a more actionable warning message when peer dependency issues are found

1 participant