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

saveShellRef callback in Chat.ts throws Null Pointer Exception when re-render is triggered. #782

Closed
marrobins opened this issue Nov 14, 2017 · 5 comments
Assignees
Labels
bug Indicates an unexpected problem or an unintended behavior.

Comments

@marrobins
Copy link
Contributor

marrobins commented Nov 14, 2017

The Chat component breaks when embedded in a React app due to a Null Pointer Exception being thrown in the saveShellRef callback.

This happens when a re-render at the parent level is triggered, causing the Chat component to be unmounted, destroyed and a new instance mounted. When this occurs, null is passed to the old saveShellRef callback as argument and the NPE is thrown, causing the new component to stop functioning.

As per this React issue [0], ref callbacks need to handle null arguments. During a re-render, the ref callback from the old component will be passed null and the ref callback from the new component instance will be passed the new ref instance.
[0] facebook/react#4533

@billba
Copy link
Member

billba commented Nov 14, 2017

Want to file a PR?

@marrobins
Copy link
Contributor Author

I've tested this PR (marrobins#1) in my codebase and it fixes the issue for me.

@compulim
Copy link
Contributor

compulim commented Dec 4, 2017

@marrobins Could you submit your PR to this repos? It would be great if you could also look at other ref calls.

React call ref with null when it is unmounting the instance, so we are practically hitting this everywhere with .getWrappedInstance().

React will call the ref callback with the DOM element when the component mounts, and call it with null when it unmounts. ref callbacks are invoked before componentDidMount or componentDidUpdate lifecycle hooks.

@compulim compulim self-assigned this Dec 4, 2017
@compulim compulim added the bug Indicates an unexpected problem or an unintended behavior. label Dec 4, 2017
@marrobins
Copy link
Contributor Author

@billba @compulim I've submitted my PR to this repo: #801

@compulim
Copy link
Contributor

PR merged, closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or an unintended behavior.
Projects
None yet
Development

No branches or pull requests

3 participants