Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duplicated MuiButtonBase (and others) in <style> elements #15610

Closed
2 tasks done
marc-polizzi opened this issue May 7, 2019 · 41 comments
Closed
2 tasks done

Duplicated MuiButtonBase (and others) in <style> elements #15610

marc-polizzi opened this issue May 7, 2019 · 41 comments
Labels
status: waiting for author Issue with insufficient information v4.x

Comments

@marc-polizzi
Copy link

marc-polizzi commented May 7, 2019

The <head> section contains several times the same material-ui internal <style> creating conflicts in styles being applied. The content of those <style> are not using unique names as before it seems: .jss346...

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

Expected Behavior 🤔

For example, .MuiButtonBase-root should be present once only.

Current Behavior 😯

.MuiButtonBase-root is added 2/3 times.

material-ui-issue

material-ui-style-content

Steps to Reproduce 🕹

Cannot reproduce with a simple piece of code; even in our application sometimes it's ok sometimes not. Tell us how to troubleshoot on our side to provide more information.

Context 🔦

Nothing special I believe. Components are badly rendered because of material-ui internal styles are being applied out of order.

Your Environment 🌎

This issue appeared since v4 beta.0 (still in beta.1): v4 alpha were ok.

Tech Version
Material-UI v4 beta.0 and 1
React 16.8.6
Browser
TypeScript 3.4.5
etc.
@oliviertassinari
Copy link
Member

@marc-polizzi How many theme providers do you use?

@marc-polizzi
Copy link
Author

@oliviertassinari the application is using several

<ThemeProvider ...

statements but in the faulty usecases we're using a single one. I've put a breakpoint in the Chrome browser and only the one of our top level component seems called. Something like:

    render() {
        return (
            <React.Fragment>
                <CssBaseline/>
                <ThemeProvider theme={UxTheme}>
                    <UxConsoleApp />
                </ThemeProvider>
            </React.Fragment>
        );
    }

@oliviertassinari oliviertassinari added the status: waiting for author Issue with insufficient information label May 7, 2019
@oliviertassinari
Copy link
Member

@marc-polizzi Two things. If you are using multiple ThemeProvider, you need to provide one at the root. You can change the class name generation to output non-global class names with the disableGlobal option, like before in v3.

@oliviertassinari
Copy link
Member

@marc-polizzi Do you use multiple React mount point?

@marc-polizzi
Copy link
Author

marc-polizzi commented May 7, 2019

If you mean we're calling several times ReactDOM.render(...), yes: one for the main application component and others for rendering dialogs. For the sake of simplicity, I've removed all of them to keep the only one in our index.tsx file that is rendering our top level component:

ReactDOM.render(app, document.getElementById('ic3-root') as HTMLElement);

Still have the same issue.

@oliviertassinari
Copy link
Member

oliviertassinari commented May 7, 2019

@marc-polizzi Then I'm out of ideas. You need to run a dichotomy on your React Tree.

@marc-polizzi
Copy link
Author

@oliviertassinari sorry not sure to understand; what do you mean by "run a dichotomy" ?

@oliviertassinari
Copy link
Member

You can use a dichotomy to find the source of the problem. Unmount half your tree, see if the problem is still present, repeat.

@oliviertassinari
Copy link
Member

@marc-polizzi Let us know when you find the source of the problem. We would be happy to understand how we can improve the implementation. I'm closing, waiting for a reproduction. Thanks for the report.

@marc-polizzi
Copy link
Author

marc-polizzi commented May 12, 2019

Here is a little example - self contained in the index.tsx file - that is failing in my project. Unfortunately, I cannot reproduce the same behavior in codesandbox.io. When running the following React app, I can see several MuiButtonBase CSS rules generated creating some unexpected conflicts.

It seems this is related to the import:

import ButtonBase from '@material-ui/core/ButtonBase/ButtonBase';

Importing as following is solving the issue:

import ButtonBase from '@material-ui/core/ButtonBase';

I'm still investigating... Any advice / feedback?

import * as React from 'react';
import * as ReactDOM from 'react-dom';

import IconButton from '@material-ui/core/IconButton';
import ButtonBase from '@material-ui/core/ButtonBase/ButtonBase';

class UxMainApp extends React.Component {

    render() {

        return (
            <>
                <div style={{border: '1px solid black'}}>
                    <IconButton>clik-me</IconButton>
                </div>
                <ButtonBase component={'a' as any}> app-link </ButtonBase>
            </>
        );

    }

}

