Add PropTypes for map, set, weakMap, weakSet, setOf

[chicoxyzzy]

I think that's not fair that only Symbol was added as PropType from ES2015 spec.

So here are

• PropTypes.set
• PropTypes.weakSet
• PropTypes.map
• PropTypes.weakMap
• PropTypes.setOf

[chicoxyzzy]

Hmm all tests pass locally on node 6. I'll check this on node 4

[satya164]

Why not just use,

PropTypes.instanceOf(Set)
PropTypes.instanceOf(WeakSet)
PropTypes.instanceOf(Map)
PropTypes.instanceOf(WeakMap)

[chicoxyzzy]

@satya164 why don't you ask why not just also use

PropTypes.instanceOf(Array);

?

I think this should be a separate question

[satya164]

@chicoxyzzy

why don't you ask 'Why not just use'

Because this PR doesn't implement array.

That aside, I think the general preference is to switch to Flow for type-checking and not add more stuff to PropTypes.

[chicoxyzzy]

It looks like Map, Set, WeakMap and WeakSet doesn't use @@toStringTag in core-js @zloirock

[chicoxyzzy]

@satya164 sure using Flow is a much better approach (personally I prefer TypeScript tho). Stuff added to PropTypes works only for dev mode so it doesn't add any overhead in production mode.

Because this PR doesn't implement array.

It is already implemented in React but looks like you are ok with it. Do you think that arrays are in some way "better" than other objects?

[satya164]

Stuff added to PropTypes works only for dev mode so it doesn't add any overhead in production mode.

Same can be said for Flow too.

It is already implemented in React but looks like you are ok with it.

It's used much more widely than Set and Map. But my point is not if React should have these in the default PropTypes. PropTypes are mostly legacy at this point, so I think we shouldn't be adding more features to it.

[chicoxyzzy]

For sake of consistency all types should be added to PropTypes until and if PropTypes will be deprecated

That's my point

[chicoxyzzy]

Also I don't agree with

It's used much more widely than Set and Map

It depends on particular project

[chicoxyzzy]

anyway IMO you should create another issue if you think that some JS types should or shouldn't be in PropTypes and their presence should depend on some use frequency (and there definitely should be some statistics)

[gaearon]

Hmm. I agree with #6780 (comment) that

PropTypes.instanceOf(Set)
PropTypes.instanceOf(WeakSet)
PropTypes.instanceOf(Map)
PropTypes.instanceOf(WeakMap)

should work fine for this scenario.

For arrays, I think the main reason we have arrayOf() is to type-check their values. Compared to instanceOf(Array), this is a convenience feature, as it adds a better type check rather than enables the type check at all.

This convenience feature already exists. However at some point we stopped adding convenience features to PropTypes to stop expanding their API surface, as we want to slowly transition to using Flow instead.

The reason we added symbol prop type is because it enables type checking symbols. It is not just a convenience feature. You can’t use instanceOf(Symbol) because it’s a new primitive type.

Unlike Symbols, Maps and Sets are not primitive types. For Sets and Mapss to have feature parity with arrays, we would need to add something like mapOf() and setOf(). However that would also be a convenience feature, like arrayOf(). Since we don’t want to expand the API surface of PropTypes, we’d rather avoid adding more convenience features. This is why I’d rather see it in a third-party package, similar in spirit to react-immutable-proptypes.

[chicoxyzzy]

we want to slowly transition to using Flow instead

I hope TS will be an option because a lot of people prefer TS over Flow

we would need to add something like mapOf() and setOf()

setOf() is what I did in this PR
mapOf() is little bit tricky...

This is why I’d rather see it in a third-party package

This is the best solution IMO. Bad thing it needs some different tweaks for different build tools.

[jimfb]

@gaearon is correct. Proptypes are mostly legacy and in maintenance mode right now. We don't want to increase the surface area of proptypes at the moment, if we can avoid it.

@chicoxyzzy These additional proptypes would be a good thing to publish as a separate third-party library on github, if you'd like to do that :).