Required Props and existence checks

Viewed 467

I see that my code breaks even though prop list is required.
So, should I check for the existence of list before mapping it as I'm doing below?

class Cart extends React.Component {
  render() {
    const { list } = this.props
    return { list && list.map(e => <div> {e} </div>) }
  }
}

Cart.propTypes = {
  list: PropTypes.array.isRequired
}

UPDATE:

I see suggestions advising to add a default value.

Does it make sense though to both have isRequired and default value set?
Isn't it implied that if a value is required then it should always exists?

But the component seems to mount even though some required props are not satisfied.

So I guess setting default value makes sense, but so isRequire is only a flag for the developer, nothing more, Correct?

5 Answers

Yes, I think you should. Other developers can still explicitly pass null to the list prop.

<Cart list={null}/>

Or ... a more real-life example:

// getListFromServer() <-- can return null
<Cart list={getListFromServer()}/>

You should use PropTypes which is imported from prop-types:

import PropTypes from 'prop-types'

Cart.propTypes = {
  list: PropTypes.array.isRequired
}

So, should I check for the existence of list before mapping it as I'm doing below?

return { list && list.map(e => {e} ) }

Yes, you should check it. Because until your component is being rendered, the list may be undefined or null. And using map on undefined or null will throw you an error. When your component gets list data then your use of map will be correct usage.

It would even be better to check its length:

return { list && list.length && list.map(e => <div> {e} </div>) }

I would also suggest you to use defaultProps:

Cart.defaultProps = {
  list: [] // or list: ['my','default','props']
}

If you use the default props, then you don't need to worry about checking it before using map. This will ensure you to map on array.

But what if user pass the props other than array?

Even in this case, user is notified through PropTypes.array.isRequired. So checking list before using map in this case is not necessary.

I think that depends on your way of programming, It's very subjective.

Some people prefer to have the responsibility on the caller to provide the right value, some prefer to be more defensive and check for all the possible values.

I prefer to have the caller provide the right value, otherwise, why have propTypes in the first place, it almost becomes useless.

Now if you can't control how your component will be called, then Yes check that the right value is passed.

I would do null checks when doing some side effects, like doing an Ajax call where I can't really control the result.

At the end, you need to do types/value checks in your program, the question is where do you do it, everywhere or where it matters.

Yes you should check whether it has an array or not because your Cart component wants list as an array always and list should not be empty array and only then do map or do that check in parent component itself before passing List props to Cart so that you no need to check again in Cart component you can directly do map

    class Cart extends React.Component {

         render() {
            const { list } = this.props; //should be inside render
             return (list && list.length>0 && list.map(e => <div> {e} </div>) 
          }
      }

Better keep your list as empty array in your parent component like for eg: this.state={list:[]} so that you no need to check whether it is undefined or null. You can just check the length of the array and do map

Could you post the code where you are passing the list to the cart component.

If nothing works you can always try this

Cart.defaultProps = {
     list: []
}

Although I would suggest to fix the underlying problem of why the code is crashing, could you provide an error log as well.

Related