Can't augment common colors of theme palette anymore

[levrik]

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

I couldn't find a way to augment the common key of a Palette anymore.

Expected Behavior 🤔

That it is possible like before.

Steps to Reproduce 🕹

Material-UI 4.9.5: https://codesandbox.io/s/peaceful-hodgkin-i05mi (working)
Material-UI 4.9.6: https://codesandbox.io/s/nameless-sun-r7nrv (not working)

Context 🔦

After upgrade to 4.9.6 of Material UI our builds started to break.

In version 4.9.6 the common colors are exported from @material-ui/core/colors/common as default and with their name CommonColors as well. This made it possible to augment the types.
Sadly TypeScript doesn't allow augmentation of default exports. That's why it isn't possible in 4.9.6 anymore since CommonColors is gone and a const named common is just exported as default.
Please see microsoft/TypeScript#14080 as reference for the mentioned TypeScript issue.

I also can't augment Palette directly since with augmentation we can't redefine an already defined key of the Palette.
I'm also wondering why the interface was removed. Before there was the interface, a type variable called common and the default export. By just getting the interface back and exported by its name like before this issue would be fixed.

Your Environment 🌎

Tech	Version
Material-UI	v4.9.6
TypeScript	4.8

[oliviertassinari]

Note that import createPalette from "@material-ui/core/styles/createPalette"; isn't a legit import, this module is private.

[oliviertassinari]

@levrik From what I understand, we can close this issue, the augmentation should happen at the theme level, not the color one.

[levrik]

@oliviertassinari For some reason I got a createPalette is not a function error when I did

import { createPalette } from '@material-ui/core/styles';

[levrik]

@oliviertassinari No. Not really. As explained in my description the augmentation from the Theme/Palette level doesn't work. I have to be able to augment the common colors definition directly.

[oliviertassinari]

createPalette is private

[eps1lon]

You can't augment core/colors anyway. If you patch your local node_modules/ then you're on your own.

Maybe you tried to augment colors the Theme? https://material-ui.com/guides/typescript/#customization-of-theme should help you achieve that.

[levrik]

@oliviertassinari That's interesting. Because TypeScript didn't complain about the import of createPalette from @material-ui/core/styles.

[levrik]

@eps1lon Yep. We've started on this guide but thought that a common blue color used throughout our application makes the most sense inside the common key of the palette. If I remember correctly for example Box can only use colors from the palette as bgcolor, not from the theme directly. It was a requirement for us to be able to use this common color for Boxes.

Also want to note that we never patched node_modules/. We were just augmenting the TypeScript types for something that was possible to change just from the outside.

[eps1lon]

Also want to note that we never patched node_modules/. We were just augmenting the TypeScript types for something that was possible to change just from the outside.

You should only augment what you actually change. In your case you told typescript that you somehow changed import { common } from '@material-ui/core/colors'. You probably didn't use this in your codebase to begin with which made the type change harmless. It's nevertheless unsound if you don't actually patch the runtime.

Augmenting the theme is less harmful since we assume that you change the context value accordingly.

[levrik]

@eps1lon Please take a look at the first Codesandbox I've posted.
That's basically exactly what we're doing. We've passed a custom palette. So we augmented what we've changed. Without the augmentation our code is broken.
Whatever. Could you suggest an alternative? As I said it seems like the Box component is reading from the palette. We want to provide a custom color through the theme to be used by Box. That's the whole reason why we started to change the palette. As long as there's no way to use colors directly defined on a theme from a Box we probably have to stay on 4.9.5. Which is something I don't want to.

[eps1lon]

So we augmented what we've changed

No, you changed colors/common which is also used by the theme when you only actually augmented the theme.

Could you suggest an alternative?

The guide I linked exactly described this. If you have further question then stackoverflow is the better place for these kind of issues.

[levrik]

@eps1lon No. The guide doesn't answer the question about the Box component.
So. How can I change the palette of a theme? Is this an unsupported thing? Because it works. But I can't make it work with TypeScript. This is basically what this issue is about.
With augmentation I can't change the type of a property of Theme. I can't augment the palette key. Augmentation works for adding fields only. Not for altering the type of existing ones.

[oliviertassinari]

How can I change the palette of a theme?

@levrik While the generic approach is documented in https://material-ui.com/guides/typescript/#customization-of-theme, I have tried to update your codesandbox with no luck. I don't know how to process, but I believe it's possible. Maybe it would be great to have a demo for it?

---

TypeScript didn't complain about the import of createPalette from @material-ui/core/styles.

Great catch, it sounds like we should keep the types in sync with the implementation. I believe this diff would do it :).

diff --git a/packages/material-ui/src/styles/index.d.ts b/packages/material-ui/src/styles/index.d.ts
index febaa6212..7414c1adc 100644
--- a/packages/material-ui/src/styles/index.d.ts
+++ b/packages/material-ui/src/styles/index.d.ts
@@ -1,10 +1,6 @@
 export * from './colorManipulator';
 export { default as createMuiTheme, ThemeOptions, Theme, Direction } from './createMuiTheme';
-export {
-  default as createPalette,
-  PaletteColorOptions,
-  SimplePaletteColorOptions,
-} from './createPalette';
+export { PaletteColorOptions, SimplePaletteColorOptions } from './createPalette';
 export { default as createStyles } from './createStyles';
 export { TypographyStyle, Variant as TypographyVariant } from './createTypography';
 export { default as makeStyles } from './makeStyles';

Do you want to send a pull request? :)

[hburrows]

@eps1lon I'm a little confused by what you're proposing the solution to this problem is. Are you saying we should just add a new property to Palette (for example, my_common)? As @levrik is saying, the way CommonColors is now defined you can't augment the common property of Palette -- it's private and it's defined as a type equal to Record<...> and you can't change the type via augmentation (so we can't define the type as Record<white | black | lightBlack | darkWhite | ...>. If that's the case why even have a common property in Palette which only has white (#fff) and black (#000) -- how often does one use pure white or black in a design.

[oliviertassinari]

You should only augment what you actually change. In your case you told typescript that you somehow changed import { common } from '@material-ui/core/colors'.

@hburrows This is important, the type should match reality ⤴️.

[hburrows]

@oliviertassinari That makes sense. Unless I'm missing something obvious that means there is no way to add additional colors to Palette.common. However, if that is possible, can you please share an example because it's lost on me. I'm OK adding a sibling to Palette.common (i.e. Palette.app_common) to hold my app's common colors, but I just don't want to do that if I'm not understanding this. It seemed like adding my app's common colors into Palette.common via Theme customization was the way to go. Again, if you can't do that then having a Palette.common with only black and white has zero value.

[eps1lon]

I was under the impression you could just add deep properties with module augmentation but seems like I misunderstood something. We'll add the ability to augment this.

Quite unfortunate that this just happened to work by augmenting colors/common. Sorry for the trouble.