-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Closed
Update selector.md #1700
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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:
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.There was a problem hiding this comment.
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: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 theshallow()
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 theclassName
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?
There was a problem hiding this comment.
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
then
shallow(<Section lines={LINES} />).children().length
should return 1, for the root<div>
returned from the component, but it actually returns 2.There was a problem hiding this comment.
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
?There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:<Foo>
and its<div class="bar">
node)<div />
)I then call
.find(".bar div")
.In
selectors.js
, it splits the selector into [selector:".bar", childCombinator:" ", selector:"div"].selector:".bar"
it:treeFilter(ROOT_NODE, FN_WHICH_FINDS_CLASS_BAR)
:.bar
; ie still the root node.childCominator:" "
it:selector:"div"
, then:matchDescendant(ROOT_NODE, FN_WHICH_FINDS_DIVS)
which:treeFilter(ROOT_NODE, FN_WHICH_FINDS_DIVS)
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:
this returns:
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.
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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.