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

Peer dependency warnings #2303

Closed
pelotom opened this issue Nov 14, 2017 · 28 comments
Closed

Peer dependency warnings #2303

pelotom opened this issue Nov 14, 2017 · 28 comments
Assignees
Labels
cleanup Minor cleanup style change that won't show up in release changelog dependencies help wanted inactive maintenance User-facing maintenance tasks

Comments

@pelotom
Copy link
Contributor

pelotom commented Nov 14, 2017

Currently I'm getting a bunch of warnings due to storybook when I install dependencies:

> yarn
yarn install v1.3.2
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
warning " > @storybook/addon-storyshots@3.2.15" has unmet peer dependency "@storybook/addons@^3.2.15".
warning " > @storybook/addon-storyshots@3.2.15" has unmet peer dependency "@storybook/channels@^3.2.15".
warning " > @storybook/addon-storyshots@3.2.15" has unmet peer dependency "babel-core@^6.26.0".
warning "@storybook/react > babel-preset-react-app@3.1.0" has unmet peer dependency "babel-runtime@^6.23.0".
warning "@storybook/react > @storybook/ui > react-komposer@2.0.0" has incorrect peer dependency "react@^0.14.7 || ^15.0.0".
warning "@storybook/react > @storybook/ui > mantra-core > react-komposer@1.13.1" has incorrect peer dependency "react@^0.14.3 || ^15.0.0".
warning "@storybook/react > @storybook/ui > mantra-core > react-simple-di@1.2.0" has incorrect peer dependency "react@^0.14.0 || ^15.0.0".
warning "@storybook/react > @storybook/ui > react-icons > react-icon-base@2.1.0" has unmet peer dependency "prop-types@*".
warning "@storybook/react > @storybook/ui > react-komposer > react-stubber@1.0.0" has incorrect peer dependency "react@^0.14.7 || ^15.0.0".

A few of these are due to react-komposer and react-simple-di not allowing React 16 as a peer dependency. These projects each have multiple open issues and PRs to fix these peer dependencies, but no one is responding; they seem to be unmaintained. Would it be possible to either transfer ownership of these projects under storybooks, or else inline needed functionality that they provide?

The others I'm not sure why they are necessary. Why do I need to add a dependency on babel-core, for example, when I used TypeScript exclusively and storybook functions just fine? Perhaps some of these things should just be dependencies/devDependencies rather than peerDependencies.

@danielduan danielduan added dependencies maintenance User-facing maintenance tasks labels Nov 14, 2017
@danielduan
Copy link
Member

danielduan commented Nov 14, 2017

I'm not sure that we should be maintaining mantra and react-komposer but I'm all for removing these peer dependency warnings.

  1. We could fork it and publish our own version.
  2. Migrate to something different or refactor to get rid of them.
  3. Anything else?

@Hypnosphi you've been dealing with a lot of dependencies. Thoughts?

@pelotom pelotom changed the title Remove dependencies on unmaintained packages Fix peer dependency warnings about React 16 Nov 14, 2017
@pelotom pelotom changed the title Fix peer dependency warnings about React 16 Fix peer dependency warnings Nov 14, 2017
@pelotom pelotom changed the title Fix peer dependency warnings Peer dependency warnings Nov 14, 2017
@Hypnosphi
Copy link
Member

I'd vote for replacing komposer/podda/mantra stack with redux. @alexandrebodin even started doing it in a branch some time ago

@Hypnosphi
Copy link
Member

I'm still intrigued about addon-storishot peer warnings. There's a chance it relies on having other addons installed

@danielduan
Copy link
Member

danielduan commented Nov 14, 2017

I think a good temporary measure would be to fork the old mantra, komposer, podda repos over and publish a React 16 version with minor bug fixes. I'd be willing to do it if it's something we want.

@Hypnosphi
Copy link
Member

@danielduan yeah, sounds like a great plan. Actually you can start with just changing their peerDeps, given that things seem to work well with React 16 as is

@ndelangen
Copy link
Member

ndelangen commented Nov 15, 2017

I'd love to be able to push out some updates of those packages, unfortunately some appear abandoned.

@danielduan
Copy link
Member

We should still investigate the storyshots dependencies though.

@pelotom
Copy link
Contributor Author

pelotom commented Nov 16, 2017

Nice work! Now down to these warnings:

> yarn
yarn install v1.3.2
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
warning " > @storybook/addon-storyshots@3.2.16" has unmet peer dependency "@storybook/addons@^3.2.16".
warning " > @storybook/addon-storyshots@3.2.16" has unmet peer dependency "@storybook/channels@^3.2.16".
warning " > @storybook/addon-storyshots@3.2.16" has unmet peer dependency "babel-core@^6.26.0".
warning "@storybook/react > babel-preset-react-app@3.1.0" has unmet peer dependency "babel-runtime@^6.23.0".
warning "@storybook/react > @storybook/ui > react-icons > react-icon-base@2.1.0" has unmet peer dependency "prop-types@*".
[4/4] 📃  Building fresh packages...
success Saved lockfile.
✨  Done in 8.88s.

