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 "Requires main queue setup" warning #18

Merged

Conversation

andriichernenko
Copy link
Contributor

@andriichernenko andriichernenko commented Jan 2, 2018

This fixes the warning in console which is produced by react-native-version-check@2.1.0 running with React Native 0.51 on iOS:

Module RNVersionCheck requires main queue setup since it overrides `constantsToExport` 
but doesn't implement `requiresMainQueueSetup`. In a future release React Native will 
default to initializing all native modules on a background thread unless explicitly opted-out of.

This fixes the warning in console which is produced by react-native-version-check@2.1.0 running with React Native 0.51:
```
RCTLog.js:48 Module RNVersionCheck requires main queue setup since it overrides `constantsToExport` but doesn't implement `requiresMainQueueSetup`. In a future release React Native will default to initializing all native modules on a background thread unless explicitly opted-out of.
```
@kimxogus
Copy link
Owner

kimxogus commented Jan 2, 2018

Thank you for contribution 👍
You can ignore CI failure. There's some issue in test settings.
Your changes looks great, but I have a question. Does this pr works with RN<0.51?

@andriichernenko
Copy link
Contributor Author

I haven't checked, but the change is purely additive, so I suppose earlier versions will just ignore this method.

@andriichernenko
Copy link
Contributor Author

andriichernenko commented Jan 4, 2018

Okay, I verified the change against RN 0.44, 0.48, 0.49 and 0.50. As expected, no weird stuff.

@kimak
Copy link

kimak commented Jan 5, 2018

@andriichernenko thanks for the changes.
@kimxogus possible to see this merged and npm publish ?

This fix is really the way to go with new versions of React Native.
Look at react-native-maps same changes: https://github.com/react-community/react-native-maps/pull/1873/files

@kimxogus
Copy link
Owner

kimxogus commented Jan 5, 2018

Sorry for late response. I'm quite busy thesedays :-(
@andriichernenko Thank you for verification! I'll merge right now
@kimak I'll publish new version within a few hours.

@kimxogus kimxogus merged commit 555ddd5 into kimxogus:master Jan 5, 2018
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.

None yet

3 participants