Skip to content

Fix LocalEventTrapMixin for Bad Tree State#2894

Merged
yungsters merged 1 commit into
facebook:masterfrom
yungsters:local-event-trap-mixin
Jan 21, 2015
Merged

Fix LocalEventTrapMixin for Bad Tree State#2894
yungsters merged 1 commit into
facebook:masterfrom
yungsters:local-event-trap-mixin

Conversation

@yungsters
Copy link
Copy Markdown
Contributor

Summary:
If getDOMNode() returns null in LocalEventTrapMixin, listener will also be null and accumulateInto will throw.

getDOMNode() can return null if a React component elsewhere in the tree fatals and causes a set of mounted components to be in a bad state.

Test Plan:
Ran unit test successfully:

npm test LocalEventTrapMixin-test.js

@yungsters yungsters force-pushed the local-event-trap-mixin branch 2 times, most recently from d1eff9e to 590e655 Compare January 21, 2015 23:25
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Add a comment here explaining that this can be null if a React component elsewhere in the tree fatals?

@sophiebits
Copy link
Copy Markdown
Collaborator

lgtm

Summary:
If `getDOMNode()` returns null in `LocalEventTrapMixin`, `listener` will also be null and `accumulateInto` will throw. This changes `LocalEventTrapMixin` to throw a more helpful error message.

Test Plan:
Ran unit test successfully:

```
npm test LocalEventTrapMixin-test.js
```
@yungsters yungsters force-pushed the local-event-trap-mixin branch from 590e655 to a7aca51 Compare January 21, 2015 23:30
yungsters added a commit that referenced this pull request Jan 21, 2015
Fix LocalEventTrapMixin for Bad Tree State
@yungsters yungsters merged commit 892e357 into facebook:master Jan 21, 2015
@yungsters yungsters deleted the local-event-trap-mixin branch January 21, 2015 23:34
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