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

[MUI v5] Type Error when using Icons from @mui/icons-material #18018

Open
2 tasks done
awanlin opened this issue May 30, 2023 · 13 comments · Fixed by #18334
Open
2 tasks done

[MUI v5] Type Error when using Icons from @mui/icons-material #18018

awanlin opened this issue May 30, 2023 · 13 comments · Fixed by #18334
Labels
bug Something isn't working will-fix We will fix this at some point

Comments

@awanlin
Copy link
Collaborator

awanlin commented May 30, 2023

📜 Description

When upgrading to MUI v5 we are getting a type error when using Icons from @mui/icons-material. We see this in the following places:

  • The icon prop of the SidebarItem, SidebarSubmenuItem, and MyGroupsSidebarItem components
  • The UNSTABLE_extraContextMenuItems prop of the EntityLayout component
  • When passing in icons to the icons section of createApp in the App.tsx

The error is:

Type 'OverridableComponent<SvgIconTypeMap<{}, "svg">> & { muiName: string; }' is not assignable to type 'ComponentType<{ fontSize?: "default" | "large" | "small" | undefined; }>'.
  Type 'OverridableComponent<SvgIconTypeMap<{}, "svg">> & { muiName: string; }' is not assignable to type 'FunctionComponent<{ fontSize?: "default" | "large" | "small" | undefined; }>'.
    Types of parameters 'props' and 'props' are incompatible.
      Type 'PropsWithChildren<{ fontSize?: "default" | "large" | "small" | undefined; }>' is not assignable to type '{ component: any; } & { children?: ReactNode; classes?: Partial<SvgIconClasses> | undefined; color?: OverridableStringUnion<"inherit" | "disabled" | "primary" | ... 5 more ... | "warning", SvgIconPropsColorOverrides> | undefined; ... 6 more ...; viewBox?: string | undefined; } & CommonProps & Omit<...>'.
        Property 'component' is missing in type 'PropsWithChildren<{ fontSize?: "default" | "large" | "small" | undefined; }>' but required in type '{ component: any; }'.t

👍 Expected behavior

You should be able to use icons from @mui/icons-material without any type errors

👎 Actual Behavior with Screenshots

We get this error:

Type 'OverridableComponent<SvgIconTypeMap<{}, "svg">> & { muiName: string; }' is not assignable to type 'ComponentType<{ fontSize?: "default" | "large" | "small" | undefined; }>'.
  Type 'OverridableComponent<SvgIconTypeMap<{}, "svg">> & { muiName: string; }' is not assignable to type 'FunctionComponent<{ fontSize?: "default" | "large" | "small" | undefined; }>'.
    Types of parameters 'props' and 'props' are incompatible.
      Type 'PropsWithChildren<{ fontSize?: "default" | "large" | "small" | undefined; }>' is not assignable to type '{ component: any; } & { children?: ReactNode; classes?: Partial<SvgIconClasses> | undefined; color?: OverridableStringUnion<"inherit" | "disabled" | "primary" | ... 5 more ... | "warning", SvgIconPropsColorOverrides> | undefined; ... 6 more ...; viewBox?: string | undefined; } & CommonProps & Omit<...>'.
        Property 'component' is missing in type 'PropsWithChildren<{ fontSize?: "default" | "large" | "small" | undefined; }>' but required in type '{ component: any; }'.t

👟 Reproduction steps

To keep things simple I'm going to base this on the main Backstage repo but you can easily do the same with a fresh instance created with npx @backstage/create-app@latest

  1. Add "@mui/icons-material": "^5.11.16" to the package/app/package.json
  2. Run yarn install
  3. In the packages/app/src/components/Root/Root.tsx change this import import HomeIcon from '@material-ui/icons/Home'; to be like this import HomeIcon from '@mui/icons-material/Home';
  4. Run yarn tsc

Notice: You'll get a type error

📃 Provide the context for the Bug.

While upgrading to MUI v5 we ran into a number of cases like this. Now we can work around this by adding as IconComponent but we feel this should probably been something that is resolved in the Backstage itself as it would make this upgrade smoother and with less manual changes

🖥️ Your Environment

This really isn't an environment specific issue more around the usage of @mui/icons-material

👀 Have you spent some time to check if this bug has been raised before?

  • I checked and didn't find similar issue

🏢 Have you read the Code of Conduct?

Are you willing to submit PR?

Yes I am willing to submit a PR!

@awanlin awanlin added the bug Something isn't working label May 30, 2023
@awanlin
Copy link
Collaborator Author

