[Typescript] PropTypes.Color does not allow new color variants

[r3dm1ke]

Related to #13875

• 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 😯

Types in packages/material-ui/src/index.d.ts, namely PropTypes.Color do not allow new color variants (success, info, warning).

Expected Behavior 🤔

PropTypes.Color should allow success, info and warning

Steps to Reproduce 🕹

Try to pass "info" as a color to any component that supports it (none right now in master, Alert in lab) and get a typescript warning.

[r3dm1ke]

This can be fixed with this diff:

--- a/packages/material-ui/src/index.d.ts
+++ b/packages/material-ui/src/index.d.ts
@@ -46,7 +46,7 @@ export interface Color {
 
 export namespace PropTypes {
   type Alignment = 'inherit' | 'left' | 'center' | 'right' | 'justify';
-  type Color = 'inherit' | 'primary' | 'secondary' | 'default';
+  type Color = 'inherit' | 'primary' | 'secondary' | 'warning' | 'info' | 'success' | 'default';
   type Margin = 'none' | 'dense' | 'normal';
 }

Let me know if I should send a PR!

[oliviertassinari]

The TypeScript definitions should match the implementation. I think that it would be better to duplicate the information until we unify it.

[eps1lon]

We need to be careful with this on and check every usage if it matches the Component.propTypes.

In the meantime you should be able to use module augmentation documented in our TypeScript guide.

[r3dm1ke]

@eps1lon @oliviertassinari I do not think that users should be augmenting the internal built-in types to make them work with internal built-in colors. But that does make sense to have definitions matching the implementation. Perhaps we could rename the existing PropTypes (and all its usages) into DEPRECATED_PropTypes and define a new PropTypes with the new colors. Then use them according to the component implementation, until all the components support the new colors and we can get rid of DEPRECATED_PropTypes?

[oliviertassinari]

@r3dm1ke I'm sorry, I don't understand. What problem do you want to solve?

[r3dm1ke]

@oliviertassinari I was looking into adding support for new colors to existing components and found out that the prop types for them do not allow the new colors. All the components reference the PropTypes.Color definition in packages/material-ui/src/index.d.ts. To add the new colors and not get typescript warnings, we need to either (1) change this definition, (2) introduce a new definition and slowly migrate the components to it or (3) hardcode the color types into each component, but I do not think it is sustainable

[oliviertassinari]

@r3dm1ke Thanks for the details. We also have this issue with the prop-types. If you add a new color, you will get a runtime warning. I was considering making the prop types a simple: 'string' in the past.

Regarding PropTypes.Color, the adoption rate seems to be at 30% in the codebase (6 components strictly use it, and 14 components' color prop don't). So I think that we can ignore it to focus on the 70% other.

[r3dm1ke]

@oliviertassinari I had followed your advice and focused on 70% of the rest. For instance, I had started with the Checkbox component, trying to add new colors to it. It seemed like a simple enough component to play out the new color support before rolling it everywhere else. However, after I had updated Checkboxes' prop-types to allow new colors and added styling, I found out that it is not enough. Under the hood, Checkbox renders the SwitchBase component. It uses PropTypes.Color, thus making it extra hard to add new colors. How do you suggest I proceed? I can hardcode color?: 'primary' | 'secondary' .... to SwitchBase prop-types, moving it away from using PropTypes.Color. There may be a simpler solution, but I cannot see it at the moment.

[oliviertassinari]

Would the best solution be to remove the Color variable and to duplicate the enum every time? Then, as we move toward the support of any color from the palette, update the type according?

[damusix]

@oliviertassinari @r3dm1ke Just an FYI, I have been searching all over the internet how to have a basic "success" button, and everything points to creating a styled component. This completely defeats the purpose of having a theme altogether. If all my forms have a success button, I would need to make a success button component and import it into my forms. We should be to be able to use <Button color='success' /> or <Chip color='success' />, and not have to completely recreate a component with font size, hover, active, disabled state, etc. It's extremely limiting to have to rely on only 3 colors.

IMO, users should either be able to define their own colors (success, warning, error, peaches, strawberries, etc), or be able to use material colors (red, green, blue, etc) and have the ability to override these shades to their hex values. I do understand the adherence to strict types and such, but not at the expense of basic features. Having a success and error button is a very very very basic thing in any UI library.

[oliviertassinari]

@damusix Agree, it's planned for v5 #13875.

[damusix]

@oliviertassinari Awesome! What's the timeline for this?

[oliviertassinari]

Closing for the updated version of this issue #24778.