-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
Implement multiple view manager lookup for the interop layer on Android #43595
Implement multiple view manager lookup for the interop layer on Android #43595
Conversation
Base commit: c574ca5 |
980aa15
to
b9fc47a
Compare
this seems very reasonable to me! it's consistent with the ios behavior, and it should help make it easier for folks to adopt the new arch. i'd love to see this land in 0.74 |
@arushikesarwani94 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@arushikesarwani94 merged this pull request in 15a5638. |
@gabrieldonadel can you add a pick request on https://github.com/reactwg/react-native-releases/issues/new/choose for this one? |
Yep, already did it yesterday reactwg/react-native-releases#172. Thanks for the remainder |
…id (#43595) Summary: When running with the new architurece and using the renderer interop layer on Android, view managers don't work if their names start with `RCT`. This happens because the `RCT` prefix is automatically removed from the component name, and inside the internal `mViewManagers` we store view managers with the `RCT` prefix. Using the `RCT` pattern as a prefix works fine with the old architecture and is actually used on the [Android Native UI Components](https://reactnative.dev/docs/next/native-components-android) tutorial in the docs, making me believe that this same patterns is used across many community libraries. This diff adds a secondary lookup logic for view managers: 1. We look for the XXXViewManager. 2. If not found, we look for RCTXXXViewManager. Quite similar to the iOS implementation introduced in #38093 --- With this change we can also remove most of the entries from FabricNameComponentMapping (I can address this in a follow up PR) https://github.com/facebook/react-native/blob/4e6eba7a2dedaa855af0bff5df3bec73a95f0fc4/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/FabricNameComponentMapping.java#L22-L45 ## Changelog: [ANDROID] [ADDED] - Implement multiple view manager lookup for the interop layer Pull Request resolved: #43595 Test Plan: Tested installing a library such as [react-native-fbsdk-next](https://github.com/thebergamo/react-native-fbsdk-next) that names its view managers starting with `RCT` <table> <tr> <th>Before</th> <th>After</th> </tr> <tr> <td> <img width="519" alt="image" src="https://github.com/facebook/react-native/assets/11707729/123de1d6-f018-424b-b6ce-38221af9d83e"> </td> <td><img width="519" alt="image" src="https://github.com/facebook/react-native/assets/11707729/0f35b369-e2e4-4bbf-b880-6471fbc05d38"> </td> </tr> </table> Reviewed By: cortinico Differential Revision: D55208396 Pulled By: arushikesarwani94 fbshipit-source-id: a1fb1f4cee8483cf91ebededd1d7c4ba7021f9d9
Summary:
When running with the new architurece and using the renderer interop layer on Android, view managers don't work if their names start with
RCT
. This happens because theRCT
prefix is automatically removed from the component name, and inside the internalmViewManagers
we store view managers with theRCT
prefix.Using the
RCT
pattern as a prefix works fine with the old architecture and is actually used on the Android Native UI Components tutorial in the docs, making me believe that this same patterns is used across many community libraries.This diff adds a secondary lookup logic for view managers:
Quite similar to the iOS implementation introduced in #38093
With this change we can also remove most of the entries from FabricNameComponentMapping (I can address this in a follow up PR)
react-native/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/FabricNameComponentMapping.java
Lines 22 to 45 in 4e6eba7
Changelog:
[ANDROID] [ADDED] - Implement multiple view manager lookup for the interop layer
Test Plan:
Tested installing a library such as react-native-fbsdk-next that names its view managers starting with
RCT