-
Notifications
You must be signed in to change notification settings - Fork 47k
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
Warning for: PropTypes.object.isRequired when prop is null
#3163
Comments
|
I agree. I can't think of very many use cases where you would want to force a user to specify a value, but would be willing to accept null as a valid value. For practical purposes, the isRequired warning for null is sensible and expected behavior. |
related: #2166 |
A common use case I see is React rendering a component before an API call for data has finished. The first render would render, for example, a list of items ( |
I am trying to do |
According to the CHANGELOG, since |
Actually, it's the opposite in 15.4.0: I am also facing this issue. I find it really useful to require a property but also allow null values. |
+1 for allowing We have a use case where our configuration is loaded via JSON, and there are several label configuration options that specify a string that should be displayed, something like this:
So when no title should be displayed, we use
(Adding parallel For JSON content, using null is a very useful way to distinguish between not defined ( |
you can allow null, set the propType to not be required, since it's not required :P |
@jquense you can allow null AND undefined, but not one or the other. That's the whole problem! Javascript provided these 2 different constructs for a reason, so forcing everyone to treat Just because I want a PropType to explicitly allow null does not mean that I should have to allow undefined as well. They are two distinct cases, and the language designed them that way on purpose. I have a PR to work around this oversight here: facebook/prop-types#90 |
I want to disallow |
@binki One way to allow In the example below, only static propTypes = {
id: PropTypes.number.isRequired,
email: function(props, propName, componentName) {
const propValue = props[propName] // the actual value of `email` prop
if (propValue === null) return
if (typeof propValue === 'string') return
return new Error(`${componentName} only accepts null or string`)
}
} I guess this solution deviates from the intent of the PropTypes library - the reason I say this is due to the code below from the PropTypes library at https://github.com/facebook/prop-types/blob/master/factoryWithTypeCheckers.js#L189. Before actually validating, a quick check is performed wherein properties set with isRequired automatically throw an error if the property value is if (props[propName] == null) {
if (isRequired) {
if (props[propName] === null) {
return new PropTypeError('The ' + location + ' `' + propFullName + '` is marked as required ' + ('in `' + componentName + '`, but its value is `null`.'));
}
return new PropTypeError('The ' + location + ' `' + propFullName + '` is marked as required in ' + ('`' + componentName + '`, but its value is `undefined`.'));
}
return null;
} else {
return validate(props, propName, componentName, location, propFullName);
} |
I agree with @jharris4 for the reasons stated. From Mozilla Developer Network:
null should be allowed, at least through |
@jquense — Removing |
I have a bunch of |
I also agree there should be a standard way to accept |
Bump. Still running into this today. Would be great to have builtin support for something like: myObj: PropType.object.isRequiredOrNull :) |
I think the priority on this is super low. Flow is the recommended way tog go. http://flow.org/ |
@Marujah your snippet is not correct. Try passing null to the component, and you'll see you'll get the warning:
The touble is that the isRequired gets evaluated FIRST and it will never let null or undefined values through. The PR to prop-types to fix the issue is linked above if you're interested. |
Oh indeed! retested again you'r right |
Requiring that a value or null must be specifically provided should absolutely be allowed via PropTypes. Here is the use case I'm running into. We have some callbacks that are required by 90% of our selectors. The few that don't need them are very specific use cases. We have some new developers that are constantly forgetting to provide all of the usual callbacks. I want to force all uses of these Components to make the conscious decision to not include specific callbacks, rather than someone just forgetting a few props. Yes, we can hack in our own more specific checks via flow, but that splits our props validation into two places, and is rather unintuitive for someone glancing at the propTypes definitions. |
Just wanted to add my use case here. I have a redux store with data along with when that data was fetched, if there was an error, etc. My component requires an 'error' prop (so it can display the error to the user if data couldn't be fetched), which is null when the data loaded successfully, but populated when there is an error. |
I'm passing a loader component ( |
I was implementing InputNumber component (wrapper for |
It expects a function: So, you should supply a function, like:
|
Having just encountered this bug, I've written a convenience function to work around it: function nullable(subRequirement) {
const check = (required, props, key, ...rest) => {
if (props[key] === null) {
return null;
}
const sub = required ? subRequirement.isRequired : subRequirement;
return sub(props, key, ...rest);
};
const fn = check.bind(null, false);
fn.isRequired = check.bind(null, true);
return fn;
} Usage: static propTypes = {
someCallbackFunction: nullable(PropTypes.func).isRequired,
}; It's possible (but rather pointless) to use My use-case is a series of components conforming to a common API, wrapped by a single component which handles callbacks. |
I took the helper I wrote before and put it in a package along with a bunch of tests to prove correctness:
Usage: import PropTypes from 'prop-types';
import nullable from 'prop-types-nullable';
[...]
static propTypes = {
thing: nullable(PropTypes.string).isRequired,
}; |
New solution: |
I am getting error. WARNING in ./ |
Actually I had a error because of that 'isRequired' at the end, being null and required at the same time is not compatible...
|
@gugol2 please note that what you have written will just disable type checking entirely (you can now pass a number to that prop, or If you want to go that route you would need something more like: PropTypes.oneOfType([
PropTypes.string.isRequired,
(props, key) => props[key] === null ? null : 'Not null'
]) Of course you could pre-define the nasty-looking helper function: const nullable = (props, key) => props[key] === null ? null : 'Not null'
// ...
PropTypes.oneOfType([PropTypes.string.isRequired, nullable]) It is also bizarrely possible (but I really wouldn't recommend it!) to just use If you can wait, there is an open PR which is moving incredibly slowly but apparently still in consideration. And until the PR is merged I'd recommend using the package I created (or the code behind it) because that way you get to put |
IMHO this is correct behavior but it ought to be documented. |
Any best way to handle this? I want this prop required BUT it's null before I get it back from the API. |
@davidje13 I encounter a little problem with this approach. Coverage testing has 1/4 case that is never covered. Let's say I have a component Login that only has one prop 'name' that can be null or a string required:
So its proptypes are:
When I test this component I really only have 2 scenarios, null OR a required string.
But coverage tells me my test has only 75% coverage. |
It sounds like your missing test case is the one where it fails the prop check? i.e. if you pass a number or undefined or something else it shouldn't have. |
No, it is not that. Passing undefined does not expand the coverage. I am not able to cover that case. |
Do we have any way for adding null and isRequired to propTypes? |
activeProject: PropTypes.oneOfType([ |
I think the problem is nicely defined here: facebook/prop-types#57 (comment) |
Looks like it was close to a solution but it didn't quite make it: facebook/prop-types#90 (review) |
As
PropTypes
is about the 'type' of prop,null
is an empty object, but still a type of object, practically.But still it warns:
I don't think this is supposed to happen.
It stops warning after it is not
null
anymore. I'm pretty sure it should only warn when it isundefined
?The text was updated successfully, but these errors were encountered: