mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.85k stars 32.25k forks source link

CssBaseline styles are duplicated on Mui v5 #33332

Closed thareshjose closed 2 years ago

thareshjose commented 2 years ago

Duplicates

Latest version

Current behavior 😯

Screenshot 2022-06-29 at 7 03 15 PM

Here are the reproducible sandboxes where the issue occurs on V5, and the same works fine on v4:

MUI v5 (where the CssBaselines are duplicated, as many times as the component is on the dom): https://codesandbox.io/s/material-ui-themes-forked-ln3qrw?file=/src/index.js

Screenshot 2022-06-29 at 6 51 13 PM

Material UI v4 (where CssBaseline styles are not duplicated, despite including it twice) : https://codesandbox.io/s/material-ui-themes-forked-3bqpcl?file=/src/index.js

Screenshot 2022-06-29 at 6 47 33 PM

Expected behavior 🤔

Even though the CssBaseline within the custom theme provider, should ideally be only one within an App, including it twice, shouldn't duplicate the CssBaseline styles on the v5 of material ui ( just like how it doesnt duplicate on the v4)

Steps to reproduce 🕹

Steps:

  1. Go to https://codesandbox.io/s/material-ui-themes-forked-ln3qrw?file=/src/index.js
  2. Select the h2 element in the DevTools.
  3. See the styles on html

Context 🔦

From the Mui docs, its clear that ideally when you create a custom theme, and include the CssBaseline, you should basically wrap your entire app with it, and there would be only one CssBaseline at any time. But we have this custom UI library with custom theme built using material ui 5, which is then imported and used on a different app that uses backbone/react. So we do have multiple react components wrapped using this custom theme, and is then added to different nodes on the DOM, and so at certain cases, more than one such react app (wrapped with the custom theme that has a CssBaseline inside) exists on the DOM at a time, and the CssBaseline styles seems to be getting duplicated, where as it does not on the v4, even with the above mentioned structure.

Your environment 🌎

npx @mui/envinfo ``` System: OS: macOS 12.0.1 Binaries: Node: 14.16.0 - /usr/local/bin/node Yarn: Not Found npm: 6.14.11 - /usr/local/bin/npm Browsers: Chrome: 103.0.5060.53 Edge: Not Found Firefox: 91.0.1 Safari: 15.1 npmPackages: @emotion/react: ^11.9.0 => 11.9.3 @emotion/styled: ^11.8.1 => 11.9.3 @mui/base: 5.0.0-alpha.83 @mui/icons-material: ^5.8.0 => 5.8.2 @mui/lab: ^5.0.0-alpha.83 => 5.0.0-alpha.84 @mui/material: ^5.8.1 => 5.8.6 @mui/private-theming: 5.8.0 @mui/styled-engine: 5.8.0 @mui/styles: ^5.8.0 => 5.8.0 @mui/system: ^5.8.1 => 5.8.2 @mui/types: 7.1.3 @mui/utils: 5.8.0 @mui/x-date-pickers: 5.0.0-alpha.1 @types/react: 18.0.9 react: ^17.0.2 => 17.0.2 react-dom: ^17.0.2 => 17.0.2 typescript: 4.6.4 ```
siriwatknp commented 2 years ago

This is expected in v5 because of how emotion works. The CssBaseline uses GlobalStyles behind the scene, so every instance of the CssBaseline will create a new stylesheet.

We should document this and/or add it to the migration guide.

thareshjose commented 2 years ago

@siriwatknp If thats the expected behavior on v5 with emotion, does the mui have any suggested way of handling such a scenario (context as mentioned above), for users migrating from v4 to v5?

siriwatknp commented 2 years ago

@siriwatknp If thats the expected behavior on v5 with emotion, does the mui have any suggested way of handling such a scenario (context as mentioned above), for users migrating from v4 to v5?

I can see 2 choices:

  1. you can create your own react context and skip the CssBaseline if it is a nested
const Nested = React.createContext();

function App() {
  return (
    <Nested.Provider value={true}>
      <NestedApp />
      <CssBaseline />
    </Nested.Provider>
  )
}

function NestedApp() {
  const nested = React.useContext(Nested)
  return (
    <div>
      {!nested && <CssBaseline />
    </div>
  )
}
  1. Don't use CssBaseline implicitly, let the developer who use your custom library decide when to use.
oliviertassinari commented 2 years ago

I was curious about this bug, I wonder if it wasn't a bug in emotion. I have found https://github.com/emotion-js/emotion/issues/2480 that covers precisely this. It seems that we can't do anything about this, other than what Jun already did by answering this issue (bring a first level of docs). So 👍 to close this issue.