-
-
Notifications
You must be signed in to change notification settings - Fork 21
Feat: Let peer dependencies work with sb@next #27
Conversation
Atm if I install `storybook-addon-next-router` on a project on sb@next, npm yells the following error at me because the restrictions on the peer-dependencies are too tight. This patch fixes the problem. ```` npm ERR! code ERESOLVE npm ERR! ERESOLVE unable to resolve dependency tree npm ERR! npm ERR! Found: @storybook/[email protected] npm ERR! node_modules/@storybook/addon-actions npm ERR! dev @storybook/addon-actions@"npm:@storybook/addon-actions@next" from the root project npm ERR! @storybook/addon-actions@"6.4.0-alpha.18" from @storybook/[email protected] npm ERR! node_modules/@storybook/addon-essentials npm ERR! dev @storybook/addon-essentials@"npm:@storybook/addon-essentials@next" from the root project npm ERR! npm ERR! Could not resolve dependency: npm ERR! dev storybook-addon-next-router@"^3.0.5" from the root project npm ERR! npm ERR! Conflicting peer dependency: @storybook/[email protected] npm ERR! node_modules/@storybook/addon-actions npm ERR! peer @storybook/addon-actions@"^6.0.0" from [email protected] npm ERR! node_modules/storybook-addon-next-router npm ERR! dev storybook-addon-next-router@"^3.0.5" from the root project npm ERR! npm ERR! Fix the upstream dependency conflict, or retry npm ERR! this command with --force, or --legacy-peer-deps npm ERR! to accept an incorrect (and potentially broken) dependency resolution. npm ERR! npm ERR! See /Users/bb/.npm/eresolve-report.txt for a full report. npm ERR! A complete log of this run can be found in: npm ERR! /Users/bb/.npm/_logs/2021-07-16T16_21_11_266Z-debug.log ````
package.json
Outdated
"@storybook/addon-actions": "^6 || >=6.0.0-0 || >=6.1.0-0 || >=6.2.0-0 || >=6.3.0-0 || >=6.4.0-0", | ||
"@storybook/addons": "^6 || >=6.0.0-0 || >=6.1.0-0 || >=6.2.0-0 || >=6.3.0-0 || >=6.4.0-0", | ||
"@storybook/client-api": "^6 || >=6.0.0-0 || >=6.1.0-0 || >=6.2.0-0 || >=6.3.0-0 || >=6.4.0-0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"@storybook/addon-actions": "^6 || >=6.0.0-0 || >=6.1.0-0 || >=6.2.0-0 || >=6.3.0-0 || >=6.4.0-0", | |
"@storybook/addons": "^6 || >=6.0.0-0 || >=6.1.0-0 || >=6.2.0-0 || >=6.3.0-0 || >=6.4.0-0", | |
"@storybook/client-api": "^6 || >=6.0.0-0 || >=6.1.0-0 || >=6.2.0-0 || >=6.3.0-0 || >=6.4.0-0", | |
"@storybook/addon-actions": "^6.0.0", | |
"@storybook/addons": "^6.0.0", | |
"@storybook/client-api": "^6.0.0", |
@benbender does this suggestion work for you as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lifeiscontent That's the failing config :) Context is automatic installation and strict enforcing of peer-deps in npm 7. See https://github.blog/2021-02-02-npm-7-is-now-generally-available/#peer-dependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benbender how are you able to even get storybook running? Last time I checked the peer deps were really messed up in storybook. do you mind putting a small project together so I can test this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tbh, I don't get your change. Thats the original, failing config... If you would like to have it shorter, the following may work too:
"@storybook/addons": "^6"
@benbender https://jubianchi.github.io/semver-check/#/^6.0.0/6.0.0-alpha.18 seems to work with alpha versions. |
@lifeiscontent as seen above, npm 7 (stable) with strict-peer-deps (now the default), begs to differ. Sorry if that sounds rude, but its your package, your choice. I don't have to convince you... |
@benbender sorry I don't mean any animosity, I'm just trying to make sense of the change which is why I asked for a minimal project to test in, I don't currently use npm 7 due to bugs with monorepos. So I don't have the context, can you please help me understand? I'm happy to make the change, I just want to understand what I'm adding here. |
No harm done ;)
Wouldn't make much sense if you don't have/want NPM 7 - otherwise a simple
Same for me ;) Let's reiterate: NPM 7 introduced strict peer-deps by default and also automatically installs them. This new default fails here (and in many other cases) with the error in the first message. Your testcase with semver-check is inaccurate as it explicitly states that it handles dependencies loose and not strict. |
@benbender thanks for the PR, I'm gonna close it due to npm 7 still having issues they need to work out, but will revisit in the future. |
@lifeiscontent could you elaborate on to which issues you refer? As far as I understand, the disruptions done with the stricter peer dependency-handling in npm 7 are intentional and, to be honest, just a result of poor practices in the web-community handling those. So im surprised that you seem to expect some sort of change I might have missed. |
@benbender nothing to do with your code, but npm itself. npm/cli#3606 there's a ton of issues with npm 7 and until it actually feels usable I don't plan to support it. |
Thinks for the pointers! Additionally I don't think that strict dependency-handling is a bug (unlike others you listed). So I would like to see my proposed fix to be merged anyways. But as also said earlier: your package, your decision ;) |
Atm if I install
storybook-addon-next-router
on a project on sb@next, npm yells the following error at me because the restrictions on the peer-dependencies are too tight.This patch fixes the problem.