search

storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
84.22k stars 9.26k forks source link

Unmet peer dependency warnings for @storybook/addon-storyshots@6.2.9 when running `yarn install` #14758

Closed alinalobast closed 3 years ago

alinalobast commented 3 years ago

When running yarn install we are getting unmet peer dependency warnings:

 warning "workspace-aggregator-... > ... > @storybook/addon-storyshots@6.2.9" has unmet peer dependency "svelte@*".
 warning "workspace-aggregator-... > ... > @storybook/addon-storyshots@6.2.9" has unmet peer dependency "vue-jest@*".
 warning "workspace-aggregator-... > ... > @storybook/addon-storyshots > react-test-renderer@17.0.2" has incorrect peer dependency "react@17.0.2".

We are using the following devDependencies in our repo:

        "storybook": "^6.2.8",
        "@storybook/addon-a11y": "^6.2.8",
        "@storybook/addon-essentials": "^6.2.8",
        "@storybook/addon-jest": "^6.2.8",
        "@storybook/addon-links": "^6.2.8",
        "@storybook/addon-storyshots": "^6.2.8",
        "@storybook/react": "^6.2.8"

as well as dependencies: 

        "react": "^16.13.1",
        "react-dom": "^16.13.1"

and also added "react-test-renderer": "^16.13.1" to try resolve one of the warnings, which did not help. Also, it is weird that it still asks forreact-test-renderer@17.0.2" and "react@17.0.2", while "react-test-renderer": "^16.13.1"and "react": "^16.13.1" have been clearly met.

Surely, the option is to add all the above as devDependencies of out package.json, but I do not think it's a viable option. There are similar issues that were discussed in yarn community https://github.com/yarnpkg/yarn/issues/5347

Would appreciate your advice on this, thanks!

shilman commented 3 years ago

@merceyz any chance you can weigh in on this one?

darekkay commented 3 years ago

Also, it is weird that it still asks for react-test-renderer@17.0.2" and "react@17.0.2", while "react-test-renderer": "^16.13.1"and "react": "^16.13.1" have been clearly met.

The caret notation ^16.13.1 will only match minor versions 16.x.x, so it does not fulfill the required "react@17.0.2" peer dependency. You would need to update your react dependency version to ^17.0.0.

If you indeed have React 16.x installed and storyshots works without any issues, then the question is whether it makes sense to lower React's peer dependency version in storyshots instead.

shilman commented 3 years ago

You could fix this by installing an earlier version of react-test-renderer. Storyshots has a dependency on:

     "react-test-renderer": "^16.8.0 || ^17.0.0",

So yarn installs the most recent version. But you could force it to use an earlier version that is compatible with your version of React.

alinalobast commented 3 years ago

Thank you all for replying!

You would need to update your react dependency version to ^17.0.0.

This is not an option for us, unfortunately, due to the usage of our package and reliance on something else that still has React v16. We tried migrating to v17 at some point, but this resulted in version conflicts.

You could fix this by installing an earlier version of react-test-renderer.

I just tried to change "react-test-renderer" to ^16.8.0, but I am still getting the following warnings:

@storybook/addon-storyshots@6.2.9" has unmet peer dependency "vue-jest@*".
@storybook/addon-storyshots > react-test-renderer@17.0.2" has incorrect peer dependency "react@17.0.2".
merceyz commented 3 years ago

@merceyz any chance you can weigh in on this one?

@shilman - https://github.com/storybookjs/storybook/issues/14758#issuecomment-829267290

Sure, I opened https://github.com/storybookjs/storybook/pull/14835 to deal with the dependencies that Storybook controls, not much can be done about the range specified by react-test-renderer here.

shilman commented 3 years ago

ZOMG!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.3.0-alpha.21 containing PR #14835 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

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

alinalobast commented 3 years ago

Thank you @shilman! I tried to install Storybook and Storyshots addons v6.3.0-alpha.21 and I am getting these 2 warnings:

@storybook/addon-storyshots > react-test-renderer@17.0.2" has incorrect peer dependency "react@17.0.2".
@storybook/addon-storyshots > preact-render-to-string@5.1.19" has unmet peer dependency "preact@>=10".

As per the conversation above, there's not much that can be done about the first one, but the second one is a new warning that started to appear... Does it mean we now need to install preact as a devDependency too? Is there a way to avoid this?

shilman commented 3 years ago

Man ... they just don't stop do they?!! PRs welcome! 🙏

merceyz commented 3 years ago

That one is already specified correctly, seems Yarn 1.x isn't able to detect it. Yarn 2+ handles it correctly

darekkay commented 3 years ago

That one is already specified correctly

Is it? It isn't specified in Storyshot's package.json. See my PR: #14932

merceyz commented 3 years ago

Yes, it's a normal dependency which has a peer dependency on preact which @storybook/addon-storyshots declares as an optional peer dependency

https://github.com/storybookjs/storybook/blob/330d800cd1ca8c4e30b99b6fe47dc0ae6d1e5784/addons/storyshots/storyshots-core/package.json#L60 https://github.com/storybookjs/storybook/blob/330d800cd1ca8c4e30b99b6fe47dc0ae6d1e5784/addons/storyshots/storyshots-core/package.json#L93 https://github.com/storybookjs/storybook/blob/330d800cd1ca8c4e30b99b6fe47dc0ae6d1e5784/addons/storyshots/storyshots-core/package.json#L126-L128

darekkay commented 3 years ago

Thanks for the explanation, this makes sense. Do you know if npm7 handles this use case this way, too?

If yarn 1 isn't handling it correctly (and it's probably not going to be fixed within yarn itself), my change could still be merged to prevent the warning, right?

merceyz commented 3 years ago

Do you know if npm7 handles this use case this way, too?

I tested npm@7.13.0 and it doesn't even install preact-render-to-string

npm install @storybook/addon-storyshots@6.3.0-alpha.21
ls -R node_modules/ | grep preact-render-to-string

cc @isaacs

my change could still be merged to prevent the warning, right?

preact-render-to-string isn't a peer dependency so it wouldn't help