-
Notifications
You must be signed in to change notification settings - Fork 358
.set and .setOf (and .map and .mapOf) #190
Comments
You can already do what you want by composing (Also, if Set was added, I'd expect Map to be added as well) |
Thanks for the feedback @ljharb.
Any thoughts on the ugly errors? |
I don’t see why it must; objectOf doesn’t take a checker for the keys, only the values. |
@ljharb That is true. In which realm does |
@Pimm it’s not that; it’s that using a Set from an iframe will return false with instanceof. It’s the reason Array.isArray exists. |
I'm interested in this. I've written a PR, though I obviously encountered the issue with the array access operator. I was able to work around it. I'm also quite sure that maps should take two validators, one for keys and one for values. It doesn't make sense to have validators for a dictionary-style object as validated by I hadn't considered the problems with
|
As a note, I have a temporarily shelved project which would use symbols to allow custom structures like |
I think the check does have to be perfect. |
also note that arrays, Maps, and Sets all have Symbol.iterator, values, and entries. |
(Presumably For me, I see there being two separate use cases at play:
In both cases, I always want to maintain the ease of passing in a The caveat to I see three options:
(Not a big deal IMO but it's worth noting that the nice array/set symmetry you get from treating them as iterables doesn't apply to object/map, since POJOs aren't, strictly speaking, iterables. But that's fine, we already have a type for POJOs.) |
Oh yeah you need a top level |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I don't think it's worth it to add these validators to prop-types, especially given react's general position on PropTypes as a concept. Here's what I'd suggest in the meantime: import isSet from 'is-set';
import isMap from 'is-map';
import { predicate, and } from 'airbnb-prop-types';
const set = predicate(isSet, 'must be a Set');
const map = predicate(isMap, 'must be a Set');
cont setOf = (validator) => and([set, validator]);
const mapOf = (validator) => and([map, validator]); You'll find General discussion about the value of PropTypes as a concept don't belong on this issue, or really on this repo at all. |
In a project I'm currently working on, I pass Sets to components.
As a fanatic user of
prop-types
to make components easier to reuse/rearrange, I writePropTypes.arrayOf(PopTypes.string.isRequired)
quite often. However, there is currently no counterpart for sets. One resorts to defining a propPropTypes.object
. This alternative doesn't have quite the positive effect on readability as.arrayOf
.I would like to discuss adding
.set
and especially.setOf
.I wrote a proof-of-concept. I am also more than happy to provide a pull request including everything from implementation to tests, but figured the wise thing to do is ask for some early feedback.
Errors in this implementation, unfortunately, look like this:
Invalid value `inside highlightedIds` of type `string` supplied to `NameList`, expected `number`.
As values inside a set are not a direct property of the set, it is more difficult to name it in the error message. If the fifth value in an array has the wrong type, the error will blab out that it's
array[4]
. In a set, I guess the best thing we could do is say that it'sinside set
.Has this been discussed before?
The text was updated successfully, but these errors were encountered: