facebook / prop-types

Runtime type checking for React props and similar objects
MIT License
4.48k stars 356 forks source link

objectOf dies when given complex types #308

Closed p closed 4 years ago

p commented 4 years ago

The readme defines objectOf validator thusly:


  // An object with property values of a certain type
  optionalObjectOf: PropTypes.objectOf(PropTypes.number),

I cannot find the definition of type in the readme, but it seems reasonable to assume that the unnamed hashes assigned to propTypes properties are types:

MyComponent.propTypes = { ... }

With this in mind, I tried to define an objectOf taking another type as the argument:


import p from 'prop-types'

let t = {
    foo: p.PropTypes.number
}

let o = {
    bar: p.PropTypes.objectOf(t).isRequired
}

var d = {bar: {foo: 1}}

p.checkPropTypes(o, d)

This failed in a way I did not expect:

Warning: Failed undefined type: Property `bar` of component `<>` has invalid PropType notation inside objectOf.

The code producing this message looks as follows:

  function createObjectOfTypeChecker(typeChecker) {
    function validate(props, propName, componentName, location, propFullName) {
      if (typeof typeChecker !== 'function') {
        return new PropTypeError('Property `' + propFullName + '` of component `' + componentName + '` has invalid PropType notation inside objectOf.');
      }

If objectOf is in fact intended to only accept JS primitive types, as enumerated in the readme on top of the usage section, it should be explicitly documented as such. But, this seems to limit usefulness of objectOf substantially.

ljharb commented 4 years ago

import p from 'prop-types' should mean that p is the PropTypes object - iow, p.objectOf, not p.PropTypes dot anything.

ljharb commented 4 years ago

Also, specifically, objectOf takes a single propType validator function - not an object of those things (you'd use PropTypes.shape for that).

p commented 4 years ago

objectOf takes a single propType validator function - not an object of those things

This is the crux of the issue I think. objectOf's argument is advertised as "a certain type", not "a single propType validator function". The "type" to me includes complex objects as well.

you'd use PropTypes.shape for that

It is my understanding that shape is different. For example how would you express the following as a shape?

{"1": {foo: 1, bar: 2}, "2": {foo: 3, bar: 4}}
ljharb commented 4 years ago

A "type" here is only a single validator function, which may validate complex objects or not.

In that case, it would be:

const fooBar = PropTypes.shape({ foo: PropTypes.number, bar: PropTypes.number });
// or PropTypes.objectOf(PropTypes.number);

PropTypes.shape({
  1: fooBar,
  2: fooBar
})
// or
PropTypes.objectOf(fooBar);

If you want to have a propType that requires all keys be numbers and all values be fooBar, say, then you'd need a custom validator (but you could also use and to compose keysOf and valuesOf from https://www.npmjs.com/package/airbnb-prop-types)

p commented 4 years ago

I got it working eventually as follows:

import p from 'prop-types'

let t = {
    foo: p.PropTypes.number
}

let o = {
    bar: p.PropTypes.objectOf(p.PropTypes.shape(t)).isRequired
}

var d = {bar: {"1": {foo: 1, bar: 2}, "2": {foo: 3, bar: 4}}}

p.checkPropTypes(o, d)

Thank you for the hint that objectOf should have its argument wrapped in a shape.

I think expanding the readme to provide some prose around what a "type" is would help users in cases like this, potentially adding this particular example where a hash is wrapped in shape to make it usable as an argument to other type functions. Neither the readme nor error messages provided guidance that helped to figure this out.

ljharb commented 4 years ago

A PR to improve the readme would be appreciated! In the meantime, this seems like it can be closed.

p commented 4 years ago

I switched to using tcomb (https://github.com/gcanti/tcomb) which I was able to get working right away.