[styles] Memoize makeStyle result to avoid rerendering

[marekscholle]

• I have searched the issues of this repository and believe that this is not a duplicate. I expected that I will find the response by googling / searching in issues, but I found nothing. Sorry if this is a duplicate.

Summary 💡

A common style of getting class names for className prop is the following pattern:

const useStyles = makeStyles(theme => {
  root: {
    padding: theme.spacing(1),
  },
})

The Material UI app is wrapped in ThemeProvider context. A component uses useStyles as follows:

function Component() {
  const classes = useStyles();
  return <Box className={classes.root}> ... </Box>
}

Changing theme object (via setTheme) causes that all makeStyles callbacks run with new theme, and it generates new set of classes used by application. The components are rerendered so that they read new class names in classes and React will then propagate the changes to browser DOM.

It looks that useStyles hook causes component to render even if there is no real change in class content. Let's take the example makeStyles above. If I can change e.g. a primary color in a theme, new run of makeStyles will return the same as before: root: { padding: 8 }. A component using useStyles could stay the same, there is no need to create new class name and rerender the component.

Proposal: At least some trivial memoizing should be added to makeStyles, so that if theme is changed, but a class returned by makeStyles has the same value, the class should stay the same; if all classes from makeStyles result are the same as before, a component using useStyles should not rerender.

Examples 🌈

const useStyles = makeStyles(theme => {
  root: {
    padding: theme.spacing(1),
  },
  child: {
    display: 'flex',
  },
})

Changing e.g. primary color in theme causes new run makeStyles, but the values of root and child are the same as before (shallow compare on root and child). A component using these useStyles should not be rendered, the MUI internals should swallow this change of theme which has no impact on this component.

const useStyles = makeStyles(theme => {
  root: {
    padding: theme.palette.primary.main,
  },
  child: {
    display: 'flex',
  },
})

Changing primary color in theme make root differ from previous run. The child may stay the same. The root class has changed, a component render must be triggered.

Motivation 🔦

I developed a quite complicated API visualizer with lot of rows, boxes, values, icons etc.; the DOM is really non-trivial. I provide a dark/light switcher. A use makeStyles / useStyles in virtually every component. Changing the theme from light to dark or vice versa has direct impact on few components; almost all makeStyles use only theme.spacing which I don't touch. Switching the theme causes that virtually all component must be recomputed, it's really slow in dev mode, and noticeable in prod mode.

If there was even a trivial optimization in makeStyles / useStyles (shallow compare on class objects), then changing of theme could be really fast, now it's lagging as whole application must be rendered again.

[oliviertassinari]

We are deprecating the withStyles and makeStyles APIs in v5. Moving to emotion and styled-components with a wrapper.

[oliviertassinari]

Regarding the rerendering cost, the current approach should be fast enough for most of the cases. For instance, if I run a quick benchmark in v4 and v5, I get:

• https://material-ui.com/components/buttons/

• https://next.material-ui.com/components/buttons/

It's improving. In the event you need high-performance updates, I would recommend looking into using CSS variables. We push it in #12827.

[marekscholle]

I would understand if you don't want to investigate this, but I would appreciate a response to the problem I tried to describe, not argumentation with some benchmark that may not be relevant for the issue.

Regarding the rerendering cost, the current approach should be fast enough for most of the cases.

Obviously, if I have useStyles in almost every component, every change of theme object means that almost every component's render is triggered by a change in theme that does not result in any actual change in most cases (class name changes, but the content of class is the same ... instead of avoiding render at all, this changes leaks to browser DOM optimizations with lot of garbage created along the way). This is inevitably slow when there are thousands of components.

Does the v5 solution avoid the problem or not?

E.g. Redux does the optimization that if the map-state-to-props function results in the same data (shallow equality), it does not render the component. As virtually every UI component is behind its "logical" wrapper, this avoids render of almost all components on any state change; the shallow-equality test on map-state-to-props result is OK for all components and helps to avoid potentially much more costly renders. We should have something similar for style classes.

[oliviertassinari]

Does the v5 solution avoid the problem or not?

We don't plan to change the theme rendering tradeoff with the styled API ("wont fix"). Looking at emotion, it's worse in v5 compared to v4 as it rerenders even if the theme is not used: https://codesandbox.io/s/admiring-dhawan-77tmu?file=/src/App.tsx:0-610. I would imagine the same thing for styled-components.

#12827 should allow the outcome you are looking for, by using CSS cascading, not React context cascading to update between the light and dark mode.