Make sure findWhere doesn't get stuck on empty string

[MikaelCarpenter]

What

• Add a findWhere test case that replicates the use case I ran into
• Update RSTTraversal.treeForEach to not call fn on empty strings
• Update treeForEach test case that references empty strings

Why

In the event that the DOM has an empty string, make sure findWhere can iterate past it and still find elements that are further down the tree.

Links

• Fixes .findWhere failing to parse all nodes in wrapper #1994
• Codesandbox that replicates the initial issue I had

[MikaelCarpenter]

would it make sense to just use if (tree) here, rather than blacklisting specific values?

[sstern6]

Thanks for reporting and contributing. I think this is a good start. Though as newer versions of react come out, more and more valid "empty" values become natively supported by react. Ie emptyString, emptyArray, emptyFragments, undefined. Adding a fix for just an empty string seems like a good start and solves your problem but i think we need to consider other use cases including the ones i mentioned above.

@ljharb thoughts on this?

[ljharb]

i mean, realistically here, this is deciding when to bypass calling childrenOfNode.

A node can only have children if it's an object, right? Could this change be effectively if (isPrimitive(tree)) {? (with function isPrimitive(val) { return !val || Object(val) !== val; })

[MikaelCarpenter]

So I tried this out and it looks like calling fn(tree) on things like strings and numbers is a desired functionality. Here are some of the tests that fail (trimmed out some of the output):

1) RSTTraversal treeForEach does not get trapped from empty strings:

2) (uses jsdom) mount .findWhere(predicate) does not pass in null or false nodes:

    AssertionError: expected [ Array(3) ] to deeply equal [ Array(4) ]
    + expected - actual

        [
          "<div>\n  foo bar\n</div>"
          true
        ]
    +  [
    +    "foo bar"
    +    [null]
    +  ]
      ]
    
    at Context.<anonymous> (packages/enzyme-test-suite/test/ReactWrapper-spec.jsx:1880:30)

3) (uses jsdom) mount .findWhere(predicate) allows `.text()` to be called on text nodes:

4) shallow .contains(node) works with strings:

5) shallow .contains(node) works with numbers:

6) shallow .contains(node) works with nested strings & numbers:

7) shallow .findWhere(predicate) allows `.text()` to be called on text nodes:

    AssertionError: expected [ Array(3) ] to deeply equal [ Array(4) ]
    + expected - actual

        [
          "<div>\n  foo bar\n</div>"
          "foo bar"
        ]
    +  [
    +    "foo bar"
    +    "foo bar"
    +  ]
      ]
    
    at Context.<anonymous> (packages/enzyme-test-suite/test/ShallowWrapper-spec.jsx:1829:31)

8) selectors (uses jsdom) mount :empty pseudo selector:

9) selectors shallow :empty pseudo selector:

    AssertionError: expected {} to have a length of 0 but got 1
    + expected - actual

    -1
    +0
    
    at Context.<anonymous> (packages/enzyme-test-suite/test/selector-spec.jsx:371:52)

[MikaelCarpenter]

So it looks like we only want to prevent falsey values. I tried out if (tree) { fn(tree) } and the tests will still pass.

[ljharb]

ah, ok - let's do that then :-)

[ljharb]

i mean, realistically here, this is deciding when to bypass calling childrenOfNode.

A node can only have children if it's an object, right? Could this change be effectively if (isPrimitive(tree)) {? (with function isPrimitive(val) { return !val || Object(val) !== val; })