facebook / prop-types

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

`objectOf` should not require `hasOwnProperty` to be a function #183

Closed tor-ocean closed 5 years ago

tor-ocean commented 6 years ago

When using objects as dictionaries, and to avoid littering code with hasOwnProperty checks, it can be useful to use object = Object.create(null) instead of object = {} when creating the object.

Unfortunately, this causes the PropTypes.objectOf checker to throw:

Warning: Failed prop type: propValue.hasOwnProperty is not a function

as the code assumes that every value will have an hasOwnProperty method in its prototype chain:

https://github.com/facebook/prop-types/blob/01634d92e4e4cb5ffaf7de96ea8cb5da9b65e4f0/factoryWithTypeCheckers.js#L305

In my case, I will have to work around it by using a regular object and refactor the rest of my own code to do the same hasOwnProperty check.

I would still argue that this is a bug and needs to be fixed. When checking that a value is an object where every own key maps to a value of a certain type, it is self-contradictory that the code simultaneously requires that one specific key is going to map to a function. What was supposed to check that the value is a dictionary of a specific type now risks crashing the app when that is the case.

Imagine the case where the object is truly being used as a dictionary, even with a regular {} object, what's to stop the key "hasOwnProperty" to map to some other value? Malicious users might even cause a component to crash for other users by exploiting this.

ljharb commented 6 years ago

This is a good fix; not because null objects are reasonable or because almost anyone would use them, but because delete Object.prototype.hasOwnProperty is a thing.

dferber90 commented 6 years ago

I am getting the same warning when defining this prop-type validation:

PropTypes.shape({
  pathname: PropTypes.string.isRequired,
  query: PropTypes.objectOf(PropTypes.string),
}),

And when the passed value is:

{
  "pathname": "/foo",
  "query": {
    "perPage": "6",
    "sortBy": "createdAt",
    "sortDirection": "desc"
  }
}

It results in this error:

Warning: Failed prop type: propValue.hasOwnProperty is not a function

However, if I only use

PropTypes.shape({
  pathname: PropTypes.string.isRequired,
  query: PropTypes.object,
})

it works fine as pointed out in the issue description.

Observed with prop-types@15.6.1.

likerRr commented 6 years ago

Any updates on it? I also faced the same issue, when validating object, created with Object.create

dferber90 commented 6 years ago

187 is approved and solves this. It's waiting for a merge still.