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.44k stars 2.73k forks source link

"unmet peer dependency" warning shown for transitive dependency that npm accepts #4675

Closed edmorley closed 7 years ago

edmorley commented 7 years ago

Hi!

Do you want to request a feature or report a bug? A bug.

STR:

  1. Clone https://github.com/edmorley/yarn-unmet-peerdependency-testcase
  2. From the repo root, run: rm -rf node_modules/ && yarn install
  3. And then: rm -rf node_modules/ && npm install

Expected:

Actual: The yarn output (example) includes:

warning "eslint-config-airbnb-base@12.0.2" has unmet peer dependency "eslint@^4.8.0".
warning "eslint-plugin-import@2.7.0" has unmet peer dependency "eslint@2.x - 4.x".

...whereas the NPM output (example) contains no such warnings.

To save having to dig through the repo, the testcase's package.json contains:

  "dependencies": {
    "neutrino": "7.1.0",
    "neutrino-preset-airbnb-base": "7.1.0"
}

...and the relevant child package's dependencies are as follows:

// neutrino-preset-airbnb-base
// https://github.com/mozilla-neutrino/neutrino-dev/blob/v7.1.0/packages/neutrino-preset-airbnb-base/package.json
  "dependencies": {
    ...
    "eslint-config-airbnb-base": "^12.0.1",
    "eslint-plugin-import": "^2.7.0",
    "neutrino-middleware-eslint": "^7.1.0"
  },

// neutrino-middleware-eslint
// https://github.com/mozilla-neutrino/neutrino-dev/blob/v7.1.0/packages/neutrino-middleware-eslint/package.json
  "dependencies": {
    ...
    "eslint": "^4.7.2",
    ...
  },

Please mention your node.js, yarn and operating system version. yarn 1.2.0 / npm 5.4.2 node 8.6.0 Windows 10 x64 using MSYS2 bash.

Many thanks :-)

edmorley commented 7 years ago

This also reproduces with yarn 1.2.1.

BYK commented 7 years ago

This doesn't seem like a bug to me. neutrino-preset-airbnb-base does not list anything related to eslint in its dependencies and I get the following warnings when I try:

warning "neutrino-preset-airbnb-base > eslint-config-airbnb-base@12.0.2" has unmet peer dependency "eslint@^4.8.0".
warning "neutrino-preset-airbnb-base > eslint-plugin-import@2.7.0" has unmet peer dependency "eslint@2.x - 4.x".

This plugin and the config are direct dependencies of neutrino-preset-airbnb-base so there should be an ESLint dependency listed compatible with these ranges in the same tree. That means either at the project root or inside neutrino-preset-airbnb-base itself. I'm guessing NPM doesn't complain about this since neutrino lists ESLint as a dependency and and it gets hoisted up to the project root. That said this is not guaranteed at all cases (hoisting is never guaranteed), so Yarn's warning is correct.

edmorley commented 7 years ago

But neutrino-preset-airbnb-base lists neutrino-middleware-eslint as a dependency, which does depend on eslint. I know this relies on hoisting to work, but this is the intended use-case - these are handy wrappers around common configuration, where the user isn't meant to have to install additional dependencies at the top level, nor do we want neutrino-preset-airbnb-base to use a different eslint version from neutrino-middleware-eslint. (And in fact, neither neutrino-preset-airbnb-base nor neutrino-middleware-eslint will directly use eslint, the neutrino main package will).

Is this just a workflow not very well supported by peerDependencies at the moment?

edmorley commented 7 years ago

Perhaps a middle ground would be for yarn to only warn in this case if it wasn't hoisting? Since if hoisting occurs then it's not actually an issue - and for Neutrino (and other ecosystems that use a similar plugin system), the hoisting is actually necessary/expected otherwise nothing would work.

mpolichette commented 6 years ago

@edmorley any chance you found a resolution to this issue? I'm using neutrino as well and it is frustrating to get a bunch of warnings we don't technically control.

edmorley commented 6 years ago

@mpolichette I haven't sadly.

@BYK, what do you think about:

Perhaps a middle ground would be for yarn to only warn in this case if it wasn't hoisting? Since if hoisting occurs then it's not actually an issue - and for Neutrino (and other ecosystems that use a similar plugin system), the hoisting is actually necessary/expected otherwise nothing would work.

mpolichette commented 6 years ago

I think this is supposed to be fixed in v1.4.0... https://github.com/yarnpkg/yarn/pull/5088

edmorley commented 6 years ago

This issue is about cases where the dep isn't at root level intentionally, but will be after hoisting. So isn't fixed in the latest release.

billybonks commented 6 years ago

and for Neutrino (and other ecosystems that use a similar plugin system), the hoisting is actually necessary/expected otherwise nothing would work.

i seem to agree with this statement.