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

Fix React Native onBlur crash #586

Merged

Conversation

audiolion
Copy link
Contributor

@audiolion audiolion commented Sep 26, 2018

What: Check for existence of this.props.environment.document before trying to access further properties on document

Why: onBlur when called in react native will try to access this.props.environment.document.activeElement, and environment.document does not exist in React Native, resulting in Downshift crashing RN.

How: Added a failing test case to showcase the bug, then added a change that fixes the failing test

Checklist:

  • [N/A] Documentation
  • Tests
  • Ready to be merged
  • Added myself to contributors table

@audiolion
Copy link
Contributor Author

I am not really sure how to capture the error in a failing test, but you can see the error getting logged out as the test passes:

https://travis-ci.org/paypal/downshift/builds/433607691#L649

image

I tried using expect(() => firstArg.onBlur(fakeEvent)).not.toThrow() but the error doesn't bubble up from that event, it occurs after the internalSetTimeout occurs

@kentcdodds
Copy link
Member

That's fine. Thanks!

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Excellent! Thank you!

@kentcdodds kentcdodds merged commit 3fb0ed2 into downshift-js:master Sep 26, 2018
@audiolion audiolion deleted the bug/fix-react-native-input-blur branch September 26, 2018 16:59
@kentcdodds
Copy link
Member

🎉 This PR is included in version 2.2.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Rendez pushed a commit to Rendez/downshift that referenced this pull request Sep 30, 2018
* Add failing test for onBlur being called in react native environment

* Check for document before trying to access document properties

* Add to all contributors
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