-
Notifications
You must be signed in to change notification settings - Fork 24.4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Reviewed By: michalgr Differential Revision: D8057885 fbshipit-source-id: 6af7f7729201d26a704adaadb15813979cd035f8
- Loading branch information
1 parent
782971f
commit 7c5845a
Showing
6 changed files
with
10 additions
and
34 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
7c5845a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious what this commit is doing, could someone provide a quick summary?
7c5845a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changelog for 0.56 references it like so "Remove native extensions - 7c5845a"
Does that mean there are no more native extensions for Android? What functionality was impacted, given the docs are still for 0.55 and mention Android Native Modules ? Are modules and extensions different?
7c5845a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I too thought it was the same thing @mikehardy which made me curious about what exactly this was doing.
7c5845a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use native modules on my projects. Do I need to find alternatives to them if I want to upgrade to React Native 0.56.0? This commit sounds odd to me...
7c5845a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked more closely and I could be wrong but it looks like i/we are having a semantic problem with the word native. I think react has the ability to run truly native code modules (like, JNI interface on Android, implemented in C++) and that's what native means here I think. When I think of react and then think native that's different and might be called platform-specific in this context. So if I'm correct in this, native (c++ code) is still possible even, just not with extensions now. And platform-specific stuff is fine. Verify for yourself but that's my read on the related code.