yarnpkg / yarn

The 1.x line is frozen - features and bugfixes now happen on https://github.com/yarnpkg/berry
https://classic.yarnpkg.com
Other
41.42k stars 2.72k forks source link

yarn add --peer --dev fails to record to peerDependencies #5287

Open hulkish opened 6 years ago

hulkish commented 6 years ago

Do you want to request a feature or report a bug? Bug

What is the current behavior? When you run yarn add --peer --dev only records the dependency to devDependencies.

If the current behavior is a bug, please provide the steps to reproduce.

~/dev/repos$ mkdir yarn-peer-bug && cd $_
~/dev/repos/yarn-peer-bug$ yarn init
~/dev/repos/yarn-peer-bug$ yarn add --silent --dev --peer react
~/dev/repos/yarn-peer-bug$ cat package.json
{
  "name": "yarn-peer-bug",
  "version": "1.0.0",
  "main": "index.js",
  "license": "MIT",
  "devDependencies": {
    "react": "^16.2.0"
  }
}

Furthermore, if you try to do them separately, yarn incorrectly does not allow it:

~/dev/repos/yarn-peer-bug$ yarn add --dev --silent react
yarn add v1.3.2
[1/4] πŸ”  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] πŸ”—  Linking dependencies...
[4/4] πŸ“ƒ  Building fresh packages...
└─ react@16.2.0
~/dev/repos/yarn-peer-bug$ yarn add --peer --silent react
yarn add v1.3.2
[1/4] πŸ”  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] πŸ”—  Linking dependencies...
[4/4] πŸ“ƒ  Building fresh packages...
success Saved 1 new dependency.
└─ react@16.2.0
warning "react" is already in "devDependencies". Please remove existing entry first before adding it to "peerDependencies".

This forces you to manually add the dependency as a peer to your package.json.

What is the expected behavior?

~/dev/repos/yarn-peer-bug$ yarn add --silent --dev --peer react
~/dev/repos/yarn-peer-bug$ cat package.json
{
  "name": "yarn-peer-bug",
  "version": "1.0.0",
  "main": "index.js",
  "license": "MIT",
  "devDependencies": {
    "react": "^16.2.0"
  },
  "peerDependencies": {
    "react": "^16.2.0"
  }
}

Please mention your node.js, yarn and operating system version.

~/dev/repos/yarn-peer-bug$ yarn versions
yarn versions v1.3.2
{ http_parser: '2.7.0',
  node: '8.9.4',
  v8: '6.1.534.50',
  uv: '1.15.0',
  zlib: '1.2.11',
  ares: '1.10.1-DEV',
  modules: '57',
  nghttp2: '1.25.0',
  openssl: '1.0.2n',
  icu: '59.1',
  unicode: '9.0',
  cldr: '31.0.1',
  tz: '2017b' }

os: macOS High Sierra 10.13.2

kochie commented 6 years ago

Just want to also mention that the same thing happens with optionalDependencies so there are probably several variations of this bug coming from a root cause.

hulkish commented 6 years ago

@koochie, I could be mistaken here, but - I believe optionalDependencies are intended to define mutually exclusive deps (same with dependencies & devDependencies).

In order words (per my personal experience); if you are wanting to add an optional dep as a regular or dev dep also - then it shouldn't actually be an optional dep.

Whereas with peerDependencies is not mutually exclusive.

kochie commented 6 years ago

@hulkish I think you're right. I'm just trying to think if there would ever be a situation where a package would both be peer and optional? Would that even matter?

hulkish commented 6 years ago

@kochie Don't think so. Because, when you define a peer dep you are saying "dont actually install this, but require it be installed at the top level package/consumer by specifying it as a dependency.".

In that event the top level package could satisfy this by installing the dep in dependencies, devDependencies or optionalDependencies. Not sure on bundleDependencies - I've never used them.

In this case, if you wanted your top level package to specify it as optional then that would be fine. But it wouldn't make sense to define both a peer and an optional dep in the same package.json

kochie commented 6 years ago

Yep that sounds reasonable. Thank @hulkish

hollg commented 6 years ago

Just want to also mention that the same thing happens with optionalDependencies so there are probably several variations of this bug coming from a root cause.

The same issue seems to arise when one package is both a devDependency and a peerDependency (I've come across this while writing a plugin for a linter, and using that linter myself)

kochie commented 6 years ago

Yeah @garyhollandxyz that's the same problem I had, do you get the same error mentioned in the OP?

hollg commented 6 years ago

D'oh, yes, of course it is! I meant to say that the same problem occurs with dependencies and peerDependencies (I originally had the package in dependencies erroneously πŸ™„). Exactly the same error.

kochie commented 6 years ago

That I believe falls under the same category as what I mentioned earlier, because it doesn't make sense to have a package as a peerDependency and a regular dependency.

Since the peerDependencies are installed in the root node_modules along side all normal dependencies - you're meant to add the package as a dependency in the root package.json if it's a peer of you're package.

Where as with regular dependencies you're only saying that this package is directly dependant on this dependency and the original package does not specifically require it.

So as far as I know a package can only be one or the other.

hulkish commented 6 years ago

Confirmed as still an issue in latest release yarn@1.5.1

~/dev/repos/yarn-peer-bug$ yarn add --peer --dev react
yarn add v1.5.1
[1/4] πŸ”  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] πŸ”—  Linking dependencies...
[4/4] πŸ“ƒ  Building fresh packages...
success Saved lockfile.
success Saved 15 new dependencies.
info Direct dependencies
└─ react@16.2.0
info All dependencies
β”œβ”€ asap@2.0.6
β”œβ”€ core-js@1.2.7
β”œβ”€ encoding@0.1.12
β”œβ”€ iconv-lite@0.4.19
β”œβ”€ is-stream@1.1.0
β”œβ”€ isomorphic-fetch@2.2.1
β”œβ”€ js-tokens@3.0.2
β”œβ”€ loose-envify@1.3.1
β”œβ”€ node-fetch@1.7.3
β”œβ”€ promise@7.3.1
β”œβ”€ prop-types@15.6.1
β”œβ”€ react@16.2.0
β”œβ”€ setimmediate@1.0.5
β”œβ”€ ua-parser-js@0.7.17
└─ whatwg-fetch@2.0.3
✨  Done in 0.86s.
~/dev/repos/yarn-peer-bug$ cat package.json
{
  "name": "yarn-peer-bug",
  "version": "1.0.0",
  "main": "index.js",
  "license": "MIT",
  "devDependencies": {
    "react": "^16.2.0"
  }
}
hulkish commented 6 years ago

Any update on this? If i could get some direction - I can try making a pr for this, myself.

MikeDabrowski commented 6 years ago

1.6.0 Still unfixed. Just returns current yarn version.

hulkish commented 6 years ago

Seems like yarn is lacking some support on important issues.

shaunc commented 5 years ago

@hulkish I don't think that is what peer dependencies mean per se; rather, they mean: "packages installed along side this one should use a compatible version of this package". It is perfectly possible to want to have peers have compatible version even if a dependency is used locally (outside the context of a parent). For instance, a package might be useable both standalone and as a library. Or it might want to be testable as standalone even if it is primarily designed as a library.

Edit Even the other categories aren't mutually exclusive so much as that "dependencies" subsume the other two. If something is both a devDependency and a dependency, it must be installed in all cases, so listing it as a devDependency isn't DRY. The same with {dependency, optionalDependency}. In theory, one could need a more specific version as a devDependency than a one needed as a dependency, but this is an edge case. Logically speaking, something could be both a devDependency and an optionalDependency, or for that matter, an optional dev dependency. I guess, practically speaking, no one jumps up and down to ask for support for such combinations, but that doesn't mean they don't make sense semantically.

zhenwenc commented 5 years ago

Here is an example usage in ReactJS react-stream:

  "peerDependencies": {
    "react": "^16.0.0"
  },

So I think it's a needed feature and part of the good practices. :-)

dreyks commented 5 years ago

still an issue yarn@1.17.3

adding a dep to both devDependencies and peerDependencies makes sense for libraries that e.g. depend on react and have react in devDeps for testing

jatwood commented 5 years ago

You can work around this by installing the peer dependency first then installing the dev one (in that order).

yarn add <package> --peer
yarn add <package> --dev

😬

Edit: at least for 1.17

thedamon commented 4 years ago

warning "foo" is already in "devDependencies". Please remove existing entry first before adding it to "peerDependencies".

This error is Very Bad Behaviourℒ️

My unit tests need the peerDependency to run. Sure I can go and rearrange the order that I run imports, or edit my package.json manually, but... this is a very valid usecase. There is a very good reason to have a dep in peer and dev dependencies (unit tests) and being ornery about what order I add things to my deps is frightfully condescending not to mention incorrect.

Plus this bug has been open for nearly two years; makes me nervous!

craigkovatch commented 4 years ago

Very ugly workaround that will add a bunch of unnecessary noise to yarn.lock and thus affect build reproducibility, e.g.:

yarn remove somepackage
yarn add somepackage@^someversion --peer --exact
yarn add somepackage@someversion --dev --exact

This allows the automated update of both dev + peer dep to a new version. But it's awful.

friederbluemle commented 2 years ago

Hello from 2022. :)

This is still an issue with the latest version of Yarn (at the time of writing, 1.22.17).

racheldotey commented 2 years ago

Hello! I was just searching for a way to add all missing peer dependencies for the dev dependencies in my package file. Just wanted to add my voice to the, that would be very handy stack.