React: React.Children.count is incorrect for false value

[bgw]

Do you want to request a _feature_ or report a _bug_?

I think this is a bug.

What is the current behavior?

If you try to use React.Children.count, and the children contains a child with the value false, the result will be off by one. For example,

function Root() {
    return (
    <Hello>
      {false}
    </Hello>
  );
}

function Hello({children}) {
  return (
    <div>
      Count: {React.Children.count(children)}
      <br />
      toArray length: {React.Children.toArray(children).length}
    </div>
  );
}

Will render

Count: 1
toArray length: 0

If you replace false with null or [] the count will be 0.

Here's a fiddle that demonstrates the issue.

What is the expected behavior?

The count shouldn't include false nodes.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

I've repro'd this on Firefox 48 and Chrome 53. I don't know if this worked in previous versions of React.

[bgw]

Update: It looks like while

{null}

and

{[]}

Both have a count of 0,

{null}
{[]}

has a count of 1 and

{null}
{null}

has a count of 2. But,

{[]}
{[]}

has a count of 0.

[aweary]

I think this goes deeper than just React.Children.count, as per the API docs count returns the total number of times a callback passed to React.Children.map or React.Children.forEach would be invoked.

Defined in those terms, it's correct, as map and forEach will iterate over children that are false , but not [].

So if this behavior is incorrect, it will need to be addressed in map and forEach as well.

[zpao]

cc @spicyj

[sophiebits]

It is intentional/known that we count empty children in the map/forEach/count but that we skip over them in toArray. We could change the older helpers but it could break stuff.

[bgw]

It is intentional/known that

It doesn't appear to be documented.

we count empty children in the map/forEach/count

Unless there's only one child, and that child is null. (Though that caveat does seem to be documented at least)

We could change the older helpers but it could break stuff.

I can't imagine someone seriously depending on this behavior. Looking through www, it looks like most places either assume the children can't be null/false/etc, or check if a child is null or undefined (often forgetting it could also be false) before processing it.

However, I could be wrong, and someone might be depending on the behavior. Is this a Resolution: Wontfix? Do we need to update the docs to contain a note about this?

[sophiebits]

I'm fine changing it if you want to send a PR, land the change in www, and take care of cleaning up any www code that might break.

[jasperjn]

An alternative solution.

React.Children.toArray(children).filter((child) => React.isValidElement(child)).length

[leFroxx]

An alternative solution.

React.Children.toArray(children).filter((child) => React.isValidElement(child)).length

This only works fine if you child is actually a valid element (as the method name says). That means that you can't simply pass e.g. a simple string as a child.

[nhunzaker]

I just closed out a similar issue to this one (https://github.com/facebook/react/issues/14564), to recap, here's where we've landed:

The behavior is intentional, even if confusing. count gives you the number of "slots" children defines. When passing null or undefined for children, React considers children to be absent. However when given an array, like [null], React considers there to be a "slot" open in children, even if the value is null.

We could also consider revising the API to be more intuitive, however we'd need to make that change in a major release (17).

[Ne-Ne]

I think .count() is a bit misleading, however my opinion does change when the null / undefined values is explained above.

Would React.Children.length make a clearer api for children that are defined?

At the moment I wanted to use this api to check if consumer had implemented this.props.children otherwise otherwise support an early return.