Material-ui: [RFC] What's the best API to override deep elements?

Created on 15 Jun 2020  路  19Comments  路  Source: mui-org/material-ui

A while back, around the alpha & beta iterations on the v1 version, we had to decide what was the best way to override the nested components. It's meant to help developers customize the rendering. We can find #11204 and #10476 as a legacy.

up until v1

So far, the tradeoff on this problem has been:

  1. Provide a XXXComponent prop when overriding the nested element can be valuable for users. For instance:

https://github.com/mui-org/material-ui/blob/c6b95d2e773088af66823f8995c3e57508c82056/packages/material-ui/src/Modal/Modal.d.ts#L8

  1. Provide a XXXProps prop when providing a custom XXXComponent component is too cumbersome. For instance:

https://github.com/mui-org/material-ui/blob/c6b95d2e773088af66823f8995c3e57508c82056/packages/material-ui/src/Modal/Modal.d.ts#L9

However, this original design constraint is increasingly more challenged by the following:

Autocomplete

The Autocomplete mixes the renderXXX approach with the XXXComponent approach. cc @oliviertassinari

https://github.com/mui-org/material-ui/blob/c6b95d2e773088af66823f8995c3e57508c82056/packages/material-ui-lab/src/Autocomplete/Autocomplete.d.ts#L164
https://github.com/mui-org/material-ui/blob/c6b95d2e773088af66823f8995c3e57508c82056/packages/material-ui-lab/src/Autocomplete/Autocomplete.d.ts#L142

Date Picker

The DatePicker mixes the renderXXX approach with the XXXComponent approach. cc @dmtrKovalenko

renderLoading?: () => React.ReactNode;

https://github.com/mui-org/material-ui-pickers/blob/c834f5cac5930df099aaad806ceb3fe4ec1669db/lib/src/views/Calendar/Calendar.tsx#L43

ToolbarComponent?: React.ComponentType<ToolbarComponentProps>;

https://github.com/mui-org/material-ui-pickers/blob/c834f5cac5930df099aaad806ceb3fe4ec1669db/lib/src/typings/BasePicker.tsx#L61

Data Drid

The DataGrid goes with xXXComponent approach, however, the name is confusing. Sometimes it's a render prop, sometimes it's a React element, it's never a component as the name suggests cc @dtassone

  paginationComponent?: (props: PaginationProps) => React.ReactNode;
  loadingOverlayComponent?: React.ReactNode;
  noRowsOverlayComponent?: React.ReactNode;
  footerComponent?: (params: ComponentParams) => React.ReactNode;
  headerComponent?: (params: ComponentParams) => React.ReactNode;

https://github.com/mui-org/material-ui-x/blob/59d533642d5837ce6912fe88cbcdfc228f621594/packages/grid/x-grid-modules/src/models/gridOptions.tsx#L93-L97

Styled components

Styled components might request something brand new. cc @mnajdova
As the experimentation of #21104 showcases. If we want to keep the CSS specificity at it's lowest possible level (meaning one level) and expose unstyled components, we will have to expose an API to inject custom component. In older experimentation, I worked around the problem with a components prop.

components?:聽{
  Root: React.ElementType<React.HTMLAttributes<HTMLDivElement>>;
  Label: React.ElementType<React.HTMLAttributes<HTMLSpanElement>>;
}

https://github.com/oliviertassinari/material-ui/blob/153c1833fb204a28ee6712db1c2ac6a9308a163e/packages/material-ui/src/TableCell/TableCell.unstyled.js#L18

The problem

I think that it would be great to defines which API works best and in which cases. I would hope that by doing so, we can provide a consistent experience for the developers using the library to build applications and websites. I also think that by reducing the number of approaches we can reduce the learning curve for new users.

When should we use a component, when should we use a render prop, etc.?

Prior-arts

breaking change discussion

Most helpful comment

flat vs deep

@dmtrKovalenko While I think that we should encourage flatten props as much as possible for the reasons mentioned in https://github.com/mui-org/material-ui/issues/21453#issuecomment-656065074. (@dtassone Yes, I very much have the DataGrid API in mind the options could be flattened :p), It's not without its limitations.

I see a couple of advantages in going deep:

  • As a developer, it makes it easier to find all the customization points (both when reading the docs and when in the editor with IntelliSense), you go look at the prop y, it's one place, always the same between all the components. You don't need to scan the whole set of props the component exposes. Take the Autocomplete, we have 59 props, take the DatePicker, we have 65 props, good luck if they aren't prefixed with the same wording, like renderX or componentX.
  • It makes it easier to forward these props deep down the React tree, you forward one y prop, not all the props.
  • These are props you will likely not use frequently, likely for the first time you want to customize your component, then you can create an abstraction and reuse it.
  • It's consistent with the classes prop.

All 19 comments

While in my initial comment was trying to draw an accurate presentation of what we have explored so far, I'm going to try to explore the pros & cons of each approach we have learned.

Component API

Historically, we went with the API because it was minimizing the complexity of the source. It was requiring less boilerplate to implement. It was making the source easier to read. I suspect there are a lot of people looking at the source, either for learning best practices, how to apply customization, to debug something, to contribute, etc.

The second advantage is that it allows using the hook API directly.

render prop API

However, we have quickly realized the limitations of the component API. The limitation is twofold:

  1. Some people create a new component in the render method, which leads to a remount of DOM nodes at each render. While the issue might hit you silently, it harms performance and is very noticeable when it impacts the transition: it breaks them.
  2. Now, if we look at why people create new components in the render method. It's because either they don't know well the distinction between a component, an element, and a function in the React landscape, or because they need to access a variable from outside the component scope.

I think that it's this latter reason that leads us to introduce more render like APIs lately. Without such API, you have two alternatives, that might be cumbersome:

Multiple props vs one prop

I'm switching gears a little bit here on a different concern: scalability. Should we have one prop per customization point or should we group the customization points under a single prop? So far, we have had different answer to this problem:

  • CSS: group the concerns under a classes prop but also expose classes.root with classNames.
  • the translation keys: flatten the props if they are less than 10, group them otherwise.
  • the component, flatten the props, have a component prop for the root, and a XxxComponent for nested elements. So far, we never had a component with more than 3 XxxComponent props.

Sharing my experience and thoughts on this matter:

as/component prop

Having a property like as/component on each component, where you can specify whatever component you want to be render as root. For example you can do the following:

<Button as="div">Hello</Button> - and the root will not be the default button, but div

renderXXX props

But then clients want to customize the different things you have inside the button, like the icon or the content... Historically we had a render props, for example if I want to render the text inside the Button with Typography from Material-UI, I could provide something like:

<Button renderContent={(Component, props) => <Typography {...props}>Hello</Typography>} />

But this created the props explosion, with each "slot" property we would have to have appropriate render property. In addition it is confusing what will happen if you provide both (we were sending as an argument the shorthand's (content) props in the renderContent callback, but it was still very confusing)

<Button content="Hello" renderContent={(Component, props) => <Typography {...props}>{props.text}</Typography>} />

children callback

For solving this, we decided to support children callback on all shorthand props, where people can customize what is rendered there:

<Button content={{ text: 'Hello', children: (C, p) => <Typography {...p}>{p.text}</Typography>}} />

createComponent/compose utility

This works great, but if you want in your application to have everywhere Typography you don't want to do this in all component instances, so you could theoretically create wrapper that will handle it, but we thought we can provide better API for this - createComponent/compose:

const CustomButton = createComponent(Button, { 
  slots: { 
    content: Typography 
  } 
}): 

The beauty of an API like this is, you can create component for each of your slots, for example using styled components, with which you can theoretically opt-out of the styling mechanism that is used otherwise.

I think we can split the problem in 3 categories

  1. Static/Dumb component swap, ie replace Icon sort in the grid
  2. Dynamic or using parent component props, ie pagination in the grid. How do we pass props between the parent components and its customised child? Render props, hooks, context, api... Do we have a standard?
  3. Tree structure, that mixes the 2 categories above, ie the footer component that includes a pagination component

FYI I have refactored the approach on the grid for the footer and header, which were the only ones using children.
All components customisation available here if you want to check it out ;)

@dtassone Thanks for sharing your exploration, once we settle on one approach we can update all the components.

Proposal number 1

  • 鈿狅笍 We aim to leverage component composition as much as possible, we try to keep a direct mapping: one DOM node <=> one React component. However, it's not always practical.
  • We reproduce the class name schema for simplicity.
  • A component prop to match the className prop. This is identical to what we already support.
  • A components prop to match the classes prop. This is new, we reproduce react-select approach exactly. For instance:
components?: {
  Root?: React.ElementType; // equivalent to classes.root
  Label?: React.ElementType; // equivalent to classes.label
};

This also means moving the existing XxxComponent props under this new umbrella. This same API will be exposed to the future unstyled components, allowing full customizability with styled components. I don't have any strong point of view on the name, we could call it slots.

  • For the cases where the component API limitations are most likely to be painful (requiring to access external states) we use a renderX API, e.g. renderInput. We flatten these props because it's not meant to be used systematically, we should never have more than 2 or 3 render props per component. If we do, they should either be moved to the components API or the component should be split to leverage composition or exposed as hooks. The first argument of the render prop contains all the required state. We can expose both the renderInput API with the components API.
  • We remove the XxxProps props.

cc @mui-org/core-team

I definitely support moving to this:

// notice slots
slots?: {
  // notice the functions
  Root?: (props)=> React.ElementType; // equivalent to classes.root
  Label?: (props)=> React.ElementType; // equivalent to classes.label
};

Matches classes and it is easier to understand what the new slots scoped prop is used for across Mui. That being said, I much rather use a function that returns the component. That will allow me to create static components, and deal with variable hoisting in the factory function rather than in the components.

That being said, I much rather use a function that returns the component.

@yordis Could you provide an example to demonstrate the benefit of a function here rather than just React.ElementType? It isn't clear to me how you would picture this working or what the benefit is, and it seems more complicated to use.

With the following:

components?: {
  Root?: React.ElementType; // equivalent to classes.root
  Label?: React.ElementType; // equivalent to classes.label
};

users can do things like:

const StyledDiv = styled.div`
  background-color: blue;
`;
...
components: {
  Root: StyledDiv 
};

It seems cumbersome and confusing to instead do

components: {
  Root: () => StyledDiv 
};

@ryancogswell hey there, let me know if the following example makes sense.

I think your example is valid, but there are some caveats to it since that example is for trivial cases, sometimes you will need to remap or access hoisted values so you can't simply do that.

Thinking out loud, the differences between a Function and React Component is technically none for the end-users, except for Mui internals and React itself, so I guess you can take a "Component" rather than a function anyway.

import ExternalComponent from 'whatever-thing-i-dont-control';

// static, it doesn't require runtime.
const SomethingNoHoisted = (props)=> {
  return <ExternalComponent remapped={props.label} title={props.title}/>
}

function MyComponent(props) {
  const title = props.title ?? 'Unknown';

  // I guess you co do this, the only thing you are missing is that Mui
  // will use `createElement` or <Something/> over Something() in the internals (or you change to createSomething), so no a big
  // deal I guess.

  // The caveat is that you lose some static declaration of the component
  // and require runtime for it, meh.

  // So personally speaking, I don't see these functions as component but factories.
  function Something(props) {
    return (
      // Has access to anything internal from MuiComponent
      // passing using props in case that some computations from inside
      /// needs to be exposed to the outside scope
      <ExternalComponent remapped={props.label}
        // Remapping in case you need it
        remapped={props.label}
        // Access to hoisted variables,
        title={title}/>
      )
  }

  return (
    <div>
      <MuiComponent
        slots={{
          // Something: Something,

          // I can't do this due to prop hoisting values, and props that needs
          // to be remapped.
          // Something: ExternalComponent

          // I don't control the rendering of the component, therefore, I can't
          // pass `title` from the hoisted variable.
          // Something: SomethingNoHoisted
        }}    
      />
    </div>
  )
}