ReactDOM.render(<UxMainApp/>, document.getElementById('ic3-root') as HTMLElement);

@oliviertassinari
Copy link
Member

oliviertassinari commented May 12, 2019

@marc-polizzi You should get a style module duplication warning in the console in this case with v4.0.0-beta.1. You might bundle two versions of the components: the CJS and ESM.

@marc-polizzi
Copy link
Author

You're right, there seems to be two modules ButtonBase as you can see in the browser tools window attached. But the console is empty.

modules

@oliviertassinari
Copy link
Member

I fear we can't help more without a reproduction.

@marc-polizzi
Copy link
Author

I've copy / pasted that file index.tsx into a codesandbox.io but got no issue. I guess something is wrong with our project configuration but have no idea where to look at. In the meantime, we've added a prebuild script that is grepping our files for this kind of imports.

@eps1lon
Copy link
Member

eps1lon commented May 14, 2019

@marc-polizzi Be sure that your path imports are not too deep.

import { Button } from '@material-ui/core'; // Ok
import Button from '@material-ui/core/Button'; // Ok
import Button from '@material-ui/core/Button/Button'; // Not Ok

If you're building with wepack you could use something like webpack-bundle-analyzer or put the created stats.json into webpack-analyse to see which modules are importing creating the duplicate module.

@oliviertassinari
Copy link
Member

I have tried to reproduce the style duplication warning without luck: https://codesandbox.io/s/affectionate-ramanujan-mnjwg.

@ChrisJVoss
Copy link

ChrisJVoss commented Jun 12, 2019

"dependencies": {
    "@material-ui/core": "^4.0.1",
    "@material-ui/icons": "^4.0.1",
    "classnames": "^2.2.6",
    "react": "^16.8.6",
    "react-dom": "^16.8.6",
...
}

I'm having this same issue. .MuiButtonBase-root is being applied twice and is overriding any other styling.
Interestingly, this will only happen some of the time. We have a component, "AccountDetails" that is passed some props and then displays the account information. Some accounts never have this issue while other accounts always do. It's the same component, just different account detail props.

I have checked the "classes" and "theme" props and those do not appear to change between different account detail views nor after the component mounts.
We are using '@material-ui/core/CssBaseline';

The MUI code:

import { Button, Grid, List, ListItem, Paper, Tooltip, Typography, Fab, MenuItem, MenuList } from @material-ui/core';
import { withStyles } from '@material-ui/core/styles';
...
const styles = theme => ({
  extendedIcon: {
    marginRight: theme.spacing(1)
  },
  paper: {
    style: { display: 'flex', flexWrap: 'wrap', textAlign: 'center', justifyContent: 'center' }
  },
  menuStyles: {
    display: 'flex',
    flexDirection: 'column',
    width: '100%',
    backgroundColor: 'ghostwhite',
    border: '1px solid darkgray',
    margin: '8px 0px'
  },
  disabled: {
    cursor: 'default',
    pointerEvents: 'none'
  },
  fabStyle: {
    '&:hover': { backgroundColor: 'transparent' }
  }
});

export default flowRight(
  connect(
    mapStateToProps,
    mapActionsToProps
  ),
  withStyles(styles, { withTheme: true })
)(AccountDetails);

These are the exact same component. Is there anything aside from the "classes" and "theme" props that might cause this?

Screen Shot 2019-06-12 at 4 17 46 PM

Also, In our components that use MUI's ExpansionPanel component, the .MuiButtonBase-root class is applied again and overrides the MUI's own .MuiExpansionPanelSummary-root styling.
We have not added any custom styling to this ExpansionPanel component

import { ExpansionPanel, ExpansionPanelSummary, ExpansionPanelDetails, Paper } from '@material-ui/core';
import { ExpandMore } from '@material-ui/icons';
...
<ExpansionPanelSummary expandIcon={<ExpandMore />}>

Screen Shot 2019-06-12 at 4 09 35 PM

This:
Screen Shot 2019-06-12 at 4 53 23 PM
vs this:
Screen Shot 2019-06-12 at 4 53 15 PM

@ChrisJVoss
Copy link

One of our dependencies also had material-ui as a dependency. We changed material-ui to a peer dependency and our issues are now fixed!

@JReinhold
Copy link

Just cross-posting mui/material-ui-pickers#1143 (comment) here, in case it helps anyone.

I got duplicate style tags, which caused some styles to be wrong (in my case, styles from the @material-ui/pickers package. Following the recommendation at #15610 (comment) solved my issue.

I think it might be possible to use the no-restricted-imports eslint rule with a glob pattern to warn against this in the future. Or, in the case of TSLint, create a regex that matches duplicate strings (with import-blacklist. Unfortunately my Regex-fu is not strong enough.


Original comment

I've experienced the same issue, tracked the cause to this: #15610. I followed #15610 (comment), and fixed all imports that was double nested, eg. changed

import Button from '@material-ui/core/Button/Button';

to

import Button from '@material-ui/core/Button';

I had a LOT of imports to fix (did a search on from '@material-ui/core/, it was a breeze). After all imports were fixed, the problem went away.

You can check if you have the same problem with duplicate imports, by going in to your Chrome DevTools > Sources > expand localhost:port > node_modules > @material-ui > core > esm. If you are doing it "wrong", you will have the same module both in root of core folder, and in core/esm folder. (see first screenshot)
If you are doing it right, you only have modules in the esm folder. (see second screenshot)

Screenshot_07-15 20 28 54

Screenshot_07-15 20 29 35

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 17, 2019

@JReinhold Thanks for sharing your experience. We are using a custom eslint rule on the repository: https://github.com/mui-org/material-ui/blob/master/packages/eslint-plugin-material-ui/src/rules/restricted-path-imports.js for our own use cases.

I would personally encourage people to import from the barrel index:

import { Button } from '@material-ui/core';

@JReinhold
Copy link

We are using a custom eslint rule on the repository: https://github.com/mui-org/material-ui/blob/master/packages/eslint-plugin-material-ui/src/rules/restricted-path-imports.js for our own use cases.

Oh wow, that's cool, is it possible that it can get published? I can see that it is on the registry, however that published version is 3 years old and doesn't contain that rule.

I would personally encourage people to import from the barrel index:

import { Button } from '@material-ui/core';

Yeah, we've restrained from doing that specifically, because in our experience, it slows down the TypeScript compiler a considerable amount. However I've just tested with a few components, and that doesn't seem to be the case anymore (maybe something has changed in v4?), so that's probably a good option.

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 18, 2019

Oh wow, that's cool, is it possible that it can get published?

@JReinhold The eslint rule was written by @eps1lon. I have no objection to publishing it. If the other maintainers agree, we can go for it.

However I've just tested with a few components, and that doesn't seem to be the case anymore (maybe something has changed in v4?), so that's probably a good option.

I'm happy to hear that the performance overhead is no longer present! With #16192, you might be able to automate the migration, at least, to try it.

@sovanjana
Copy link

  1. run npm ls @material-ui/styles command to check if there is any duplicate material-ui in your code.
  2. if any of your package's using material-ui then make sure material-ui package is in devDependency and peerDependency.
    example:
    "peerDependencies": {
    "@material-ui/core": ">=4.7"
    },
    "devDependencies": {
    "@material-ui/core": "4.7.2"
    },

@brandonccx
Copy link

The issue also exists in my development environment when I use npm link. For example:

We have two projects A and B
A depends on @material-ui/core
A depends on B
B depends on @material-ui/core
They both have installed node_modules

I get used to link B to A for debugging. When I run the dev-server on project A, B will look for the module from the nearest path which is installed in directory of B:

/A/node_modules/
    |-- @material-ui/core/...
    |-- B/node_modules/@material-ui/core/...

Therefore, the <style> tags are added twice from the two @material-ui/core modules. I have to do following things with the issue:

  • When I develop A with the linked B, I run mv node_modules/@material-ui node_modules/@material-ui_bak
  • When I develop B separately,I move them back

Hope for any better solution, thank you.

@oliviertassinari
Copy link
Member

@brandonccx #15610 (comment) is a great solution.

@jeromeSH26
Copy link

jeromeSH26 commented Apr 23, 2020

Hi guys,
although this issue is closed, just to confirm that applying some import refactoring and build configuration in webpack/babel, fixed the problem of multi time the same classes to be applied
On top of that, as far as I'm concerned,it fixed also the issue that the global theme defined in the APP is well applied to some components I have created in a separate library I'm importing in the main APP.

It turned out that I had to change the way to build my react/material-Ui components library @mylibrairy/reactcomponentscommon.

1- Make sure that in the library, all imports where such as import { Button} from "@material-ui/core" and not for example import Button from "@material-ui/core/Button"

2- Remove the usage offile-loader pluginin the .babelrc to make sure it doesn't change the way to import material-ui components

3- Push @material-ui/core and @material-ui/icons as dev and peer dependencies in the package.json of the library.

4- Rebuilt the library using webpack and babel to compile typescript tsx to js.

Then, importing the library in the main APP, creating the theme and applying it at the root of the App worked like a charm, whereas I had stronge behavior from one component to another before applying this method.

However to be fair, I do not really understand the reason why this refactoring and the move from dependency to dev and peer fixed the problem.

nickpalmer added a commit to nick-codes/tubular-react that referenced this issue Jun 26, 2020
* Fix an issue which caused multiple styles to be applied in the
  incorrect order.

  For details about the duplicated style issue please see this issue in
  material-ui: mui/material-ui#15610

* Add a lint check to ensure the issue never returns.

* Fix linter execution so it should pass on CI.
kadosh pushed a commit to unosquare/tubular-react that referenced this issue Jun 26, 2020
* Fix an issue which caused multiple styles to be applied in the
  incorrect order.

  For details about the duplicated style issue please see this issue in
  material-ui: mui/material-ui#15610

* Add a lint check to ensure the issue never returns.

* Fix linter execution so it should pass on CI.
@vilanz
Copy link

vilanz commented Aug 11, 2020

It's closed, but just my two cents:
My package had @material-ui/core/esm in the node_modules of the built code, which caused the duplication issue. I couldn't make my package use MUI as a peerDependency, so adding this to my Webpack config solved it for me.

resolve: {
  alias: {
    '@material-ui/core': path.resolve(__dirname, 'node_modules', '@material-ui/core')
  }
}

@ghost
Copy link

ghost commented Aug 13, 2020

We're running into the same issue, but in our case with components that come from 'lab':

Neither of the following seem to work:

import ToggleButton from '@material-ui/lab/ToggleButton';
import ToggleButtonGroup from '@material-ui/lab/ToggleButtonGroup';
// import { ToggleButtonGroup, ToggleButton } from '@material-ui/lab';

There's always duplicate MuiBaseButton style definitions. Any thoughts? 🙏 @oliviertassinari maybe?

EDIT: w/ the following versioning btw

├─ @material-ui/core@4.11.0
├─ @material-ui/icons@4.9.1
├─ @material-ui/lab@4.0.0-alpha.56

@aheidelberg
Copy link

I also have a duplicate MuiButtonBase definition.

I'm pretty sure the problem comes from two bundles that are included on the same page as mentioned here.

Since the CommonsChunkPlugin isn't there anymore I tried to replicate the example with the SplitChunksPlugin.
Here is the part I added to my webpack.config.js

optimization: {
    splitChunks: {
        cacheGroups: {
            vendor: {
                test: /[\\/]node_modules[\\/](@material-ui\/styles)[\\/]/,
                name: 'vendor',
                chunks: 'all',
            },
        },
    },
},

Then I included the vendor.js bundle before the two application bundles.

This did not fix the issue. Is there anything I need to change with this config?

As far as I can tell (via the Chrome DevTools > Sources page and by looking through the code) I don't have any wrong imports.
Screenshot 2020-08-20 at 08 01 09

@nkpz
Copy link

nkpz commented Sep 14, 2020

My use case is similar to @jeromeSH26 - creating a library of common components to be used in multiple projects within an organization. His advice is good, but in order to get this fully working for my setup, I also had to declare all material UI components as externals in the webpack config for my library, and I did this with a regex like so:

  externals: [
    {
      react: 'commonjs react',
      'react-dom': 'commonjs react-dom',
    },
    /@material-ui\/.*/
  ]

Everything is now working as expected. Hope this helps someone.

@avelosa
Copy link

avelosa commented Sep 14, 2020

@nkpz does using webpack externals require including a script tag (in html where bundles and entry point is )with src being a CDN url?

@nkpz
Copy link

nkpz commented Sep 15, 2020

@avelosa No, defining an external will just tell it not to bundle the dependency and to look for it elsewhere in the bundle at runtime. It's useful for authoring a component library as an addon to mui because everything using your library will already have its own version of react, react-dom, material-ui etc

@areisle
Copy link

areisle commented Dec 17, 2020

I'm having this issue when using mui components with aggrid. I'm not sure if it's mui or aggrid causing the issue, but I've made a min repo showing the issue https://github.com/areisle/aggrid-mui-styles-issue and deployed it here https://aggrid-mui-issue.herokuapp.com/

@thediveo
Copy link

thediveo commented Jan 7, 2021

It's getting weirder by the yarn start cycle ... after having refactored all imports, where applicable, to be in the form of import { Blafasel } from '@material-ui/core' I still had duplicate <style/> nodes. Then I tripped upon React: CssBaseLine component doesn't update after Material UI theme changed which notices that <React.StrictMode> triggers this when using theme changing. And in my case that got me rid of the duplicate <style/> nodes; related is: Style nodes leaking when switching global theme in StrictMode #20708.

@miriamso
Copy link

miriamso commented Feb 2, 2021

I had same issue
Fixed by adding these lines to webpack aliases

resolve: {
  alias: {
   '@material-ui/styles': path.resolve('./node_modules/@material-ui/styles'),
   '@material-ui/core': path.resolve('./node_modules/@material-ui/core'),
  }
}

@pzi
Copy link
Contributor

pzi commented Mar 22, 2021

My use case is similar to @jeromeSH26 - creating a library of common components to be used in multiple projects within an organization. His advice is good, but in order to get this fully working for my setup, I also had to declare all material UI components as externals in the webpack config for my library, and I did this with a regex like so:

  externals: [
    {
      react: 'commonjs react',
      'react-dom': 'commonjs react-dom',
    },
    /@material-ui\/.*/
  ]

Everything is now working as expected. Hope this helps someone.

I had to change the webpack config to the following to get it to work for our component library:

  externals: [
    {
      react: 'commonjs2 react',
      'react-dom': 'commonjs2 react-dom',
      '@material-ui/core': 'commonjs2 @material-ui/core',
      '@material-ui/lab': 'commonjs2 @material-ui/lab',
      '@material-ui/pickers': 'commonjs2 @material-ui/pickers',
      '@material-ui/styles': 'commonjs2 @material-ui/styles',
    },
  ],

When I used the regex rule, it was complaining about an invalid @ character when bundling. This seemed to have something to do with another webpack config in output.libraryTarget: 'commonjs-module'. Not sure what 'commonjs-module' does compared to 'commonjs2'.

@ehimare369
Copy link

My use case is similar to @jeromeSH26 - creating a library of common components to be used in multiple projects within an organization. His advice is good, but in order to get this fully working for my setup, I also had to declare all material UI components as externals in the webpack config for my library, and I did this with a regex like so:

  externals: [
    {
      react: 'commonjs react',
      'react-dom': 'commonjs react-dom',
    },
    /@material-ui\/.*/
  ]

Everything is now working as expected. Hope this helps someone.

I had to change the webpack config to the following to get it to work for our component library:

  externals: [
    {
      react: 'commonjs2 react',
      'react-dom': 'commonjs2 react-dom',
      '@material-ui/core': 'commonjs2 @material-ui/core',
      '@material-ui/lab': 'commonjs2 @material-ui/lab',
      '@material-ui/pickers': 'commonjs2 @material-ui/pickers',
      '@material-ui/styles': 'commonjs2 @material-ui/styles',
    },
  ],

When I used the regex rule, it was complaining about an invalid @ character when bundling. This seemed to have something to do with another webpack config in output.libraryTarget: 'commonjs-module'. Not sure what 'commonjs-module' does compared to 'commonjs2'.

I am glad it worked for you. Please give an update if you later find a concrete difference between 'commonjs module' and 'commonjs2'.

@zhuhang-jasper
Copy link

Any solution for CRA? (we don't have webpack config)

@thediveo
Copy link

See #15610 (comment); I'm using CRA.

@zhuhang-jasper
Copy link

See #15610 (comment); I'm using CRA.

This doesn't help. I already have my "strictMode" removed long ago.
Actually this issue was fixed when I first faced it when I uses @material-ui/core.
Today I use @material-ui/lab, and this problem happens again.

I already tried moving both these packages into dev & peer dependencies.

@mui mui locked as resolved and limited conversation to collaborators Aug 14, 2021
@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 14, 2021

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: waiting for author Issue with insufficient information v4.x
Projects
None yet
Development

No branches or pull requests