Material-ui: `createMuiTheme` return type doesn't take into account merged objects

[mcobzarenco]

• [x] The issue is present in the latest release.
• [x] I have searched the issues of this repository and believe that this is not a duplicate.

Thank you so much for this project -- it is amazing!

Current Behavior 😯

I would like to add custom fields to the Theme object as described in the docs. However I struggle to get this to typecheck. The current Typescript definition for createMuiTheme is

export default function createMuiTheme(options?: ThemeOptions, ...args: object[]): Theme;

createMuiTheme returns a Theme object, rather than the type of deep merging options and args together.

Expected Behavior 🤔

The return type of createMuiTheme would reflect that args was merged in. For example the following wouldn't fail (today it says Theme doesn't have field foo).

createMuiTheme({}, {foo: "bar"}).foo

Proposed Solution

One possible solution would be to change the return type of createMuiTheme to

export default function createMuiTheme<T>(
  options?: ThemeOptions,
  ...args: Array<Partial<T>>
): Theme & T;

[eps1lon]

Customization is explained in our typescript guide. Does this work for you?

Changing the signature of createMuiTheme would be incomplete since it wouldn't affect the type you read from context.

[blasterpistol]

@mcobzarenco it can works if you use this typed theme object directly, but not when it goes from the context

[mcobzarenco]

@eps1lon @testarossaaaaa Thank you so much for you guidance!

Customization is explained in our typescript guide. Does this work for you?

:+1: I'll try the solution described in the guide using declaration merging -- I'd argue it is preferable to express the relationship in the type system directly if possible.

Changing the signature of createMuiTheme would be incomplete since it wouldn't affect the type you read from context.

@mcobzarenco it can works if you use this typed theme object directly, but not when it goes from the context

ThemeProvider is already declared as generic in the type of Theme:

export default function ThemeProvider<T = DefaultTheme>(
  props: ThemeProviderProps<T>,
): React.ReactElement<ThemeProviderProps<T>>;

and similary the useTheme hook

export default function useTheme<T = DefaultTheme>(): T;

Such that something like the snippet below works ok with my proposed type signature for createMuiTheme:

const MY_THEME = createMuiTheme({}, {foo: "bar"});

function Component() {
  const foo = useTheme<typeof MY_THEME>().foo;
}

<ThemeProvider theme={MY_THEME}>
  <Component/>
</ThemeProvider>

I may be missing what you mean, I do apologize if that's the case, I'm new to Material UI -- is the issue useTheme<typeof MY_THEME>, i.e. explicitly specifying the return type?

[eps1lon]

I would recommend not using useTheme<MyTheme>(). We're probably removing this anyway since it's abusing generics. What you're actually doing is type casting so you should be explicit about it:

-useTheme<MyTheme>();
+useTheme() as MyTheme;

I'd argue it is preferable to express the relationship in the type system directly if possible.

Sure but you still need to tell all the context consumers what the type is. With module augmentation we provide the safest way to do that.

I'm going to close this issue since it's more suited for a discussion platform e.g. StackOverflow.

[mcobzarenco]

I would recommend not using useTheme(). We're probably removing this anyway since it's abusing generics. What you're actually doing is type casting so you should be explicit about it:

:+1:

FWIW when using declaration merging as suggested by the guide, I still have to cast to Theme when using the useTheme hook, i.e.

useTheme().foo

fails as useTheme() returns DefaultTheme, not Theme -- @eps1lon should the guide suggest declaration merging for DefaultTheme rather than Theme?

[eps1lon]

You're probably importing from @material-ui/styles not @material-ui/core/styles. Would be nice to include in the docs what is affect and what not :+1: