Prevent falsy nodes from being counted as children #251

nishitaniyuki · 2016-03-13T16:40:46Z

I have a problem like below.

test('component should have no children', t => {
    const component = shallow(
        <div>
            {undefined}
            {undefined}
        </div>
    );
    t.same(component.children().length, 0) // This should be success, but fail since length is 1.
});

I expect null, undefined and false should not be counted as a child node.
What do you think??

lelandrichardson · 2016-03-13T17:51:45Z

src/ShallowTraversal.js

  const result = [];
-  React.Children.forEach(maybeArray, child => result.push(child));
+  React.Children.forEach(filtered, child => result.push(child));


a simpler solution here might be to just check and see if child is false, null, or undefined before pushing it into the result? The recursive nature of children will be taken care of in React.Children.forEach in this case.

Exactly.
Can I rewrite & send new PR?(I'm new to github...)

@nishitaniyuki yep! Update your code and then push the changes to github. this PR should update automatically.

When you update your code, it's best for a change like this if you can make all of your changes into a single atomic commit. Let me know if you need any extra help with this.

FYI, you can change the code and then use git commit --amend to combine the changes into the latest commit so that it stays as a single commit.

I've combined the changes, thank you!
Please check it.

lelandrichardson · 2016-03-13T17:53:08Z

Thanks for working on this. I believe we omit these nodes from being passed into methods such as .findWher(), so omitting them here feels more consistent.

ljharb · 2016-03-13T17:59:01Z

We omit null nodes, but not usually all falsy nodes - what about the empty string and NaN?

lelandrichardson · 2016-03-13T21:00:43Z

@ljharb i'm not too worried about NaN, but we should have defined behavior for all things that React considers non-renderable: false, null, and undefined.

We explicitly don't traverse false and null. It looks like we might (incorrectly) handle undefined though:

https://github.com/airbnb/enzyme/blob/master/src/ShallowTraversal.js#L32

nishitaniyuki · 2016-03-14T01:13:14Z

i'm not too worried about NaN, but we should have defined behavior for all things that React considers non-renderable: false, null, and undefined.

I agree.

We explicitly don't traverse false and null. It looks like we might (incorrectly) handle undefined though: https://github.com/airbnb/enzyme/blob/master/src/ShallowTraversal.js#L32

I expect this also should skip undefined.

lelandrichardson · 2016-03-14T02:54:48Z

@nishitaniyuki great, thanks!

Prevent falsy nodes from being counted as children

lelandrichardson reviewed Mar 13, 2016
View reviewed changes

Prevent falsy nodes from being counted as children

0e5ace1

lelandrichardson added a commit that referenced this pull request Mar 14, 2016

Merge pull request #251 from nishitaniyuki/patch-1

c711f13

Prevent falsy nodes from being counted as children

lelandrichardson merged commit c711f13 into enzymejs:master Mar 14, 2016

nishitaniyuki deleted the patch-1 branch November 10, 2016 13:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prevent falsy nodes from being counted as children #251

Prevent falsy nodes from being counted as children #251

nishitaniyuki commented Mar 13, 2016

lelandrichardson Mar 13, 2016

nishitaniyuki Mar 14, 2016

lelandrichardson Mar 14, 2016

lelandrichardson Mar 14, 2016

nishitaniyuki Mar 14, 2016

lelandrichardson commented Mar 13, 2016

ljharb commented Mar 13, 2016

lelandrichardson commented Mar 13, 2016

nishitaniyuki commented Mar 14, 2016

lelandrichardson commented Mar 14, 2016

Prevent falsy nodes from being counted as children #251

Prevent falsy nodes from being counted as children #251

Conversation

nishitaniyuki commented Mar 13, 2016

lelandrichardson Mar 13, 2016

Choose a reason for hiding this comment

nishitaniyuki Mar 14, 2016

Choose a reason for hiding this comment

lelandrichardson Mar 14, 2016

Choose a reason for hiding this comment

lelandrichardson Mar 14, 2016

Choose a reason for hiding this comment

nishitaniyuki Mar 14, 2016

Choose a reason for hiding this comment

lelandrichardson commented Mar 13, 2016

ljharb commented Mar 13, 2016

lelandrichardson commented Mar 13, 2016

nishitaniyuki commented Mar 14, 2016

lelandrichardson commented Mar 14, 2016