emotion-js / emotion

👩‍🎤 CSS-in-JS library designed for high performance style composition
https://emotion.sh/
MIT License
17.43k stars 1.11k forks source link

Make types which use Theme generic #2452

Open avendiart opened 3 years ago

avendiart commented 3 years ago

The problem

The current approach of declaring theme types by overriding the internal emotion theme type definition doesn't scale beyond simple use cases. E.g. monorepos or applications which might be using two component libraries built with emotion with different theme type definition, which results in incorrect types and/or loss of static analysis benefits.

Proposed solution

Make types which use Theme internally generic. For example:

export type StyledTags<Theme = DefaultTheme> = {
  [Tag in keyof JSX.IntrinsicElements]: CreateStyledComponent<
    {
      theme?: Theme
      as?: React.ElementType
    },
    JSX.IntrinsicElements[Tag]
  >
}

Additional context

One consideration here is backwards compatibility. In general, generics support default values, as shown in the example above, with DefaultTheme being the internal Theme interface, which is currently being used to provide theme types, which means there is probably no need for a major version release.

iChenLei commented 3 years ago

https://github.com/emotion-js/emotion/pull/2352#issuecomment-821769442

feat: useTheme support typescript generic

Emotion maintainer opposed to this matter.

Andarist commented 3 years ago

A more type-safe way (but not completely) to achieve this would be to namespace your themes in the theme object and implement custom hooks for each one that would select that "slice" of a theme. In that way, mistakes could only be made by forgetting to include appropriate providers but not at the consuming site of things - which seems like a big risk when doing refactorings.

avendiart commented 3 years ago

I don't see this approach working for design systems / component libraries. Ideally, there would be a way to create an own instance of styled tags and have a way to provide the context which should be used for theme access. This way both the context provider and theme could be properly typed and not interfere with other libraries or components within a given application.

Andarist commented 2 years ago

Yes, mixing different libraries using the same theming object is a problem and a shortcoming of the styled API. I think it's possible to wrap our styled though in a way that would isolate your components from this. You can check how others implement stuff on top of our styled here. With such wrapper in place it's just a matter of wrapping all function expressions with your functions that would provide your own theme from your context