emotion-js / emotion

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

Typescript issue with CSSObject in version 11.11.5 of @emotion/styled #3174

Open valleywood opened 5 months ago

valleywood commented 5 months ago

Current behavior:

Update @emotion/styled from 11.11.0 => 11.11.5

To reproduce: We have several styled components in our project and we started to get TS errors on a lot of them after this update:

Example component that doesn't work:

import styled from '@emotion/styled';

export const StyledHeader = styled('div')(() => {
  return {
    textAlign: `center`,
    position: `relative`,
  };
});

Gives this error:

error TS2769: No overload matches this call.
  Overload 1 of 3, '(template: TemplateStringsArray, ...styles: Interpolation<{ theme?: Theme | undefined; as?: ElementType<any, keyof IntrinsicElements> | undefined; } & ClassAttributes<...> & HTMLAttributes<...> & { ...; }>[]): StyledComponent<...>', gave the following error.
    Argument of type '() => { textAlign: string; position: string; }' is not assignable to parameter of type 'TemplateStringsArray'.
  Overload 2 of 3, '(template: TemplateStringsArray, ...styles: Interpolation<{ theme?: Theme | undefined; as?: ElementType<any, keyof IntrinsicElements> | undefined; } & ClassAttributes<...> & HTMLAttributes<...> & { ...; }>[]): StyledComponent<...>', gave the following error.
    Argument of type '() => { textAlign: string; position: string; }' is not assignable to parameter of type 'TemplateStringsArray'.
  Overload 3 of 3, '(...styles: Interpolation<{ theme?: Theme | undefined; as?: ElementType<any, keyof IntrinsicElements> | undefined; } & ClassAttributes<HTMLDivElement> & HTMLAttributes<...> & { ...; }>[]): StyledComponent<...>', gave the following error.
    Argument of type '() => { textAlign: string; position: string; }' is not assignable to parameter of type 'Interpolation<{ theme?: Theme | undefined; as?: ElementType<any, keyof IntrinsicElements> | undefined; } & ClassAttributes<HTMLDivElement> & HTMLAttributes<...> & { ...; }>'.
      Type '() => { textAlign: string; position: string; }' is not assignable to type 'FunctionInterpolation<{ theme?: Theme | undefined; as?: ElementType<any, keyof IntrinsicElements> | undefined; } & ClassAttributes<HTMLDivElement> & HTMLAttributes<...> & { ...; }>'.
        Type '{ textAlign: string; position: string; }' is not assignable to type 'Interpolation<{ theme?: Theme | undefined; as?: ElementType<any, keyof IntrinsicElements> | undefined; } & ClassAttributes<HTMLDivElement> & HTMLAttributes<...> & { ...; }>'.
          Type '{ textAlign: string; position: string; }' is not assignable to type 'CSSObject'.
            Types of property 'position' are incompatible.
              Type 'string' is not assignable to type 'Position | readonly NonNullable<Position | undefined>[] | readonly Position[] | undefined'.

18 export const StyledHeader = styled('div')(() => {

This is a breaking change.

Strangely it seems to depend on which css-attributes you are styling. This works for example:

export const StyledRecentSearch = styled('div')(() => {
  return {
    display: 'flex',
    justifyContent: 'space-between',
    alignItems: 'flex-start',
  };
});

There are several ways around this for example these variants seem to work:

export const StyledHeader = styled.div`
    textAlign: 'center',
    position: 'relative',  
`;

or

 export const StyledHeader = styled('div')(() => {
  return {
    textAlign: 'center',
    position: 'relative',
  } as CSSObject;
});

Expected behavior:

There should be no breaking changes in patch versions (in this case TS errors)

Code like this should work since it returns a valid CSSObject:

export const StyledHeader = styled('div')(() => {
  return {
    textAlign: `center`,
    position: `relative`,
  };
});

Environment information:

Andarist commented 5 months ago

As a workaround, you can use as const on the returned object. I suspect the reason why it fails now is that we have reordered styled overloads.

TypeScript, currently, caches some types resolved while trying to match against the first overload. This is a long-standing issue and there is a recent-ish fix for this open here. You can even quickly verify that it does indeed fix this issue since there is a TS playground with that build available: TS playground

valleywood commented 5 months ago

Thanks for rapid action on this! 🙏 Can confirm that as const also fixes/hides the issue (just as as CSSObject did) Perhaps the issue can be fixed anyway with a new release as it forces code changes otherwise? 🤔

Andarist commented 5 months ago

Perhaps the issue can be fixed anyway with a new release as it forces code changes otherwise? 🤔

Yeah, maybe - we have to investigate this further to determine which tradeoff is better and if we can do anything to mitigate the issue on our side without reverting the change that changed this.

valleywood commented 5 months ago

Perhaps the issue can be fixed anyway with a new release as it forces code changes otherwise? 🤔

Yeah, maybe - we have to investigate this further to determine which tradeoff is better and if we can do anything to mitigate the issue on our side without reverting the change that changed this.

Alternatively make a release that indicates a breaking change and update documentation accordingly

Cerber-Ursi commented 5 months ago

Thanks for notifying, looking into it. Looks really strange for now, to be honest, since I can't understand why this worked before if it's broken now.

Strangely it seems to depend on which css-attributes you are styling.

Essentially, every attribute with the closed set of values (like testAlign, which has the type Globals | "center" | "end" | "justify" | "left" | "match-parent" | "right" | "start") is broken, while the attribute with open set (i.e. the one which can handle any string, like fontFamily, which is Globals | DataType.GenericFamily | (string & {})) works correctly.

Cerber-Ursi commented 5 months ago

Minimized example of the same error, in case someone wants to investigate too:

interface Styles {
    position?: 'left';
}

type StylesCreator = () => Styles;

interface Styled {
    // (creator: StylesCreator): void // uncomment this to remove error
    (styles: Styles): void
    (creator: StylesCreator): void
}

declare const styled: Styled

styled(() => ({ position: 'left' })) // type `string` is not assignable to type `"left"`
Andarist commented 5 months ago

Yeah, I mentioned above that it's related to signature caching that happens when resolving the first overload. The fix would be to add a leading overload that would be capable of introducing proper contextual types for those creator functions

KittyGiraudel commented 1 month ago

As a workaround, you can use as const on the returned object. I suspect the reason why it fails now is that we have reordered styled overloads.

This being a workaround is debatable when talking about projects with hundreds or thousands of rules containing the text-align declaration. 😅