Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Peer dependency warnings are never seen when using legacy-peer-deps #6227

Open
2 tasks done
sscaff1 opened this issue Mar 7, 2023 · 8 comments
Open
2 tasks done
Labels
Bug thing that needs fixing Needs Discussion is pending a discussion Needs Triage needs review for next steps Release 9.x work is associated with a specific npm 9 release

Comments

@sscaff1
Copy link

sscaff1 commented Mar 7, 2023

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

When using legacy peer deps, npm no longer throws a warning for uninstalled peer dependencies.

Screenshot 2023-03-07 at 2 38 52 PM

Notice no warning for eslint-config-airbnb's peers.

Expected Behavior

You should see a warning. Notice that yarn throws a warning:

Screenshot 2023-03-07 at 2 37 09 PM

Steps To Reproduce

  1. Go to codesandbox
  2. Create a new node project
  3. yarn add eslint-config-airbnb - note you get warnings
  4. rm -rf node_modules
  5. npm i eslint-config-airbnb --legacy-peer-deps=true - note you get no warnings
  6. Look in the package-lock.json and note that none of the peers have been installed

Environment

  • npm: latest
  • Node.js: LTS
  • OS Name: N/A
  • System Model Name: N/A
  • npm config: N/A
@sscaff1 sscaff1 added Bug thing that needs fixing Needs Triage needs review for next steps Release 9.x work is associated with a specific npm 9 release labels Mar 7, 2023
@ljharb
Copy link
Contributor

ljharb commented Mar 7, 2023

Good catch. You should also, ofc, not use legacy peer deps :-)

@darcyclarke
Copy link
Contributor

darcyclarke commented Mar 8, 2023

+1 @ljharb. --legacy-peer-deps was meant as a stopgap solution for the ecosystem to catch up/fix issues. I think it's time to rip off the bandaid. I imagine the npm CLI team should be queuing up deprecating the flag for v10 🙏🏻 I know that pnpm will be installing peer deps by default in v8 (ref. https://twitter.com/ZoltanKochan/status/1630956825012064256) so I think it's time to drop any config that avoids/hides these conflicts.

@fritzy fritzy added the Needs Discussion is pending a discussion label Mar 16, 2023
@valiant-code
Copy link

If you are discussing deprecating the flag then we should definitely fix this to throw warnings so that people can use those warnings to help inform them on how to resolve the conflicts and be aware how many issues are potentially building up by using the legacy flag.

@Susccy
Copy link

Susccy commented Apr 26, 2024

This just cost me hours of angry debugging because I didn't understand why my peer deps weren't installing and not even showing any warnings in the console. I eventually found that legacy-peer-deps has been silently enabled for months in the .npmrc of our project and everyone forgot about it because npm never tells you anywhere about it being enabled.

However, even with knowing that option was enabled I still didn't understand why npm install and npm ci wouldn't even show any warnings about missing peer deps. It just succeeded the install like everything was perfectly fine. But of course the code wouldn't run due to missing deps. So frustrating!

@EliezerB123
Copy link

This just cost me hours of angry debugging because I didn't understand why my peer deps weren't installing and not even showing any warnings in the console. I eventually found that legacy-peer-deps has been silently enabled for months in the .npmrc of our project and everyone forgot about it because npm never tells you anywhere about it being enabled.

However, even with knowing that option was enabled I still didn't understand why npm install and npm ci wouldn't even show any warnings about missing peer deps. It just succeeded the install like everything was perfectly fine. But of course the code wouldn't run due to missing deps. So frustrating!

This. Same thing happened to us. One of our developers had silently enabled --legacy-peer-deps without telling anyone, and because npm didn't throw any warnings or errors, nobody noticed the problem.

@EliezerB123
Copy link

EliezerB123 commented Aug 14, 2024

....Does anyone have any workaround, to get npm to display the peerDependencies that it's skipping?

Edit: As a workaround, I did:

  1. Delete node_modules and package-lock.json.
  2. npm install --legacy-peer-deps
  3. npm install --verbose
  4. Open the debug log of the most recent npm install. (It'll say in the output, but if you don't find it, it'll be located in someplace like: D:\Users\USERPROFILE\AppData\Local\npm-cache_logs\TIMESTAMP-debug-0.log)
  5. Inside the debug log, search for the phrase, silly ADD. It should be written several times, next to every package newly installed (which, because it was only installed the second time, was not installed during the npm install --legacy-peer-deps).

I hope that helps someone in the future.

@silvenon
Copy link

silvenon commented Dec 27, 2024

Adding legacy-peer-deps=true to .npmrc should be fixed to emit warnings until #3666 is fixed, otherwise we cannot use either — if I'm using the default behavior eventually I'm going to hit a brick wall when npm is refusing to install a dependency with incorrect claims of mismatched peer dependencies, and if I'm using legacy-peer-deps=true I'm not even going to know about mismatches at all.

Considering that fixing incorrect claims is obviously more complicated, I recommend fixing this first instead. Warnings are going to bother us and we'll be inclined to get rid of them, so it's going to be beneficial for the ultimate goal.

@silvenon
Copy link

silvenon commented Dec 27, 2024

It seems like force=true instead of legacy-peer-deps=true is achieving the desired effect: emitting warnings + letting dependencies install.

They install a bit differently, not sure in what way, but seems to be a good workaround for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Needs Discussion is pending a discussion Needs Triage needs review for next steps Release 9.x work is associated with a specific npm 9 release
Projects
None yet
Development

No branches or pull requests

8 participants