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

[Fiber] Fix findDOMNode and findAllInRenderedTree #8450

Merged
merged 2 commits into from
Dec 3, 2016

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Nov 29, 2016

This strategy finds the current fiber. It traverses back up to the root if the two trees are completely separate and determines which one is current. If the two trees converge anywhere along the way, we assume that is the current tree. We find the current child by searching the converged child
set.

This could fail if there's any way for both return pointers to point backwards to the work in progress. I don't think there is but I could be wrong.

This may also fail on coroutines where we have reparenting situations.

This is failing in Fiber without the fix. Because we have no deletions to rely
on in this case and the placement effects have already happened.
@sebmarkbage
Copy link
Collaborator Author

Ok added tests. Done.

This strategy finds the current fiber. It traverses back up to the root if
the two trees are completely separate and determines which one is current.
If the two trees converge anywhere along the way, we assume that is the
current tree. We find the current child by searching the converged child
set.

This could fail if there's any way for both return pointers to point
backwards to the work in progress. I don't think there is but I could be
wrong.

This may also fail on coroutines where we have reparenting situations.
Copy link
Collaborator

@sophiebits sophiebits left a comment

Choose a reason for hiding this comment

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

Let's try it?

b = parentB;
invariant(
a.alternate === b,
'Return fibers should always be each others\' alternates.'
Copy link
Collaborator

Choose a reason for hiding this comment

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

other's (alternates of each other)

'Unable to find node on an unmounted component.'
);
if (a.stateNode.current === a) {
// We've determined that A is the current branch.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm having trouble convincing myself why this is true.

@sebmarkbage sebmarkbage merged commit 0723007 into facebook:master Dec 3, 2016
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.

4 participants