Fix MantineThemeOverride

[AlexisN94]

MantineThemeOverride is curently broken because of how imports are being made in your code. You're using relative imports of MantineThemeOverride instead of importing from @mantine/code, which means module augmentation on our end won't be applied.

Also some types, like CSSVariablesResolver, use MantineTheme incorrectly instead of MantineThemeOverride:

// Current 
import { MantineTheme } from '../theme.types';
export type CSSVariablesResolver = (theme: MantineTheme) => ConvertCSSVariablesInput;

// Fixed
import { MantineThemeOverride } from '@mantine/core'; // replaced the relative import
export type CSSVariablesResolver = (theme: MantineThemeOverride) => ConvertCSSVariablesInput; // replaced MantineTheme by MantineThemeOverride

Also create-theme.d.ts needs to be fixed:

// Current 
import { MantineThemeOverride } from '../theme.types';
export declare function createTheme(theme: MantineThemeOverride): MantineThemeOverride;

// Fixed
import { MantineThemeOverride } from '@mantine/core'; // replaced the relative import
export declare function createTheme(theme: MantineThemeOverride): MantineThemeOverride;

Finally, useMantineTheme also needs changing.

// Current 
import { MantineTheme, MantineThemeOverride } from '../theme.types';
...
export declare function useMantineTheme(): MantineTheme;

//Fixed
import { MantineTheme } from '../theme.types';
import { MantineThemeOverride } from '@mantine/core';
...
export declare function useMantineTheme(): MantineTheme & MantineThemeOverride;

[rtivital]

What kind of module augmentation does not work? Can you please send a link to the documentation that describes it? I've checked theme.colors and theme.other types overrides, and they are working correctly.

[alexis-exaud]

This isn’t something that needs a link to documentation or any reproduction steps, as it’s a basic matter of how Typescript handles module augmentation and I expected you to grasp the problem right away just from looking at the imports.

Module augmentation only applies when the type is imported from the original module – in this case @mantine/core –, not when you use relative imports like ../theme.types. Relative imports are treated as different types from the ones defined in the module and this breaks the augmentation chain. This is happening in quite a few places in your code, such as the ones that I exposed in my initial message. This means you're ignoring the MantineThemeOverride from the augmented module in methods such as createTheme.

The override for theme.colors works because MantineThemeColors directly extends MantineThemeColorsOverride, so that will apply regardless; it's not related to the problem with MantineThemeOverride.

To add injury to insult, useMantineTheme returns MantineTheme which completely disregards MantineThemeOverride. I'm suprised this wasn't covered by any of your tests.

[rtivital]

useMantineTheme is supposed to return MantineTheme type as it is implied by its name. MantineThemeOverride type is not meant to be used for types augmentation, it is a deep partial of MantineTheme that is used for passing partial of the theme object to override theme via MantineProvider.

[alexis-exaud]

I understand now. In that case it seems you don't have a full, flexible theme overriding implementation in place. Allowing only theme.colors and theme.others to be overridden is very limiting. It makes no sense having to do theme.others.fontSizes.myCustomSize when theme.fontSizes already exists. It should be possible to extend as theme.fontSizes.myCustomSize. Naturally this doesn't apply just to font sizes, but also spacing, breakpoints, and so forth.

On another note, a secondary color should be able to live in theme.secondaryColor to be consistent with theme.primaryColor, instead of theme.others.secondaryColor. Which highlights again the need for a more flexible theme overriding experience. Just do away with theme.others entirely and allow adding properties directly to theme instead, by leveraging MantineThemeOverride better.

People who might like keeping custom properties neatly separated from the base theme ones, could always extend the theme themselves to include theme.others, or theme.custom, or theme.extend, or theme.whatever. It's all about providing flexibility and being less opinionated.

As for me, I've patched your code to have MantineThemeOverride working on my end as I just described, so I'm all good, but it still would be nice if the team implemented this officially. Having patches feels hacky and might break with future updates.

[alexis-exaud]

Closing the issue without any feedback on whether this will be considered doesn't feel very considerate. Could you provide some clarity on whether flexible theme overrides will be addressed in the future?

[rtivital]

GitHub issues are used to report bugs with the library. This is not a bug with the library, you've made a wrong assumption about the meaning of the MantineThemeOverride type. If you wonder whether MantineThemeOverride will be used the way you've described – no, this type is reserved for the purpose described above and changing that will lead to a breaking change. If this feature is implemented in the future, it can only be done differently.