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.

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

[utils] Drop componentPropType in favor of PropTypes.elementType #14602

Merged
merged 2 commits into from
Feb 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions docs/src/modules/utils/generateMarkdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,9 @@ function generatePropType(type) {
return generatePropType(chained.type);
}

if (type.raw === 'componentPropType') {
return 'Component';
// this should be fixed at some point in react-docgen
Copy link
Member

Choose a reason for hiding this comment

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

if (type.raw === 'PropTypes.elementType') {
return 'element type';
}

return type.raw;
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@
"nyc": "^13.0.0",
"postcss": "^7.0.0",
"prettier": "^1.8.2",
"prop-types": "^15.6.0",
"prop-types": "^15.7.2",
"puppeteer": "^1.5.0",
"raw-loader": "^1.0.0",
"react": "^16.8.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui-lab/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
"@material-ui/utils": "^4.0.0-alpha.0",
"clsx": "^1.0.2",
"keycode": "^2.1.9",
"prop-types": "^15.6.0"
"prop-types": "^15.7.2"
},
"devDependencies": {},
"sideEffects": false,
Expand Down
3 changes: 1 addition & 2 deletions packages/material-ui-lab/src/Slider/Slider.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import clsx from 'clsx';
import withStyles from '@material-ui/core/styles/withStyles';
import ButtonBase from '@material-ui/core/ButtonBase';
import { fade } from '@material-ui/core/styles/colorManipulator';
import { componentPropType } from '@material-ui/utils';
import clamp from '../utils/clamp';

export const styles = theme => {
Expand Down Expand Up @@ -616,7 +615,7 @@ Slider.propTypes = {
* The component used for the root node.
* Either a string to use a DOM element or a component.
*/
component: componentPropType,
component: PropTypes.elementType,
/**
* If `true`, the slider will be disabled.
*/
Expand Down
3 changes: 1 addition & 2 deletions packages/material-ui-lab/src/SpeedDial/SpeedDial.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import PropTypes from 'prop-types';
import clsx from 'clsx';
import keycode from 'keycode';
import warning from 'warning';
import { componentPropType } from '@material-ui/utils';
import { withStyles } from '@material-ui/core/styles';
import Zoom from '@material-ui/core/Zoom';
import { duration } from '@material-ui/core/styles/transitions';
Expand Down Expand Up @@ -343,7 +342,7 @@ SpeedDial.propTypes = {
/**
* The component used for the transition.
*/
TransitionComponent: componentPropType,
TransitionComponent: PropTypes.elementType,
/**
* The duration for the transition, in milliseconds.
* You may specify a single timeout for all transitions, or individually with an object.
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui-styles/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
"jss-plugin-props-sort": "^10.0.0-alpha.7",
"jss-plugin-rule-value-function": "^10.0.0-alpha.7",
"jss-plugin-vendor-prefixer": "^10.0.0-alpha.7",
"prop-types": "^15.6.0",
"prop-types": "^15.7.2",
"warning": "^4.0.1"
},
"sideEffects": false,
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui-system/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"dependencies": {
"@babel/runtime": "^7.2.0",
"deepmerge": "^3.0.0",
"prop-types": "^15.6.0",
"prop-types": "^15.7.2",
"warning": "^4.0.1"
},
"devDependencies": {},
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
},
"dependencies": {
"@babel/runtime": "^7.2.0",
"prop-types": "^15.6.0",
"prop-types": "^15.7.2",
"react-is": "^16.8.0"
},
"devDependencies": {},
Expand Down
48 changes: 0 additions & 48 deletions packages/material-ui-utils/src/componentPropType.js

This file was deleted.

92 changes: 0 additions & 92 deletions packages/material-ui-utils/src/componentPropType.test.js

This file was deleted.

1 change: 0 additions & 1 deletion packages/material-ui-utils/src/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
export { default as componentPropType } from './componentPropType';
export { default as chainPropTypes } from './chainPropTypes';
export { default as exactProp } from './exactProp';
export { default as getDisplayName } from './getDisplayName';
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
"jss-vendor-prefixer": "^7.0.0",
"normalize-scroll-left": "^0.1.2",
"popper.js": "^1.14.1",
"prop-types": "^15.6.0",
"prop-types": "^15.7.2",
Copy link
Member

Choose a reason for hiding this comment

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

PropTypes.elementType was introduced in 15.7.0. Can we lose a bit the version requirement?
https://github.com/facebook/prop-types/blob/master/CHANGELOG.md

Copy link
Member Author

Choose a reason for hiding this comment

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

We already had 15.7.1 in the lockfile anyway and 15.7.2 had an important bug fix for browserify build. I would rather propagate that to downstream packages. The only time this would cause duplication (assuming this was bad) is when someone actually pins their prop-types to "prop-types": "15.7.1" and I don't see any reason for that.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I was looking for potential dependency deduplication for our users.

Copy link
Member Author

Choose a reason for hiding this comment

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

That can always happen which is why I always review lockfiles and adjust them if I see deduplication potential. I think both viewpoints are perfectly valid but I would insist in this case because 15.7.2 had a critical bug fix.

"react-event-listener": "^0.6.2",
"react-transition-group": "^2.2.1",
"warning": "^4.0.1"
Expand Down
3 changes: 1 addition & 2 deletions packages/material-ui/src/Avatar/Avatar.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React from 'react';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import { componentPropType } from '@material-ui/utils';
import withStyles from '../styles/withStyles';

export const styles = theme => ({
Expand Down Expand Up @@ -123,7 +122,7 @@ Avatar.propTypes = {
* The component used for the root node.
* Either a string to use a DOM element or a component.
*/
component: componentPropType,
component: PropTypes.elementType,
/**
* Attributes applied to the `img` element if the component
* is used to display an image.
Expand Down
3 changes: 1 addition & 2 deletions packages/material-ui/src/Badge/Badge.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React from 'react';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import { componentPropType } from '@material-ui/utils';
import withStyles from '../styles/withStyles';
import { capitalize } from '../utils/helpers';

Expand Down Expand Up @@ -142,7 +141,7 @@ Badge.propTypes = {
* The component used for the root node.
* Either a string to use a DOM element or a component.
*/
component: componentPropType,
component: PropTypes.elementType,
/**
* If `true`, the badge will be invisible.
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React from 'react';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import { componentPropType } from '@material-ui/utils';
import warning from 'warning';
import withStyles from '../styles/withStyles';

Expand Down Expand Up @@ -76,7 +75,7 @@ BottomNavigation.propTypes = {
* The component used for the root node.
* Either a string to use a DOM element or a component.
*/
component: componentPropType,
component: PropTypes.elementType,
/**
* Callback fired when the value changes.
*
Expand Down
3 changes: 1 addition & 2 deletions packages/material-ui/src/Breadcrumbs/Breadcrumbs.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React from 'react';
import warning from 'warning';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import { componentPropType } from '@material-ui/utils';
import withStyles from '../styles/withStyles';
import Typography from '../Typography';
import BreadcrumbCollapsed from './BreadcrumbCollapsed';
Expand Down Expand Up @@ -141,7 +140,7 @@ Breadcrumbs.propTypes = {
* Either a string to use a DOM element or a component.
* By default, it maps the variant to a good default headline component.
*/
component: componentPropType,
component: PropTypes.elementType,
/**
* If max items is exceeded, the number of items to show after the ellipsis.
*/
Expand Down
3 changes: 1 addition & 2 deletions packages/material-ui/src/Button/Button.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import React from 'react';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import { componentPropType } from '@material-ui/utils';
import withStyles from '../styles/withStyles';
import { fade } from '../styles/colorManipulator';
import ButtonBase from '../ButtonBase';
Expand Down Expand Up @@ -258,7 +257,7 @@ Button.propTypes = {
* The component used for the root node.
* Either a string to use a DOM element or a component.
*/
component: componentPropType,
component: PropTypes.elementType,
/**
* If `true`, the button will be disabled.
*/
Expand Down
3 changes: 1 addition & 2 deletions packages/material-ui/src/ButtonBase/ButtonBase.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React from 'react';
import PropTypes from 'prop-types';
import ReactDOM from 'react-dom';
import clsx from 'clsx';
import { componentPropType } from '@material-ui/utils';
import ownerWindow from '../utils/ownerWindow';
import withStyles from '../styles/withStyles';
import NoSsr from '../NoSsr';
Expand Down Expand Up @@ -348,7 +347,7 @@ ButtonBase.propTypes = {
* The component used for the root node.
* Either a string to use a DOM element or a component.
*/
component: componentPropType,
component: PropTypes.elementType,
/**
* If `true`, the base button will be disabled.
*/
Expand Down
3 changes: 1 addition & 2 deletions packages/material-ui/src/CardContent/CardContent.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React from 'react';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import { componentPropType } from '@material-ui/utils';
import withStyles from '../styles/withStyles';

export const styles = {
Expand Down Expand Up @@ -34,7 +33,7 @@ CardContent.propTypes = {
* The component used for the root node.
* Either a string to use a DOM element or a component.
*/
component: componentPropType,
component: PropTypes.elementType,
};

CardContent.defaultProps = {
Expand Down
3 changes: 1 addition & 2 deletions packages/material-ui/src/CardHeader/CardHeader.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React from 'react';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import { componentPropType } from '@material-ui/utils';
import withStyles from '../styles/withStyles';
import Typography from '../Typography';

Expand Down Expand Up @@ -114,7 +113,7 @@ CardHeader.propTypes = {
* The component used for the root node.
* Either a string to use a DOM element or a component.
*/
component: componentPropType,
component: PropTypes.elementType,
/**
* If `true`, the children won't be wrapped by a Typography component.
* This can be useful to render an alternative Typography variant by wrapping
Expand Down
3 changes: 1 addition & 2 deletions packages/material-ui/src/CardMedia/CardMedia.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React from 'react';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import warning from 'warning';
import { componentPropType } from '@material-ui/utils';
import withStyles from '../styles/withStyles';

export const styles = {
Expand Down Expand Up @@ -63,7 +62,7 @@ CardMedia.propTypes = {
* Component for rendering image.
* Either a string to use a DOM element or a component.
*/
component: componentPropType,
component: PropTypes.elementType,
/**
* Image to be displayed as a background image.
* Either `image` or `src` prop must be specified.
Expand Down