facebook / prop-types

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

Please expose PropTypeError #269

Closed KevinGhadyani-minted closed 2 years ago

KevinGhadyani-minted commented 5 years ago

I was writing a few custom prop type checkers and noticed in the source, PropTypeError was used instead of Error, but this isn't exposed in the library.

PropTypes.oneOfType looks for this kind of error when it does:

{
  var checkerResult = checker(props, propName, componentName, location, propFullName, ReactPropTypesSecret);
  if (checkerResult == null) {
    return null;
  }
  if (checkerResult.data.hasOwnProperty('expectedType')) {
    expectedTypes.push(checkerResult.data.expectedType);
  }
}
var expectedTypesMessage = (expectedTypes.length > 0) ? ', expected one of type [' + expectedTypes.join(', ') + ']': '';
return new PropTypeError('Invalid ' + location + ' `' + propFullName + '` supplied to ' + ('`' + componentName + '`' + expectedTypesMessage + '.'));

The checkerResult could just throw new Error, but it could also throw a special PropTypeError that contains a data prop with an expectedType prop. I'd like to be able to utilize this when wrapping my custom prop type in a PropTypes.oneOfType.

In this particular case, I have multiple custom prop types I'm checking; for instance, if they throw in an array or number or object, I have to process them all differently:

PropTypes.oneOfType([
    gridColumnCount,
    gridColumnSizes,
    PropTypes.shape(breakpointValues),
])

While complex, it fulfills the need that this component needs for this group of developers.

ljharb commented 5 years ago

It looks like it looks for an expectedType property on a data property, which doesn’t require it be a PropTypeError - can you explain a bit more about why having this constructor available would help?

KevinGhadyani-minted commented 5 years ago

If you guys change something in the future, now I have to make that change in my code as well.

I'd create my own error object that I maintain separately from your code which has to mimic functionality already available in your code. That's why I'd rather just use your object instead of creating one myself.

ljharb commented 5 years ago

That constructor would still need to take an argument for the expected types, to which your code would have to couple just the same.

KevinGhadyani-minted commented 5 years ago

Coupling to the function args of PropTypeError is different from coupling to the output of PropTypeError.

In one case, you can change the innards without me having to change my code. In the latter, I need to modify my code anytime the innards of PropTypeError change.

I would have no way of knowing. But if the args change, there's a chance you guys will handle the compatibility or at least note it in a changelog. If the innards of PropTypeError change, then I will have to be in-sync with your source code to understand what is and isn't working.

Since you're allowing the creation of custom prop types, then you should also allow the ability to hook into your existing error logging logic as well. Intuitively, I would believe I could use PropTypes.oneOfType with my custom prop type.

If I have to do something special, I'd rather that special thing come from the library. If it's from my own implementation, creating custom prop types becomes a hack as it wouldn't be resilient to any library changes. Since I had to read the source code to figure out why my custom prop type errors weren't working like the official ones, I'm already under the impression custom prop types are a hack.

ljharb commented 5 years ago

No longer reading expectedTypes would be a breaking change, which you'd be able to see in a semver-major changelog entry.

ljharb commented 2 years ago

Closing; there doesn't seem to be any value in exposing PropTypeError unless it constituted an unbreakable contract where the current one was breakable, and the likelihood of breakage for either is equally remote.