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

Update selector.md #1700

Closed
wants to merge 1 commit into from
Closed

Conversation

ReactiveRaven
Copy link
Contributor

Clarify where Enzyme descendant selectors diverge from CSS descendant selectors, referencing #1680 .

Clarify where Enzyme descendant selectors diverge from CSS descendant selectors, referencing enzymejs#1680 .
@@ -34,6 +34,10 @@ enzyme also gives support for the following contextual selectors
.foo input
```

**Note**

* In certain cases, the descendant selector (`.foo .bar`) does not exclude parents like a CSS selector would; if the left and right both match the parent, both matching children _and parent_ will be returned (eg: `.foo span` on `<div class="foo"><span /></div>` finds one result, but `.foo div` on `<div class="foo"><div /></div>` finds two results). In order to maintain backwards compatibility, there are [no plans to update this](https://github.com/airbnb/enzyme/pull/1680#issuecomment-401677554). You can work around this by using `.children()`, ie `.find(".foo").children().find("div")`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't quite accurate. it's not that selectors behave differently, it's that the tree isn't "just the DOM".

the tree might be like this:

<Foo className="a">
  <div className="a">
    <div />
  </div>
</Foo>

In this case, selecting on .a will return the div - but not the Foo (since the wrapper is the Foo).

I would still expect .foo .bar to return any .bar inside any .foo - just like CSS does - it's just that "any" refers to more than just host nodes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, that is surprising. Before you showed me that full example (thank you), I still didn't understand that the tree included the input to shallow() (hence my mistake with .find(".foo").children().find("div"), in stead of .children().find(".foo div")).

In the example I gave in #1680 , I had tried shallow(<Section lines={LINES} />).debug() which returned:

<div className="section">
  <div className="heading">
    Hello world
  </div>
  <div className="paragraph">
    Hello world
  </div>
</div>

Which doesn't include the <Section> (I might expect .html() to strip it, but it seems odd that a function called "debug" omits part of the actual state).

Although, that does raise some questions:

I didn't pass className="section" to the shallow() call; does this mean although <Section> and the root <div> are considered separate nodes in the tree, they are still sharing the same attributes/props?

If that is true, I'm also confused why .find(".section").length returns one, not two, if the className is being 'copied upwards'. I'd assume it should find <Section className="section">...</Section> and <div className="section">...</div>.

Could you shed any light please?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm. No, I think I must have misunderstood. If the tree actually did have the structure

<Section> <!-- input to shallow -->
    <div> <!-- root of render call -->
        ... <!-- more children -->
    </div>
</Section>

then shallow(<Section lines={LINES} />).children().length should return 1, for the root <div> returned from the component, but it actually returns 2.

Copy link
Contributor Author

@ReactiveRaven ReactiveRaven Jul 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the initial question is:

Why would <Section lines={} /> match the selector .section?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that's a good question :-) Want to add a failing test case to this PR? We could turn this into a bugfix.

Copy link
Contributor Author

@ReactiveRaven ReactiveRaven Jul 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading deeper, I don't think it does.

I've come back to thinking that the code in selectors.js is wrong, and the PR was on the right track. I'm working it through, maybe you can find what I'm missing.

Lets say I have a component <Foo> that renders <div class="bar"><div /></div>.

After passing through react-test-renderer/shallow (via the react 16 adaptor) the tree would look like:

  • root node (this is simultaneously the <Foo> and its <div class="bar"> node)
    • child node (this is the inner <div />)

I then call .find(".bar div").

In selectors.js, it splits the selector into [selector:".bar", childCombinator:" ", selector:"div"].

  • Starting with selector:".bar" it:
    • calls treeFilter(ROOT_NODE, FN_WHICH_FINDS_CLASS_BAR):
      • which walks the tree, finding anything with .bar; ie still the root node.
  • Then with childCominator:" " it:
    • notices it is a combinator, and also swallows selector:"div", then:
    • runs matchDescendant(ROOT_NODE, FN_WHICH_FINDS_DIVS) which:
      • via a reduce statement,
      • calls treeFilter(ROOT_NODE, FN_WHICH_FINDS_DIVS)
        • which walks the tree, finding both the root node, and the child node.

notice matchDescendant is not restricting to actual descendants at any point.

And this is not some property of the root node being special because it is both a component and an element.

For example:

export default class SillyExample extends React.Component {
    render() {
        return (
            <div className={"outer"}>
                <div className={"middle"}>
                    <div className={"inner"} />
                </div>
            </div>
        )
    }
}
shallow(<SillyExample/>).find("div div div").length
shallow(<SillyExample/>).find("div div div").debug()

this returns:

3
<div className="outer">
  <div className="middle">
    <div className="inner" />
  </div>
</div>


<div className="middle">
  <div className="inner" />
</div>


<div className="inner" />

When the only sensible results should either be ['inner'] (as with css), or ['middle', 'inner'] (if components are special).

Am I missing something obvious? I don't understand why a selector like "div div div" would just return every div in a tree, regardless of how many matching ancestors they have.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The root is the component - the div (what the component renders) is a child of it. There's not two root nodes, in other words, just the one. (for mount - for shallow, the root is what the component renders).

However, yes, I do see that in your example, div div div shouldn't be able to return more than one node. Can you add that as a test case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, sure thing, I've pushed this commit 7fcf3f5 and reopened PR #1680 .

If you can describe/provide a test case for the intended behaviour when working with non-host-nodes, I'll happily get that PR fixed up.

@ReactiveRaven
Copy link
Contributor Author

Returned to #1680

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants