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

Information: solving jss classnames production build conflict with external MUI dependent component #11628

Closed
nihaux opened this issue May 29, 2018 · 13 comments
Assignees
Labels
docs Improvements or additions to the documentation

Comments

@nihaux
Copy link

nihaux commented May 29, 2018

So I spend the all afternoon reading the related issues and debugging my code so I am posting this here in case it helps someone.
I can make a PR for the doc if you want but don't know where it should go as it is not directly a MUI issue.

Use case:

  • I have an external component "mui-tree-list" which use mui v1.
  • The lib use rollup to package the component.
  • in the rollup config I put
    external: [ 'react', '@material-ui/core', '@material-ui/icons', 'react-dom' ],
    WRONG ! ;p

If you do this, you'll end up having jss classname conflicts in the production build of you final app.

why ?

cause @material-ui/core does not avoid @material-ui/core/List for instance to be included in your dist package.

and @material-ui/core/List will include withStyles which will cause jss class naming conflict in your final package !

So the solution is, you should put this in your rollup config:
external: id => /react|react-dom|material-ui\/.*/.test(id),

this way all material-ui components will be excluded from your build... lost some hair with this one :p .

@nihaux nihaux changed the title Information: solving jss production conflict with external MUI dependent component Information: solving jss classnames production build conflict with external MUI dependent component May 29, 2018
@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label May 29, 2018
@oliviertassinari
Copy link
Member

oliviertassinari commented May 29, 2018

@nihaux Bundling withStyles twice is definitely not a good idea. A less optimal alternative would have been to use <JssProvider /> to share the same class name generator. Do you have a reproduction example? Maybe we could think of a way to automatically warn about this issue.

@nihaux
Copy link
Author

nihaux commented May 29, 2018

hello @oliviertassinari so I put up a minimal demo to reproduce.

https://github.com/nihaux/demo-double-bundle-withstyles
Is a material-ui component packaged as a library

import * as React from 'react';
import List from '@material-ui/core/List';
import ListItem from '@material-ui/core/ListItem';
import ListItemText from '@material-ui/core/ListItemText';

const DummyComponent = ({ items }: { items: string[] }) => (
  <List>
    {items.map(item => (
      <ListItem>
        <ListItemText primary={item} />
      </ListItem>
    ))}
  </List>
);

export default DummyComponent;

I put the faulty conf in the rollup config external: ['react', 'react-dom', '@material-ui/core'],

I published it on npm. I also commited the dist for the demo purpose.

Then this repo use it:
https://github.com/nihaux/dummy-demo
It's a create-react-app with material-ui + the DummyComponent

I used the AppBar demo from the doc to show the css problem.

class ButtonAppBar extends React.Component {
  
  state = {
    showDummy: false,
  };
  
  componentDidMount() {
    setTimeout(() => this.setState({ showDummy: true }), 5000);
  }
  
  render() {
    const {classes} = this.props;
    return (
      <div className={classes.root}>
        <AppBar position="static">
          <Toolbar>
            <Typography variant="title" color="inherit" className={classes.flex}>
              Title
            </Typography>
            <Button color="inherit">Login</Button>
          </Toolbar>
        </AppBar>
        { this.state.showDummy && <DummyComponent items={['toto', 'titi', 'tata']}/>}
      </div>
    );
  }
}

Everything works great when you're in dev mode but when you build the project and serve it (using https://github.com/zeit/serve) you'll see all the css bugging as soon as the faulty comp is loaded.

@nihaux
Copy link
Author

nihaux commented May 29, 2018

pushed a non uglified/minified etc version of the build to make it easier to read
https://github.com/nihaux/dummy-demo/blob/master/build/static/js/main.8086f9c5.js

@mlindwall

This comment has been minimized.

@sli

This comment has been minimized.

@oliviertassinari oliviertassinari self-assigned this Jun 1, 2018
@oliviertassinari
Copy link
Member

@nihaux The reproduction example is perfect. Thanks. Let's see how we can improve the warnings.

@oliviertassinari
Copy link
Member

Also, I'm gonna add a not in the frequently asked question. People have been facing this issue for months: #8223 (comment)

@oliviertassinari
Copy link
Member

You should have a warning in the console next time.

@marco-silva0000
Copy link
Contributor

I have this exact issue, but I use webpack.

All my libs must not package with the material-ui libs, but the main app, which imports the libs, should have it bundled right?

@Klabauterman
Copy link

I have the same Issue, does some1 have a solution that does not involve uncommenting code from material-ui?

@marton-levay
Copy link

Ohai

If anybody is still looking for a solution with webpack (for us it was an ejected create-react-app with material-ui elements, shared as a library, and then used in a similar cra app with material elements):
You have to externalize your whole material-ui dependency using regex in webpack config, otherwise various material scripts - withStyles, withTheme, etc... - will load twice (or more if you have more libs) which will cause the screen to fall apart.

https://webpack.js.org/guides/author-libraries/#external-limitations
So in webpack.config.prod.js add something similar:

externals: [
    'react',
    'react-dom',
    //this will externalize everything which start with '@material-ui'
    /^@material-ui\/.+$/,
    'prop-types',
    'react-router-dom'
  ],

Of course this means you want to indicate in your /build/package json that these are peerDependecies. You have to have them in the app where you wish to use this lib.

@ghost
Copy link

ghost commented Jan 21, 2020

So I spend the all afternoon reading the related issues and debugging my code so I am posting this here in case it helps someone.
I can make a PR for the doc if you want but don't know where it should go as it is not directly a MUI issue.

Use case:

  • I have an external component "mui-tree-list" which use mui v1.
  • The lib use rollup to package the component.
  • in the rollup config I put
    external: [ 'react', '@material-ui/core', '@material-ui/icons', 'react-dom' ],
    WRONG ! ;p

If you do this, you'll end up having jss classname conflicts in the production build of you final app.

why ?

cause @material-ui/core does not avoid @material-ui/core/List for instance to be included in your dist package.

and @material-ui/core/List will include withStyles which will cause jss class naming conflict in your final package !

So the solution is, you should put this in your rollup config:
external: id => /react|react-dom|material-ui\/.*/.test(id),

this way all material-ui components will be excluded from your build... lost some hair with this one :p .

Where exactly in the rollup.config.js should we put that ? please show a sample.

@nckswt
Copy link

nckswt commented Aug 4, 2020

If anybody is still looking for a solution with webpack (for us it was an ejected create-react-app with material-ui elements, shared as a library, and then used in a similar cra app with material elements):
You have to externalize your whole material-ui dependency using regex in webpack config, otherwise various material scripts - withStyles, withTheme, etc... - will load twice (or more if you have more libs) which will cause the screen to fall apart.

https://webpack.js.org/guides/author-libraries/#external-limitations
So in webpack.config.prod.js add something similar:

externals: [
    'react',
    'react-dom',
    //this will externalize everything which start with '@material-ui'
    /^@material-ui\/.+$/,
    'prop-types',
    'react-router-dom'
  ],

Of course this means you want to indicate in your /build/package json that these are peerDependecies. You have to have them in the app where you wish to use this lib.

This fix worked for me -- I'm surprised I haven't seen more people commenting! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation
Projects
None yet
Development

No branches or pull requests

8 participants