Uh oh!
There was an error while loading. Please reload this page.
Implement multiple view manager lookup for the interop layer on Android - #43595
Closed
gabrieldonadel wants to merge 1 commit into
Closed
Implement multiple view manager lookup for the interop layer on Android#43595gabrieldonadel wants to merge 1 commit into
gabrieldonadel wants to merge 1 commit into
Conversation
Base commit: c574ca5 |
gabrieldonadelforce-pushed
the
@gabrieldonadel/fix-rct-view-manager-android
branch
from
March 21, 2024 16:36
980aa15 to
b9fc47aComparebrentvatne
commented
Mar 21, 2024
Collaborator
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 |
facebook-github-bot
commented
Mar 21, 2024
Contributor
@arushikesarwani94 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
facebook-github-bot
commented
Mar 22, 2024
Contributor
@arushikesarwani94 merged this pull request in 15a5638. |
cortinico
commented
Mar 22, 2024
Contributor
@gabrieldonadel can you add a pick request on https://github.com/reactwg/react-native-releases/issues/new/choose for this one? |
gabrieldonadel
commented
Mar 22, 2024
CollaboratorAuthor
Yep, already did it yesterday reactwg/react-native-releases#172. Thanks for the remainder |
huntie pushed a commit
that referenced
this pull request
Mar 25, 2024
…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
This was referenced Apr 5, 2024
This was referenced Jun 28, 2024
Merged
This file contains hidden or 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
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 theRCTprefix is automatically removed from the component name, and inside the internalmViewManagerswe store view managers with theRCTprefix.Using the
RCTpattern 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)
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
Test Plan:
Tested installing a library such as react-native-fbsdk-next that names its view managers starting with
RCT