Addon-docs: Vue dependencies cause npm warnings on npm install when not using Vue

[vdh]

Describe the bug

npm WARN @egoist/vue-to-react@1.1.0 requires a peer of vue@^2.6.10 but none is installed. You must install peer dependencies yourself.
npm WARN vue-docgen-loader@1.1.0 requires a peer of vue-dogen-api@^3.26.0 but none is installed. You must install peer dependencies yourself.

To Reproduce
Setup Storybook with addon-docs with a different framework (e.g. React)

Expected behavior
Maybe there needs to be separate packages for Vue / each framework support?

System:
Please paste the results of npx -p @storybook/cli@next sb info here.

Environment Info:

  System:
    OS: macOS 10.15
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
  Binaries:
    Node: 10.15.3 - ~/.nvm/versions/node/v10.15.3/bin/node
    Yarn: 1.19.0 - /usr/local/bin/yarn
    npm: 6.13.0 - ~/.nvm/versions/node/v10.15.3/bin/npm
  Browsers:
    Chrome: 78.0.3904.108
    Safari: 13.0.2
  npmPackages:
    @storybook/addon-actions: ^5.3.0-alpha.40 => 5.3.0-beta.0
    @storybook/addon-docs: ^5.3.0-alpha.40 => 5.3.0-beta.0
    @storybook/addon-links: ^5.3.0-alpha.40 => 5.3.0-beta.0
    @storybook/addon-storyshots: ^5.2.6 => 5.2.6
    @storybook/addons: ^5.3.0-alpha.40 => 5.3.0-beta.0
    @storybook/react: ^5.3.0-alpha.40 => 5.3.0-beta.0

[shilman]

@ndelangen @Hypnosphi @tmeasday any idea what to do about this? don't want to release separate versions of docs per framework...

[Hypnosphi]

@shilman move @egoist/vue-to-react to dependencies of @storybook/vue?

[shilman]

@Aaron-Pool WDYT? ☝️

[Aaron-Pool]

Yeah, that should be fine. Although, of course, it means non-doc users will be installing a dependency they never use 🤷‍♂️

[Hypnosphi]

non-doc users will be installing a dependency they never use 🤷‍♂️

Right now it's the same with @storybook/react and babel-plugin-react-docgen

[Aaron-Pool]

True, seems like a problem worth solving eventually, perhaps. But probably not a big issue.

[stale]

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 30 days. Thanks!

[stale]

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!

[tx-nestor]

npm WARN @egoist/vue-to-react@1.1.0 requires a peer of vue@^2.6.10 but none is installed. You must install peer dependencies yourself.

Let me get this straight: I don't use vue (I use React), but I want to use Storybook docs addon, so I'm forced to install vue. Is this correct?

[Aaron-Pool]

@tx-nestor that is incorrect. It is, in fact, the inverse of that. Vue users that want to use storybook at all are forced to install react (but only as a dev dependency).

[tx-nestor]

Thanks for the speedy response @Aaron-Pool ! Much appreciated.

Unfortunately, that doesn’t seem accurate or align with what we’re seeing in our project.

We do not use Vue in our application at all. We use React and Storybook React and have been doing so without issue for a long time, until we installed the storybook docs add on.

At that point, we started seeing errors from npm suggesting that vue is required by vue-to-react. The later of which is a dependency of storybook docs.

I’m not in front of a computer at the moment, but I will try to create a reference/sample repo I can point you to, so you can see for yourself or at least clarify what I’m doing wrong.

Thanks again for you consideration.

[shilman]

@tx-nestor that's a warning not an error. 😉 we're still working out the package structure, but feel free to ignore that warning for now.

[Aaron-Pool]

@tx-nestor Correct! It's throwing that warning because vue is a peer dependency of vue-to-react and it (vue) isn't installed. So, yes, there are some vue-related dependencies that are installed with addon-docs, but they're relatively small and it certainly isn't the main vue library. Unless I'm mistaken somehow 🤷‍♂

[tx-nestor]

Hello,

So, after installing @storybook/addon-docs, I see the following when doing npm list

npm ERR! peer dep missing: vue@^2.6.10, required by @egoist/vue-to-react@1.1.0

That's not a warning. That's an error -- npm stating there is an unmet dependency.

If I print out the dependency tree, I see this...

├─┬ @storybook/addon-docs@5.3.13
│ ├── @egoist/vue-to-react@1.1.0
│ ├── UNMET PEER DEPENDENCY vue@^2.6.10

So, @storybook/addon-docs has a hard dependency on @egoist/vue-to-react, which has a peer dependency on vue. As such, via the peer dependency, @storybook/addon-docs effectively has a dependency on vue which it does not satisfy on its own and therefore is being pushed up to the end-user of @storybook/addon-docs.

Therefore, as a pure React user, if I want to use @storybook/addon-docs, I'm forced to install the core vue framework into my node_modules. Moreover, the vue package does in fact seem to be the core framework.

I completely understand if you're still figuring out the intricacies of the package dependencies and I'm not necessarily saying this needs to be fixed ASAP. But, at minimum, I think this issue should be reopened as I think it's a serious problem for users to have to install the entirety of vue when their application and/or UI kit doesn't use it.

[shilman]

@tx-nestor you don't need to install "the entirety of vue". or any part of vue at all, for that matter.

[tx-nestor]

@shilman I think there's a misunderstanding here.

Indeed, this is not breaking Storybook itself. Storybook still runs without installing vue. So, if you're a pure React user, you don't necessarily need to install vue to get Storybook to work properly. Sorry I didn't make that clear.

While that is the case, that's not the issue I'm trying to raise here.

This is a matter of breaking the integrity of the npm dependency tree. Generally speaking, if a package introduces a runtime (non-dev) dependency (as @storybook/addon-docs has done here with @egoist/vue-to-react), it is responsible for satisfying all of that dependency's peer dependencies by either installing it as its own runtime dependency or declaring it a peerDependency (in its own package.json). @storybook/addon-docs does neither.

We're using hundreds of other packages in our project, none of them break the integrity of the dependency tree except for @storybook/addon-docs. Before installing @storybook/addon-docs, an npm list shows all dependencies have been met. After installing @storybook/addon-docs, an npm list shows that there are unmet peer dependencies introduced by @storybook/addon-docs via @egoist/vue-to-react.

I suppose whether or not one thinks this is an issue is subjective. But, I feel strongly this breaks the intended use/behavior of npm package dependencies. I can only hope the community agrees. If not, we'll work around it -- maybe we'll even begrudgingly install vue to fix the dep tree because we do appreciate the power of having Storybook docs.

[Hypnosphi]

satisfying all of that dependency's peer dependencies by either installing it as its own runtime dependency

It's impossible. Peer dependencies can only be satisfied on end app level

[tx-nestor]

@Hypnosphi It’s not impossible. Peer dependencies can be satisfied at any point in the hierarchy in which they are introduced. The issue is @storybook/addon-docs neither satisfies the vue dependency itself nor does it specify it as a peerDependency. It just ignores it and let’s the end user deal with it (“end app level”, as you called it). Which is annoying because my project doesn’t use vue at all — yet I would have to install it if I want to maintain a fully satisfied dependency graph/tree. Perhaps people just don’t care about this as much as we do at our company — and that’s fine.

I’m willing to concede this isn’t a problem with Storybook itself. But it is a problem with how they’re introducing dependencies in this particular add-on.

[Hypnosphi]

Peer dependencies can be satisfied at any point in the hierarchy in which they are introduced

Here's why it's not true:
npm/npm#19944
npm/npm#19877

[Aaron-Pool]

@tx-nestor let's shift the conversation a bit from who's right or wrong here. What would you propose we do to fix the problem? The only solution I see is to divide the docs add-on into a ton of framework specific packages (like the core storybook app), but that's going to introduce a lot of overhead and headache to the maintenance burden for the add-on, and I don't think the team is willing to do that just to preserve the purity of the npm dependency tree.

Is that the solution you had in mind? Or did you have a simpler alternative to propose?

[tx-nestor]

@Aaron-Pool Yeah, sorry, I wasn’t trying to prove I’m right, but rather express my concern for this issue (which was closed).

The more I read into it (thanks for links @Hypnosphi), the more I’m uncovering that npm’s dependency graph and resolution is complex and imperfect (particularly re: their peer and optional dependency feature).

Re: how to fix...

The only solution I see is to divide the docs add-on into a ton of framework specific packages (like the core storybook app), but that's going to introduce a lot of overhead and headache to the maintenance burden for the add-on

Yes, that may be the only way to resolve this given the way npm dependencies work at the moment.

FWIW, I think the Storybook team has done a great job with figuring out how to support multiple frameworks — honestly, might be one of the best case studies on this sort of thing I’ve seen.

Would you need to create a separate docs add on for every framework upfront? Or, could you just create one for any framework that breaks the mold? Also, I see Storybook is using Lerna — Does that not eliminating the overheard of introducing new packages under the @storybook scope?

Humor me for a minute, for my wild idea of the day... Could you potentially solve this problem with dynamic imports against a Storybook CDN? Or, maybe even just unpkg or cdnjs. As opposed to relying on static bundling.

[Hypnosphi]

We could also make @egoist/vue-to-react an optional peer dependency in 6.0. That way, you'd have to install it on your side, but only if you actually use vue

@shilman WDYT?

[Hypnosphi]

Also, this seems like a viable option, not requiring a breaking change: #8936 (comment)

[Aaron-Pool]

Yeah, I like moving it to the storybook/Vue package over making it a peer dependency. I think peer dependencies are confusing for most end users and should only be used in cases that really necessitate it.

[shilman]

If we move vue-to-react into @storybook/vue, I think we're simply trading off one set of evils for another (I'm OK with some evil--AFAIK we're hitting up against some fundamental limitations of NPM):

1. We're moving the size burden of the vue-to-react install from "all Docs users" (regardless of whether they use vue), to "all Vue users". Since I want everybody to use Docs, I'm OK with this. However, in general, there will be many more addons than frameworks, so it's not a good general solution.

2. AFAIK, a package needs to depend on another package to use it directly. Otherwise there will be linting errors that we'll need to ignore. We'd fix one form of "integrity error" but introduce another.

So I think we actually need both "optional peer deps" (to deal with point 2) AND to move vue-to-react to @storybook/vue. Will merge a PR with this change if everybody agrees this is a step forward.

[Hypnosphi]

@shilman @storybooks/vue can re-export what's needed from vue-to-react to deal with 2. Or we could even move the entire prepareForInline definition to @storybooks/vue

[tomdev10]

Thanks @shilman - confirming I'm happy with the approach above after reading this thread. Fully agree in it's current form it's at best confusing and at worse poor practise. Thanks for all thea activity and willingness to fix!

[ndelangen]

@tomdev10 Would you be able to contribute the fix perhaps?

[stale]

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 30 days. Thanks!

[stale]

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!

[stale]

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 30 days. Thanks!

[dfernandez-asapp]

Perhaps this issue can be resolved by using peerDependenciesMeta?

See npm/cli#224

[tx-nestor]

@dfernandez-asapp That's interesting. I have not used it before but worth a shot.

cc: @shilman @Hypnosphi Any idea if this would work and resolve this problem?

[shilman]

@tx-nestor yes, agree it looks promising. my current thinking is to put all framework-specific dependencies in the framework package (e.g. vue-docgen-api => @storybook/vue) and then make it an optional peer dependency of @storybook/addon-docs. i'm under water with 6.0
blockers, but would gladly take a PR for this.

[tx-nestor]

@shilman No worries re: v6 blockers.

I've pretty swamped myself. BUT, I think I can spare some time do some experimenting locally by hacking with node_modules and seeing how npm behaves. I can also publish a package to our internal private package repository and see if the peerDependenciesMeta works as documented. If my experiments look promising, I'll open an official PR in this repo.

[shilman]

FWIW we're using this successfully in Storybook already, but I didn't connect the dots until @dfernandez-asapp 's comment:

https://github.com/storybookjs/storybook/blob/next/addons/storyshots/storyshots-puppeteer/package.json

[stale]

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 30 days. Thanks!

[shilman]

Son of a gun!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.0.0-rc.22 containing PR #11759 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

Closing this issue. Please re-open if you think there's still more to do.

[tx-nestor]

@shamin Confirmed fixed when I install the v6 release candidate. Thanks SO MUCH!

Can't wait to see what else is in store for Storybook v6!

[shilman]

FYI this has been fixed properly in https://github.com/storybookjs/storybook/releases/tag/v6.5.0-alpha.48

To test it out:

npx sb upgrade --prerelease