awanlin commented May 30, 2023

I've been playing around to see if I could come up with a fix for this before I submit a PR. The type for the IconComponent is located here:

export type IconComponent = ComponentType<{

If I change this to be like this the errors for away:

export type IconComponent =
  | ComponentType<{
      fontSize?: 'default' | 'small' | 'large' | undefined;
    }>
  | ComponentType<{}>;

But I think this might be very naive on my part?

@tudi2d
Copy link
Member

tudi2d commented Jun 13, 2023

This is a good discovery. I ran into a similar issue when hacking on MUI v5 core-components. I temporarily solved it like the following:

type SidebarItemBaseProps = {
    icon: typeof SvgIcon;
    text?: string`
...

We might want to rethink the IconComponent type as the 2 year old comment says:

"If you have the need to forward additional props from SvgIconProps, you can

  • open an issue or submit a PR to the main Backstage repo. When doing so please
  • also describe your use-case and reasoning of the addition."

@awanlin
Copy link
Collaborator Author

awanlin commented Jun 13, 2023

Yeah, I'm open to any suggestions with this one. But I do think we want to try and resolve this soon as it's the one that everyone who upgrades to MUI v5 will run in to

@tudi2d
Copy link
Member

tudi2d commented Jun 15, 2023

yes, you are right! it's not great. hopefully I have some time tommorrow to look at this & related MUI 5 issues

@freben
Copy link
Member

freben commented Jun 15, 2023

I think we're generally open and grateful for the help investigating and suggesting a fix. I know that the IconComponent was added quite some time back because of similar type issues and we wanted to narrow the type to the absolute minimum that we needed at the time to avoid minor inconsequential type differences causing issues. Now, it seems like even this very narrow slice of the type still can cause issues :)

Sidestepping it with some clever typeof sounds compelling - if it actually works in the mixed-mui-major case etc.

@tudi2d
Copy link
Member

tudi2d commented Jun 19, 2023

added a PR with a possible solution, which worked for me - happy about feedback

@awanlin
Copy link
Collaborator Author

awanlin commented Jan 5, 2024

Sadly it seems this error has not been resolved or has cropped up in some other variation. This came up again recently in this Discord thread: https://discord.com/channels/687207715902193673/1192389833235120239. I took a run at migrating the Demo site as that needs to happen eventually anyway and ran into the error there as well: backstage/demo#212

@awanlin awanlin reopened this Jan 5, 2024
@awanlin
Copy link
Collaborator Author

awanlin commented Jan 5, 2024

What's interesting is that the original error message in this Issue is only there once but the others are now just this:

Type 'OverridableComponent<SvgIconTypeMap<{}, "svg">> & { muiName: string; }' is not assignable to type 'IconComponent'.

See the detailed errors here: https://github.com/backstage/demo/actions/runs/7425731911/job/20207991237?pr=212

@rmwondolleck
Copy link

In the process of upgrading and running into the same issue as well.

$ tsc
packages/app/src/App.tsx:101:9 - error TS2322: Type 'OverridableComponent<SvgIconTypeMap<{}, "svg">> & { muiName: string; }' is not assignable to type 'IconComponent'.

101         code: CodeIcon,
            ~~~~

packages/app/src/App.tsx:102:9 - error TS2322: Type 'OverridableComponent<SvgIconTypeMap<{}, "svg">> & { muiName: string; }' is not assignable to type 'IconComponent'.

102         alarm: AlarmIcon,
            ~~~~~

Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Mar 10, 2024
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 17, 2024
@awanlin awanlin reopened this Mar 18, 2024
@awanlin awanlin added will-fix We will fix this at some point and removed stale labels Mar 18, 2024
@awanlin
Copy link
Collaborator Author

awanlin commented Mar 18, 2024

This needs to be fixed as it's blocking clean migrations to MUI v5

@praphull-purohit
Copy link
Contributor

I tried migrating to MUI v5 today, before upgrading backstage to 1.24.0. I've resolved all other issues, but stuck on this one. If there's any workaround available, please share. For now, I'm postponing MUI v5 migration in our instance for later

@webark
Copy link
Contributor

webark commented Mar 20, 2024

we do like this.

icon={() => <Category />}

so you get like

  icons: {
    alert: () => <Alarm />,
    menuBook: () => <MenuBook />,
    code: () => <Code />,
    'kind:product': () => <Flare />,
    'kind:feature': () => <DonutSmall />,
  },

or

<SidebarItem icon={() => <Category />} to="catalog" text="Catalog" />

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working will-fix We will fix this at some point
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants