webpack and its plugins should be peer dependencies of @storybook/builder-webpack5 #26825
-
Describe the bugCurrently, many of webpack-related dependencies are marked as direct dependencies of @storybook/builder-webpack5. This causes a problem with consumers of this package that already have their own version of these packages. To ReproduceHave a Webpack project with your own SystemStorybook Environment Info:
System:
OS: macOS 14.3
CPU: (10) arm64 Apple M1 Max
Shell: 3.6.1 - /opt/homebrew/bin/fish
Binaries:
Node: 18.18.2 - ~/Library/Caches/fnm_multishells/65622_1701365893085/bin/node
npm: 9.8.1 - ~/Library/Caches/fnm_multishells/65622_1701365893085/bin/npm
pnpm: 8.14.3 - ~/Library/Caches/fnm_multishells/65622_1701365893085/bin/pnpm <----- active
Browsers:
Chrome: 123.0.6312.105
Edge: 123.0.2420.65
Safari: 17.3
npmPackages:
@storybook/addon-essentials: 7.6.17 => 7.6.17
@storybook/addons: 7.6.17 => 7.6.17
@storybook/api: 7.6.17 => 7.6.17
@storybook/components: 7.6.17 => 7.6.17
@storybook/core-events: 7.6.17 => 7.6.17
@storybook/react: 7.6.17 => 7.6.17
@storybook/react-webpack5: 7.6.17 => 7.6.17
@storybook/testing-library: 0.2.2 => 0.2.2
@storybook/theming: 7.6.17 => 7.6.17
storybook: 7.6.17 => 7.6.17 Additional context |
Beta Was this translation helpful? Give feedback.
Replies: 4 comments
-
Hi @mxdvl Thank you very much for raising this issue. In a less complex world, I would definitely agree with you. Today, though, the frontend development landscape is pretty fragmented, and this also applies to the number of package managers and their different behaviors. Declaring all webpack-related dependencies as peer dependencies only works if users have already installed webpack. In more restricted modes, like yarn pnp, they have to be even direct dependencies of the user instead of transitive ones, which meta frameworks like Next.js may bring. If the user still needs to install them as a direct dependency, the package managers decide whether they are automatically installed. Some do this by default (npm), and some don't (yarn berry). Your problem currently seems to be that The same applies in this case:
If all that doesn't help, you can always regenerate your lock files by deleting them and executing a fresh install. Then you should have I hope this helps. I will close this issue since we are not planning to declare webpack-related dependencies as peer dependencies. If you still don't agree with me, let me know, and then we can discuss some solutions that cover the above-mentioned problems. |
Beta Was this translation helpful? Give feedback.
-
Thanks for the detailed response @valentinpalkovic. First off, I want to reassure you that we have addressed the CVE issue and only mentioned it because it was how we discovered that Storybook declares these peer dependencies as direct. I am very disappointed that you team chooses to take an incorrect approach to dependencies management due to the fragmentation of the JS ecosystem. If the major players do not follow the rules, who will? I am well aware that different package managers work differently, but I would wager that this is a consumer's choice and trying to address their quirks is not the direct concern of Storybook. While I'm sure you've balanced this choice carefully with your user base, I think it's important for your team to be aware that there are consumers who would prefer the correct approach. Lastly,
We've chosen option 2 due to our use of |
Beta Was this translation helpful? Give feedback.
-
Thank you for taking the time to give a lengthy answer! Let's hypothetically assume that all Storybook webpack-related dependencies are declared as peer-dependencies. I would love to go through some scenarios with you and see how, from a user perspective, the experience with Storybook might be: Init Storybook into a Next.js projectNext.js doesn't provide On the other hand, though, I definitely see inconsistencies in Storybook itself. For the Vite builder, we define Vite and its related packages as peer dependencies. But we are only talking about Allow multiple major versions of
|
Beta Was this translation helpful? Give feedback.
-
Thank you so much for the further insight into how such a small change can have wide-ranging repercussions. You've given a very detailed answer and I can see how my comment may come as short or rude. I apologise for the tone and want to reassure you that apart from this small annoyance my experience with Storybook and its team has been very positive.
Balancing priorities is hard and as you mentioned the ecosystem is quite fragmented. It does seem that your are restricted by Next's decision to equally not declare I cannot speak for other repositories and would love to know if there are any other developers out there that would want the peer dependencies to be “correct”. If not, this is a clear indication that your current decision was the best 😉 |
Beta Was this translation helpful? Give feedback.
Hi @mxdvl
Thank you very much for raising this issue.
In a less complex world, I would definitely agree with you. Today, though, the frontend development landscape is pretty fragmented, and this also applies to the number of package managers and their different behaviors.
Declaring all webpack-related dependencies as peer dependencies only works if users have already installed webpack. In more restricted modes, like yarn pnp, they have to be even direct dependencies of the user instead of transitive ones, which meta frameworks like Next.js may bring.
If the user still needs to install them as a direct dependency, the package managers decide whether they are automatically installed. Some d…