fix: give precedence to pnp over node_modules, close #288

[noahnu]

Fixes #288.

Ideally, node_modules should not even be considered if the pnp api is enabled, however it looks like that's intentional according to #273

[codecov]

Codecov Report

Merging #289 (20caa8a) into master (f08fe3f) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #289   +/-   ##
=======================================
  Coverage   94.93%   94.93%           
=======================================
  Files          39       39           
  Lines        1579     1581    +2     
=======================================
+ Hits         1499     1501    +2     
  Misses         80       80           

Impacted Files	Coverage Δ
lib/ResolverFactory.js	97.12% <100.00%> (+0.03%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f08fe3f...20caa8a. Read the comment docs.

[alexander-akait]

hm, why developers have node_modules and use yarn PnP, I think yarn should print error/warning here

[merceyz]

It happens when using the portal: protocol to a project that uses node_modules, for example Next.js uses Yarn 1 with node_modules but has e2e tests using Yarn 2 with PnP that uses the built sources from the Yarn 1 project

https://github.com/vercel/next.js/blob/15133b47ec7bfc413aa068d92fb678626cf47571/test-pnp.sh#L40

[noahnu]

Another example which prompted this PR/issue:

At my work we have 2 projects. Project A using yarn pnp and Project M using node_modules. Project M is a monorepo. Right now if you try to link a package from Project M into Project A, the peer dependencies of the Project M package are resolved using Project M's node_modules, rather than Project A's dependencies. If the peer deps include React, you get singleton issues preventing React hooks from working, among other features.

[merceyz]

Since Next.js started using webpack 5 by default this is also affecting their e2e tests https://github.com/vercel/next.js/runs/2373726821#step:5:222, applying the change in this PR fixes their e2e tests cc @sokra

[sokra]

Thanks