3

I have an issue where I render lots of components, where each of these components have a useStyles. We want the components props to affect the styling, so we combine props and theme settings. Works fine, except that it's incredible slow and I have narrowed it down to it being caused by having prop methods in makeStyles.

It actually doesn't matter if the class is used or not. If all classes are static it's fast, but if I add an unused class that is a method it's slow. The prop doesn't matter either, like if I add test: { fontWeight: () => 'bold' } it's going to be slow again.

Very weird indeed. I'm using Material UI ^4.11.0. Not the latest version of v4. Bug? Anyone heard of this? Anyone got a better solution for adding prop based styles and using the theme at the same time? Anyone got a performance improvement by using useMemo or anything? (It had not effect when I tried).

 const useStyles = makeStyles(({ typography }) => ({
  fontFamily: {
    fontFamily: ({ fontFamily }: any) => fontFamily,
  },
  fontFamilySecondary: {
    fontFamily: typography.fontFamilySecondary,
  },
  ...filterCustomVariants(typography),
  bold: {
    fontWeight: typography.fontWeightBold,
  },
  boldScondary: {
    fontWeight: typography.fontWeightBoldSecondary,
  },
  italic: {
    fontStyle: "italic",
  },
  thisIsNotEvenUsed: {
    fontWeight: () => {
      console.log("Running running running") // Runs many times anyway!
      return "bold"
    },
  },
}))

And the implementation:

const styleProps = { fontFamily: fontFamily }
const classes = useStyles(styleProps)

EDIT: Here are a CodeSandbox that shows this behavour, using latest MUI v4 and without my Apps complexity: https://codesandbox.io/s/black-monad-tr05l?file=/src/App.tsx

NearHuscarl
  • 66,950
  • 18
  • 261
  • 230
Tony Gustafsson
  • 828
  • 1
  • 7
  • 18
  • Looks like you use `makeStyles` the wrong way. Similar question: https://stackoverflow.com/q/69608311/9449426 – NearHuscarl Oct 23 '21 at 06:54
  • @NearHuscarl You were right, I did use it the wrong way. I've updated my code example now above. It didn't make things any faster I'm afraid and the class thisIsNotEvenUsed is still running hundreds of times even if the class it not used. – Tony Gustafsson Oct 23 '21 at 08:54
  • Something else must be wrong in your code. Can you add the whole component that uses `useStyles`? – NearHuscarl Oct 23 '21 at 12:28
  • @NearHuscarl No, take a look at my sandbox link here: https://codesandbox.io/s/black-monad-tr05l?file=/src/App.tsx – Tony Gustafsson Oct 24 '21 at 09:16

1 Answers1

3

This is not a bug, makeStyles in v5 behaves the same way. The dynamic styles using callback is a JSS feature. But JSS dynamic performance has been known to be pretty terrible (see the performance section for more detail, it's about 6 times slower than emotion). That's one of the reason the MUI team decided to abandon it in favor of emotion in v5 which uses dynamic styles heavily.

And yes, it always invokes the callback even if you don't pass anything inside and don't make use of the classes object. It traverses the style object and invoke every callbacks it found when you call useStyles. For more detail about how makeStyles works, I recommend you reading this answer from Ryan.

const useStyles = makeStyles({
  thisIsNotEvenUsed: {
    fontWeight: () => {
      console.log("Running running running"); // Runs many times anyway!
      return "bold";
    }
  }
});
const unusedClasses = useStyles(); // generate the styles

There are some strategies to improve the performance. The easiest one (depend on the situation) is to upgrade to v5 to leverage emotion and its capability to generate dynamic styles efficiently. Another approach is to use the inline styles:

<Typography style={{ fontWeight: props.fontWeight }}>

But if you don't want to pollute your component with inline styles, you can still use makeStyles using the 'style injection' method by creating a nested styles in the container that holds a large amount of elements inside (Grid, List, Stack...)

const useStyles = makeStyles({
  root: {
    "& MuiTypography-root": {
      fontWeight: () => 'bold'
    }
  }
});
<div className={classes.root}>
  {[...Array(100)].map((_, index) => {
    return <Text key={`text_${index}`}>Text Component</Text>;
  })}
</div>

Codesandbox Demo

NearHuscarl
  • 66,950
  • 18
  • 261
  • 230
  • Such a good answer! Didn't expect that really. Thank you very much for your insights. It's clear that we want styled component in the future but it's a large task for a large project. Style injection is a great suggestion I'll try directly. I'll probably use more inline styles and start with components that is used multiple times everywhere. – Tony Gustafsson Oct 25 '21 at 06:04
  • This is still slow though and runs a lot, so we'll go with inline styles. "& > .thisIsNotEvenUsed": { fontWeight: () => { console.log("Running running running") // Runs many times anyway! return "bold" }, }, – Tony Gustafsson Oct 25 '21 at 06:11
  • @TonyGustafsson are you having a nested list or too many containers? the dynamic styles if applied in the container only generate once per render. In the codesandbox if you click the render button to force the app to re-render, it only prints the message once. – NearHuscarl Oct 25 '21 at 06:20