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

Warning for: PropTypes.object.isRequired when prop is null #3163

Closed
TrySpace opened this issue Feb 16, 2015 · 43 comments
Closed

Warning for: PropTypes.object.isRequired when prop is null #3163

TrySpace opened this issue Feb 16, 2015 · 43 comments

Comments

@TrySpace
Copy link

As PropTypes is about the 'type' of prop, null is an empty object, but still a type of object, practically.

But still it warns:

 Warning: Required prop `profile` was not specified in `Element`. Check the render method of `OtherElement`.

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 is undefined ?

@syranide
Copy link
Contributor

null is the equivalent of no value and is not an object (empty or otherwise). If you don't want it to warn for null then don't make it required, it has the same effect. Testing for the presence of a key is not something I would recommend.

@jimfb
Copy link
Contributor

jimfb commented Feb 18, 2015

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.

@jimfb jimfb closed this as completed Feb 18, 2015
@ptim
Copy link

ptim commented Mar 3, 2016

related: #2166
(this issue still shows up prominently in search results)

@wardpenney
Copy link

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 (items: null). Then the API call finishes, and now items is populated with an array.

@Noitidart
Copy link

I am trying to do PropTypes.oneOfType([null, PropTypes.object]).isRequired, so either null or an object, is this not possible right now?

@GabrielDelepine
Copy link

According to the CHANGELOG, since 15.4.0 it's suppose to be possible

@misantronic
Copy link

Actually, it's the opposite in 15.4.0: Required PropTypes now fail with specific messages for null and undefined.

I am also facing this issue.
@Noitidart 's workaround is not working for me. It throws an error saying:
Failed prop type: The prop valueis marked as required inSelect, but its value is null.

I find it really useful to require a property but also allow null values.

@jharris4
Copy link

+1 for allowing null somehow.

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:

{
  "title": "my title"
}

So when no title should be displayed, we use null to denote that case:

{
  "title": null
}

(Adding parallel hasTitle: false would be prohibitive as we have dozens of these settings.)

For JSON content, using null is a very useful way to distinguish between not defined (undefined) versus deliberately omitted (null).

@jquense
Copy link
Contributor

jquense commented Jul 17, 2017

you can allow null, set the propType to not be required, since it's not required :P

@hzhu
Copy link

hzhu commented Jul 17, 2017

@jquense thanks that's super helpful! I had deleted my previous comment because this SO answer said the same thing.

@jharris4
Copy link

@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 null === undefined for PropTypes is an artificial limitation.

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

@binki
Copy link

binki commented Jul 19, 2017

I want to disallow undefined because that means there is a typo and allow null because that means that the caller explicitly passed in null. That’s the point of this issue. I understand that this issue is closed because it is recommended to just switch to flow, which I will look into.

@puiutucutu
Copy link

puiutucutu commented Jul 20, 2017

@binki One way to allow null but not undefined is to use your own PropType validator function.

In the example below, only null or string is permitted. The PropTypes library uses typeof internally to check for strings, so I did the same. One benefit is that you can move this function outside your component and call it as needed.

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 null. In other words they believe that a required property being null is erroneous whereas I consider it a valid use case to have have required nulls.

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);
}

@Findiglay
Copy link

Findiglay commented Jul 21, 2017

I agree with @jharris4 for the reasons stated. null is not the same as undefined. It is standard to use it as a placeholder.

From Mozilla Developer Network:

The value null represents the intentional absence of any object value.

null is not an identifier for a property of the global object, like undefined can be. Instead, null expresses a lack of identification, indicating that a variable points to no object. In APIs, null is often retrieved in a place where an object can be expected but no object is relevant.

null should be allowed, at least through PropTypes.oneOfType([null, PropTypes.string]).isRequired.

@brandondurham
Copy link

@jquense — Removing isRequired means you should probably set a default. Does this mean you’d now be setting a default for the initial value in the component and in the reducer, all to avoid allowing null as a value for a prop?

@erfanio
Copy link

erfanio commented Aug 2, 2017

I have a bunch of data || '' (isRequired accepts empty string '', empty object {}, etc.) in my selectors cause while waiting for API calls to finish null is the perfect thing to tell the component the data is coming just wait a bit! (but I can't do that...)
@puiu91 has a good work around for now... I'll have make a utility function to use across our codebase... since it looks like this issue isn't getting reopened any time soon sigh

@ianmartorell
Copy link

I also agree there should be a standard way to accept null as a value, for the same reasons @jharris4 and @Findiglay stated, but this is not the place to continue the discussion anymore. Not only is this issue closed, but it belongs to facebook/prop-types. I'm following the pull request here facebook/prop-types#90.

@Keltin42
Copy link

Keltin42 commented Mar 5, 2018

Bump. Still running into this today. Would be great to have builtin support for something like:

myObj: PropType.object.isRequiredOrNull :)

@Noitidart
Copy link

I think the priority on this is super low. Flow is the recommended way tog go. http://flow.org/

@jharris4
Copy link

@Marujah your snippet is not correct.

Try passing null to the component, and you'll see you'll get the warning:

Warning: Failed prop type: The prop 'theProp' is marked as required in 'TheComponent', but its value is null.

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.

@Marujah
Copy link

Marujah commented Mar 15, 2018

Oh indeed! retested again you'r right

@pushitreal
Copy link

pushitreal commented Apr 27, 2018

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.

@shepherdjerred
Copy link

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.

@MrToph
Copy link

MrToph commented Aug 14, 2018

I'm passing a loader component (PropTypes.node) as props, and when I don't want to render a loader I pass null.
Afaik, a render function should return null instead of undefined when not rendering anything. So passing null as the value looks like the correct way to do it to me.

@Aliance
Copy link

Aliance commented Sep 13, 2018

I was implementing InputNumber component (wrapper for <input type="number">), so I had propTypes for my prop valuePropTypes.number.isRequired, and when I used my component, I always passed it property. But today I need to pass there a nullable by default link to value, and my component adds a warning. The only decision I could imagine is to change propTypes for my prop value to PropTypes.oneOfType([PropTypes.number, PropTypes.string]) and set the defaultProps as null. But I feel it's not the correct one because input type=number should work with numbers only.

@wbednarski
Copy link

I am trying to do PropTypes.oneOfType([null, PropTypes.object]).isRequired, so either null or an object, is this not possible right now?

It expects a function:Warning: Invalid argument supplied to oneOfType. Expected an array of check functions, but received null at index 1.

So, you should supply a function, like:

PropTypes.oneOfType([
  () => null,
  PropTypes.object
]).isRequired

@davidje13
Copy link

davidje13 commented Mar 6, 2019

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 nullable without isRequired. The reason I make it compatible with isRequired is so that it will work with the react/require-default-props eslint rule.

My use-case is a series of components conforming to a common API, wrapped by a single component which handles callbacks. null callbacks mean 'read only', so the wrapper component sometimes intentionally passes nulls. At the same time, it's important that every property is passed to the subcomponents to ensure that if a new property is added, it doesn't get missed. I also don't want to provide defaultProps of null for every component which conforms to this API; as far as they are aware, the caller must have specified a value.

@davidje13
Copy link

I took the helper I wrote before and put it in a package along with a bunch of tests to prove correctness:

npm install --save git+https://github.com/davidje13/prop-types-nullable.git#semver:^1.0.0

Usage:

import PropTypes from 'prop-types';
import nullable from 'prop-types-nullable';

[...]

static propTypes = {
  thing: nullable(PropTypes.string).isRequired,
};

@LukaGiorgadze
Copy link

New solution: PropTypes.oneOfType([PropTypes.object]).isRequired

@gangeshRoy
Copy link

I am getting error.
how to fix.

WARNING in .//prop-types/prop-types.js Critical dependencies: 1:482-489 This seems to be a pre-built javascript file. Though this is possible, it's not recommended. Try to require the original source to get better results. @ .//prop-types/prop-types.js 1:482-489

@gugol2
Copy link

gugol2 commented Apr 19, 2020

I am trying to do PropTypes.oneOfType([null, PropTypes.object]).isRequired, so either null or an object, is this not possible right now?

It expects a function:Warning: Invalid argument supplied to oneOfType. Expected an array of check functions, but received null at index 1.

So, you should supply a function, like:

PropTypes.oneOfType([
  () => null,
  PropTypes.object
]).isRequired

Actually I had a error because of that 'isRequired' at the end, being null and required at the same time is not compatible...
This is what worked for me:

PropTypes.oneOfType([ 
	PropTypes.string.isRequired, 
	() => null 
])

@davidje13
Copy link

davidje13 commented Apr 19, 2020

@gugol2 please note that what you have written will just disable type checking entirely (you can now pass a number to that prop, or undefined, or anything); the function you provide should return null if validation succeeds and non-null if it fails.

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 PropTypes.oneOfType([PropTypes.string.isRequired]). This feels like a bug though and I wouldn't expect code like that to survive later versions. Also note that it is similar to, but not the same as, a previously suggested method which does not work.


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 .isRequired at the end of the line, which makes it compatible with linting rules.

@benatkin
Copy link

IMHO this is correct behavior but it ought to be documented.

@adil-waqar
Copy link

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 (items: null). Then the API call finishes, and now items is populated with an array.

Any best way to handle this? I want this prop required BUT it's null before I get it back from the API.

@gugol2
Copy link

gugol2 commented May 2, 2020

PropTypes.oneOfType([
  PropTypes.string.isRequired,
  (props, key) => props[key] === null ? null : 'Not null'
])

@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:

const Login = ({name}) => {
  return <div>{name}</div>
} 

So its proptypes are:

Login.propTypes = {
  name: PropTypes.oneOfType([
    PropTypes.string.isRequired,
    (props, key) => (props[key] === null ? null : 'Not null'),
  ]),
};

When I test this component I really only have 2 scenarios, null OR a required string.

render(<Login name={null} />
render(<Login name={'anyName'} />

But coverage tells me my test has only 75% coverage.
I wonder what will be the official approach for this.

@davidje13
Copy link

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.

@gugol2
Copy link

gugol2 commented May 4, 2020

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.
And I can't pass a number anyway, it is not an allowed value.

I am not able to cover that case.

@LaxmanPache123
Copy link

LaxmanPache123 commented Nov 10, 2022

image
i validated this proptype but still getting warning
image

@mahawar0709
Copy link

Do we have any way for adding null and isRequired to propTypes?
activeProject: PropTypes.oneOfType([PropTypes.object]).isRequired <-----I want to add null too as one of the type

@LaxmanPache123
Copy link

Do we have any way for adding null and isRequired to propTypes? activeProject: PropTypes.oneOfType([PropTypes.object]).isRequired <-----I want to add null too as one of the type
you can try using -

activeProject: PropTypes.oneOfType([
PropTypes.object,
PropTypes.oneOf([null]) // Allow null
]).isRequired,

@LaxmanPache123
Copy link

LaxmanPache123 commented Jan 23, 2024

image i validated this proptype but still getting warning image

This issue is resolved. in data i am getting Array of objects so correct way for validate proptype is
allOverview: PropTypes.arrayOf(PropTypes.oneOfType([PropTypes.object])) .isRequired

@benatkin
Copy link

I think the problem is nicely defined here: facebook/prop-types#57 (comment)

@benatkin
Copy link

Looks like it was close to a solution but it didn't quite make it: facebook/prop-types#90 (review)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests