search

facebook / prop-types

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

mapOf and setOf validators #289

Open conartist6 opened 5 years ago

conartist6 commented 5 years ago

Fixes #190

I also intend to add iterableOf to this.

conartist6 commented 5 years ago

One of the things that I'm sure is worth discussing is the structure of the mapOf call, currently PropTypes.mapOf([keyValidator, valueValidator]). I thought it was appropriate to use an array, because this will be closely related to PropTypes.iterableOf. In fact quite likely I will delegate, making mapOf([k, v]) internally call iterableOf([k, v]). It feels internally consistent to me for these two validators to be related.

conartist6 commented 5 years ago

So in order to allow validators to delegate to each other, e.g. mapOf delegating to iterableOf I'm going to need to do some further work with secret. Currently the design is that the validators themselves don't have the secret, so they can't possibly delegate. I assume the reason they don't have it is because that call site also would feed props into a custom validator, and the intent is to avoid leaking the secret that way.

EDIT: I'm dumb, the secret lives in a lib module. I can just use it.

conartist6 commented 5 years ago

I'm going to add a tupleOf validator as well since I need it to implement mapOf (partly) as iterableOf(tupleOf([keyType, valueType])).

conartist6 commented 5 years ago

@ljharb I guess I can't dismiss your review as stale, but I believe I have made the changes that you requested, including your improved method for detecting Map and Set instances.

Please don't ship this yet though. I have to finish writing the tests. I'm just waiting to do it until I have more of an idea whether you would accept these changes.

conartist6 commented 5 years ago

OK I take back what I said. All the tests are here. CHANGELOG and README are updated. When you get to looking at this, it is ready to merge from my perspective.