Material-ui: jest snapshot className issues

[sakulstra]

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

Eventually this is related to #9433 , but i'm not sure.

Expected Behavior

Snapshots should succeed when nothing was changed.

Current Behavior

Whenever introducing a new withStyles() Component or simply updating material-ui to the latest beta our jest test fails due to things like:

-          className="MuiTypography-root-104 MuiTypography-body1-113"
+          className="MuiTypography-root-111 MuiTypography-body1-120"
// or
-        className="MuiExpansionPanelSummary-content-169"
+        className="MuiExpansionPanelSummary-content-176"

basically the appended number changes.

Steps to Reproduce (for bugs)

Sorry I think there is no easy sandbox reproduction possible.

Context

We switched to material-ui from our existing global-css/deterministic className solution where we were able to easily snapshot components and check if they were altered. Now it seems like we have to adjust our setup, but I don't now how.

Your Environment

| Tech | Version |
|--------------|---------|
| Material-UI | basically all the v1 beta branches since we started using it (~8+) |
| React | 16 |

[oliviertassinari]

I'm closing for #9433. Some hint, you need to change the className generator.

[sakulstra]

@oliviertassinari not sure if this is the right place to ask this, but i'm using createGenerateClassName now, but still face some issues with custom classNames related to https://github.com/mui-org/material-ui/blob/c171287d490e05a6bf23028468dea5b60047cddc/src/styles/createGenerateClassName.js#L62 (when adding new classes the count obviously increases the count which leads to failing snapshot tests)... so my question is - why are we doing this https://github.com/mui-org/material-ui/blob/c171287d490e05a6bf23028468dea5b60047cddc/src/styles/createGenerateClassName.js#L58 can't we just remove the suffix for all classes when dangerouslyUseGlobalCSSis set?

[oliviertassinari]

@sakulstra When the classNamePrefix match ^Mui, we have the guarantee that it will be unique. It's no true otherwise, the suffix is important to prevent class name clashes.

[oliviertassinari]

@sakulstra I guess, we could host a class name generator specifically tuned for the snapshot testing. However, it requires wrapping the components. If we are going to wrap the component we want to test with a JssProvider, it would be even simpler to reset the counter for each tests.

[sakulstra]

would you guys be willing to accept a pr for a new snapshot flag?

It could be as easy as:

if (prefix.match(/^Mui/) || snapshot) {
  return prefix + '-' + rule.key;
}

i guess.

Best Regards,
Lukas

[oliviertassinari]

@sakulstra How are you going to use this new flag? Why don't you reset the class counter between each snapshot instead?

[sakulstra]

Good question, I'm using this with storybook/storyshots which generates one big snapshot - so I don't know it it's somehow possible to reset the counter between each story snapshot (I'll have a look and let you know). The idea was that i can do sth similar to this:

import { createGenerateClassName, jssPreset } from 'material-ui/styles';

// make class names deterministic
const jss = create(jssPreset());
const generateClassName = createGenerateClassName({ dangerouslyUseGlobalCSS: true, snapshot: true });

const Decorator = fn => (
  <JssProvider generateClassName={generateClassName} jss={jss}>
    <AppLayout>{fn()}</AppLayout>
  </JssProvider>
);

not sure if this is the best approach (just the easiest one i could imagine)

[oliviertassinari]

@sakulstra Your example is already resetting the class name counter. No need to change the implementation :).

[sakulstra]

@oliviertassinari sadly it is not :) I have to look into how storyshots is generating the snapshots/applying the decorator and how i can alter the behavior, but currently it seems like decorator is only mounted once and caused by this the rule counter runs over all stories without resetting in between (at the last story it's around ~11000).

Anyways I guess you're right and this should be somehow fixed in the testing setup - not material-ui itself.
Best Regards,
Lukas

[oliviertassinari]

sadly it is not :)

@sakulstra Maybe this will do it:

import { createGenerateClassName } from 'material-ui/styles';

const Decorator = fn => (
  <JssProvider generateClassName={createGenerateClassName()}>
    <AppLayout>{fn()}</AppLayout>
  </JssProvider>
);

[oldwin]

For Storybook snapshots I use next thing:

_.storybook/config.js_

import React from 'react';
import JssProvider from 'react-jss/lib/JssProvider';

const generateClassName = (rule, styleSheet) =>
  `${styleSheet.options.classNamePrefix}-${rule.key}`;

addDecorator(story => (
   <JssProvider generateClassName={generateClassName}>
      {story()}
   </JssProvider>
));

...

As result I got the same class names but without counter suffix

Does anything wrong?

[lb-]

@oldwin thank you for this - helped me out heaps! worked great in my set up.
Just a reminder to anyone else that finds this - you need to import addDecorator at the start of the storybook config file.

eg.
import { configure, addDecorator } from '@storybook/react';

This creates a global decorator that will decorate ALL of your stories as per docs:
https://storybook.js.org/basics/writing-stories/#using-decorators

[stellarhoof]

It doesn't seem to work inside some HOCs: https://github.com/cssinjs/react-jss/issues/242.
So far my testing reveals it doesn't work inside injectSheet from react-jss and when shallow rendering a component without first deep rendering it. Shallow rendering also uses HOCs.

[joseph-allen]

this issue is being referenced from multiple other posts, let's reopen it.

[oliviertassinari]

@joseph-allen On what grounds? Isn't the issue addressed?

[joseph-allen]

there are still ongoing discussions in #6834 and you have told them to talk somewhere more recent, this is the most recent issue related to this topic, and I am having the issue too.

[oliviertassinari]

@joseph-allen Did you try the solution proposed in the discussion? Do you have a reproduction showing the issue?

[joseph-allen]

it's the same issue except mine is when I run my tests on circleCI, works fine locally.

I am not totally sure where I am supposed to be implementing the above solution?

[GerritVK]

I am seeing similar issues but in a different context, I am not using storybook. So the above suggestion does not apply.

When running my tests locally the snapshot test passes but when it is tested again on Bitbucket Pipeline fails.

Snapshot example Error

              <span
    -           className="MuiTypography-root-137 MuiTypography-headline-142 MuiCardHeader-title-135"
    +           className="MuiTypography-root-138 MuiTypography-headline-143 MuiCardHeader-title-136"
              >

Snapshot Test

  test('Snapshot Renders Correctly', () => {
    const component = renderer.create(
      Object.keys(groupedDeviceList).map(category => (
        <DeviceListExpansionPanel category={category} key={category}>
          <div>Device Cards</div>
          <div>Device Cards</div>
        </DeviceListExpansionPanel>
      ))
    );
    const tree = component.toJSON();
    expect(tree).toMatchSnapshot();
  });

Any guidance would be appreciated.

[oldwin]

@GerritVK you just need to wrap your test environment with custom JssProvider, for example in your test just try to wrap it with custom JssProvider as mentioned here:
https://github.com/mui-org/material-ui/issues/9492#issuecomment-368205258

In Storybook we're doing it with addDecorator() it should be possibility to add the same in your freamwork/lib. Next example should work, but you need to find a way to put this JssProvider globally

  import JssProvider from 'react-jss/lib/JssProvider';

  const generateClassName = (rule, styleSheet) =>
    `${styleSheet.options.classNamePrefix}-${rule.key}`;

  test('Snapshot Renders Correctly', () => {
    const component = renderer.create(
      <JssProvider generateClassName={generateClassName}>
        {Object.keys(groupedDeviceList).map(category => (
          <DeviceListExpansionPanel category={category} key={category}>
            <div>Device Cards</div>
            <div>Device Cards</div>
          </DeviceListExpansionPanel>
        ))}
      </JssProvider>
    );
    const tree = component.toJSON();
    expect(tree).toMatchSnapshot();
  });

[joseph-allen]

@oldwin thank you for the much needed snippet, the above solves my issues too!

[eddiemonge]

The problem with @oldwin's solution is that I have to include react-jss to use it. I wasn't using that before and it adds to the already bloated dependency list.

[oldwin]

@eddiemonge if you do not use react-jss, you will not see the problem described in this issue. Because the issue about JSS inside Material UI ))

https://github.com/mui-org/material-ui/blob/master/package.json#L135

[eddiemonge]

if you do not use react-jss, you will not see the problem described in this issue. Because the issue about JSS inside Material UI ))

Not true. I wasn't using react-jss and Material-UI causes the classes to be unique. Whether that is through its use of JSS or some other means, it is still the one doing it, not react-jss.

[jesusmaldonado]

there needs to be a flag to generate non-counter classNames with NODE_ENV === 'test' or something, because this makes material-UI largely unusable in a snapshot context without wrapping it in <JSSProvider> everytime.

[gnapse]

I've been struggling with this issue too, and after digging into these and related issues and PRs in this repo, realizing that I now have to wrap the rendered component trees in hundreds of test cases is disappointing. Even if I create a helper render function that wraps the one from react-testing-library, I'd still have to change the import in tenths of test files.

I really hope there's a more transparent way of doing this globally without having to wrap stuff into something. What @jesusmaldonado mentions would be ideal:

there needs to be a flag to generate non-counter classNames with NODE_ENV === 'test' or something, because this makes material-UI largely unusable in a snapshot context without wrapping it in everytime.

And even if it's something such that the counter can't be ditched and has to be reset after each test, as some comments here or elsewhere seem to suggest, instead of wrapping each test tree inside something that resets it, why not something like:

import { resetClassNameGenerator } from 'material-ui/styles';

beforeEach(resetClassNameGenerator);

That would be something that could be setup globally.

[oliviertassinari]

there needs to be a flag to generate non-counter classNames with NODE_ENV === 'test'

@sakulstra It's one way to do it. Emotion used a custom Jest serializer and styled-components a global reset. #14013 will improve the story with the static styles.

[denyo]

Comments and snippets in this issue refer to JssProvider.
The same works with material's StylesProvider like

const generateClassName = (rule, styleSheet) => `${styleSheet.options.classNamePrefix}-${rule.key}`;

<StylesProvider generateClassName={generateClassName}>
    <ThemeProvider theme={theme}>
        <Component />
    </ThemeProvider>
</StylesProvider>

[chinhnguyen]

Hi! While waiting for the official release on static name, storybook does provide the addDecorator that should serve the purpose:

import React from 'react';
import { configure, addDecorator } from '@storybook/react'
import requireContext from 'require-context.macro'
import '../src/index.css'
import { StylesProvider } from '@material-ui/styles';

const generateClassName = (rule, styleSheet) => `${styleSheet.options.classNamePrefix}-${rule.key}`;

addDecorator(storyFn => (
  <StylesProvider generateClassName={generateClassName}>
    {storyFn()}
  </StylesProvider>
));

const req = requireContext('../src/components', true, /\.stories.js$/)
function loadStories() {
  req.keys().forEach(filename => req(filename))
}

configure(loadStories, module)

[javidjamae]

One thing to note is that if you use classes on a non-MUI component, like a div, it will just prefix your component with makeStyles-.

If you remove the generated numbers from the class name, then you'll start seeing a lot of style collisions on these components.

For example, if you define a root classname on two components and you nest one under the other, they'll both generate makeStyles-root, and the styles will collide in Storybook.

<MyParentComponent className={ useStylesParent.root } >
  <MyChildComponent className={ useStylesChild.root } />
</MyParentComponent>

To help avoid this, you'll want to make sure to provide a name for any custom components styles you define using the name option you can pass in to makeStyles:

const useStylesParent = makeStyles({
  root: { ... },
  label: { ... },
}, { name: 'MyParentComponent' })


const useStylesChild = makeStyles({
  root: { ... },
  label: { ... },
}, { name: 'MyChildComponent' })

Also, if you're using Storyshots, you can make sure to only add the decorator that strips out the generated id in your Storyshots test file, instead of putting it in your .storybook/config.js file as proposed by @oldwin above. That way components will still render properly in the Storybook UI, and the generated id will only be stripped off when running the tests.

[chriswetterman]

Providing my implementation based off what has been discussed here to hopefully help others. As @oliviertassinari mentioned, you can reset the counter yourself. I have each of my mounted test components wrapped in this HOC.

const generateClassName = () => {
  let counter = 0
  return (rule, styleSheet) => `${styleSheet.options.classNamePrefix}-${rule.key}-${counter++}` 
}


export default function WithStableMuiClassnames({children}) {
  return (
    <StylesProvider generateClassName={generateClassName()}>
      {children}
    </StylesProvider>
  )
}