And I can't agree that it is cumbersome and confusing due to my personal experience, I can only speak from my perspective.

And probably you are right, I am thinking in FP practices for function composition that we don't need in some cases and will create harder to understand code for some people.

I take back my suggestion, doesn't matter to me.

@yordis It seems that the main problem you are trying to solve (or at least a different way to frame the problem) is how to pass additional props to your custom component. One way this has been solved in Material-UI is a corresponding XXXProps prop.

For instance, Modal (in Olivier's intial comment) has a BackdropProps prop in addition to a BackdropComponent prop. With the new structure, it might look more like:

<Modal components={Backdrop: CustomBackdropComponent} BackdropProps: { customProp1: props.valueFromParentScope } ...>...</Modal>

or if we take the complicated case in your example (the one requiring a function) it could look like:

import ExternalComponent from 'whatever-thing-i-dont-control';

// This can now still be static.
function Something({label, title, ...other}) {
    return (
      <ExternalComponent remapped={label} title={title} {...other}/>
      );
}

function MyComponent(props) {
  const title = props.title ?? 'Unknown';
  return (
    <div>
      <MuiComponent components={{ SomeMuiNestedComponent: Something}} SomeMuiNestedComponentProps={{ title }} />
    </div>
  )
}

In this scenario MUI would merge the props specified via SomeMuiNestedComponentProps with the props that it would send to SomeMuiNestedComponent.

As a side note, I prefer the name components over slots since I think it more clearly communicates the type expected (assuming the type is React.ElementType) and retains more consistency with prior Material-UI naming conventions.

<Modal components={Backdrop: CustomBackdropComponent} BackdropProps: { customProp1: props.valueFromParentScope } ...>...</Modal>

That example is exactly what I am trying to prevent since I been there. This is why either to use a factory function which I personally prefer or a dynamic component in the scope. I have been bitten by that example multiple times and the problems that bring in the future.

Reasons

  1. It definitely increases the error-prune by mismatching
  2. Computations are wasted in cases where the SomeMuiNestedComponent component is not being rendered, but you computed all the properties for it regardless (nested loops doing these practices, the refactoring was a nightmare to track since happened at multiple levels).
  3. Mui doesn't need to take such complexity to make sure to proxy the right props to the right component at the right time.
  4. The functional style is friendly to test environments since you can always isolate the hoisting out of the component test, potentially testing the data transformation in isolation if needed and compose more functions to create the component factory function (meh).

@yordis To take your example, using props from the outside would look like this, with the first proposal. We would use the context:

import ExternalComponent from 'whatever-thing-i-dont-control';

const Context = React.createContext();

function Something(props) {
  const { title } = React.useContext(Context);

  return (
    <ExternalComponent remapped={props.label}
      remapped={props.label}
      title={title} />
  )
}

function MyComponent(props) {
  const title = props.title ?? 'Unknown';

  return (
    <div>
      <Context.Provider value={{ title }}>
        <MuiComponent
          components={{
            Something,
          }}
        />
      </Context.Provider>
    </div>
  )
}

It comes with two drawbacks compared to the XxxProps props: 1. It forces you to import the correct element for the slot, you have to find it (can probably be solved with great conventions and documentation), 2. It's more boilerplate.

A second possible proposal

The same one as the first, however, we introduce a prop to match the current use cases for the XxxProps props.

slotComponents?: {
  Root?: React.ElementType<RootProps>;
  Label?: React.ElementType<LabelProp>;
};
slotProps?: {
   Root?: RootProps;
   Label?: LabelProp;
};

This option has the advantage of allowing to customize nested components without even needing to import them.

A third possible proposal

This time with render props, all in. For the cases where the render prop is mandatory, like renderInput we would expose it independently.

renders?: {
  root?: (props: RootProps) => React.ReactNode;
  label?: (props: LabelProp) => React.ReactNode; 
};

I see two downsides with this approach

  1. The developers will need to import the correct components. This can be mitigated with awesome documentation and great conventions.
  2. The source of our components will look "strange", we will no longer be able to use the JSX syntax. Imagine we were only using the React.createElement syntax, it would look like the same. It might be disorienting for the developers looking at our source to figure customizability issue or inspiration.
const Button = React.forwardRef(function Button(props, ref) {
  const {
    children,
    classes,
    className,
    color = 'default',
    component = 'button',
    renders = { root = props => <ButtonBase {...props} />, label: props => <span {...props} /> },
    disabled = false,
    disableElevation = false,
    disableFocusRipple = false,
    endIcon: endIconProp,
    focusVisibleClassName,
    fullWidth = false,
    size = 'medium',
    startIcon: startIconProp,
    type = 'button',
    variant = 'text',
    ...other
  } = props;

  const startIcon = startIconProp && (
    <span className={clsx(classes.startIcon, classes[`iconSize${capitalize(size)}`])}>
      {startIconProp}
    </span>
  );
  const endIcon = endIconProp && (
    <span className={clsx(classes.endIcon, classes[`iconSize${capitalize(size)}`])}>
      {endIconProp}
    </span>
  );

  return renders.root({
    className: clsx(
      classes.root,
      classes[variant],
      {
        [classes[`${variant}${capitalize(color)}`]]: color !== 'default' && color !== 'inherit',
        [classes[`${variant}Size${capitalize(size)}`]]: size !== 'medium',
        [classes[`size${capitalize(size)}`]]: size !== 'medium',
        [classes.disableElevation]: disableElevation,
        [classes.disabled]: disabled,
        [classes.fullWidth]: fullWidth,
        [classes.colorInherit]: color === 'inherit',
      },
      className,
    ),
    component,
    disabled: true,
    focusRipple: !disableFocusRipple,
    focusVisibleClassName: clsx(classes.focusVisible, focusVisibleClassName),
    ref,
    type,
    ...other,
    children: render.span({
      className: classes.label,
    })
  })
});

On a related note, during my exploration of the unstyled story, I ended up trying a prop to solve a similar problem:

forwardProps?: (slot: 'root' | 'label', state) => props;

The prop (or something similar) is required to give the nested styled-components access to the external props and internal state of the component, to style it accordingly. It also assumes that we want to keep the CSS specificity at 1.

My personal opinion:

How to store the props?

Here we have only 2 possible options: flat vs deep. I believe that flat option is more useful because it is

  • Easier to document
  • Easier to find with a pure editor (e.g. when you need to override label you can only type the label and will get renderLabel in the editor result
  • Easier to manage, because we have fewer rules on the codebase

Components vs functions

Personally I have no preference on that. There is no performance difference even on the big amount of nodes.

Also could say that render functions look more expressive from the reading side. In this example, if you are looking for overrides rendering you could miss the components prop because it is just an object

<Something
  SelectComponent={Select}
/>

And here your editor will shout you that something is rendering over here.

<Something
  renderSelect={props => <Select {...props} />} 
/>

Reading your examples and opinions, and based on my experience, I definitely will stick to factory/render function to tackle this problem.

Something else we could do is to adopt some practices from the Vue community if I am not mistaken they use factory/render functions to tackle this issue.

I like that they use slots (close to what the HTML spec suppose to do) and the naming is normally a noun instead of a verb (label vs renderLabel). I wouldn't mind align with them in this regard, it looks clean personally speaking, and it tackles all the technical use cases so far.

// some generics
interface MuiUniversalProps <T = unknown> {
  slots?: T;
}

interface Slot<T = unknown> {
  (props: T): React.Child;
}
// button component
interface ButtonSlots {
  root: Slot<{}>;
  label: Slot<{}>;
}

interface ButtonProps extends MuiUniversalProps<ButtonSlots> {
  ...
}

I don't think I have much to contribute at this point, I trust your judgment.

flat vs deep

@dmtrKovalenko While I think that we should encourage flatten props as much as possible for the reasons mentioned in https://github.com/mui-org/material-ui/issues/21453#issuecomment-656065074. (@dtassone Yes, I very much have the DataGrid API in mind the options could be flattened :p), It's not without its limitations.

I see a couple of advantages in going deep:

  • As a developer, it makes it easier to find all the customization points (both when reading the docs and when in the editor with IntelliSense), you go look at the prop y, it's one place, always the same between all the components. You don't need to scan the whole set of props the component exposes. Take the Autocomplete, we have 59 props, take the DatePicker, we have 65 props, good luck if they aren't prefixed with the same wording, like renderX or componentX.
  • It makes it easier to forward these props deep down the React tree, you forward one y prop, not all the props.
  • These are props you will likely not use frequently, likely for the first time you want to customize your component, then you can create an abstraction and reuse it.
  • It's consistent with the classes prop.

Thanks @oliviertassinari :)

I think both ways are very discussable
if we go deep we have clean and tidy props such as, options, events components...
However, properties like loading which just turn the loading overlay for the grid, or disabled don't really belong in options. I mean they could be there. But I find it unpractical to put them in a nested object.
Could we go for an hybrid approach, should we flatten state boolean props, or should we just flatten everything?

If we decide to flatten the props, then, inside the component, we will probably have to restructure some of the props together, so we can process or observe them together.

Another kind of drawback is that we mind end up with a very long list of props for some components and it might be difficult to find what we want in the middle of everything.

Events, components are quite limited, but some components might have a long list of configuration options. So should we consider to flatten everything except configuration options...

I'm leaning toward the second proposal. The 3rd one seems to be a no-go regarding the readability of the source at scale. Readability is still OK for one-off cases. The 1st one is quite limiting for leveraging props from the outside. it's cumbersome to leverage the context. Hence the second as a tradeoff. It's basically the 1st proposal but extended with slotComponents and slotProps).

Has anyone taken a look at an approach like React Aria is using?:

import {useButton} from '@react-aria/button';

function Button(props) {
  let ref = React.useRef();
  let {buttonProps} = useButton(props, ref);

  return (
    <button {...buttonProps} ref={ref}>
      {props.children}
    </button>
  );
}

<Button onPress={() => alert('Button pressed!')}>Press me</Button>

I've used a couple APIs like this (another is react-swipeable). Perhaps, irrespective of whatever deeper-level abstraction is picked from what's being discussed above, material-ui can incorporate an API that sort of "handles the details" for you, by using this technique.

That way, if a user wants to override something, they can dig in and find out more about the structure, but if they don't then they don't have to. React Aria's API is like this in most places, so take a look there for more concrete examples of it working in practice.

@dimitropoulos This RFC doesn't aim to solve the whole problem of customizability. There are different levels of abstraction that are viable, each with pros & cons, I would put them in 3 buckets:

  1. A hook first approach. This gives maximum flexibility, great for power users that understand the value of a11y. e.g. https://www.downshift-js.com/.
  2. A component first approach with components close to the DOM nodes. This is great for composition and can be more practical for users than hooks. e.g. https://reach.tech/combobox.
  3. A high-level component approach. Sometimes, they would be too many components to compose to make it feasible. Imagine react-table scaling to the needs of ag-grid: no way. e.g. https://react-select.com/.

The RFC focuses on improving 3. This is a problem we face with the upcoming DataGrid and DatePicker components. The RFC is hopefully also something I hope we can use to provide unstyled components or different styles engine.

For 1. it will require a different effort. But I definitely agree that it would make sense to unbundled the component we have, like the angular CDK. Do you have any interest in helping us with this?
For 2. it's the approach we have tried to follow (but with exceptions).

sure sure, yeah, I get that, I was just saying that whatever solution this RFC lands on - it would be cool to keep the React Aria-style API above in mind because it offers a lot of flexibility which I (at least tangentially) view to be a deeper goal of this RFC: so I thought I'd mention it.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

zabojad picture zabojad  路  3Comments

newoga picture newoga  路  3Comments

sys13 picture sys13  路  3Comments

revskill10 picture revskill10  路  3Comments

reflog picture reflog  路  3Comments