Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: React.Node should not have true, or PropTypes.node warns it #17871

Closed
ypresto opened this issue Jan 19, 2020 · 7 comments
Closed

Bug: React.Node should not have true, or PropTypes.node warns it #17871

ypresto opened this issue Jan 19, 2020 · 7 comments
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug Type: Discussion

Comments

@ypresto
Copy link

ypresto commented Jan 19, 2020

React version: 16.8.6

Steps To Reproduce

import PropTypes from 'prop-types';

type Props = {
  component: React.Node
};

function Container(props: Props) {
  return ...;
}

Container.propTypes = {
  component: PropTypes.node
}

<Container component={true} />

The current behavior

Flow does not emit error (as React.Node accepts true), but PropTypes.node emits below runtime error in browser's console.

Invalid prop `component` supplied to `Container`, expected a ReactNode.

The expected behavior

Flow emits type error.


According to a maintainer of prop-types, formerly React itself did not accept true for JSX node (in flow React.Node) (so not changing prop-types for now), is it right?

@ypresto ypresto added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Jan 19, 2020
@ypresto ypresto changed the title Bug: ReactNode should not have true, or PropTypes.node warns it Bug: React.Node should not have true, or PropTypes.node warns it Jan 19, 2020
@ljharb
Copy link
Contributor

ljharb commented Jan 19, 2020

Note; I’m not claiming that, I’m suggesting it as a possible history. Clarification would be appreciated.

@bvaughn
Copy link
Contributor

bvaughn commented Jan 19, 2020

The expected behavior

Flow emits type error.

Why is this the expected behavior?

The React docs say:

Booleans, Null, and Undefined Are Ignored

false, null, undefined, and true are valid children. They simply don’t render. These JSX expressions will all render to the same thing:

<div />
<div></div>
<div>{false}</div>
<div>{null}</div>
<div>{undefined}</div>
<div>{true}</div>

The docs have said this since back through 2016 (when I stopped tracing).

type Props = {
  component: React.Node
};

IMO this prop type says "I require something React can render to be passed as a component prop" - a case which true satisfies.

@ljharb
Copy link
Contributor

ljharb commented Jan 19, 2020

I’m wondering if perhaps true was invalid prior to react 15?

@bvaughn
Copy link
Contributor

bvaughn commented Jan 20, 2020

Not sure! I wasn't on the team before v15. 🙂

I don't think that would change my stance on this issue though. I don't think this is a bug or requires any change from React.

@ljharb
Copy link
Contributor

ljharb commented Jan 20, 2020

It requires a change in prop-types, however, (which i maintain) so it’d be useful to determine exactly when it changed. That said, there’s probably nothing actionable for react to do at this point and the issue can be closed.

@bvaughn
Copy link
Contributor

bvaughn commented Jan 21, 2020

Right, my comment was only meant for React. Looks like there's a separate issue filed against prop-types (facebook/prop-types#310). I'm going to go ahead and close this issue down.

@bvaughn bvaughn closed this as completed Jan 21, 2020
@ljharb
Copy link
Contributor

ljharb commented Jan 21, 2020

Did some testing; looks like React 15 disallowed true; React 16 allows it. I'll update the prop-types package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug Type: Discussion
Projects
None yet
Development

No branches or pull requests

3 participants