TS2589 Error throws in TS3.6 if SVGIcon component being wrapped with styled-components

[leoyli]

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

import styled from 'styled-components';
import SearchIcon from '@material-ui/icons/Search';
import InputBase from '@material-ui/core/InputBase';

const StyledSearchIcon = styled(SearchIcon)`
  position: relative;
  cursor: pointer;
  left: -2px;
`;

const MyInput = (props) => {
  return (<InputBase
    {...props}
    endAdornment={<StyledSearchIcon />}
              // wwwwwwwwwwwwwwwwwwwww
              // TS2589 Type instantiation is excessively deep and possibly infinite.
  />)
}

Expected Behavior 🤔

TS3.5 gives no errors, probably is due to this (comments).

Steps to Reproduce 🕹

Steps:

The code is straightforward, just copy paste and use in any component to see this TS error.

Context 🔦

Other components seems okay but SVG components are doomed. Perhaps we can simplify a bit the type declaration so it is less deep.

Your Environment 🌎

Tech	Version
Material-UI	v4.4.0
React	v16.8.x
TypeScript	v3.6.2

[eps1lon]

Thank you for opening this issue and trying out the new typescript release.

As far as I can tell this is an issue with TypeScript. Since microsoft/TypeScript#33132 is labelled as a bug I'd like to see if a workaround is found. If anybody has a solution I'm happy to review it but I'm not actively looking for a solution that is caused by two external dependencies just yet.

[leoyli]

Thank you very much.

Yes, this is introduced by the latest TypeScript and looks like there are other typing regression bugs (e.g. one in RxJS).

So far it’s unclear if TS would really fixes this since the core team member there also said it is likely to be by design. If that is the case, I do think MUI should simply the typing so it became extendable to the user, or documented as caveats. That’s why I open an issue here.

---

Edited by @eps1lon: Restored actual comment. I actually overwrote the post I wanted to respond to instead of replying. 🤦‍♂️

[eps1lon]

If that is the case, I do think MUI should simply the typing so it became extendable to the user, or documented as caveats.

That is always a goal (not just for the user but maintainer). We have to figure out how this might be possible first though. Currently it doesn't "look" very complicated it's just a export { default } from '@material-ui/core/SvgIcon' where SvgIcon is typed as
React.ComponentType<SvgIconProps> where SvgIconProp is a simple interface shape.

It might not be a bad idea in the first place to get rid of ComponentType and export it as a plain function component. All the statics aren't meant to be applied anyway. People straight up calling function components shouldn't be an issue anymore since this is now actually forbidden in a hooks world.

Did you check if you encounter this issue with other components decorated with styled?

[leoyli]

@eps1lon,

Nope, I'm actually using styled-components heavily, and none of one I wrapped with MUI-core gives error but the one from MUI-icon. And the interface of SvgIconProps is actually non-trivial:

export interface SvgIconProps
  extends StandardProps<React.SVGProps<SVGSVGElement>, SvgIconClassKey> {
  color?: PropTypes.Color | 'action' | 'disabled' | 'error';
  component?: React.ElementType<React.SVGProps<SVGSVGElement>>;
  fontSize?: 'inherit' | 'default' | 'small' | 'large';
  htmlColor?: string;
  shapeRendering?: string;
  titleAccess?: string;
  viewBox?: string;
}

export type StandardProps<
  C,
  ClassKey extends string,
  Removals extends keyof C = never,
  AcceptsRef = true
> = Omit<C, 'classes' | Removals> &
  StyledComponentProps<ClassKey> & {
    className?: string;
    style?: React.CSSProperties;
  } & {
    ref?: AcceptsRef extends true
      ? C extends { ref?: infer RefType }
        ? RefType
        : React.Ref<unknown>
      : never;
  };

I guess there are some overheads for typescript language services. If keep tracking down, we do see some conditional types for ref and other stuffs.

---

But I don't think it is hitting the language parsing limits, or it could really a regression from TS?!

[eps1lon]

And the interface of SvgIconProps is actually non-trivial:

Oh, well, it is compared to most of our types. What I'm getting at is that StandardProps is used in other components as well e.g. AppBar, Card, Checkbox etc. So if it does cause the issue I wonder why the other components work.

[merceyz]

It might not be a bad idea in the first place to get rid of ComponentType and export it as a plain function component

@eps1lon This actually solves the issue

[eps1lon]

@eps1lon This actually solves the issue

Great. This should also improve type perf for every component that isn't polymorphic and we can get rid of implicit children.

[leoyli]

So it looks like PropTypes.Color makes it hits the limit in TS3.6 when wrapped with sc. Not sure if other component have the same issue, but so far I haven't encountered, e.g. when wrapping with Button. The one in PR #17288 do fix the issue but it is kind of magic and mysterious to me...

[eps1lon]

So what fixed this now specifically? color or ComponentType or both?

[merceyz]

It works when leaving color as is and exporting as a plain function component.

[el1f]

Did you check if you encounter this issue with other components decorated with styled?

This seems to affect TextField too. Should we open a separate issue for that one?

[Juansasa]

Yes Textfield is also affected

[brad-decker]

Dialog is also affected