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

@types/styled-components should be an optional peerDep and not a dependency #2894

Closed
mattcosta7 opened this issue Feb 11, 2023 · 2 comments · Fixed by #3399
Closed

@types/styled-components should be an optional peerDep and not a dependency #2894

mattcosta7 opened this issue Feb 11, 2023 · 2 comments · Fixed by #3399
Assignees
Labels
bug Something isn't working major release breaking changes react size: sand

Comments

@mattcosta7
Copy link
Collaborator

mattcosta7 commented Feb 11, 2023

Description

Since @types/styled-components is a dependency of @primer/react it's installed transitively, with a non-fixed version for consumers, which means that different manners of installing or instally the same packages at different times can resolve the version such that the types in a consuming application can break, even though they work fine for other installations.

In dotcom, we've recently noticed this https://github.com/github/github/pull/259218#discussion_r1103645591 where an explicit install of a library with a higher version dependency on types/styled-components, caused the resolution for all code to use that newer dep.

it would be clearer to force typescript consumers to take a direct dependency on @types/styled-components similar to what we do with styled-components to ensure that a consistent version can be resolved between various installs

Steps to reproduce

...

Version

latest

Browser

No response

@mattcosta7 mattcosta7 added the bug Something isn't working label Feb 11, 2023
@joshblack
Copy link
Member

Great point, adding this to https://github.com/github/primer/issues/1294 in case this requires a major release, semantically.

Would you say that a good rule of thumb here is that any package that we list as a peerDependency should also have its types listed there, as well? (In cases where they must be installed from DefinitelyTyped)

@mattcosta7
Copy link
Collaborator Author

Great point, adding this to github/primer#1294 in case this requires a major release, semantically.

Would you say that a good rule of thumb here is that any package that we list as a peerDependency should also have its types listed there, as well? (In cases where they must be installed from DefinitelyTyped)

I think that's probably a good rule of thumb in this case!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working major release breaking changes react size: sand
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants