Material-ui: `makeStyles` API creates thousands of `<style>` tags synchronously

Created on 26 Jul 2019  ·  10Comments  ·  Source: mui-org/material-ui

We use hooks with dynamic props extensively.
I might even say it completely got out of hand, our codebase basically has hooks for every CSS property you can set, for example:

    const { widthClass } = useWidth({ width })
    const { fullWidthClass } = useFullWidth({ fullWidth })
    const { maxWidthClass } = useMaxWidth({ maxWidth })
    const { flexChildClass } = useFlexChild({ flexGrow, flexShrink, alignSelf })
    const { spacingClass } = useSpacing(props)
    const { verticalSpacingClass } = useVerticalSpacing({ verticalSpacing })
    const { horizontalSpacingClass } = useHorizontalSpacing({ horizontalSpacing })
    const { overflowClass } = useOverflow({ overflow })

However, this extensive usage is super bad for performance, as it seems the hooks API creates and injects 1) empty styles 2) synchronously.

The profile starts here:
https://github.com/mui-org/material-ui/blob/d4c7c0586bb7853705ed1337f3f8392eb892a1bc/packages/material-ui-styles/src/makeStyles/makeStyles.js#L225
and ends up here:
https://github.com/cssinjs/jss/blob/0b17c5d9717141565e466ea0dafdf3ccbe86df5a/packages/jss/src/DomRenderer.js#L335

However, up to 99% of the <style> tags end up empty:

> [...document.querySelectorAll('[data-jss][data-meta="makeStyles"]')].length
1227
> [...document.querySelectorAll('[data-jss][data-meta="makeStyles"]')].filter(e => e.innerHTML.trim() === '').length
1215
> 1215 / 1227
0.9902200488997555

  • [x] This is not a v0.x issue.
  • [x] I have searched the issues of this repository and believe that this is not a duplicate.

Possibly related to https://github.com/mui-org/material-ui/issues/16111 maybe?

Expected Behavior 🤔

  • jss should not create empty style tags
  • jss should group DOM updates

Current Behavior 😯

  • 99% of the <style> tags created by jss are empty
  • the style tags are inserted inside useSynchronousEffect and do DOM thrashing

Steps to Reproduce 🕹


Link: https://www.eversports.style/design/icons

Just loading the page takes ages, navigating between Typography and Icons sections takes in the order of 2-3 seconds, on a beefy desktop browser.

sorry for not providing a reduced testcase, but meh, its quite clear whats going on.

Context 🔦

Your Environment 🌎

| Tech | Version |
|--------------|---------|
| Material-UI | v4.2.1 |
| React | v16.8.6 |
| jss | v10.0.0-alpha.17 |

styles performance

Most helpful comment

@wereHamster oh nice, I didn’t know…

Then it becomes:

Array.from(document.styleSheets).map(s => s.rules.length).length
1266
Array.from(document.styleSheets).map(s => s.rules.length).filter(l => l > 0).length
1249
Array.from(document.styleSheets).map(s => s.rules.length).filter(l => l > 1).length
48

So most of these are single rule stylesheets.

[...document.styleSheets].flatMap(s => [...s.cssRules]).length
1416
new Set([...document.styleSheets].flatMap(s => [...s.cssRules]).map(r => r.cssText)).size
1347

Most of the rules however are unique. That kind of surprises me…
But actually looking at some of the rules makes me facepalm even more…

[...document.styleSheets].flatMap(s => [...s.cssRules]).map(r => r.cssText)
​​1087: ".c61 { font-weight: normal; }"
​​1088: ".c91 { font-weight: normal; }"
… this goes on for hundreds of rules…
​​​​1138: ".c94 { }"
​​1139: ".c107 { }"
… hundreds of these as well…
​​​201: ".c103.c103 > * { margin-right: 8px; }"
​​​202: ".c103.c103 > :last-child { margin-right: 0px; }"
… hm, some specificity hacks maybe?

So there is no deduplication going on …

Lets investigate further…