@danielduan
Copy link
Member

danielduan commented Nov 16, 2017

I get that peer dependencies reduce the amount of packages installed, but I don't think the rest of these need to be. React is one thing because you can't share an instance of react or else it will cause problems.

  • React icons is outside our control, maybe we can just include prop-types with ui
  • Babel preset react app is outside our control too, but I believe babel-core is installed somewhere so it shouldn't really be a warning.
  • Investigate storyshots because I think those can just be installed as regular dependencies.

@Hypnosphi
Copy link
Member

maybe we can just include prop-types with ui
We do, but looks like it's not enough. Maybe it's a bug in yarn

As for @storybook/addons, I think it actually should be a peer dependency in all the addons. It's already a direct dependency in all the apps, and you can't use addons without an app anyway. So it shouldn't break anything actually. And with @storybook/addons we kinda have the same situation as with React: we really need that the addons and the app share the same instance of it to prevent the "nonexistent channel" error.

@danielduan
Copy link
Member

Is there anything we can do about the babel ones? If not, we can just close this ticket and say the rest of those can't really be fixed.

@Hypnosphi
Copy link
Member

We already have babel-core@6.26.0 as @storybook/react's direct dependency, so looks like it's the same issue as with prop-types

@Hypnosphi
Copy link
Member

BTW, here is the relevant (I think) yarn bug: yarnpkg/yarn#2688 (comment)

@danielduan
Copy link
Member

I wonder if a transitive dependency counts as a peer dependency in yarn's definition in general.

I'm gonna close this ticket so we can worry about more important things. This issue is not 100% fixed, but good enough I think. If anyone has a fix and would like to help out, please comment and open a PR.

@pelotom
Copy link
Contributor Author

pelotom commented Nov 16, 2017

Can these just be removed from peerDependencies please? I'm not really seeing the downside of that. Otherwise it puts downstream users in the position of either having to add a bunch of irrelevant dependencies at their top level or just living with spurious warnings (which could cause useful warnings to go unnoticed).

@Hypnosphi
Copy link
Member

Hypnosphi commented Nov 17, 2017

@pelotom which of them do you mean exactly? We’re only in control of the ones in @storybook/storyshots, and I believe all of them are relevant

@pelotom
Copy link
Contributor Author

pelotom commented Nov 18, 2017

@Hypnosphi I believe you that they're relevant, but can't they just be regular dependencies? I'd rather not pollute the top-level dependencies of my app with things that only storybook needs.

@Hypnosphi
Copy link
Member

Hypnosphi commented Nov 18, 2017

We really need a single instance of @storybook/addons shared between app and addons, that's why we're going to make it a peerDep in more places: #2335 .
I'll investigate what we can do with @storybook/channels and babel-core though

@Hypnosphi Hypnosphi reopened this Nov 18, 2017
@Hypnosphi Hypnosphi added the cleanup Minor cleanup style change that won't show up in release changelog label Nov 18, 2017
@davidhouweling
Copy link

Also related would be the use of react-transition-group (present libraries are using version 1, whereas version 2 is available).

@Hypnosphi Hypnosphi assigned Hypnosphi and unassigned danielduan Dec 3, 2017
@pelotom
Copy link
Contributor Author

pelotom commented Jan 10, 2018

Any news on this front? I've added a dependency on @storybook/addons which makes sense for the reason given above, but I still have these:

warning " > @storybook/addon-storyshots@3.3.7" has unmet peer dependency "babel-core@^6.26.0 || ^7.0.0-0".
warning " > @storybook/react@3.3.7" has unmet peer dependency "babel-core@^6.26.0 || ^7.0.0-0".
warning "@storybook/react > babel-loader@7.1.2" has unmet peer dependency "babel-core@6 || 7 || ^7.0.0-alpha || ^7.0.0-beta || ^7.0.0-rc".
warning "@storybook/react > babel-preset-react-app@3.1.0" has unmet peer dependency "babel-runtime@^6.23.0".
warning "@storybook/react > @storybook/ui > react-icons > react-icon-base@2.1.0" has unmet peer dependency "prop-types@*".

I'm currently on @storybook/*@3.3.7.

@Hypnosphi
Copy link
Member

Hypnosphi commented Jan 10, 2018

^6.26.0 || ^7.0.0-0

Actually this is the reason why we need babel-core to be a peer dep: to be compatible with different versions

@stale
Copy link

stale bot commented Feb 24, 2018

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 60 days. Thanks!

