production build - classnames conflicts

[darkowic]

The css class names definitions are duplicated for some components - in my case it is duplicated (I guess from my debugging) for MuiIconButton and MuiModal - check current behavior

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

Expected Behavior

The class names should not be duplicated across components.

Current Behavior

My button styles: The class is duplicated. Styles definition:

It is working in development mode: My buttons styles:

and found the definitions:

and from modal:

Steps to Reproduce (for bugs)

I can try to prepare the environment to reproduce the problem but right now I just wanted to report it here.

Context

I'm trying to release my application with the production environment. ## Your Environment

Tech	Version
Material-UI	1.0.0-beta.10
React	15.6.1
browser	any
webpack	^3.3.0

I need some hints where may be the problem. I'm not using withStyles solution anywhere - I use styled components for styles overriding.

[oliviertassinari]

I have already seen some issue around this problem. It was always linked to duplicated className generator instances. I can't help more without a reproduction.

[oliviertassinari]

I'm closing the issue for now as not actionable. Let me know if you have a reproduction example.

[darkowic]

@oliviertassinari I was able to reproduce the problem. Here is the webpack bin - https://www.webpackbin.com/bins/-KuO6ia3h-JDpBOJncZ3

In my case, I have 2 application roots which are mounted independently to 2 different div. Both use the same material-ui ThemeProvider wrapped with JssProvider to override insertionPoint from jss.

Based on createGenerateClassName function, you use counter to have unique class names

export default function createGenerateClassName(): generateClassName {
  let ruleCounter = 0;

  return (rule: Rule, sheet?: StyleSheet): string => {
    ruleCounter += 1;
    warning(
      ruleCounter < 1e10,
      [
        'Material-UI: you might have a memory leak.',
        'The ruleCounter is not supposed to grow that much.',
      ].join(''),
    );

    if (process.env.NODE_ENV === 'production') {
      return `c${ruleCounter}`;
    }

    if (sheet && sheet.options.meta) {
      return `${sheet.options.meta}-${rule.key}-${ruleCounter}`;
    }

    return `${rule.key}-${ruleCounter}`;
  };
}

And my screenshot confirms that for some reason the counter is duplicated.

I need help. I don't know what I'm doing wrong right now.

[oliviertassinari]

@darkowic You need to share the jss instance between the different react trees. You should be good with such change.

[darkowic]

@oliviertassinari I think I'm doing it using my custom ThemeProvider

  <JssProvider registry={context.sheetsRegistry} jss={context.jss}>
    <MuiThemeProvider
      theme={context.theme}
      sheetManage={context.sheetsManager}
      {...props}
    />
  </JssProvider>

I wrap my every react tree with this.

[darkowic]

This issue should be opened.

[oliviertassinari]

Sure, let's sum up what we know so far. This issue arise as soon as you are using multiple react rendering tree. The jss provider is going to create two class name generators, one for each tree. Hence we end up with class name conflicts. @kof Should we extend the JssProvider from react-jss to accept a class name generator?

[darkowic]

WORKAROUND for client-side application: You can create your own createGenerateClassName and move ruleCounter outside the wrapper function

import warning from 'warning';

// Returns a function which generates unique class names based on counters.
// When new generator function is created, rule counter is reset.
// We need to reset the rule counter for SSR for each request.
//
// It's an improved version of
// https://github.com/cssinjs/jss/blob/4e6a05dd3f7b6572fdd3ab216861d9e446c20331/src/utils/createGenerateClassName.js
//
// Copied from material-ui due to issue https://github.com/callemall/material-ui/issues/8223

// This counter is moved outside from `createGenerateClassName` to solve the issue
let ruleCounter = 0;

export default function createGenerateClassName() {
  return (rule, sheet) => {
    ruleCounter += 1;
    warning(
      ruleCounter < 1e10,
      [
        'Material-UI: you might have a memory leak.',
        'The ruleCounter is not supposed to grow that much.'
      ].join('')
    );

    if (process.env.NODE_ENV === 'production') {
      return `c${ruleCounter}`;
    }

    if (sheet && sheet.options.meta) {
      return `${sheet.options.meta}-${rule.key}-${ruleCounter}`;
    }

    return `${rule.key}-${ruleCounter}`;
  };
}

[kof]

Nice one, we had such a use case on the server and I was already planing to for a bold hint in the documentation see #5

But now you actually found a good reason to support 2 parallel running providers.

[oliviertassinari]

We need to research if there are use cases for a strong need of multiple parallel JssProviders. If there are, we need to think of something to support it.

@kof You have just found a use case for a multiple parallel JssProviders on the client side. And I think the proposed solution is simple to implement :).

[kof]

move ruleCounter outside the wrapper function

This will mean that on the server, ruleCounter will never get reseted. We can't do that.

[oliviertassinari]

On the server side, the JssProviders definitely needs to support concurrent async rendering of a react tree (strong need). But the current implementation makes it already supported :)

[oliviertassinari]

@darkowic Proposed a workaround having no access to the underlying stack. We do. We can do better with this extra flexibility: accepting a class name generator instance.

[kof]

Also request to the same endpoint should always respond with same class names.

[kof]

@kof Should we extend the JssProvider from react-jss to accept a class name generator?

1. Yeah one possible way would be to allow JssProvider accept a class name generator like this:

import {createGenerateClassName} from 'react-jss'

const generateClassName = createGenerateClassName()

<JssProvider generateClassName={generateClassName}>
  <App1 />
</JssProvider>

<JssProvider generateClassName={generateClassName}>
  <App2 />
</JssProvider>

2. Another potential option would be to provide some prefix, like an application name. This could work if we assume we don't have unpredictable amount of applications.

<JssProvider classNamePrefix="app-1">
  <App1 />
</JssProvider>

<JssProvider classNamePrefix="app-2">
  <App2 />
</JssProvider>

Pro 1:

• User doesn't need to provide the prefix

Pro 2:

• User can have something meaningful in the DOM class names that identifies the app during development
• User have more control about the prefix.
• It is relatively easy to have a non-conflicting prefix. Either you have just a few subtrees and naming is already easy enough or you can add you own number prefix to those names and reset them on each request.

[kof]

Actually it makes sense to have both props classNamePrefix and generateClassName. First for debugging, second for automated uniqueness of classnames.

[tlvince]

I've ran into this issue via https://github.com/facebookincubator/create-react-app/issues/3173 (and the linked reduced test case).

In my case, a material UI-dependant component (v1.0.0-beta.11) was included into an app that also uses material-ui (same versions). In development, this works fine, but in production the layout is broken due to conflicting class names.

I'm not sure if this qualifies as "multiple react rendering trees" and moving var ruleCounter = 0; before createGenerateClassName() did not workaround the issue, but commenting out the following, did work:

https://github.com/callemall/material-ui/blob/2361339fd6df9bfbb85ed2ee4da9d42ee6fee71c/src/styles/createGenerateClassName.js#L26-L28

[robophil]

Sorry, I couldn't provide more info at the time I opened #7855.😊 This comment basically covers the issue I faced when running production build.

A workaround for this in the pipeline?

[kof]

Created a PR which should fix this in react-jss https://github.com/cssinjs/react-jss/pull/155

[oliviertassinari]

So let's sum up.

• @darkowic's issues root cause is described here: https://github.com/callemall/material-ui/issues/8223#issuecomment-331076580 and will be fixed thanks to @kof's PR: cssinjs/react-jss#155 and a new warning will be added to prevent this situation in the future: #8341.

• @tlvince's issue symptoms are the same, but the root cause is different. You are using a single react render tree. It could be linked to a duplicated Material-UI version installed. Check your warnings and if you can figure out exactly what's going on, please open another issue.

• @Robophil's issue is too vague to be actionable.

[iamhosseindhv]

I had the colliding classnames and adding a prefix in createGenerateClassName options solved the problem.

Used the great, comprehensive doc here

[Sewdn]

@oliviertassinari thanks for the summary!

In a single react render tree, this issue is most likely caused by different material-ui versions in your build. Make sure to check your dependency tree and check for material-ui versions being used.

yarn list | grep material-ui

[lilliesAndRoses]

My webpack config is as follows

module.exports = {
    entry: {
        FirstComp: path.join(paths.JS, '/components/FirstComponent/MyFirstComp.js'),
        SecondComp: path.join(paths.JS, '/components/SecondComponent/MySecondComp.js'),
    },
}

I have these two components and a common component that is built using the splitChunks option. I wrapped the default exported component in MyFirstComp and MySecondComp using JSSProvider. How should I be using the JSSProvider around the common component?

[christiaanwesterbeek]

@Sewdn I also think it can be caused by different material-ui versions. This className conflict problem was introduced in my app yesterday when I started to migrate to hooks and the useStyles hook created by makeStyles from @material-ui/styles which is at 3.0.0-alpha.1 Also I was using "@material-ui/core" which is at 3.6.0

Suddenly my app classes and the material-ui classes both started with jss1, counting up in parallel. That would mix everything up. My header for example with jss5 was also styled with the jss5 of let's say MuiListItem.

After following this solution https://github.com/mui-org/material-ui/issues/8223#issuecomment-412349569 by @iamhosseindhv the classes of the mui-components got prefixed with a c and the problem was solved.

[oliviertassinari]

@christiaanwesterbeek Are you using the installation step correctly? We use dependency injection. install() call needs to run before all the higher order component imports.

[christiaanwesterbeek]

@oliviertassinari I'm afraid I don't know what step you are referring to. Also, I don't know what install() is part of and where it needs to be called. I'd be glad to learn more about it, but I need to know where it's documented.

[oliviertassinari]

@christiaanwesterbeek I'm referring to https://material-ui.com/css-in-js/basics/#migration-for-material-ui-core-users.

[zheeeng]

@oliviertassinari Seems that install() should be called in every component's file? I have thought that we just need to call it once in the entry file. Maybe we need a more clear description.

[zheeeng]

Maybe I'm wrong, install can't resolve conflicts. To switch from the default style implementation to this newest version, you need to execute the following code before importing any Material-UI's components:

import { install } from '@material-ui/styles';

install();

@oliviertassinari How to install it before importing any Material-UI's components? importing is always be handled by babel and tsc statically

[oliviertassinari]

@zheeeng ES modules imports are hoisted to the top of the module and resolved synchronously in the order they are defined. We have been facing the same issue with the Material-UI documentation. The solution was to package everything into a bootstrap.js module and import it first: https://github.com/mui-org/material-ui/blob/cb30b43e9c6cd49f9b16536a125456f5ea3a85c5/docs/src/modules/components/bootstrap.js#L1-L13 If the problem persists, please, let's move to the discussion to a different issue.

[zheeeng]

@oliviertassinari I moved the importing order from

// entry index.js
import * as React from 'react'
import * as ReactDOM from 'react-dom'
import CssBaseline from '@material-ui/core/CssBaseline'
import { install } from '@material-ui/styles'

to

import * as React from 'react'
import * as ReactDOM from 'react-dom'
import { install } from '@material-ui/styles'
import CssBaseline from '@material-ui/core/CssBaseline'

the code reports error Cannot read property 'text' of undefined. By inspecting the theme object, it is empty. If I switch back the importing order, this part works fine.

const useStyles = makeStyles(
  (theme: Theme) => ({
    root: {
      flexShrink: 0,
      color: theme.palette.text.secondary,
    },
  }),
)

[zheeeng]

Same problem happens when I wrap the whole application with <StylesProvider>

[christiaanwesterbeek]

@zheeeng

the code reports error Cannot read property 'text' of undefined. By inspecting the theme object, it is empty.

I had the same problem. It was fixed by either upgrading "@material-ui/core" to 3.6.0 or @material-ui/styles which to 3.0.0-alpha.1. I forgot which. Better do both.

However the exact theme that the function passed to makeStyles would receive, was not the theme that I had created with createMuiTheme. Instead it got the default theme. What I ended up doing was to not rely on whatever theme that would get passed. Instead I imported the theme in every file that the styles needed it.

[zheeeng]

@christiaanwesterbeek I installed @material-ui/core@3.6.1 and @material-ui/styles/.0.0-alpha.2 and still have this issue.

[christiaanwesterbeek]

This seems unrelated to this issue (8223) at hand. But here goes anyway. Just import theme and don't rely on it being passed to the function that you pass to makeStyles. And you're done.

[christiaanwesterbeek]

Can somebody confirm whether the install step is still required with v3.7.0 ( https://github.com/mui-org/material-ui/releases/tag/v3.7.0 )

[oliviertassinari]

@christiaanwesterbeek yes, it's required. We will remove the installation step in Material-UI v4.

[w3nda]

@oliviertassinari hey, I'm experiencing this issue with latest v3.9.0, and I am not using the @material-ui/styles, I am still using withStyles from core I have two questions.

1. should I migrate now to mui/styes or wait to v4 release. (large scale app)
2. using this workaround https://github.com/mui-org/material-ui/issues/8223#issuecomment-331081785 is the proper way of handling this problem? it does work fine in build finally but can't seem to understand why it is happening in the first place.

Thanks

[oliviertassinari]

I'm experiencing this issue with latest v3.9.0

@w3nda This is too generic, there is a million different reason for this problem to happen.

should I migrate now to mui/styes or wait to v4 release. (large scale app)

We might revert the classname hashing logic. While it greatly improves the performance on the server, it has a significant cost on the client. Maybe we will provide a flag to enable it instead. So no, I think that the best option is to solve the core issue. Did you try this page https://material-ui.com/guides/server-rendering/#troubleshooting?

[w3nda]

@oliviertassinari Thanks for the fast response, I really don't know how to debug the problem and the link you mentioned isn't relevant to me because I serve it as a static site.. Tho the comment I referenced helped me, shouldn't it tell the cause of this issue in my case?

[oliviertassinari]

@w3nda Static website has the same issue. You need to generate the HTML, so it will use the server-side rendering API. If the index counter leaks between two pages, the class name won't be correct. Well, I think that a slower class name generator is a good tradeoff compared to the pain it is to debug such a problem (& how frequent it is). So, yes, you can upgrade to @material-ui/styles, it's a simple escape hatch.

[leolozes]

I just had a similar problem and it was an old material-ui import that was in one of our libraries. It worked fine in development mode but was broken in production. I'm pretty sure this was working before, I don't know if a warning could be issued in this case, even if it's clearly our fault. I updated the versions to match them between projects and everything worked again.

[kopax]

Hi, I am using material just for a single input, button and form on my site and I had to use a <JssProvider /> following this comment https://github.com/mui-org/material-ui/issues/8223#issuecomment-331412432

It's annoying to need this jss provider, is there another fix we can do that would not increase the final build size?

[oliviertassinari]

@kopax What are you using JssProvider for?

[kopax]

Hi @oliviertassinari, In production before visiting another route (we use react-router):

In production after visiting a route or in development

Something similar here happens with a form (weird box shadows), we need to visit another page to have the proper css, that happens only in production:

Both issues are fixed if we add JssProvider. (fixed: we have the same css in production than in development)

[oliviertassinari]

I have no idea. I can't help with the provided information.

[andrewkslv]

Everything is broken too. I observe wrong order jssXX classes in production and as result the wrong styling. It happens after refreshing the page.

Can't find the reason yet.

[kopax]

@oliviertassinari perhaps those version number used by react-admin@2.6.4 we are using would help you tell us what's going on:

        "@material-ui/core": "^1.4.0",
        "@material-ui/icons": "^1.0.0",
        "material-ui-chip-input": "1.0.0-beta.6 - 1.0.0-beta.8",

[oliviertassinari]

Could you make sure that Material-UI is only bundled once? Using two versions at the same time can create the same buggy behavior.

[andrewkslv]

Well. It's possible. We're using react-admin which has recommended version ~1.5.

I solved the bug on production by adding JssProvider. Now I can refresh the page normally.

import React from "react";
import { Admin, Resource } from "react-admin";
import JssProvider from "react-jss/lib/JssProvider";

const App = () => (
  <JssProvider>
    <Admin dataProvider={dataProvider}>
      <Resource name="trip" list={RequestsList} className="resourceItem" />
    </Admin>
  </JssProvider>
);

export default App;