Having two versions of MUI in node-modules duplicates class names on production

[Kmaschta]

Because of JSS, if we have two different versions of material-ui in the node modules, there is conflict in CSS classes on production.

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

Expected Behavior

Say, you have an app that requires Material UI and another module that requires another version of Material UI called dependency.

node_modules/dependency/package.json

{
    "version": "1.0.0",
    "dependencies": { "@material-ui/core": "1.4.0" }
}

package.json

{
    "private": true,
    "dependencies": {
        "@material-ui/core": "1.5.0",
        "dependency": "1.0.0"
    }
}

If you are using the styling and theming features of Material UI in both of them, there will be two versions of JSS and they will minify their classes in the same fashion on prod: jss1, jss2, etc.

This leads to a class names conflicts that appears only when NODE_ENV is production.

Context

This impacts all library maintainers relying on Material-UI and their users.

We have an example of that behavior on react-admin, see https://github.com/marmelab/react-admin/issues/1782

There is workarounds like checking for older versions of MUI or warn users on the doc but it would be great that the root cause would be fixed.

Your Environment

Tech	Version
Material-UI	v1.x.x
React	16
Browser	All
TypeScript	N

[eps1lon]

I think we could add the package version to https://github.com/mui-org/material-ui/blob/047dab5064ae971e23a318bdd609077eef6d6efa/packages/material-ui/src/styles/withStyles.js#L233

But now all the styles are duplicated across your app. We could version our components separately but how would that have to look to be maintainable?

If I override the style of a component in a root ThemeProvider would that only apply to child components with the same version or everything?

And I'm not convinced we should warn about conflicting versions. Again the version is library wide but components might be compatible across major releases and this would lead to warnings that can be ignored and now every other warning that might cause actual bugs might also be ignored.

Would adding the version as a class to all components help you deal with conflicting styles?

[oliviertassinari]

Some thoughts.

1. People should be able to recover from the withStyles duplication quite easily: https://material-ui.com/getting-started/faq/#why-aren-39-t-my-components-rendering-correctly-in-production-builds- ?
2. I would encourage third-party libraries using Material-UI to have @material-ui/core as a peer dependency. Or maybe we should scope the singletong issue to a @material-ui/styles peer dependency.
3. You should have a warning like https://github.com/mui-org/material-ui/blob/047dab5064ae971e23a318bdd609077eef6d6efa/packages/material-ui/src/styles/createGenerateClassName.js#L35-L44
4. @eps1lon jss work around the problem with a moduleId counter: https://github.com/cssinjs/jss/blob/master/packages/jss/src/utils/moduleId.js. While it's solving the UI broken part, it hidding the root problem, there is code duplication. To see if it's a good tradeoff for us.
5. We could make the default className generator of Material-UI a global, shared between different versions.

[eps1lon]

~Shouldn't this happen in development, too?~

https://github.com/mui-org/material-ui/blob/047dab5064ae971e23a318bdd609077eef6d6efa/packages/material-ui/src/styles/createGenerateClassName.js#L7 I think this should be initialized on invocation if undefined. If one lazy loads this it will be reset.

[Kmaschta]

Thanks for the quick answers!

And I'm not convinced we should warn about conflicting versions.

Neither am I. It can be fixed from JSS or MUI in my humble opinion.

Would adding the version as a class to all components help you deal with conflicting styles?

I don't know if it's the best solution, but it would definitely fix the issue!

@oliviertassinari Yep, I read that, but all modules can't use peerDependencies. For example in the case of react-admin, it's a "out-of-the-box" lib and its goal of to be bootstraped as fast as possible. Is there a solution that can be done at the level of Material UI to avoid that class names conflict ?

[rosskevin]

but all modules can't use peerDependencies. For example in the case of react-admin, it's a "out-of-the-box" lib

Then you could use a webpack alias to point to a single mui in node_modules to force resolution.

[Kmaschta]

@rosskevin We don't use webpack on react-admin directly, it's up to the user (usually behind a create-react-app). And we can't tell to all our users to set an alias for material-ui.

[rosskevin]

create-react-app is webpack. This is not an uncommon userland issue and aliases are simple to add. It addresses your direct comment about situations when peerdependencies are not or cannot be used.