Skip to content
This repository has been archived by the owner on Jan 31, 2024. It is now read-only.

Widen peer dependency range to include prereleases #26

Merged
merged 2 commits into from
Jun 3, 2021

Conversation

IanVS
Copy link
Member

@IanVS IanVS commented May 28, 2021

Fixes #25

This uses a more liberal >= semver range for peer dependencies, to allow installations with pre-release versions of storybook (and react for that matter) with npm 7. It's also just good practice to be as open as possible with peer dependencies, to avoid problems like reach-router has right now (reach/router#432 (comment))

📦 Published PR as canary version: 0.0.17--canary.26.e11efee.0

✨ Test out this PR locally via:

npm install @storybook/testing-react@0.0.17--canary.26.e11efee.0
# or 
yarn add @storybook/testing-react@0.0.17--canary.26.e11efee.0

@IanVS IanVS requested a review from yannbf May 28, 2021 00:13
@yannbf
Copy link
Member

yannbf commented May 28, 2021

Hey @IanVS thanks for your contribution! However I'm not sure if this is the best approach. This would open up this library to have possible version mismatches with future React/Storybook versions without proper support being added first.

Also, in https://semver.npmjs.com/ I see that >=6.0.0 does not take prereleases into account, so I believe this would not fix the problem.

In the yarn docs you can see this explanation about ranges and prereleases:
image

So I think the only way to support alphas/betas would be to add a range like this, and keep updating it with subsequent minor versions:

^6 || >=6.0.0-0 || 6.1.0-0 || >=6.2.0-0 || >=6.3.0-0

I'm not sure it's the best approach as well. @shilman what would you suggest? It's not nice that this lib breaks people's code when they're trying out Storybook prerelease versions

@IanVS
Copy link
Member Author

IanVS commented May 28, 2021

Ugh, why are peer dependencies so awful? Is there really no good way to do this? I see that of course you're correct, when experimenting with https://semver.npmjs.com/. It sounds like this nightmare is "working as intended" npm/cli#3135

@shilman
Copy link
Member

shilman commented Jun 1, 2021

@yannbf that proposal looks pretty good to me!

@IanVS
Copy link
Member Author

IanVS commented Jun 1, 2021

It feels kind of wrong, but to avoid having to update this package every time there's a new storybook version, I wonder if that pattern could even be extended out into the future, with something like:

^6 || >=6.0.0-0 || >=6.1.0-0 || >=6.2.0-0 || >=6.3.0-0 || >=6.4.0-0 || >= 6.5.0-0 

Interestingly, ^6 || >=6.3.0-0 does not seem to include the 6.3.0 prerelease tags on https://semver.npmjs.com, but the others like ^6 || >=6.2.0-0 show up correctly. I wonder if it's a bug on that site, or really some kind of strange resolution issue.

@IanVS
Copy link
Member Author

IanVS commented Jun 3, 2021

@yannbf I've adopted your proposal here.

@yannbf
Copy link
Member

yannbf commented Jun 3, 2021

@yannbf I've adopted your proposal here.

Awesome! Could you test the canary version that was generated to see if it fixes the problem for you?

@IanVS
Copy link
Member Author

IanVS commented Jun 3, 2021

Yes, I have confirmed that 0.0.17--canary.26.e11efee.0 does indeed install correctly with npm@7, whereas trying to install 0.0.16 fails.

@yannbf
Copy link
Member

yannbf commented Jun 3, 2021

Yes, I have confirmed that 0.0.17--canary.26.e11efee.0 does indeed install correctly with npm@7, whereas trying to install 0.0.16 fails.

Awesome! Thanks a lot for your contribution, Ian!

@yannbf yannbf merged commit 2cb9104 into main Jun 3, 2021
@yannbf yannbf deleted the 25-peer-dependencies branch June 3, 2021 16:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Peer dependency failure with beta storybook
3 participants