Closed jesusmaldonado closed 3 years ago
Tests are failing on my pull, and solving them is a bit murky.
Another solution I would like to try is passing an option to withStyles(<Component />, { ignoreCounter: true })
, but I don't know how to track that in some kind of context that's available to children from a parent component.
Seems like a good opportunity to do so but the context does not stay the same, and this is an old context API. I also see two withStyles
defined within the repo and I am not sure if that would be changing the right one.
It is not appropriate to wrap every object in a as stated in the docs of https://material-ui.com/customization/css-in-js/#creategenerateclassname-options-class-name-generator because tests should mirror their native implementation.
Why is it not appropriate? When testing you mock, fake, setup etc all sorts of things. Whether you wrap your tests in a custom JSSProvider or change the className logic for test environments inside the implementation changes nothing about the fact that you don't "mirror native implementation".
emotion has jest-emotion
and styled-components has styled-components-jest
.
Seems like it would be more consistent with the ecosystem if we would release our own package for snapshot testing instead of changing the implementation for everyone. Maybe there exists already snapshot serializer that makes the classnames in snapshots deterministic and is framework agnostic.
@eps1lon Or, we could use emotion or styled-components internally.
But I believe it's a good tradeoff.
I don't. There is no precedent in the ecosystem and an existing solution.
@eps1lon What do you mean by existing solution? I believe that we would have to build our own serializer package. We could very well decide to stop investing time and ressource in JSS and to start looking into a first class integration with emotion or styled-components?
Wrapping in a JSSProvider
with a custom class name generator as was outlined in #9492.
Don't know if it can help @oliviertassinari
I actually use this mock
const styles = jest.requireActual('@material-ui/core/styles');
const theme = styles.createMuiTheme();
const withStyles = style => component => {
const classes = typeof style === 'function' ? style(theme) : style;
component.defaultProps = { ...component.defaultProps, classes };
return component;
};
module.exports = { ...styles, withStyles };
This way I have snapshot like this one
exports[`[Component] Header should render correctly 1`] = `
<header
className={
Object {
"alignItems": "center",
"backgroundColor": "#282c34",
"color": "#ffffff",
"display": "flex",
"flexDirection": "column",
"fontSize": "calc(10px + 2vmin)",
"justifyContent": "center",
"minHeight": "100vh",
}
}
>
<img
alt="logo"
className={
Object {
"animation": "spin infinite 20s linear",
"height": "40vmin",
}
}
src="logo.svg"
/>
<p>
Title
</p>
</header>
`;
I prefer having the css properties instead of a classname to detect UI regressions.
With the v4 I now use this mock
import theme from '../../src/theme/muiTheme';
const styles = jest.requireActual('@material-ui/styles');
const makeStyles = style => props => {
// Apply theme to classes
const classes = typeof style === 'function' ? style(theme) : style;
// Apply props to every key of each class, which is every key of classes
const classesByProps = {};
Object.keys(classes).forEach(classKey => {
const classByProps = {};
Object.keys(classes[classKey]).forEach(key => {
classByProps[key] =
typeof classes[classKey][key] === 'function' ? classes[classKey][key](props) : classes[classKey][key];
});
classesByProps[classKey] = classByProps;
});
return classesByProps;
};
module.exports = { ...styles, makeStyles };
@VincentLanglet Thanks, it helps.
@oliviertassinari No problem. BTW, since this discussion https://github.com/mui-org/material-ui/issues/14797, considering this is the best way to write style
const useStyles = makeStyles(
(theme) => ({
option: (props) => ({
backgroundColor: props.isRed ? 'red' : '',
}),
}),
);
Instead of
const useStyles = makeStyles(
(theme) => ({
option: ({
backgroundColor: (props) => props.isRed ? 'red' : '',
}),
}),
);
Because of issue with the actual types definitions of StyleRule/CSSProperties, ...
I now use this (simpler) mock
import theme from '../../src/theme/muiTheme';
const styles = jest.requireActual('@material-ui/styles');
const makeStyles = style => props => {
// Apply theme to classes
const classes = typeof style === 'function' ? style(theme) : style;
// Apply props to every class, which is every key of classes
const classesByProps = {};
Object.keys(classes).forEach(classKey => {
classesByProps[classKey] = typeof classes[classKey] === 'function' ? classes[classKey](props) : classes[classKey];
});
return classesByProps;
};
module.exports = { ...styles, makeStyles };
Any progress on this issue? @VincentLanglet can you point where should we put that mock and how to make it work?
@andrey-semin __mocks__/@material-ui/styles.js
, there is nothing more to do.
@VincentLanglet how would you import createMuiTheme inside your theme?
I'm getting the following error since i'm assuming '@material-ui/styles' is being mocked and the '@material-ui/core/styles' is importing '@material-ui/styles
TypeError: (0 , _styles.withThemeCreator) is not a function
> 1 | import { createMuiTheme } from '@material-ui/core/styles';
| ^
2 |
3 | const theme = createMuiTheme({});
at Object.<anonymous> (node_modules/@material-ui/core/styles/withTheme.js:14:46)
at Object.<anonymous> (node_modules/@material-ui/core/styles/index.js:116:41)
at Object.<anonymous> (src/theme.js:1:1)
Looks like it's failing on _styles.withThemeCreator in withTheme.js
@Kaustix I did not import createMuiTheme
but import theme from '../../src/theme/muiTheme';
But if you want to import a non-mocked function, use jest.requireActual
Like I did for const styles = jest.requireActual('@material-ui/styles');
@VincentLanglet sorry don't think i explained very well.
I'm importing createMuiTheme inside my themes.js which is causing the issue so not sure how to fix the error.
// mock/styles.js
import theme from '../../../src/theme';
const styles = jest.requireActual('@material-ui/styles');
const makeStyles = style => (props) => {
const classes = typeof style === 'function' ? style(theme) : style;
const classesByProps = {};
Object.keys(classes).forEach((classKey) => {
classesByProps[classKey] = classKey;
});
return classesByProps;
};
module.exports = { ...styles, makeStyles };
// src/theme.js
import { createMuiTheme } from '@material-ui/core/styles';
const theme = createMuiTheme({ // my theme overrides });
export default theme;
causes the error:
TypeError: (0 , _styles.withThemeCreator) is not a function
> 1 | import { createMuiTheme } from '@material-ui/core/styles';
| ^
2 |
3 | const theme = createMuiTheme({
4 | overrides: {
at Object.<anonymous> (node_modules/@material-ui/core/styles/withTheme.js:14:46)
at Object.<anonymous> (node_modules/@material-ui/core/styles/index.js:116:41)
at Object.<anonymous> (src/theme.js:1:1)
It's been month I didn't touch my project. This is what I had
// __mocks__/@material-ui/styles.js
import theme from '../../src/theme/muiTheme';
const styles = jest.requireActual('@material-ui/styles');
const makeStyles = style => props => {
// Apply theme to classes
const classes = typeof style === 'function' ? style(theme) : style;
// Apply props to every class, which is every key of classes
const classesByProps = {};
Object.keys(classes).forEach(classKey => {
classesByProps[classKey] = typeof classes[classKey] === 'function' ? classes[classKey](props) : classes[classKey];
});
return classesByProps;
};
module.exports = { ...styles, makeStyles };
// src/theme/muiTheme.ts
import { amber, blueGrey, red } from '@material-ui/core/colors';
import createMuiTheme from '@material-ui/core/styles/createMuiTheme';
const theme = createMuiTheme({
palette: {
background: {
default: blueGrey[50],
},
primary: red,
secondary: amber,
},
spacing: 4,
});
export default theme;
I see you wrote '../../../src/theme';
and not '../../src/theme';
.
What is the path of your mock ?
I mock @material-ui/styles.js
, not @material-ui/core/styles.js
. That can change a lot of thing.
@VincentLanglet thank you so much!! it was the import statement
import createMuiTheme from '@material-ui/core/styles/createMuiTheme';
vs
import { createMuiTheme } from '@material-ui/core/styles';
since the latter was going after index.js which brought '@material-ui/styles'
I'm going to bump this.
This is one of the major cons of using material-ui. My team are currently building a design system based on @material-ui/styles
, though we can't do proper snapshots.
I looked into styled-components-jest
, where I believe most of the code could be adopted. That approach would require to export the store, in which the stylesheet is living.
Would that be a possibility?
@Thisen Did you try the solution proposed by @VincentLanglet?
@oliviertassinari, I don't want a mock, I want an actual serializer for Jest. Can't we expose the stylesheet as PRIVATE, like styled-components?
@Thisen see #6115, people using @material-ui/styles
should be able to migrate to react-jss without much effort, as for Material-UI, we will rely on styled-components. However, our priority is to add more components, we won't work on it before +6 months. I would suggest that you raise the concern to JSS.
@oliviertassinari What does mean that you're going to deprecate @material-ui/styles
in favor of styled-components
?
@Thisen we will try to move the maintainance of the package to react-jss. We will work on making the migration is as easy as possible.
I'm totally agree with @Thisen, ideally we should have serializer for Jest, like jest-styled-components
does. I guess the issue related to JSS
(cssinjs/jss#804), but not to Material-UI
.
But as a temporal solution, I modified workaround from @VincentLanglet and mock makeStyles
and withStyles
from @material-ui/styles
as a result, a don't want to generate styles object and put it to classes
and className
, because it looks a bit strange when you have style object in className
or classes
.
// __mocks__/@material-ui/styles.js
import theme from '../../muiTheme';
const styles = jest.requireActual('@material-ui/styles');
const makeStyles = (style, options) => () => {
// Apply theme to classes
const classes = typeof style === 'function' ? style(theme) : style;
// Generate class name with correct component name without any index
const classesByProps = {};
Object.keys(classes).forEach((key) => {
classesByProps[key] = options ? `${options.name}-${key}` : key;
});
return classesByProps;
};
const withStyles = (style, options) => {
const useStyles = makeStyles(style, options);
return (Component) => {
const classes = useStyles();
const MockComponent = (props) => (
<Component classes={classes} {...props} />
);
MockComponent.displayName = (options && options.name) || Component.displayName;
return MockComponent;
};
};
module.exports = {
...styles,
makeStyles,
withStyles,
};
Don't forget to add name
option as a second parameter to makeStyles
/withStyles
:
const useStyles = makeStyles((theme) => ({
root: {
backgroundColor: theme.palette.primary.main,
},
}), { name: 'Button' });
export default withStyles((theme) => ({
root: {
backgroundColor: theme.palette.primary.main,
},
}), { name: 'Button' })(MuiButton);
as mentioned by @Kaustix, we need to import createMuiTheme
from '@material-ui/core/styles/createMuiTheme'
as a result, we have generated snapshot:
I took an approach where I mocked @testing-library/react to return all render results with StyleProvider and the generateClassName function that takes away the numerical suffixes
import { StylesProvider } from '@material-ui/styles'
import React from 'react'
const react = jest.requireActual('@testing-library/react')
const generateClassName = (rule, styleSheet) =>
`${styleSheet.options.classNamePrefix}-${rule.key}`
const render = args => {
return react.render(
<StylesProvider generateClassName={generateClassName}>
{args}
</StylesProvider>,
)
}
module.exports = { ...react, render }
Some random extra notes here https://github.com/GMOD/jbrowse-components/pull/503
I am fine with using a custom generateClassName that does not include the numbers and I am already doing that.
That said, the ORDER of the class names is ALSO coming out different on CI vs on my Dev machine.
- <div class="LoginForm-error"> </div><button class="MuiButtonBase-root MuiButton-root LoginForm-btn MuiButton-contained MuiButton-containedSecondary" tabindex="0" type="button"><span class="MuiButton-label">Log In</span><span class="MuiTouchRipple-root"></span></button><span class="MuiTypography-root LoginForm-or MuiTypography-body1">or</span><a href="/forgot"><span class="MuiTypography-root LoginForm-link MuiTypography-body1">Forgot Password</span></a>
+ <div class="LoginForm-error"> </div><button class="MuiButtonBase-root MuiButton-root MuiButton-contained MuiButton-containedSecondary LoginForm-btn" tabindex="0" type="button"><span class="MuiButton-label">Log In</span><span class="MuiTouchRipple-root"></span></button><span class="MuiTypography-root LoginForm-or MuiTypography-body1">or</span><a href="/forgot"><span class="MuiTypography-root LoginForm-link MuiTypography-body1">Forgot Password</span></a>
Using the code snippets provided in this thread, I had to make a couple of tweaks to get withStyles
to work in my project (I'm using the default theme at the moment).
// __mock__/@material-ui/core/styles.js
import React from "react";
import createMuiTheme from "@material-ui/core/styles/createMuiTheme";
export const styles = jest.requireActual("@material-ui/core/styles");
export const makeStyles = (style, options) => () => {
// Apply classes
const classes = typeof style === "function" ? style(createMuiTheme()) : style;
// Generate class name with correct component name without any index
const classesByProps = {};
Object.keys(classes).forEach(key => {
classesByProps[key] = options ? `${options.name}-${key}` : key;
});
return classesByProps;
};
export const withStyles = (style, options) => {
const useStyles = makeStyles(style, options);
return Component => {
const classes = useStyles();
const MockComponent = props => <Component classes={classes} {...props} />;
MockComponent.displayName =
(options && options.name) || Component.displayName;
return MockComponent;
};
};
Hope this helps someone!
Guys,
I'm using this as withStyles
mock:
withStyles: style => component => {
const classes = typeof style === 'function' ? style(theme) : style
component.defaultProps = { ...component.defaultProps, classes }
return component
}
But I'm having a prop-types error since I'm using a className
with object instead of string. Also, my style values are missing from the snapshots:
`
<div>
<div
class="MuiAvatar-root MuiAvatar-circle color backgroundColor width height MuiAvatar-colorDefault"
>
V
</div>
</div>
`
Warning: Failed prop type: Invalid prop `className` of type `object` supplied to `ForwardRef(Avatar)`, expected `string`.
I could manage to fix it by using JSON.stringify
in my classes but the snapshot output is not nice:
withStyles: (style: any) => (component: any) => {
const classes = typeof style === 'function' ? style(theme) : style
Object.entries(classes).forEach(([className, classValue]) => {
classes[className] = JSON.stringify(classValue)
})
component.defaultProps = { ...component.defaultProps, classes }
return component
}
`
<div>
<div
class="MuiAvatar-root MuiAvatar-circle {\\"color\\":\\"#fff\\",\\"backgroundColor\\":\\"#ff5722\\",\\"width\\":30,\\"height\\":30} MuiAvatar-colorDefault"
>
V
</div>
</div>
`
What am I missing to get an snapshot like this?
exports[`[Component] Header should render correctly 1`] = `
<header
className={
Object {
"alignItems": "center",
"backgroundColor": "#282c34",
"color": "#ffffff",
"display": "flex",
"flexDirection": "column",
"fontSize": "calc(10px + 2vmin)",
"justifyContent": "center",
"minHeight": "100vh",
}
}
>
<img
alt="logo"
className={
Object {
"animation": "spin infinite 20s linear",
"height": "40vmin",
}
}
src="logo.svg"
/>
<p>
Title
</p>
</header>
`;
Any help would be appreciated, thanks in advance.
Just adding my opinion that this is a major time suck. Hours of Googling across multiple days. Trying and failing to find a solution to get consistent snapshots.
I finally find this and #9492 only to see it's been an issue for 3 years and it seems core devs think this is fine. I want my 3 days back. Instead I now have to figure out which of the hacks on this page may minimize additional time required.
@michaelpward I'm sorry to hear that you such a frustrating experience. https://github.com/mui-org/material-ui/issues/9492#issuecomment-368205258 is an example implementation that should cover 90% of the use cases. What did you find lacking from this example?
The reason why we're hesitant to add documentation for this is that we
<button className="redAndBold" />
and not <button className="red bold" />
. Both of these could render the exact same thing. So are you interested in classes or styles? If you're interested in styles then we recommend proper visual regression testing since we do rely to some extend on styles cascading. Serializing snapshots of each generic container and their styles implementation details is very likely to break between releases anyway.Considering the number of upvotes and that nobody released a helper package is indicative that this is either already sufficiently solved by createGenerateClassName or low popularity of snapshot testing. This means that isn't a priority for us to address.
Guys,
I'm using this as
withStyles
mock:withStyles: style => component => { const classes = typeof style === 'function' ? style(theme) : style component.defaultProps = { ...component.defaultProps, classes } return component }
But I'm having a prop-types error since I'm using a
className
with object instead of string. Also, my style values are missing from the snapshots:` <div> <div class="MuiAvatar-root MuiAvatar-circle color backgroundColor width height MuiAvatar-colorDefault" > V </div> </div> `
Warning: Failed prop type: Invalid prop `className` of type `object` supplied to `ForwardRef(Avatar)`, expected `string`.
I could manage to fix it by using
JSON.stringify
in my classes but the snapshot output is not nice:withStyles: (style: any) => (component: any) => { const classes = typeof style === 'function' ? style(theme) : style Object.entries(classes).forEach(([className, classValue]) => { classes[className] = JSON.stringify(classValue) }) component.defaultProps = { ...component.defaultProps, classes } return component }
` <div> <div class="MuiAvatar-root MuiAvatar-circle {\\"color\\":\\"#fff\\",\\"backgroundColor\\":\\"#ff5722\\",\\"width\\":30,\\"height\\":30} MuiAvatar-colorDefault" > V </div> </div> `
What am I missing to get an snapshot like this?
exports[`[Component] Header should render correctly 1`] = ` <header className={ Object { "alignItems": "center", "backgroundColor": "#282c34", "color": "#ffffff", "display": "flex", "flexDirection": "column", "fontSize": "calc(10px + 2vmin)", "justifyContent": "center", "minHeight": "100vh", } } > <img alt="logo" className={ Object { "animation": "spin infinite 20s linear", "height": "40vmin", } } src="logo.svg" /> <p> Title </p> </header> `;
Any help would be appreciated, thanks in advance.
I can get a pretty output if I use react-test-renderer:
import React from 'react';
import {create} from 'react-test-renderer';
const TestComponent = () => (
<div className={{ root: { backgroundColor: 'red'}}}>
<p className={{ test: 123 }} style={{ background: 'yellow'}}>Adsfds</p>
</div>
);
test('renders learn react link', () => {
const wrapper = create(<TestComponent />);
expect(wrapper.toJSON()).toMatchSnapshot();
});
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`renders learn react link 1`] = `
<div
className={
Object {
"root": Object {
"backgroundColor": "red",
},
}
}
>
<p
className={
Object {
"test": 123,
}
}
style={
Object {
"background": "yellow",
}
}
>
Adsfds
</p>
</div>
`;
@effortlesscoding your example doesn't include withStyles though? that is the main cause of the issue here
Hi devs, I'm having issues with the generated class names too. But I noticed that the difference of a new class name appears to be consistent, not a random generated each time you run a test. So I took a quick look to createGenerateClassName
and saw that the suffix
is
const suffix = `${rule.key}-${ruleCounter}`;
So I please correct me if I'm wrong @oliviertassinari:
createGenerateClassNameHash()
will be deterministic as well? If that's the case, maybe having those diffs in the snapshots is not so bad 🤔
@hisapy I think that the suggestion by @VincentLanglet is the best so far on this thread. At this point, we have a working workaround, we should focus on improving the DX. Ideally, the problem would be handled by default. But if not possible, we would combine some code + documentation.
Thx! For the moment, if the generated suffix in the className is not random then I think I can live with it for a while 😄
In case anyone is interested in another approach, I managed to remove the incremental suffix using a snapshot serialiser.
First, I followed the solution proposed by @cmdcolin here. But it does not work fine with @material-ui
v3
. Actually, v3.9.4
.
The problem lies in the StyleProvider
component. Core components (Button, Icon, etc...) relies in the old withStyles
, which do not have access to the context created by StylesProvider
. So, only custom components using the new withStyles
are going to make use the generatedClassName
provided in context by StylesProvider
. The rest stay as they are.
...stuff
<button
aria-label="Delete"
class="MuiButtonBase-root-8 MuiIconButton-root-2 CustomButton-root"
tabindex="0"
type="button"
>
...more stuff
Based on how jest-styled-components
manages the snapshots, I decided to take the same approach. Basically, I reused most of the code of the styled components serializer. Right below is the mentioned serializer.
const KEY = '_mui_cleaned_'
const getNodes = (node, nodes = []) => {
if (typeof node === 'object') {
nodes.push(node)
}
if (node.children) {
Array.from(node.children).forEach(child => getNodes(child, nodes))
}
return nodes
}
const getClassNamesFromDOM = node => Array.from(node.classList)
const getClassNamesFromProps = node => {
const classNameProp = node.props && (node.props.class || node.props.className)
if (classNameProp) {
return classNameProp.trim().split(/\s+/)
}
return []
}
const getClassNames = nodes =>
nodes.reduce((classNames, node) => {
let newClassNames = null
if (global.Element && node instanceof global.Element) {
newClassNames = getClassNamesFromDOM(node)
} else {
newClassNames = getClassNamesFromProps(node)
}
newClassNames.forEach(className => classNames.add(className))
return classNames
}, new Set())
const markNodes = nodes => nodes.forEach(node => (node[KEY] = true))
const removeIncrementalSuffix = (code, classNames) =>
Array.from(classNames)
.filter(className => className.match(/-\d+$/))
.reduce(
(acc, val) => acc.replace(val, val.slice(0, val.lastIndexOf('-'))),
code
)
expect.addSnapshotSerializer({
test: function(val) {
return (
val &&
!val[KEY] &&
(val.$$typeof === Symbol.for('react.test.json') ||
(global.Element && val instanceof global.Element))
)
},
print: function(val, print) {
const nodes = getNodes(val)
const classNames = getClassNames(nodes)
markNodes(nodes)
return removeIncrementalSuffix(print(val), classNames)
}
})
Now the snapshot looks like this:
...stuff
<button
aria-label="Delete"
class="MuiButtonBase-root MuiIconButton-root CustomButton-root"
tabindex="0"
type="button"
>
...more stuff
Hope it helps. Buena suerte.
In case anyone is interested in another approach, I managed to remove the incremental suffix using a snapshot serialiser.
First, I followed the solution proposed by @cmdcolin here. But it does not work fine with
@material-ui
v3
. Actually,v3.9.4
.The problem lies in the
StyleProvider
component. Core components (Button, Icon, etc...) relies in the oldwithStyles
, which do not have access to the context created byStylesProvider
. So, only custom components using the newwithStyles
are going to make use thegeneratedClassName
provided in context byStylesProvider
. The rest stay as they are....stuff <button aria-label="Delete" class="MuiButtonBase-root-8 MuiIconButton-root-2 CustomButton-root" tabindex="0" type="button" > ...more stuff
Based on how
jest-styled-components
manages the snapshots, I decided to take the same approach. Basically, I reused most of the code of the styled components serializer. Right below is the mentioned serializer.const KEY = '_mui_cleaned_' const getNodes = (node, nodes = []) => { if (typeof node === 'object') { nodes.push(node) } if (node.children) { Array.from(node.children).forEach(child => getNodes(child, nodes)) } return nodes } const getClassNamesFromDOM = node => Array.from(node.classList) const getClassNamesFromProps = node => { const classNameProp = node.props && (node.props.class || node.props.className) if (classNameProp) { return classNameProp.trim().split(/\s+/) } return [] } const getClassNames = nodes => nodes.reduce((classNames, node) => { let newClassNames = null if (global.Element && node instanceof global.Element) { newClassNames = getClassNamesFromDOM(node) } else { newClassNames = getClassNamesFromProps(node) } newClassNames.forEach(className => classNames.add(className)) return classNames }, new Set()) const markNodes = nodes => nodes.forEach(node => (node[KEY] = true)) const removeIncrementalSuffix = (code, classNames) => Array.from(classNames) .filter(className => className.match(/-\d+$/)) .reduce( (acc, val) => acc.replace(val, val.slice(0, val.lastIndexOf('-'))), code ) expect.addSnapshotSerializer({ test: function(val) { return ( val && !val[KEY] && (val.$$typeof === Symbol.for('react.test.json') || (global.Element && val instanceof global.Element)) ) }, print: function(val, print) { const nodes = getNodes(val) const classNames = getClassNames(nodes) markNodes(nodes) return removeIncrementalSuffix(print(val), classNames) } })
Now the snapshot looks like this:
...stuff <button aria-label="Delete" class="MuiButtonBase-root MuiIconButton-root CustomButton-root" tabindex="0" type="button" > ...more stuff
Hope it helps. Buena suerte.
Thanks for this. :+1:
Passing a string to replace
only catches the first occurrence, however, so it fails if the serializer's input has multiple occurrences of the same MUI classname. i.e:
<Styled(WithStyles(ForwardRef(Container)))>
<WithStyles(ForwardRef(Container))
className="WithStyles(ForwardRef(Container))-root-120 WithStyles(ForwardRef(Container))-root-249"
>
<ForwardRef(Container)
className="WithStyles(ForwardRef(Container))-root-120 WithStyles(ForwardRef(Container))-root-249"
>
<div
className="MuiContainer-root WithStyles(ForwardRef(Container))-root-120 WithStyles(ForwardRef(Container))-root-249 MuiContainer-maxWidthLg"
Since String.replaceAll
doesn't have much support yet we can use a global RegExp for the search string; it just needs to be escaped first because of all the ()
s and whatnot.
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions#Escaping
const escapeRegExp = (string) => string.replace(/[.*+\-?^${}()|[\]\\]/g, '\\$&');
const removeIncrementalSuffix = (code, classNames) => Array.from(classNames)
.filter((className) => className.match(/-\d+$/))
.reduce(
(acc, val) => {
const exp = new RegExp(escapeRegExp(val), 'g');
return acc.replace(exp, val.slice(0, val.lastIndexOf('-')));
},
code,
);
@emoriarty Great idea for components using the legacy withStyles and thank you @jalovatt for the regex change. For me though I am still not having consistent snapshots. I'm also generating classnames too without a counter appended. I'm at the point, just like others have said that I am going to say pencils down and just walk away from snapshots as I've unfortunately spent entirely too much time on this. It's just not worth the headache here and if I end up writing more functional tests then that's even better. I'm sure at some point this will be solved for 100% of use cases, or something new will come along. All the best.
Don't know if it can help @oliviertassinari
I actually use this mock
const styles = jest.requireActual('@material-ui/core/styles'); const theme = styles.createMuiTheme(); const withStyles = style => component => { const classes = typeof style === 'function' ? style(theme) : style; component.defaultProps = { ...component.defaultProps, classes }; return component; }; module.exports = { ...styles, withStyles };
This way I have snapshot like this one
exports[`[Component] Header should render correctly 1`] = ` <header className={ Object { "alignItems": "center", "backgroundColor": "#282c34", "color": "#ffffff", "display": "flex", "flexDirection": "column", "fontSize": "calc(10px + 2vmin)", "justifyContent": "center", "minHeight": "100vh", } } > <img alt="logo" className={ Object { "animation": "spin infinite 20s linear", "height": "40vmin", } } src="logo.svg" /> <p> Title </p> </header> `;
I prefer having the css properties instead of a classname to detect UI regressions.
With the v4 I now use this mock
import theme from '../../src/theme/muiTheme'; const styles = jest.requireActual('@material-ui/styles'); const makeStyles = style => props => { // Apply theme to classes const classes = typeof style === 'function' ? style(theme) : style; // Apply props to every key of each class, which is every key of classes const classesByProps = {}; Object.keys(classes).forEach(classKey => { const classByProps = {}; Object.keys(classes[classKey]).forEach(key => { classByProps[key] = typeof classes[classKey][key] === 'function' ? classes[classKey][key](props) : classes[classKey][key]; }); classesByProps[classKey] = classByProps; }); return classesByProps; }; module.exports = { ...styles, makeStyles };
Thanks much for sharing it. Still found it helpful. Just had to add a small change to it otherwise it throws error with the types.
(Warning: Failed prop type: Invalid prop
className
of typeobject
supplied toForwardRef(Typography)
, expectedstring
.) (Material-UI: The keyroot
provided to the classes prop is not valid for ForwardRef(Typography). You need to provide a non empty string instead of: [object Object].)
classesByProps[classKey] = JSON.stringify(classByProps);
For those still trying to solve this, I wrote a utility that loops through all children of a rendered tree and regex replace class names to remove the integers on the end. I'd rather have this as part of a mock function, but I preferred having style names match prod exactly but without number. Utility:
const cleanClasses = (tree) => {
// There are occasions where MUI has a "null" child or entire tree, skip those for our purposes
if(tree) {
// Some item trees are returned as arrays instead of objects
// If it's an array, make sure we clean each item in that array
if(tree.length > 1) {
for (let i = 0; i < tree.length; i++) {
cleanClasses(tree[i]);
}
}
// If this level of the tree has children
// Clean the children's className
// And pass any children of that child back to this function
if(tree.children) {
for(let i = 0; i < tree.children.length; i++) {
if(tree.children[i].props && tree.children[i].props.className) {
tree.children[i].props.className = tree.children[i].props.className.replace(/-[0-9]{1,}/g, '');
}
if(tree.children[i].children && tree.children[i].children) {
for(let ii = 0; ii < tree.children[i].children.length; ii++) {
cleanClasses(tree.children[i].children[ii]);
}
}
}
}
// Clean the className of the current item in the nest
// Makes sure the top-level is always covered
if(tree.props && tree.props.className) {
tree.props.className = tree.props.className.replace(/-[0-9]{1,}/g, '');
}
}
// Return the cleaned tree
return tree;
}
export default cleanClasses;
Then in a snapshot, import the utility and:
const tree = renderer
.create(
<YourComponentHere />
)
.toJSON();
expect(cleanClasses(tree)).toMatchSnapshot();
@angusmccloud : this worked with minor adjustments, thanks so much!
const cleanClasses = (tree: any) => {
const r = new RegExp("-[0-9a-zA-Z]{1,}( .*)?", "g");
// There are occasions where MUI has a "null" child or entire tree, skip those for our purposes
if(tree) {
// Some item trees are returned as arrays instead of objects
// If it's an array, make sure we clean each item in that array
if(tree.length > 1) {
for (let i = 0; i < tree.length; i++) {
cleanClasses(tree[i]);
}
}
// If this level of the tree has children
// Clean the children's className
// And pass any children of that child back to this function
if(tree.children) {
for(let i = 0; i < tree.children.length; i++) {
if(tree.children[i] && tree.children[i].className) {
try
{
tree.children[i].className = tree.children[i].className.replace(r, '');
} catch (e) {
}
}
if(tree.children[i].children && tree.children[i].children) {
for(let ii = 0; ii < tree.children[i].children.length; ii++) {
cleanClasses(tree.children[i].children[ii]);
}
}
}
}
// Clean the className of the current item in the nest
// Makes sure the top-level is always covered
if(tree && tree.className) {
try
{
tree.className = tree.className.replace(r, '');
}
catch (e) {}
}
}
// Return the cleaned tree
return tree;
}
export default cleanClasses;
Closing as in v5, we rely on emotion and styled-components that have their own plugin https://emotion.sh/docs/testing, https://github.com/styled-components/jest-styled-components.
@oliviertassinari I'm not sure I understand. If I use material-ui components, I can use emotion testing?
This was a previously #9492 however it does not actually provide a fix for the core problem which is that in testing environments we should be able to
mount(<Component>)
that are created fromwithStyles()
and have snapshots be reliable without wrapping them.Expected Behavior 🤔
Snapshots should create reliable labels for withStyles() used by both custom and defaut material-ui components like in the red portion of this Snapshot:
Current Behavior 😯
One can modify a custom component to have
However, this only modifies overwritten styles in material UI, default ones are actually not modified at all:
Steps to Reproduce 🕹
This can't be easily reproduced in a sandbox environment. Link:
ruleCounter
increases.Context 🔦
I would like to be able to mount() and call Jest's native as stated in the docs of https://material-ui.com/customization/css-in-js/#creategenerateclassname-options-class-name-generator because tests should mirror their native implementation.
.toMatchSnapshot()
without worrying about the ruleCounter adding random numbers. It is not appropriate to wrap every object in aYour Environment 🌎