Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[test] Remove test-only class wrappers for higher-order components #15017

Merged
merged 2 commits into from
Mar 23, 2019
Merged

[test] Remove test-only class wrappers for higher-order components #15017

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4077c2f
[test] Use actual withStyles implementation
eps1lon Mar 22, 2019
0e661ba
[MenuList] Adjust clientHeight test
eps1lon Mar 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions package.json
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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 },
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/cc @ryancogswell

Not entirely sure why. I think it has something to do with ref now being attached to the actual host component instead of the class wrapper. Without this change browser test were failing: https://circleci.com/gh/mui-org/material-ui/79900

Copy link
Member

@oliviertassinari oliviertassinari Mar 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prior to the change, no CSS was injected in the page, now, it is. I have already seen this behavior in the past. The newly injected CSS change the layout values, now, they are correct.
The new styles come from the List component.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes the change even better I suppose.

{ direction: 'ltr' },
);
assert.strictEqual(list.style.paddingRight, '');
Expand Down
8 changes: 4 additions & 4 deletions packages/material-ui/src/Popover/Popover.test.js
View file
Open in desktop
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
View file
Open in desktop
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
Loading