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

Depend on prop-types/checkPropTypes, not prop-types itself #11420

Merged
merged 3 commits into from Nov 1, 2017

Conversation

Projects
None yet
3 participants
@gaearon
Member

gaearon commented Oct 31, 2017

Two small changes.

  1. Instead of reimplementing prop-types/checkPropTypes we can just use it. I think it didn't exist when this copy was written but now we can switch to it.

  2. Importing prop-types itself is going to be harder to DCE in DEV only once we move to ES Modules. It is doing some funky business in entry points, and builds up exports in a way that Rollup might consider side effectful. It seems much simpler to just inline the implementation since we only need a subset and there are no other places where we need it. We can delete it—see below.

  3. Actually I just checked, and the onChange annotation can be removed altogether. That check existed before we had DEV-time hooks verifying prop types. So currently we fire the warning twice:

screen shot 2017-10-31 at 23 34 24

This removes the unnecessary extra warning, so now it is handled once just like all other host elements:

screen shot 2017-10-31 at 23 34 02

So this both makes DEV bundle smaller, and removes an unnecessary noisy warning.

gaearon added some commits Oct 31, 2017

Remove renderer dependency on top-level PropTypes
This annotation is unnecessary because we already warn for bad event listener types.
@trueadm

trueadm approved these changes Nov 1, 2017

LGTM

@gaearon gaearon merged commit 9d75a62 into facebook:master Nov 1, 2017

1 check passed

ci/circleci Your tests passed on CircleCI!
Details

Ethan-Arrowood added a commit to Ethan-Arrowood/react that referenced this pull request Dec 8, 2017

Depend on prop-types/checkPropTypes, not prop-types itself (facebook#…
…11420)

* Remove prop-types/checkPropTypes reimplementation

* Remove renderer dependency on top-level PropTypes

This annotation is unnecessary because we already warn for bad event listener types.

* Record sizes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment