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

fix(deps): add typescript as a peer dependency #189

Merged
merged 2 commits into from
Feb 12, 2025

Conversation

TomKalina
Copy link
Contributor

@TomKalina TomKalina commented Jan 28, 2025

What Changed

I had a problem with dependencies in my project using yarn 3 which I had to solve manually.
This should be the correct solution to the problem.

➤ YN0000: eslint-plugin-storybook@npm:0.11.2 [c8dbd] doesn't provide typescript, breaking the following requirements:

➤ YN0000: @typescript-eslint/typescript-estree@npm:8.21.0 [cab32] → >=4.8.4 <5.8.0 ✘
➤ YN0000: @typescript-eslint/utils@npm:8.21.0 [4cb6b]             → >=4.8.4 <5.8.0 ✘
➤ YN0000: ts-api-utils@npm:2.0.0 [8a201]                          → >=4.8.4        ✘

Checklist

Check the ones applicable to your change:

  • Ran pnpm run update-all
  • Tests are updated
  • Documentation is updated

Change Type

Indicate the type of change your pull request is:

  • maintenance
  • documentation
  • patch
  • minor
  • major

Sorry, something went wrong.

@Sidnioulz Sidnioulz added bug Something isn't working dependencies Update one or more dependencies version labels Feb 11, 2025
Copy link
Member

@Sidnioulz Sidnioulz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with the proposed change: TS ESLint utils already are a runtime dependency, and they have a peer dependency on TS, so it makes sense to hint to package managers that we are aware of this unmet dependency and are leaving it to upstream to satisfy it.

This being said, it seems to me that the peer dependency requirement should be >=4.8.4 <5.8.0 like for the utils package. The code we depend on declares it will work with TS 4.8.4, and we shouldn't force end users to upgrade to TS 5 for an ESLint plugin.

@TomKalina
Copy link
Contributor Author

I agree with the proposed change: TS ESLint utils already are a runtime dependency, and they have a peer dependency on TS, so it makes sense to hint to package managers that we are aware of this unmet dependency and are leaving it to upstream to satisfy it.

This being said, it seems to me that the peer dependency requirement should be >=4.8.4 <5.8.0 like for the utils package. The code we depend on declares it will work with TS 4.8.4, and we shouldn't force end users to upgrade to TS 5 for an ESLint plugin.

Yeah, good point. Fixed in cec50ec

@Sidnioulz
Copy link
Member

Thanks @TomKalina. I'll bring this one up at our next triage session to have it merged!

@kasperpeulen kasperpeulen merged commit 3e61b84 into storybookjs:main Feb 12, 2025
Copy link

🚀 PR was released in v0.11.3 🚀

@github-actions github-actions bot added the released This issue/pull request has been released. label Feb 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working dependencies Update one or more dependencies version released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants