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

[typescript] Material UI components are incompatible to emotion and styled-components #14191

Closed
Mario-Eis opened this issue Jan 15, 2019 · 6 comments · Fixed by #14482
Closed

Comments

@Mario-Eis
Copy link

Mario-Eis commented Jan 15, 2019

When using styled components >=4.1.0 or emotion >=10 there will be type errors if material ui components are styled.

The reason seems to be the usage of React.ComponentType that is weather ComponentClass OR FunctionComponent but not explicitly typed.

declare const Typography: React.ComponentType<TypographyProps>;
....
type ComponentType<P = {}> = ComponentClass<P> | FunctionComponent<P>;

I think this should be related to DefinitelyTyped/DefinitelyTyped#29832 and possibly DefinitelyTyped/DefinitelyTyped#30942 on styled components side. And emotion-js/emotion#1167 on emotions side.

There are possible workarounds, but they all require to bypass type checking by e.g. casting to any. As if that is not bad enough, it has to be done for all styled components individually. In a very large code base this means to implement a workaround in many (many!) places.

Expected Behavior 🤔

No build or type errors are expected when styling material ui components with styled components or emotion.

Current Behavior 😯

A lot of type errors like:

TS2322: Type '{ color: "primary"; }' is not assignable to type '(IntrinsicAttributes & TypographyProps & ClassAttributes<Component<TypographyProps, any, any>> & Pick<(TypographyProps & ClassAttributes<Component<TypographyProps, any, any>>) | (TypographyProps & Attributes), "hidden" | ... 259 more ... | "variant"> & { ...; } & { ...; }) | (IntrinsicAttributes & ... 4 more ... & {...'.
  Type '{ color: "primary"; }' is not assignable to type 'IntrinsicAttributes & TypographyProps & Attributes & Pick<(TypographyProps & ClassAttributes<Component<TypographyProps, any, any>>) | (TypographyProps & Attributes), "hidden" | ... 259 more ... | "variant"> & { ...; } & { ...; }'.
    Type '{ color: "primary"; }' is missing the following properties from type 'Pick<(TypographyProps & ClassAttributes<Component<TypographyProps, any, any>>) | (TypographyProps & Attributes), "hidden" | "dir" | "slot" | "style" | "title" | "color" | ... 254 more ... | "variant">': style, classes, className, innerRef, headlineMapping

Steps to Reproduce 🕹

Just do something simple like

const StyledTypography = styled(Typography)`
     margin-right: 10px;
`;

and use StyledTypography in a render function passing a prop like color. Typescript will throw errors.

So far, any material ui component I tried, was affected.

Your Environment 🌎

Tech Version
Material-UI v3.9.0
React 16.7
TypeScript 3.2.2
Styled Components >=4.1
Emotion >=10
@eps1lon eps1lon changed the title [typescript] Material UI components are incompatible to major CSS-in-JS solutions [typescript] Material UI components are incompatible to emotion and styled-components Jan 17, 2019
@eps1lon
Copy link
Member

eps1lon commented Jan 17, 2019

I cannot reproduce the error for styled-components. Could you please include your lockfile, tsconfig and verify that you're using the latest version of @types/react and @types/styled-components?

Edit: I get a different error though. Will work on a fix.

@Mario-Eis
Copy link
Author

I'm using "@types/react": "^16.7.20" and "@types/styled-components": "^4.1.6".

Here is the exact error message from a styled icon with styled components:

import AccountCircle from "@material-ui/icons/AccountCircle";

...

const UserIcon = styled(AccountCircle)`
  && {
    margin-right: 10px;
  }
`;

...
  <UserIcon color="disabled"/>
...

TS2740: Type '{ color: "disabled"; }' is missing the following properties from type 'Pick<Pick<(Pick<SvgIconProps, "string" | "max" | "accumulate" | "end" | "hanging" | "alphabetic" | "ideographic" | "media" | "style" | "clipPath" | "cursor" | "filter" | "mask" | ... 408 more ... | "titleAccess"> & RefAttributes<...>) | (Pick<...> & { ...; }), "string" | ... 421 more ... | "titleAccess"> & Partial<....': string, max, accumulate, end, and 418 more.

This is my tsconfig.json

{
  "compilerOptions": {
    "outDir": "out/dist",
    "module": "esnext",
    "target": "es5",
    "esModuleInterop": true,
    "allowSyntheticDefaultImports": true,
    "lib": [
      "es6",
      "dom"
    ],
    "sourceMap": true,
    "allowJs": true,
    "jsx": "react",
    "moduleResolution": "node",
    "rootDir": "src",
    "noImplicitReturns": true,
    "noImplicitThis": true,
    "noImplicitAny": true,
    "strictNullChecks": false
  },
  "exclude": [
    "node_modules",
    "build"
  ]
}

@Mario-Eis
Copy link
Author

Mario-Eis commented Jan 18, 2019

I THINK the comment from @Jessidhia here DefinitelyTyped/DefinitelyTyped#29832 (comment) is the best description of the problem. As soon as i edit
declare const Typography: React.ComponentType<TypographyProps>;
to be
declare const Typography: React.FunctionalComponent<TypographyProps>;
in the material ui types, the error goes away.

@Mario-Eis
Copy link
Author

Mario-Eis commented May 8, 2019

I still see the issue in 4.0.0-beta.1
Should this be fixed?

The merge message contains some information about a css prop in the documentation? Yet all I can find there is

The styled() method works perfectly on all of our components.

@eps1lon
Copy link
Member

eps1lon commented May 8, 2019

@Mario-Eis We have type tests for latest emotion and material-ui types that pass. Please open a separate issue with the code that is failing.

@Mario-Eis
Copy link
Author

Mario-Eis commented May 11, 2019

I posted a demo project in #15609
I think its the right place for it. Please correct me if I'm wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants