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

Add @ReactModule annotation and NAME constant #745

Merged
merged 1 commit into from
Aug 29, 2019

Conversation

janicduplessis
Copy link
Contributor

Description

Add @ReactModule annotation and NAME constant. This will eventually be used by TurboModules annotation processor.

Compatibility

N/A

Checklist

  • I have tested this on a device/simulator for each compatible OS
  • I added the documentation in README.md
  • I mentioned this change in CHANGELOG.md
  • I updated the typings files (deviceinfo.d.ts, deviceinfo.js.flow)
  • I updated the dummy web/test polyfill (default/index.js)
  • I added a sample use of the API (example/App.js)

This will eventually be used by TurboModules annotation processor.
@mikehardy
Copy link
Collaborator

Seems fine - and thank you! - a couple questions:

  • when does TurboModules come in and what's the effect? A pointer to a good doc you'd recommend is sufficient, I can read up :-)
  • when did this API come into existence? Are we cutting off compatibility to some older react-native versions by adding this? If so which ones?

@janicduplessis
Copy link
Contributor Author

There is some info about it here react-native-community/discussions-and-proposals#40. Basically this enables using TurboReactPackage more easily, it allows loading native modules lazily at the moment they are used in JS.

The annotation class was added in September 2016 so it should be fine.

@mikehardy
Copy link
Collaborator

Awesome thanks for both bits of info - I'm traveling so please be patient but I'll double check and likely merge and release, next opportunity. I appreciate the help!

@mikehardy mikehardy changed the base branch from master to v3 August 29, 2019 17:31
@mikehardy mikehardy merged commit a3cff96 into react-native-device-info:v3 Aug 29, 2019
@mikehardy
Copy link
Collaborator

Thanks for your patience! Shaping up a v3 release with most of the backlog now

@mikehardy
Copy link
Collaborator

@janicduplessis
The only thing I don't like about this, is that now Android Studio can't find the import or the ReactModule symbol. Is there something we need to also add in build.gradle in order for Android Studio state to be sane again?

@janicduplessis
Copy link
Contributor Author

Hmm that’s strange, it should work fine. Maybe its a cache issue or something? I’ll try to run the sample from the repo to see if I get the issue too.

@mikehardy
Copy link
Collaborator

mikehardy commented Aug 29, 2019

Oh it runs fine - as in you can use it in a project that depends on react-native, but in this repo and react-native-localize at least, if you open <library>/android in Android Studio by itself to work on the library, it can't find the symbol. That's puzzling to me

@mikehardy
Copy link
Collaborator

@janicduplessis I hope you haven't spent much if any time looking yet - it helps to actually have a devDependency on react-native and include it in the build.gradle if you, you know, want Android Studio to have access to symbols you want to use 🤦‍♂️ 😅

All good now.

@janicduplessis janicduplessis deleted the patch-1 branch September 2, 2019 17:08
@janicduplessis
Copy link
Contributor Author

Haha I forgot about this. Awesome!!

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

2 participants