Skip to content

Commit

Permalink
chore: Use .checkPropTypes instead of render + propTypes
Browse files Browse the repository at this point in the history
  • Loading branch information
eps1lon committed Apr 7, 2020
1 parent b6f44b7 commit 9265f8c
Show file tree
Hide file tree
Showing 14 changed files with 121 additions and 73 deletions.
5 changes: 4 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,10 @@ module.exports = {
// test to `beforeEach`.
// `beforeEach`+`afterEach` also means that the `beforeEach`
// is cleaned up in `afterEach` if the test causes a crash
'mocha/no-hooks-for-single-case': 'off'
'mocha/no-hooks-for-single-case': 'off',

// They are accessed to test custom validator implementation with PropTypes.checkPropTypes
'react/forbid-foreign-prop-types': 'off',
},
},
{
Expand Down
10 changes: 6 additions & 4 deletions packages/material-ui-styles/src/styled/styled.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,13 @@ describe('styled', () => {
});

it('warns if it cant detect the secondary action properly', () => {
mount(
<StyledButton clone component="div">
<div>Styled Components</div>
</StyledButton>,
PropTypes.checkPropTypes(
StyledButton.propTypes,
{ clone: true, component: 'div' },
'prop',
'StyledButton',
);

assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.include(
consoleErrorMock.messages()[0],
Expand Down
8 changes: 6 additions & 2 deletions packages/material-ui-styles/src/withTheme/withTheme.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,12 @@ describe('withTheme', () => {

it('is deprecated', () => {
const ThemedDiv = withTheme('div');

mount(<ThemedDiv innerRef={React.createRef()} />);
PropTypes.checkPropTypes(
ThemedDiv.propTypes,
{ innerRef: React.createRef() },
'prop',
'ThemedDiv',
);

assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.include(
Expand Down
1 change: 1 addition & 0 deletions packages/material-ui-utils/src/chainPropTypes.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ describe('chainPropTypes', () => {

beforeEach(() => {
consoleErrorMock.spy();
PropTypes.resetWarningCache();
});

afterEach(() => {
Expand Down
11 changes: 8 additions & 3 deletions packages/material-ui/src/ButtonBase/ButtonBase.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -929,11 +929,16 @@ describe('<ButtonBase />', () => {
return <button type="button" {...props} />;
}

// cant match the error message here because flakiness with mocha watchmode
render(<ButtonBase component={Component} />);
PropTypes.checkPropTypes(
// @ts-ignore `Naked` is internal
ButtonBase.Naked.propTypes,
{ classes: {}, component: Component },
'prop',
'MockedName',
);

expect(consoleErrorMock.messages()[0]).to.include(
'Invalid prop `component` supplied to `ForwardRef(ButtonBase)`. Expected an element type that can hold a ref',
'Invalid prop `component` supplied to `MockedName`. Expected an element type that can hold a ref',
);
});

Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/src/CardMedia/CardMedia.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ describe('<CardMedia />', () => {
});

it('warns when neither `children`, nor `image`, nor `src`, nor `component` are provided', () => {
mount(<CardMedia />);
PropTypes.checkPropTypes(CardMedia.Naked.propTypes, { classes: {} }, 'prop', 'MockedName');

assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.include(
Expand Down
26 changes: 14 additions & 12 deletions packages/material-ui/src/ExpansionPanel/ExpansionPanel.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,23 +145,25 @@ describe('<ExpansionPanel />', () => {
});

it('requires at least one child', () => {
if (!/jsdom/.test(window.navigator.userAgent)) {
// errors during mount are not caught by try-catch in the browser
// can't use skip since this causes multiple errors related to cleanup of the console mock
return;
}

assert.throws(() => mount(<ExpansionPanel>{[]}</ExpansionPanel>));
assert.strictEqual(consoleErrorMock.callCount(), 3);
PropTypes.checkPropTypes(
ExpansionPanel.Naked.propTypes,
{ classes: {}, children: [] },
'prop',
'MockedName',
);

assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.include(consoleErrorMock.messages()[0], 'Material-UI: expected the first child');
});

it('needs a valid element as the first child', () => {
mount(
<ExpansionPanel>
<React.Fragment />
</ExpansionPanel>,
PropTypes.checkPropTypes(
ExpansionPanel.Naked.propTypes,
{ classes: {}, children: <React.Fragment /> },
'prop',
'MockedName',
);

assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.include(
consoleErrorMock.messages()[0],
Expand Down
10 changes: 6 additions & 4 deletions packages/material-ui/src/IconButton/IconButton.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,11 +136,13 @@ describe('<IconButton />', () => {
});

it('should raise a warning', () => {
render(
<IconButton>
<svg onClick={() => {}} />
</IconButton>,
PropTypes.checkPropTypes(
IconButton.Naked.propTypes,
{ classes: {}, children: <svg onClick={() => {}} /> },
'prop',
'MockedName',
);

expect(consoleErrorMock.callCount()).to.equal(1);
expect(consoleErrorMock.messages()[0]).to.include(
'you are providing an onClick event listener',
Expand Down
16 changes: 11 additions & 5 deletions packages/material-ui/src/ListItem/ListItem.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,11 +150,17 @@ describe('<ListItem />', () => {
});

it('warns if it cant detect the secondary action properly', () => {
render(
<ListItem>
<ListItemSecondaryAction>I should have come last :(</ListItemSecondaryAction>
<ListItemText>My position doesn not matter.</ListItemText>
</ListItem>,
PropTypes.checkPropTypes(
ListItem.Naked.propTypes,
{
classes: {},
children: [
<ListItemSecondaryAction>I should have come last :(</ListItemSecondaryAction>,
<ListItemText>My position doesn not matter.</ListItemText>,
],
},
'prop',
'MockedName',
);

expect(consoleErrorMock.callCount()).to.equal(1);
Expand Down
18 changes: 11 additions & 7 deletions packages/material-ui/src/Popper/Popper.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -284,15 +284,19 @@ describe('<Popper />', () => {
});

it('should warn if anchorEl is not valid', () => {
mount(<Popper {...defaultProps} open anchorEl={null} />);
PropTypes.checkPropTypes(
Popper.propTypes,
{
...defaultProps,
open: true,
anchorEl: null,
},
'prop',
'MockedPopper',
);

assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.include(consoleErrorMock.messages()[0], 'It should be an HTML Element instance');
});

// it('should warn if anchorEl is not visible', () => {
// mount(<Popper {...defaultProps} open anchorEl={document.createElement('div')} />);
// assert.strictEqual(consoleErrorMock.callCount(), 1);
// assert.include(consoleErrorMock.messages()[0], 'The node element should be visible');
// });
});
});
18 changes: 16 additions & 2 deletions packages/material-ui/src/Slider/Slider.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -497,14 +497,28 @@ describe('<Slider />', () => {
});

it('should warn if aria-valuetext is provided', () => {
render(<Slider value={[20, 50]} aria-valuetext="hot" />);
PropTypes.checkPropTypes(
Slider.Naked.propTypes,
{ classes: {}, value: [20, 50], 'aria-valuetext': 'hot' },
'prop',
'MockedSlider',
);

expect(consoleErrorMock.callCount()).to.equal(1);
expect(consoleErrorMock.messages()[0]).to.include(
'you need to use the `getAriaValueText` prop instead of',
);
});

it('should warn if aria-label is provided', () => {
render(<Slider value={[20, 50]} aria-label="hot" />);
PropTypes.checkPropTypes(
Slider.Naked.propTypes,
{ classes: {}, value: [20, 50], 'aria-label': 'hot' },
'prop',
'MockedSlider',
);

expect(consoleErrorMock.callCount()).to.equal(1);
expect(consoleErrorMock.messages()[0]).to.include(
'you need to use the `getAriaLabel` prop instead of',
);
Expand Down
40 changes: 23 additions & 17 deletions packages/material-ui/src/SwipeableDrawer/SwipeableDrawer.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { assert } from 'chai';
import { spy } from 'sinon';
import { createMount } from '@material-ui/core/test-utils';
import describeConformance from '@material-ui/core/test-utils/describeConformance';
import PropTypes from 'prop-types';
import PropTypes, { checkPropTypes } from 'prop-types';
import consoleErrorMock from 'test/utils/consoleErrorMock';
import Drawer from '../Drawer';
import SwipeableDrawer, { reset } from './SwipeableDrawer';
Expand Down Expand Up @@ -525,36 +525,42 @@ describe('<SwipeableDrawer />', () => {
});

it('warns if a component for the Paper is used that cant hold a ref', () => {
mount(
<SwipeableDrawer
onOpen={() => {}}
onClose={() => {}}
open={false}
PaperProps={{ component: () => <div />, elevation: 4 }}
/>,
checkPropTypes(
SwipeableDrawer.propTypes,
{
onOpen: () => {},
onClose: () => {},
open: false,
PaperProps: { component: () => <div />, elevation: 4 },
},
'prop',
'MockedSwipeableDrawer',
);

assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.include(
consoleErrorMock.messages()[0],
'Warning: Failed prop type: Invalid prop `PaperProps.component` supplied to `ForwardRef(SwipeableDrawer)`. Expected an element type that can hold a ref.',
'Warning: Failed prop type: Invalid prop `PaperProps.component` supplied to `MockedSwipeableDrawer`. Expected an element type that can hold a ref.',
);
});

it('warns if a component for the Backdrop is used that cant hold a ref', () => {
mount(
<SwipeableDrawer
onOpen={() => {}}
onClose={() => {}}
open={false}
ModalProps={{ BackdropProps: { component: () => <div />, 'data-backdrop': true } }}
/>,
checkPropTypes(
SwipeableDrawer.propTypes,
{
onOpen: () => {},
onClose: () => {},
open: false,
ModalProps: { BackdropProps: { component: () => <div />, 'data-backdrop': true } },
},
'prop',
'MockedSwipeableDrawer',
);

assert.strictEqual(consoleErrorMock.callCount(), 1);
assert.include(
consoleErrorMock.messages()[0],
'Warning: Failed prop type: Invalid prop `ModalProps.BackdropProps.component` supplied to `ForwardRef(SwipeableDrawer)`. Expected an element type that can hold a ref.',
'Warning: Failed prop type: Invalid prop `ModalProps.BackdropProps.component` supplied to `MockedSwipeableDrawer`. Expected an element type that can hold a ref.',
);
});
});
Expand Down
26 changes: 12 additions & 14 deletions packages/material-ui/src/TablePagination/TablePagination.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -288,20 +288,18 @@ describe('<TablePagination />', () => {
});

it('should raise a warning if the page prop is out of range', () => {
mount(
<table>
<TableFooter>
<TableRow>
<TablePagination
page={2}
count={20}
rowsPerPage={10}
onChangePage={noop}
onChangeRowsPerPage={noop}
/>
</TableRow>
</TableFooter>
</table>,
PropTypes.checkPropTypes(
TablePagination.Naked.propTypes,
{
classes: {},
page: 2,
count: 20,
rowsPerPage: 10,
onChangePage: noop,
onChangeRowsPerPage: noop,
},
'prop',
'MockedTablePagination',
);

assert.strictEqual(consoleErrorMock.callCount(), 1);
Expand Down
3 changes: 2 additions & 1 deletion packages/material-ui/src/utils/deprecatedPropType.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ describe('deprecatedPropType', () => {

beforeEach(() => {
consoleErrorMock.spy();
PropTypes.resetWarningCache();
});

afterEach(() => {
Expand All @@ -30,7 +31,7 @@ describe('deprecatedPropType', () => {
});

it('should warn once', () => {
const propName = `children${new Date()}`;
const propName = `children`;
const props = {
[propName]: 'yolo',
};
Expand Down

0 comments on commit 9265f8c

Please sign in to comment.