[Bug?]: yarn with node-linker does not correctly install nested dependencies

bradzacher commented 3 months ago

Self-service

[ ] I'd be willing to implement a fix

Describe the bug

$ yarn install
$ yarn why @types/eslint

Expected:

It should list that @stylistic/eslint-plugin depends on @types/eslint@9.6.0
It should create node_modules/@stylistic/eslint-plugin/node_modules/@types/eslint containing @types/eslint@9.6.0

Actual:

$ yarn why @types/eslint
└─ root-workspace-0b6124@workspace:.
   └─ @types/eslint@npm:8.56.11 (via npm:^8)

$ ls -1a node_modules/@stylistic/eslint-plugin
.
..
LICENSE
README.md
dist
package.json

To reproduce

package.json

{
  "devDependencies": {
    "@stylistic/eslint-plugin": "latest",
    "@types/eslint": "^8"
  },
  "packageManager": "yarn@4.4.0"
}

.yarnrc.yml

nodeLinker: node-modules

Environment

System:
    OS: macOS 14.5
    CPU: (10) arm64 Apple M1 Max
  Binaries:
    Node: 20.15.0 - /private/var/folders/l4/bc59q80d0xn_j4pkq97cn7rc0000gn/T/xfs-62c13eda/node
    Yarn: 4.4.0 - /private/var/folders/l4/bc59q80d0xn_j4pkq97cn7rc0000gn/T/xfs-62c13eda/yarn
    npm: 10.7.0 - /opt/homebrew/bin/npm
    pnpm: 8.15.8 - /opt/homebrew/bin/pnpm
    bun: 1.1.12 - /opt/homebrew/bin/bun

Additional context

Looking at the lockfile I can see:

"@stylistic/eslint-plugin@npm:latest":
  version: 2.6.1
  resolution: "@stylistic/eslint-plugin@npm:2.6.1"
  dependencies:
    "@stylistic/eslint-plugin-js": "npm:2.6.1"
    "@stylistic/eslint-plugin-jsx": "npm:2.6.1"
    "@stylistic/eslint-plugin-plus": "npm:2.6.1"
    "@stylistic/eslint-plugin-ts": "npm:2.6.1"
    "@types/eslint": "npm:^9.6.0"
  peerDependencies:
    eslint: ">=8.40.0"
  checksum: 10c0/87cbbac0ea84161e6ccc03b7c5298279c2d5a0100be63af5a83f979ae0fa5328df371443a3da9eac833d58a1708bae2e5d8615530048c888e7f5b01d3ebf654a
  languageName: node
  linkType: hard

and

"@types/eslint@npm:^9.6.0":
  version: 9.6.0
  resolution: "@types/eslint@npm:9.6.0"
  dependencies:
    "@types/estree": "npm:*"
    "@types/json-schema": "npm:*"
  checksum: 10c0/69301356bc73b85e381ae00931291de2e96d1cc49a112c592c74ee32b2f85412203dea6a333b4315fd9839bb14f364f265cbfe7743fc5a78492ee0326dd6a2c1
  languageName: node
  linkType: hard

So yarn knows about the dependency and lists it as expected -- however it's just not installing it.

yarn.lock

bradzacher commented 3 months ago

We discovered this because this is causing one of our tests to break.

We have an integration test with the following package.json: https://github.com/typescript-eslint/typescript-eslint/blob/146179334b576141aa3d4179866bb63680adde85/packages/integration-tests/fixtures/flat-config-types-%40types__eslint-v8/package.json

It is intended to allow us to test community plugins to ensure that we don't get unexpected type errors. But this bug is causing its own unexpected type error -- causing our tests to fail https://github.com/typescript-eslint/typescript-eslint/actions/runs/10284026780/job/28461467116

arcanis commented 3 months ago

Indeed, I can repro, and I found where it comes from.

For context, if you write the TS-compatible package my-lib which has a peer dependency on, say, lodash, it implies you must also have a peer dependency on @types/lodash (otherwise your .d.ts file that would import ... from 'lodash' wouldn't work). Unfortunately library authors not always respect this as they're concerned of adding TypeScript-specific dependencies. To workaround that, Yarn automatically adds a @types optional peer dependency for every non-@types peer dependency.

The code checks whether an @types peer dependency already exists (so as not to overwrite it), but in the case of @stylistic/eslint-plugin the @types package is instead provided through a regular dependency. It's quite unusual, and we forgot to add a check for this particular case. As a result the automatic peer dependency on @types/eslint is added, and overrides the regular dependency (peer dependency + regular dependency is interpreted as "use the peer dependency if provided by the parent, otherwise use the regular dependency").

Ref: https://github.com/yarnpkg/berry/blob/13d5b3041794c33171808fdce635461ff4ab5c4e/packages/yarnpkg-core/sources/Configuration.ts#L1958-L1974

I'll make a fix later today.

jaskp commented 3 weeks ago

Has this been fixed?

yarnpkg / berry