ljharb / prop-types-tools

Custom React PropType validators
MIT License
671 stars 50 forks source link

`or` with classic `oneOf` losing prettiness of the warning message #32

Closed qfox closed 5 years ago

qfox commented 6 years ago

When I call this

PropTypes.checkPropTypes({
  theme: AirbnbPropTypes.or([
    PropTypes.oneOf(['normal', 'pseudo']), // basic set of values, comes from library
    PropTypes.oneOf(['my-custom']) // local addition to base component
  ])
}, {theme: 'abnormal'}, 'theme', 'Button');

... and I see this: (actual):

Warning: Failed theme type: Button: invalid value supplied to theme.

But want to see this (expected):

Warning: Failed theme type: Invalid theme `theme` of value `abnormal` supplied to `Button`,
expected one of ["normal","pseudo"] or one of ["my-custom"].

Or even (ideal):

> PropTypes.checkPropTypes({theme: PropTypes.oneOf(['normal', 'pseudo', 'my-custom'])}, {theme: 'abnormal'}, 'theme', 'Button')
Warning: Failed theme type: Invalid theme `theme` of value `abnormal` supplied to `Button`,
expected one of ["normal","pseudo","my-custom"].

Versions:

ljharb commented 6 years ago

What error do you get with PropTypes.oneOfType([PropTypes.oneOf(['normal', 'pseudo']), PropTypes.oneOf(['my-custom'])])?

ljharb commented 6 years ago

In general, the oneOf validator function is opaque; i can't reliably know what values it had (altho it might set .typeChecker)

qfox commented 6 years ago

What error do you get with

This one:

Warning: Failed theme type: Invalid theme `theme` supplied to `Button`.

Feels like this problem on the other side?

qfox commented 6 years ago

In general, the oneOf validator function is opaque; i can't reliably know what values it had (altho it might set .typeChecker)

Well, yes, but I though if we run each validator and catch every message anyway we should have a way to combine something from the result. Much harder to do this with and operator, but guess it's fine for and to have just one (the first) fail message from it.

ljharb commented 6 years ago

In other words, it seems like PropTypes.oneOfType lacks this feature - I'm not sure the complexity is worth it, there or in or.