facebook / prop-types

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

List transitive failures on failure of oneOfType #398

Open erikerikson opened 1 year ago

erikerikson commented 1 year ago

using "prop-types": "15.8.1"

Previously discussed at https://github.com/facebook/react/issues/8901, #226, and (sort of) #9. (there are probably some I've missed)

I am opening this to suggest that the message produced during a failure against PropTypes.oneOfType should list the transitive failures for each failing type. If it had been, it would have saved me time and fiddling. I would guess other users would also benefit.

I have a component with prop data that can be a string, an object (of type A), or an object with attributes including an array of objects [of type A]). This depends on the use case I am employing the component in, clearly. Error and code follows.

Error:

Warning: Failed prop type: Invalid prop `data` supplied to `<component>`, expected one of type [string, string].

(in fact, there were three types, the second and third were object types /shrug)

Code:

const A = PropTypes.objectOf(
  PropTypes.shape({
    attrA0: PropTypes.number.isRequired,
    attr...: PropTypes.number.isRequired,
    attrAN: PropTypes.number.isRequired,
  }),
)
const O = PropTypes.shape({
  attrO0: PropTypes.string.isRequired,
  attrO1: PropTypes.arrayOf(A).isRequired,
  attrO2: PropTypes.number.isRequired,
})
Report.propTypes = {
  data: PropTypes.oneOfType([
    PropTypes.string,
    A,
    O,
  ]).isRequired,

I was silly and made some mental mapping mistakes (I needed to add PropTypes.objectOf(...) to A) as well as forgetting an attribute's type s/string/number/ for attrO2. However, while it would have been trivial to figure out the problems given an array of failure messages, being new to React I was just confused for a while, poking around in the dark, until I isolated the cases where each type came in and changed things to debug those:

const A = PropTypes.objectOf(
  PropTypes.shape({
    attrA0: PropTypes.number.isRequired,
    attr...: PropTypes.number.isRequired,
    attrAN: PropTypes.number.isRequired,
  }),
)
const O = PropTypes.shape({
  attrO0: PropTypes.string.isRequired,
  attrO1: PropTypes.arrayOf(A).isRequired,
  attrO2: PropTypes.number.isRequired,
})
Report.propTypes = {
  data: PropTypes.string.isRequired,
  // data: PropTypes.oneOfType([
  //   PropTypes.string,
  //   A,
  //   O,
  // ]).isRequired,
}

then...

Report.propTypes = {
  data: A.isRequired,
  // data: PropTypes.oneOfType([
  //   PropTypes.string,
  //   A,
  //   O,
  // ]).isRequired,
}

then...

Report.propTypes = {
  data: O.isRequired,
  // data: PropTypes.oneOfType([
  //   PropTypes.string,
  //   A,
  //   O,
  // ]).isRequired,
}

Doing that made my errors obvious. Choosing to do that was not immediately obvious due to my inability to understand and reason back from the warning I was seeing. I believe this was exacerbated by the swallowing of the individual errors.

It would have taken a fraction of the time to fix my failures if, in the case a oneOfType fails, the failure for every evaluated type was listed. Even (or especially) if it were 100 types.