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 #94

Merged
merged 1 commit into from
Apr 8, 2020

Conversation

janicduplessis
Copy link
Contributor

Add @ReactModule annotation, this will eventually be used by TurboModules annotation processor.

Add `@ReactModule` annotation, this will eventually be used by TurboModules annotation processor.
@jdnichollsc
Copy link
Member

Hi @janicduplessis,
So it looks like we need to revert this change? dcbfaef#diff-7ae5a9093507568eabbf35c3b0665732

What do you think? 🤔

@janicduplessis
Copy link
Contributor Author

@jdnichollsc Hmm why do we need to revert this? Should not be related to this PR.

@jdnichollsc
Copy link
Member

So is com.facebook.react.module.annotations already included? 🤔

@janicduplessis
Copy link
Contributor Author

@jdnichollsc Yes it is included in react native already, there is no need to add dependencies.

@jdnichollsc
Copy link
Member

@janicduplessis thanks for your help mate, Do you know any link to see the documentation of this? 👍

@janicduplessis
Copy link
Contributor Author

There is not much documentation yet except react-native-community/discussions-and-proposals#40. Basically the annotation allows generating some info about the native module (mostly the name) that can be used by TurboReactPackage. The annotation was added a long time ago so there's no issue with backwards compatibility.

@janicduplessis
Copy link
Contributor Author

Also some info in this twitter thread https://twitter.com/janicduplessis/status/1157172268495179776

@jdnichollsc
Copy link
Member

Hey mate, thanks for your help! In order to merge this PR, how can we avoid an issue like this? react-native-async-storage/async-storage#153

@janicduplessis
Copy link
Contributor Author

Hmm I think this might just happen as a result of a weird issue when upgrading. It is mentionned in the issue that it does not happen on a new project.

@jdnichollsc jdnichollsc merged commit ec289ed into proyecto26:master Apr 8, 2020
@jdnichollsc
Copy link
Member

Thanks for your help! 👍

phuongwd pushed a commit to phuongwd/react-native-inappbrowser that referenced this pull request Jul 31, 2020
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