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

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

Closed
edmorley opened this issue Oct 10, 2017 · 9 comments

Comments

@edmorley
Copy link
Contributor

edmorley commented Oct 10, 2017

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:

  • unmet peer dependency warnings aren't shown for dependencies that are installed transitively.
  • yarn and npm errors/warnings are roughly the same for the same package.json.

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
Copy link
Contributor Author

This also reproduces with yarn 1.2.1.

@BYK
Copy link
Member

BYK commented Oct 16, 2017

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.

@BYK BYK closed this as completed Oct 16, 2017
@edmorley
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link

@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
Copy link
Contributor Author

@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
Copy link

I think this is supposed to be fixed in v1.4.0... #5088

@edmorley
Copy link
Contributor Author

edmorley commented Mar 17, 2018

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants