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

Strip PropTypes checkers in production build #8066

Merged
merged 2 commits into from
Oct 24, 2016

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Oct 23, 2016

Resubmitting #7651 because I accidentally nuked my fork.
Feedback from #7651 is addressed.

This removes the warning about PropTypes and actually replaces them with a single PropType function that throws in production.

Difference from master:

   raw     gz Compared to last run                                             
     =      = build/react-dom-fiber.js                                         
     =      = build/react-dom-fiber.min.js                                     
  -652   -133 build/react-dom-server.js                                        
  -134    -82 build/react-dom-server.min.js                                    
  -652    -84 build/react-dom.js                                               
  -129    -41 build/react-dom.min.js                                           
  -379   -103 build/react-with-addons.js                                       
 -3837  -1022 build/react-with-addons.min.js                                   
  -379   -105 build/react.js                                                   
 -3838  -1050 build/react.min.js                                               

@acdlite
Copy link
Collaborator

acdlite commented Oct 24, 2016

Looks good

@aweary aweary added this to the 16.0 milestone Oct 24, 2016
@gaearon gaearon merged commit 14156e5 into facebook:master Oct 24, 2016
@gaearon gaearon deleted the strip-prop-types branch October 24, 2016 17:43
gaearon added a commit to gaearon/react that referenced this pull request Jan 31, 2017
…k#7132)""

This reverts commit be71f76.

In other words, now we again have the warning if you attempt to call PropTypes manually.
It was removed in facebook#8066 but we shouldn't have done this since we still want to avoid people accidentally calling them in production (and even more so since now it would throw).

Fixes facebook#8080.
gaearon added a commit to gaearon/react that referenced this pull request Feb 7, 2017
…k#7132)""

This reverts commit be71f76.

In other words, now we again have the warning if you attempt to call PropTypes manually.
It was removed in facebook#8066 but we shouldn't have done this since we still want to avoid people accidentally calling them in production (and even more so since now it would throw).

Fixes facebook#8080.
gaearon added a commit that referenced this pull request Feb 8, 2017
* Revert "Revert "Warn if PropType function is called manually (#7132)""

This reverts commit be71f76.

In other words, now we again have the warning if you attempt to call PropTypes manually.
It was removed in #8066 but we shouldn't have done this since we still want to avoid people accidentally calling them in production (and even more so since now it would throw).

Fixes #8080.

* Pass secret in ReactControlledValuePropTypes

* Record tests
acusti pushed a commit to brandcast/react that referenced this pull request Mar 15, 2017
* Strip PropTypes in production build

* Revert "Warn if PropType function is called manually (facebook#7132)"

This reverts commit e75e8dc.
@bvaughn bvaughn mentioned this pull request Aug 1, 2017
@peterbraden-amana
Copy link

FWIW, this broke our code in production, but not locally as we had some code that compared propTypes to determine behaviour (ie. if (foo.propTypes[x] == PropTypes.number){ ... })

There is a distinct lack of documentation about the behaviour of prop-types in production, changing them from a distinct symbol to something that is equal for all types is fairly problematic.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants