mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.76k stars 32.24k forks source link

TS Module augmentation for adding variants to components not working #28244

Open MarksCode opened 3 years ago

MarksCode commented 3 years ago

It says in the docs here: https://next.material-ui.com/customization/theme-components/, adding a variant to a component when using typescript requires you add this:

declare module '@mui/material/Button' {
  export interface ButtonPropsVariantOverrides {
    actionHover: true;
  }
}

However, this doesn't work. You'll still get an error in your createTheme():

Type '"actionHover"' is not assignable to type '"contained" | "outlined" | "text"'.ts(2322)
Button.d.ts(83, 5): The expected type comes from property 'variant' which is declared here on type 'Partial<ButtonProps<"button", {}>>'
(property) variant?: "text" | "outlined" | "contained"

In order to fix this you need to also add the line to your definitions file:

import '@mui/material/Button';

I'm not sure why this is the case but this should probably be added to the docs.

mnajdova commented 3 years ago

It doesn't look like this bug report has enough info for one of us to reproduce it.

Please provide a CodeSandbox (https://material-ui.com/r/issue-template-next), a link to a repository on GitHub, or provide a minimal code example that reproduces the problem.

Here are some tips for providing a minimal example: https://stackoverflow.com/help/mcve

sveggiani commented 3 years ago

I'm having the same issue when customizing or adding a variant for Buttons following the docs.

MarksCode commented 3 years ago

@sveggiani Do you mind making a Code Sandbox showing the issue?

sveggiani commented 3 years ago

@MarksCode I'll try when I have a moment.

In the meantime I've found a workaround forcing the variant to any:

{
  props: { variant: 'secondary' as any },
  style: {
    backgroundColor: baseThemeColors.common.white,
  }
}
awgv commented 3 years ago

@MarksCode

I think the issue here is that when a file with module augmentation doesn’t have import/export statements, you’re defining an ambient module—a shape that does not have an actual implementation—which is not the same as augmenting something that does have an implementation, like the Button.

Although, I’ve just quickly tested it in a CodeSandbox and TypeScript seems to recognize the new variant even if you replace import '@mui/material/Button'; with a useless export, like export default '';—perhaps it’s unnecessary to import the module itself, and the only thing that matters is for the file to be a module with at least one import/export statement, which is bizarre.

In any case, developers that aren’t aware of this will benefit from a better documentation—even if it’ll be reiterating the obvious, which, I think, it won’t, because TypeScript’s documentation on module augmentation has never been clear in the first place.

mnajdova commented 2 years ago

The correct way to go about this is shutting off automatic export, for example:

// shut off automatic exporting for the `Theme` above
export {};

For example, take a look at how we are doing it for the @mui/lab: https://github.com/mui/material-ui/blob/master/packages/mui-lab/src/themeAugmentation/components.d.ts