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

Removing a ref callback works differently on HostComponents vs ClassComponents #12177

Closed
rhagigi opened this issue Feb 7, 2018 · 0 comments · Fixed by #12178
Closed

Removing a ref callback works differently on HostComponents vs ClassComponents #12177

rhagigi opened this issue Feb 7, 2018 · 0 comments · Fixed by #12178
Assignees

Comments

@rhagigi
Copy link
Contributor

rhagigi commented Feb 7, 2018

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

If you remove the ref from a mounted HostComponent on an update, it calls the original ref with null. If you remove the ref from a mounted ClassComponent on an update, it does not call the original ref with null.

https://codesandbox.io/s/q8k7776pz9 Note that it doesn't call ref1 (passed to FooInner as ref) with null

https://codesandbox.io/s/849538y8m8 Note that it does call ref1 (passed to div as ref) with null

If you modify the ref from either a HostComponent or a ClassComponent (change from ref={this.ref1} to ref={this.ref2}), the original ref (ref1) is called with null and the new ref (ref2) is called with the instance.

(take the sandboxes above and change the second <Foo> to ref={this.ref2}

What is the expected behavior?

Honestly not sure but it should be consistent. The reason this is different is because for HostComponents, we decide to schedule a Ref effect on completeWork as long as current.ref !== workInProgress.ref. For ClassComponents, this decision is made on beginWork as long as workInProgress.ref && (!current || current.ref !== workInProgress.ref) -- which means if workInProgress.ref is null, the behavior is different (hence this issue).

We should probably pick one and make it consistent.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

Unsure - it for sure is an issue in latest/Fiber, but I haven't dug deep enough to see what happened in <16.

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 a pull request may close this issue.

2 participants