frenic / csstype

Strict TypeScript and Flow types for style based on MDN data
MIT License
1.7k stars 69 forks source link

[3.1.3] typing issue with @emotion/serialize #189

Open jlowcs opened 7 months ago

jlowcs commented 7 months ago

Not sure if this should be a @emotion issue, but a patch should probably not break a build like this, so I'm posting the issue here for now 😅

Bumping this to 3.1.3 results in a lot of type errors related to @emotion/serialize:

image

For reference, here's the @emotion/serialize file (node_modules/@emotion/serialize/types/index.d.ts):

// Definitions by: Junyoung Clare Jang <https://github.com/Ailrun>
// TypeScript Version: 2.8

import { RegisteredCache, SerializedStyles } from '@emotion/utils'
import * as CSS from 'csstype'

export { RegisteredCache, SerializedStyles }

export type CSSProperties = CSS.PropertiesFallback<number | string>
export type CSSPropertiesWithMultiValues = {
  [K in keyof CSSProperties]:
    | CSSProperties[K]
    | Array<Extract<CSSProperties[K], string>>
}

export type CSSPseudos = { [K in CSS.Pseudos]?: CSSObject }

export interface ArrayCSSInterpolation extends Array<CSSInterpolation> {}

export type InterpolationPrimitive =
  | null
  | undefined
  | boolean
  | number
  | string
  | ComponentSelector
  | Keyframes
  | SerializedStyles
  | CSSObject

export type CSSInterpolation = InterpolationPrimitive | ArrayCSSInterpolation

export interface CSSOthersObject {
  [propertiesName: string]: CSSInterpolation
}

export interface CSSObject
  extends CSSPropertiesWithMultiValues,
    CSSPseudos,
    CSSOthersObject {}

export interface ComponentSelector {
  __emotion_styles: any
}

export type Keyframes = {
  name: string
  styles: string
  anim: number
  toString: () => string
} & string

export interface ArrayInterpolation<Props>
  extends Array<Interpolation<Props>> {}

export interface FunctionInterpolation<Props> {
  (props: Props): Interpolation<Props>
}

export type Interpolation<Props> =
  | InterpolationPrimitive
  | ArrayInterpolation<Props>
  | FunctionInterpolation<Props>

export function serializeStyles<Props>(
  args: Array<TemplateStringsArray | Interpolation<Props>>,
  registered?: RegisteredCache,
  props?: Props
): SerializedStyles
MadCcc commented 7 months ago

Same issue here.

matthewjameslockwood commented 6 months ago

Hi, same issue here. This looks like a breaking change rather than a minor update. We've had to manually force the csstype in our package-lock.json to 3.1.2 which is not a viable solution.

sebherrerabe commented 6 months ago

Same issue, using the library react-select @JedWatson/

heath-freenome commented 6 months ago

Same issue when using material-ui with emotion

abvolatile commented 6 months ago

I came across something similar a while back, and I can't remember where I found the original solution, but you can use "resolutions" (for yarn) or "overrides" (for npm) in your package.json to handle issues like this.

probably a good idea to review them fairly often, but much better than modifying package-lock.json or yarn.lock

in package.json add this:

"resolutions": {
    "csstype": "3.1.2"
  }

or if using npm instead of yarn:

 "overrides": {
     "csstype": "3.1.2"
  }
Cerber-Ursi commented 6 months ago

I did a little digging into the types in question, and here's the simplified reproduction code (minimized from emotion's code base):

import * as CSS from 'csstype';

type CssValue = number | string;

interface CSSOthersObject {
  [propertiesName: string]: CssValue | CssValue[]
}

interface CSSObject extends CSS.PropertiesFallback<CssValue>, CSSOthersObject {
}

If we use csstype@3.1.2, cvode compiles successfully, but with csstype@3.1.3 it throws a bunch of errors. To debug this, I've added the code which originally triggered the error in our own project (where we use react-select):

const stylesMapper: (base: CSSObject) => CSSObject = base => ({...base, zIndex: 2});

...which generated more helpful error:

index.tsx:14:69 - error TS2322: Type '{ zIndex: number; accentColor?: readonly string[] | CSS.Property.AccentColor; alignContent?: readonly string[] | CSS.Property.AlignContent; alignItems?: readonly string[] | CSS.Property.Alig
nItems; ... 820 more ...; vectorEffect?: CSS.Property.VectorEffect | readonly NonNullable<...>[]; }' is not assignable to type 'CSSObject'.
  Property 'accentColor' is incompatible with index signature.
    Type 'readonly string[] | AccentColor' is not assignable to type 'CssValue | CssValue[]'.
      Type 'readonly string[]' is not assignable to type 'CssValue | CssValue[]'.
        The type 'readonly string[]' is 'readonly' and cannot be assigned to the mutable type 'CssValue[]'.

Then, a single change has fixed all the errors present:

interface CSSOthersObject {
  [propertiesName: string]: CssValue | (readonly CssValue[])
}

I agree that breakage in patch release is really unexpected, but probably it's not that big deal. For emotion, in particular, the fix seems to be quite simple - https://github.com/emotion-js/emotion/pull/3141.

sacummings91 commented 6 months ago

Confirmed @abvolatile your solution worked for me with overrides in package.json. Will prob just use that for now until this gets sorted out

benjdlambert commented 6 months ago

Hey :wave: is there any chance that we can get this change rolled forward and undone? Granted it's a small breaking change, but still breaking either way and looks like it's broken a lot of libraries?

We don't have the ability to use resolutions over in https://github.com/backstage/backstage as it puts more maintenance load on our end users having to remember to remove this resolution when this issue is eventually fixed.