-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
Conversation
eef688c
to
99c970d
Compare
@@ -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", |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
@@ -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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Breaking change
Switched the wording from
Component
toelement type
in the docs. This is more in line with the flow types and nowprop-types
.