slots overrides do not merge with the theme

[flaviendelangle]

Problem

How does the theme default props work

Users can define default props for there components using the theme as follow:

const theme = createTheme({
  components: {
    MuiDatePicker: {
      defaultProps: {
        closeOnSelect: true,
      },
    },
  },
});

The props passed to the theme, will then be merged with the props passed to the component with a shallow merge.

material-ui/packages/mui-utils/src/resolveProps.ts

Lines 13 to 17 in 50e2b94

	Object.keys(defaultProps).forEach((propName: keyof T) => {
	if (output[propName] === undefined) {
	output[propName] = defaultProps[propName];
	}
	});

What is the problem with the slots ?

The way the merge currently works, makes it very difficult, or even impossible for users to override a slot component (or some slot component props) directly from the theme.

For instance, if a user wants to customize the left and right arrow icons on all the date pickers of its application, he will not be able to use the theme.

Since the merge is a shallow merge, the props.components passed to DatePicker overrides the one stored in the theme default props instead of being merged with it.

Example with the props: working

Example with the theme: not working

When is this problem blocking ?

I did not find any actual use case on the core and I don't think users should always override the slots through the theme.
But on bigger components like the one provided by X, the ability of overriding some slot once and for all can make some customization a lot easier.

Here are a few example I have in mind:

DataGrid + Joy

@siriwatknp did a POC during our retreat (https://github.com/mui/mui-x/pull/5360/files).
The behavior could be even better if users could just import a method to populate the theme with joy slots.
This could look something like:

import { addJoyDataGrid } from '@mui/x-data-grid/joy';

const theme = createTheme({ /** Some theme customization not related to the data grid **/ });
const themeWithJoyDataGrid = addJoyDataGrid(theme);

// Each data grid usage does not have to care about the theme
const UserList = () => {
  const users = useUsers();
  
  return (
    <DataGrid {...users} />
  );
};

export default function App() {
  return (
    <ThemeProvider theme={themeWithJoyDataGrid}>
      <UserList />
    </ThemeProvider>
  );
}

Custom DatePicker with custom input

The current version of the pickers requires a renderInput prop.
This prop can be defined in the theme to avoid defining it on each picker (with @ts-ignore to avoid having TS complain that the prop is required)
Working example

In v6, we are discussing replacing this prop with a component slot to be coherent with the other component customization.
But it would prevent users from defining it in the theme, which would be a clean solution I would like to encourage.

Proposal

For the components prop, increase the depth of the merge of one.

--- a/packages/mui-utils/src/resolveProps.ts
+++ b/packages/mui-utils/src/resolveProps.ts
@@ -4,14 +4,20 @@
  * @param {object} props
  * @returns {object} resolved props
  */
-export default function resolveProps<T extends { className?: string } & Record<string, unknown>>(
-  defaultProps: T,
-  props: T,
-) {
+export default function resolveProps<
+  T extends { className?: string; components?: Record<string, unknown> } & Record<string, unknown>,
+>(defaultProps: T, props: T) {
   const output = { ...props };
 
   Object.keys(defaultProps).forEach((propName: keyof T) => {
-    if (output[propName] === undefined) {
+    if (propName === 'components') {
+      if (defaultProps[propName] !== undefined) {
+        output[propName] =
+          output[propName] === undefined
+            ? defaultProps[propName]
+            : resolveProps(defaultProps[propName] as any, output[propName]);
+      }
+    } else if (output[propName] === undefined) {
       output[propName] = defaultProps[propName];
     }
   });

Advantages

• Users will be able to define the component slot on the theme
• We will be able to provide methods to enrich the theme and customize our X components for Joy

Disadvantages

• It adds some additional complexity
• How can user override there default props on a specific component to fallback on the built-in default value ?

Follow up

I focused this RFC on the components prop, but the same can be discussed for the componentsProps prop.

[siriwatknp]

In general, this makes sense to me. We can do a POC with Joy UI in the next quarter together with the Data Grid integration.

[flaviendelangle]

Would you consider it a breaking change ? Technically it's a behavior change but I don't know who would currently use the theme to define components and expect the current behavior.

[siriwatknp]

Would you consider it a breaking change ?

I don't think so. If my understanding is correct, the change is basically merging components prop from the theme and on the instance together, right?

Technically it's a behavior change but I don't know who would currently use the theme to define components and expect the current behavior.

Yeah, I feel that it will benefit the X components more than in general usage. I agree with the proposal that the components should be merged. It is just a matter of when we should create a PR, may be let's wait for @mnajdova's opinion first.

[flaviendelangle]

the change is basically merging components prop from the theme and on the instance together, right?

Yes

[mnajdova]

Agree with the proposal. I would generally consider this as a bug fix, not a breaking change. I would assume this is what people would expect, but I also think this is not widely used so far, so it may even go unnoticed.

[ZeeshanTamboli]

A PR was opened for it in #35088