@stale stale bot added the inactive label Feb 24, 2018
@stale
Copy link

stale bot commented Mar 11, 2018

Hey there, it's me again! I am going close this issue to help our maintainers focus on the current development roadmap instead. If the issue mentioned is still a concern, please open a new ticket and mention this old one. Cheers and thanks for using Storybook!

@Laslo89
Copy link

Laslo89 commented Jun 14, 2019

it would be nice, if you could clean the peer dependencies again :)
btw. i am not using react obviously

warning "@storybook/addon-a11y > @storybook/components@5.1.7" has unmet peer dependency "react-dom@*".
warning "@storybook/addon-a11y > @storybook/theming@5.1.7" has unmet peer dependency "react-dom@*".
warning "@storybook/addon-a11y > react-sizeme@2.6.7" has unmet peer dependency "prop-types@^15.0.0-0".
warning "@storybook/addon-a11y > react-sizeme@2.6.7" has unmet peer dependency "react-dom@^0.14.0 || ^15.0.0-0 || ^16.0.0".
warning "@storybook/addon-notes > markdown-to-jsx@6.10.2" has unmet peer dependency "react@>= 0.14.0".
warning "@storybook/addon-a11y > @storybook/api > @storybook/router@5.1.4" has unmet peer dependency "react-dom@*".
warning "@storybook/addon-a11y > @storybook/api > @storybook/router > @reach/router@1.2.1" has unmet peer dependency "react-dom@15.x || 16.x || 16.4.0-alpha.0911da3".
warning "@storybook/addon-a11y > @storybook/api > @storybook/theming@5.1.4" has unmet peer dependency "react-dom@*".
warning " > @storybook/addon-console@1.1.0" has unmet peer dependency "prop-types@*".
warning " > @storybook/addon-console@1.1.0" has unmet peer dependency "react@*".
warning " > @storybook/addon-knobs@5.1.7" has unmet peer dependency "react@*".
warning "@storybook/addon-knobs > react-select@2.4.4" has unmet peer dependency "react@^15.3.0 || ^16.0.0".
warning "@storybook/addon-knobs > react-select@2.4.4" has unmet peer dependency "react-dom@^15.3.0 || ^16.0.0".
warning "@storybook/addon-links > @storybook/router@5.1.7" has unmet peer dependency "react@*".
warning "@storybook/addon-links > @storybook/router@5.1.7" has unmet peer dependency "react-dom@*".
warning "@storybook/addon-knobs > react-color > @icons/material@0.2.4" has unmet peer dependency "react@*".
warning "@storybook/addon-knobs > react-select > react-input-autosize@2.2.1" has unmet peer dependency "react@^0.14.9 || ^15.3.0 || ^16.0.0-rc || ^16.0".
warning "@storybook/addon-knobs > react-select > react-transition-group@2.9.0" has unmet peer dependency "react@>=15.0.0".
warning "@storybook/addon-knobs > react-select > react-transition-group@2.9.0" has unmet peer dependency "react-dom@>=15.0.0".
warning " > @storybook/addon-links@5.1.7" has unmet peer dependency "react@*".
warning " > @storybook/addon-notes@5.1.7" has unmet peer dependency "react@*".
warning " > @storybook/addon-options@5.1.7" has unmet peer dependency "react@*".
warning " > @storybook/addon-viewport@5.1.7" has unmet peer dependency "react@*".
warning "@storybook/vue > @storybook/core@5.1.7" has unmet peer dependency "react@*".
warning "@storybook/vue > @storybook/core@5.1.7" has unmet peer dependency "react-dom@*".

@Hypnosphi
Copy link
Member

Hypnosphi commented Jun 14, 2019

I am not using react obviously

But Storybook UI (stories tree, addon panels) does. Please install the missing dependencies on your side as devDependencies (we probably should include it in documentation and CLI scripts, feel free to open a separate issue or PR for that)

@esr360
Copy link

esr360 commented Feb 17, 2020

I am not using react obviously

But Storybook UI (stories tree, addon panels) does. Please install the missing dependencies on your side as devDependencies (we probably should include it in documentation and CLI scripts, feel free to open a separate issue or PR for that)

If Storybook UI requires React, and can be used on projects that are not React, shouldn't it be added as a dependency not a peer dependency? Or perhaps an optional dependency?

@Hypnosphi
Copy link
Member

Actually, right now it is:
https://github.com/storybookjs/storybook/blob/next/lib/ui/package.json#L52-L53

@ndelangen @shilman do you think that we can remove peer dependencies on React from manager-side packages, given that manager uses its own verion of React anyway?

@ndelangen
Copy link
Member

YES

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Minor cleanup style change that won't show up in release changelog dependencies help wanted inactive maintenance User-facing maintenance tasks
Projects
None yet
Development

No branches or pull requests

7 participants