new Set([...document.styleSheets].flatMap(s => [...s.cssRules]).map(r => r.cssText).filter(t => t.startsWith('.c')).map(t => t.replace(/^\.c\d+/, ''))).size
84
new Set([...document.styleSheets].flatMap(s => [...s.cssRules]).map(r => r.cssText).filter(t => t.startsWith('.c')).map(t => t.replace(/^(\.c\d+)+/, ''))).size
66

So we are down to < 100 unique classname rules, even less when we remove duplicates due to specificity…

Well not only do we have all these mostly duplicate rules injected into the head, they also have uselessly unique classnames, which defeats the whole purpose style sharing via classes, and blows up the generated html as well…

All 10 comments

Bildschirmfoto von 2019-07-26 11-02-02

This is how the profile of a single navigation / render looks like.

Related to #16180.

for example:

@Swatinem I wouldn't do that! Have you considered not using @material-ui/styles, using for instance, styled-components? The main reason for the existence of @material-ui/styles is to avoid the need to duplicate a CSS-in-JS runtime. If you are OK with +14 kB gzipped, you should use styled-components instead.

Related to #6115 and #16947.

Have you considered not using @material-ui/styles, using for instance, styled-components?

We have discussed this internally today, @priscilawebdev will evaluate moving our uses to styled-components, and keep an eye on how the performance changes…

@Swatinem I don't think these style tags are actually empty. When you create a <style> element and add the rules to it via JS API (which many of the CSS-in-JS solutions do, due to performance reasons), then the resulting CSS won't show up as the innerHTML string. You should use document.styleSheets to enumerate all stylesheets and then check .rules.length to see if they are really empty.

@wereHamster oh nice, I didn’t know…

Then it becomes:

Array.from(document.styleSheets).map(s => s.rules.length).length
1266
Array.from(document.styleSheets).map(s => s.rules.length).filter(l => l > 0).length
1249
Array.from(document.styleSheets).map(s => s.rules.length).filter(l => l > 1).length
48

So most of these are single rule stylesheets.

[...document.styleSheets].flatMap(s => [...s.cssRules]).length
1416
new Set([...document.styleSheets].flatMap(s => [...s.cssRules]).map(r => r.cssText)).size
1347

Most of the rules however are unique. That kind of surprises me…
But actually looking at some of the rules makes me facepalm even more…

[...document.styleSheets].flatMap(s => [...s.cssRules]).map(r => r.cssText)
​​1087: ".c61 { font-weight: normal; }"
​​1088: ".c91 { font-weight: normal; }"
… this goes on for hundreds of rules…
​​​​1138: ".c94 { }"
​​1139: ".c107 { }"
… hundreds of these as well…
​​​201: ".c103.c103 > * { margin-right: 8px; }"
​​​202: ".c103.c103 > :last-child { margin-right: 0px; }"
… hm, some specificity hacks maybe?

So there is no deduplication going on …

Lets investigate further…

new Set([...document.styleSheets].flatMap(s => [...s.cssRules]).map(r => r.cssText).filter(t => t.startsWith('.c')).map(t => t.replace(/^\.c\d+/, ''))).size
84
new Set([...document.styleSheets].flatMap(s => [...s.cssRules]).map(r => r.cssText).filter(t => t.startsWith('.c')).map(t => t.replace(/^(\.c\d+)+/, ''))).size
66

So we are down to < 100 unique classname rules, even less when we remove duplicates due to specificity…

Well not only do we have all these mostly duplicate rules injected into the head, they also have uselessly unique classnames, which defeats the whole purpose style sharing via classes, and blows up the generated html as well…

Well not only do we have all these mostly duplicate rules injected into the head, they also have uselessly unique classnames, which defeats the whole purpose style sharing via classes, and blows up the generated html as well…

@Swatinem did you ever get an explanation for this? Did you ultimately move to styled components to improve performance?

I have moved on to different things by now, maybe @treylon can give you an answer.

@tlambrou We ultimately moved back to styled components and stopped investigating further.

Duplicate of #17370

Was this page helpful?
0 / 5 - 0 ratings

Related issues

mattmiddlesworth picture mattmiddlesworth  ·  3Comments

FranBran picture FranBran  ·  3Comments

mb-copart picture mb-copart  ·  3Comments

finaiized picture finaiized  ·  3Comments

chris-hinds picture chris-hinds  ·  3Comments