PropTypes.node forbids true but flow typing accepts it, causing runtime warning

[ypresto]

PropTypes.node does not accept true according to #109.

But flow typing in React declares export type ReactEmpty = null | void | boolean; https://github.com/facebook/react/blob/b6173e643a4311b9b1cf039824b2f3d7b974b8cf/packages/shared/ReactTypes.js#L18 so it accepts true without error for type checking, but prop-types shows warning for it.

(Actually I'm using equivalent TypeScript type definition along with material-ui library which uses prop-types.)

[ljharb]

Sounds like the flow types (which are maintained in a separate package) have a bug. I’d suggest filing this issue there :-)

[ypresto]

ReactEmpty flow type is in facebok/react repo and which is just following React's spec to include true. So I'm filling issue on facebook/react to change that spec, is it right?

[ljharb]

Ah, I misunderstood. You're saying that react itself changed to allow true at some point, and PropTypes.node does not properly allow it?

It'd be great if we could figure out in which version that changed.

[ypresto]

Perhaps the type has not been changed since 2014: (the first commit which contains ReactEmpty (in the doc)) https://github.com/facebook/react/commit/d36d26a57454366132e8cd23adddefe3595535da#diff-191cf9292ca3ab67015443b913e019cbR179

type ReactEmpty = null | undefined | boolean;

[ljharb]

Perhaps the first step then is to file an issue on react to ask them to update the flow type. Once that's updated, and once it's clear when that change in React proper happened, it should be straightforward to update it here.

[ypresto]

AFAIK current state is:

• React: accepts true and render empty at least since 2017/11
• React's flow type: accepts true at least since 2014
• @types/react: accepts true
• PropTypes.node: rejects true

So React and its flow type are already in sync for true. My guess is that React accepts true from the beginning. But I'm in doubt, so filled https://github.com/facebook/react/issues/17871 .

[ljharb]

Did some testing; looks like prior to React 16, true was disallowed. I'll update this package.

[Hypnosphi]

@ljharb

Did some testing; looks like prior to React 16, true was disallowed. I'll update this package.

Can you please share your test method? If you were trying to return true from component, it would fail just because React 15 didn't allow primitives as component return values. So true isn't different from 'foo' in that aspect. A proper test would be to pass true as children, and it was possible with React 15 AFAIR: <div>{true}</div>

UPD: it works in React 15 without any warnings: https://codesandbox.io/s/boring-goldberg-6qgnh?fontsize=14&hidenavigation=1&theme=dark

[ljharb]

(similar to #109)

@Hypnosphi i just loaded up a bunch of react versions in a code sandbox and verified which ones gave a runtime warning or not. I was using true as a child of a div.

In other words, what i said was “prior to react 16, it was disallowed” which does not conflict with “works in 15”.

[Hypnosphi]

Sorry, but how is 15 not prior to 16?

[Hypnosphi]

verified which ones gave a runtime warning or not

So which versions gave warnings?

[ljharb]

“disallowed < 16” means “does not work on 15”, I’m not sure where the confusion is.

Every version 15 and below gave warnings.

[Hypnosphi]

Every version 15 and below gave warnings.

Can you please share a link to codesandbox? I have posted one above, and it gives no warnings

“does not work on 15”

does not conflict with “works in 15”

It doesn't make sense to me, sorry. Did you mean "does conflict" or something?

[ljharb]

I discarded the link, so i have no link to share.

No, i meant does NOT conflict. React 16 worked, react 15 didn’t, with true as a child. I see that you are claiming that react 15 works, but your own sandbox has an invariant warning in the console, so react 15 seems to not work.

[Hypnosphi]

Oh, looks like I had to fork the shared sandbox before playing with it, my bad. I reverted the changes, please check again

https://codesandbox.io/s/boring-goldberg-6qgnh

UPD: also, React 0.14: https://codesandbox.io/s/relaxed-fermi-7kgc4

[ljharb]

Fair, those seem to work. Can you provide a react 0.13 sandbox with a class component to verify if that one fails?

[Hypnosphi]

Can you provide a react 0.13 sandbox with a class component to verify if that one fails?

Nope

0.13 with class: https://codesandbox.io/s/sad-haze-yykbq 0.12 with createClass: https://codesandbox.io/s/weathered-waterfall-q8fm7

[ljharb]

Then I’m very confused.

If true has always been allowed, then i don’t know why the node propType ever disallowed it.

I’m still happy to loosen it to accept true, but before i do id like to understand the disparity.