-
Notifications
You must be signed in to change notification settings - Fork 47.7k
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
Add “Don't Call PropTypes” warning #7219
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,93 @@ | ||
--- | ||
title: Don't Call PropTypes Warning | ||
layout: single | ||
--- | ||
|
||
In a future major release of React, the code that implements PropType validation functions will be stripped in production. Once this happens, any code that calls these functions manually (that isn't stripped in production) will throw an error. | ||
|
||
### Declaring PropTypes is still fine | ||
|
||
The normal usage of PropTypes is still supported: | ||
|
||
```javascript | ||
Button.propTypes = { | ||
highlighted: React.PropTypes.bool | ||
}; | ||
``` | ||
|
||
Nothing changes here. | ||
|
||
### Don’t call PropTypes directly | ||
|
||
Using PropTypes in any other way than annotating React components with them is no longer supported: | ||
|
||
```javascript | ||
var apiShape = React.PropTypes.shape({ | ||
body: React.PropTypes.object, | ||
statusCode: React.PropTypes.number.isRequired | ||
}).isRequired; | ||
|
||
// Not supported! | ||
var error = apiShape(json, 'response'); | ||
``` | ||
|
||
If you depend on using PropTypes like this, we encourage you to use or create a fork of PropTypes (such as [these](https://github.com/aackerman/PropTypes) [two](https://github.com/developit/proptypes) packages). | ||
|
||
If you don't fix the warning, this code will crash in production with React 16. | ||
|
||
### If you don't call PropTypes directly but still get the warning | ||
|
||
Inspect the stack trace produced by the warning. You will find the component definition responsible for the PropTypes direct call. Most likely, the issue is due to third-party PropTypes that wrap React’s PropTypes, for example: | ||
|
||
```js | ||
Button.propTypes = { | ||
highlighted: ThirdPartyPropTypes.deprecated( | ||
React.PropTypes.bool, | ||
'Use `active` prop instead' | ||
) | ||
} | ||
``` | ||
|
||
In this case, `ThirdPartyPropTypes.deprecated` is a wrapper calling `React.PropTypes.bool`. This pattern by itself is fine, but triggers a false positive because React thinks you are calling PropTypes directly. The next section explains how to fix this problem for a library implementing something like `ThirdPartyPropTypes`. If it's not a library you wrote, you can file an issue against it. | ||
|
||
### Fixing the false positive in third party PropTypes | ||
|
||
If you are an author of a third party PropTypes library and you let consumers wrap existing React PropTypes, they might start seeing this warning coming from your library. This happens because React doesn't see a "secret" last argument that [it passes](https://github.com/facebook/react/pull/7132) to detect manual PropTypes calls. | ||
|
||
Here is how to fix it. We will use `deprecated` from [react-bootstrap/react-prop-types](https://github.com/react-bootstrap/react-prop-types/blob/0d1cd3a49a93e513325e3258b28a82ce7d38e690/src/deprecated.js) as an example. The current implementation only passes down the `props`, `propName`, and `componentName` arguments: | ||
|
||
```javascript | ||
export default function deprecated(propType, explanation) { | ||
return function validate(props, propName, componentName) { | ||
if (props[propName] != null) { | ||
const message = `"${propName}" property of "${componentName}" has been deprecated.\n${explanation}`; | ||
if (!warned[message]) { | ||
warning(false, message); | ||
warned[message] = true; | ||
} | ||
} | ||
|
||
return propType(props, propName, componentName); | ||
}; | ||
} | ||
``` | ||
|
||
In order to fix the false positive, make sure you pass **all** arguments down to the wrapped PropType. This is easy to do with the ES6 `...rest` notation: | ||
|
||
```javascript | ||
export default function deprecated(propType, explanation) { | ||
return function validate(props, propName, componentName, ...rest) { // Note ...rest here | ||
if (props[propName] != null) { | ||
const message = `"${propName}" property of "${componentName}" has been deprecated.\n${explanation}`; | ||
if (!warned[message]) { | ||
warning(false, message); | ||
warned[message] = true; | ||
} | ||
} | ||
|
||
return propType(props, propName, componentName, ...rest); // and here | ||
}; | ||
} | ||
``` | ||
|
||
This will silence the warning. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Do you think we'll ever ship our own standalone package? Should we mention that we might do that alongside the major version change?
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.
Do you mean we could delegate to that package normally but replace with throwing shims in prod?
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.
I thought we talked about potentially splitting the PropTypes out from the main package and shipping it as a standalone thing. But I guess that's tangential and not worth mentioning until/if we do it. Just ignore me.