Skip to content

Commit

Permalink
[test] Remove test-only class wrappers for higher-order components (#…
Browse files Browse the repository at this point in the history
…15017)

* [test] Use actual withStyles implementation

* [MenuList] Adjust clientHeight test

Don't know why the bounding box of
the ref element changed in the browser tests
  • Loading branch information
eps1lon authored and oliviertassinari committed Mar 23, 2019
1 parent 6e750ac commit 1d91e0a
Show file tree
Hide file tree
Showing 16 changed files with 90 additions and 126 deletions.
8 changes: 4 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@
"dtslint": "^0.4.0",
"emotion-theming": "^10.0.5",
"enzyme": "^3.5.0",
"enzyme-adapter-react-16": "^1.3.0",
"enzyme-adapter-react-16": "~1.10.0",
"eslint": "^5.9.0",
"eslint-config-airbnb": "^17.0.0",
"eslint-config-prettier": "^4.0.0",
Expand Down Expand Up @@ -160,10 +160,10 @@
"prop-types": "^15.7.2",
"puppeteer": "^1.5.0",
"raw-loader": "^1.0.0",
"react": "^16.8.0",
"react": "^16.8.5",
"react-autosuggest": "^9.3.2",
"react-docgen": "^4.0.1",
"react-dom": "^16.8.0",
"react-dom": "^16.8.5",
"react-draggable": "^3.0.5",
"react-final-form": "^4.0.2",
"react-frame-component": "^4.0.2",
Expand All @@ -174,7 +174,7 @@
"react-router-dom": "^4.2.2",
"react-select": "^2.0.0",
"react-swipeable-views": "^0.13.0",
"react-test-renderer": "^16.1.1",
"react-test-renderer": "^16.8.5",
"react-text-mask": "^5.0.2",
"react-virtualized": "^9.21.0",
"recast": "^0.17.0",
Expand Down
2 changes: 0 additions & 2 deletions packages/material-ui-styles/src/styled/styled.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ describe('styled', () => {

before(() => {
mount = createMount();
global.disableShallowSupport = true;
StyledButton = styled('button')({
background: 'linear-gradient(45deg, #FE6B8B 30%, #FF8E53 90%)',
borderRadius: 3,
Expand All @@ -28,7 +27,6 @@ describe('styled', () => {

after(() => {
mount.cleanUp();
global.disableShallowSupport = false;
});

it('should work as expected', () => {
Expand Down
40 changes: 3 additions & 37 deletions packages/material-ui-styles/src/withStyles/withStyles.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@ import React from 'react';
import PropTypes from 'prop-types';
import warning from 'warning';
import hoistNonReactStatics from 'hoist-non-react-statics';
import { chainPropTypes, getDisplayName, ponyfillGlobal } from '@material-ui/utils';
import { chainPropTypes, getDisplayName } from '@material-ui/utils';
import makeStyles from '../makeStyles';
import getThemeProps from '../getThemeProps';
import useTheme from '../useTheme';
import mergeClasses from '../mergeClasses';
import getStylesCreator from '../getStylesCreator';

// Link a style sheet with a component.
// It does not modify the component passed to it;
Expand Down Expand Up @@ -46,7 +44,7 @@ const withStyles = (stylesOrCreator, options = {}) => Component => {
...stylesOptions,
});

let WithStyles = React.forwardRef(function WithStyles(props, ref) {
const WithStyles = React.forwardRef(function WithStyles(props, ref) {
const { classes: classesProp, innerRef, ...other } = props;
const classes = useStyles(props);

Expand All @@ -72,39 +70,6 @@ const withStyles = (stylesOrCreator, options = {}) => Component => {
return <Component ref={innerRef || ref} classes={classes} {...more} />;
});

if (process.env.NODE_ENV === 'test' && !ponyfillGlobal.disableShallowSupport) {
// Generate a fake classes object.
const stylesCreator = getStylesCreator(stylesOrCreator);
const styles = stylesCreator.create(defaultTheme, name);
const classes = Object.keys(styles).reduce((acc, key) => {
acc[key] = `${name}-${key}`;
return acc;
}, {});

class WithStylesTest extends React.Component {
render() {
// eslint-disable-next-line react/prop-types
const { classes: newClasses, innerRef, ...other } = this.props;
const more = other;

if (withTheme && !more.theme) {
more.theme = defaultTheme;
}

return (
<Component
ref={innerRef}
classes={mergeClasses({ baseClasses: classes, newClasses })}
{...more}
/>
);
}
}
WithStylesTest.Original = WithStyles;
WithStyles = WithStylesTest;
WithStyles.classes = classes;
}

WithStyles.propTypes = {
/**
* Override or extend the styles applied to the component.
Expand Down Expand Up @@ -137,6 +102,7 @@ const withStyles = (stylesOrCreator, options = {}) => Component => {
// Exposed for test purposes.
WithStyles.Naked = Component;
WithStyles.options = options;
WithStyles.useStyles = useStyles;
}

return WithStyles;
Expand Down
2 changes: 0 additions & 2 deletions packages/material-ui-styles/src/withStyles/withStyles.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,10 @@ describe('withStyles', () => {

before(() => {
mount = createMount();
global.disableShallowSupport = true;
});

after(() => {
mount.cleanUp();
global.disableShallowSupport = false;
});

it('hoist statics', () => {
Expand Down
15 changes: 2 additions & 13 deletions packages/material-ui-styles/src/withTheme/withTheme.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import PropTypes from 'prop-types';
import hoistNonReactStatics from 'hoist-non-react-statics';
import { chainPropTypes, getDisplayName, ponyfillGlobal } from '@material-ui/utils';
import { chainPropTypes, getDisplayName } from '@material-ui/utils';
import useTheme from '../useTheme';

export function withThemeCreator(options = {}) {
Expand All @@ -17,23 +17,12 @@ export function withThemeCreator(options = {}) {
);
}

let WithTheme = React.forwardRef(function WithTheme(props, ref) {
const WithTheme = React.forwardRef(function WithTheme(props, ref) {
const { innerRef, ...other } = props;
const theme = useTheme() || defaultTheme;
return <Component theme={theme} ref={innerRef || ref} {...other} />;
});

if (process.env.NODE_ENV === 'test' && !ponyfillGlobal.disableShallowSupport) {
class WithThemeTest extends React.Component {
render() {
// eslint-disable-next-line react/prop-types
const { innerRef, ...other } = this.props;
return <Component theme={defaultTheme} ref={innerRef} {...other} />;
}
}
WithTheme = WithThemeTest;
}

WithTheme.propTypes = {
/**
* @deprecated
Expand Down
2 changes: 0 additions & 2 deletions packages/material-ui-styles/src/withTheme/withTheme.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,10 @@ describe('withTheme', () => {

before(() => {
mount = createMount();
global.disableShallowSupport = true;
});

after(() => {
mount.cleanUp();
global.disableShallowSupport = false;
});

it('should inject the theme', () => {
Expand Down
14 changes: 9 additions & 5 deletions packages/material-ui/src/ButtonBase/ButtonBase.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ describe('<ButtonBase />', () => {
wrapper.simulate('blur', {});

assert.strictEqual(instanceWrapper.instance().ripple.stop.callCount, 1);
assert.strictEqual(instanceWrapper.state().focusVisible, false);
assert.strictEqual(wrapper.find('button').hasClass(classes.focusVisible), false);
});
});

Expand Down Expand Up @@ -334,7 +334,11 @@ describe('<ButtonBase />', () => {
let clock;

function getState() {
return wrapper.find('ButtonBase').state();
/**
* wrapper.find('ButtonBase').state()
* throws '::state() can only be called on class components'
*/
return instance.state;
}

beforeEach(() => {
Expand Down Expand Up @@ -404,7 +408,7 @@ describe('<ButtonBase />', () => {
wrapper.setProps({
disabled: true,
});
assert.strictEqual(instanceWrapper.state().focusVisible, false);
assert.strictEqual(instanceWrapper.instance().state.focusVisible, false);
});

it('should not apply disabled on a span', () => {
Expand Down Expand Up @@ -693,13 +697,13 @@ describe('<ButtonBase />', () => {
});

it('should not rerender the TouchRipple', () => {
const wrapper = mount(<ButtonBase.Original>foo</ButtonBase.Original>);
const wrapper = mount(<ButtonBase>foo</ButtonBase>);
wrapper.setProps({
children: 'bar',
});
assert.strictEqual(
rerender.updates.filter(update => update.displayName !== 'NoSsr').length,
2,
1,
);
});
});
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/FormControl/FormControl.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ describe('<FormControl />', () => {
}

function getState(wrapper) {
return wrapper.find('FormControl').state();
return wrapper.find('FormControl').instance().state;
}

before(() => {
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/InputBase/InputBase.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ describe('<InputBase />', () => {
wrapper.setProps({
disabled: true,
});
assert.strictEqual(wrapper.find('InputBase').state().focused, false);
assert.strictEqual(wrapper.find('InputBase').instance().state.focused, false);
assert.strictEqual(handleBlur.callCount, 1);
});

Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/MenuList/MenuList.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ describe('<MenuList />', () => {
assert.strictEqual(list.style.paddingLeft, '');
assert.strictEqual(list.style.width, '');
menuListActionsRef.current.adjustStyleForScrollbar(
{ clientHeight: 10 },
{ clientHeight: 20 },
{ direction: 'ltr' },
);
assert.strictEqual(list.style.paddingRight, '');
Expand Down
8 changes: 4 additions & 4 deletions packages/material-ui/src/Popover/Popover.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ describe('<Popover />', () => {

return new Promise(resolve => {
const component = (
<Popover.Original
<Popover
{...defaultProps}
anchorEl={anchorEl}
anchorOrigin={anchorOrigin}
Expand All @@ -346,7 +346,7 @@ describe('<Popover />', () => {
}}
>
<div />
</Popover.Original>
</Popover>
);
wrapper = mount(component);
wrapper.setProps({ open: true });
Expand Down Expand Up @@ -482,7 +482,7 @@ describe('<Popover />', () => {
openPopover = anchorOrigin =>
new Promise(resolve => {
wrapper = mount(
<Popover.Original
<Popover
{...defaultProps}
anchorReference="anchorPosition"
anchorPosition={anchorPosition}
Expand All @@ -494,7 +494,7 @@ describe('<Popover />', () => {
}}
>
<div />
</Popover.Original>,
</Popover>,
);
wrapper.setProps({ open: true });
});
Expand Down
8 changes: 3 additions & 5 deletions packages/material-ui/src/TextField/TextField.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,10 @@ describe('<TextField />', () => {

describe('with an outline', () => {
it('should set outline props', () => {
wrapper = mount(<TextField variant="outlined" classes={{}} />);
wrapper = mount(<TextField variant="outlined" />);

assert.strictEqual(wrapper.props().variant, 'outlined');
assert.strictEqual(
wrapper.find(OutlinedInput).props().labelWidth,
wrapper.instance().inputLabelNode ? wrapper.instance().inputLabelNode.offsetWidth : 0,
);
assert.strictEqual(typeof wrapper.find(OutlinedInput).props().labelWidth, 'number');
});

it('should set shrink prop on outline from label', () => {
Expand Down

0 comments on commit 1d91e0a

Please sign in to comment.