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

Removed Bridge dependency from image props on Fabric (iOS) #607

Merged
merged 14 commits into from
Jul 3, 2022

Conversation

grahammendick
Copy link
Owner

The React Native Slider component has image props, for example, trackImage. When first migrating to Fabric, tried to copy the Slider implementation for the image props on BarButton and TabBarItem components. But it uses c++ state which isn’t documented anywhere. So used the ImageLoader module. But it was only a temp solution because it relied on RCTBridge+Private.h. This PR replaces the temp solution with the Slider's c++ implementation.

The ‘Write You Own State’ solution suggested by React Native put me off trying c++ state because it didn’t seem right hand-writing all the codegen classes. But discovered the interfaceOnly flag setting in the NativeComponent.js spec. This still codegens the Props and EventEmitter - classes that change when the spec changes - but not the ComponentDescriptor, State and ShadowNode. These can be handwritten to implement custom State like image props.

Had to update the source_files of the podspec so these hand-written classes were included in the iOS project. But getting these files included on Android is much harder. Even though these files aren’t used on Android, the build fails without them. Had to include them in the Android.mk of the app (user changes).

LOCAL_C_INCLUDES := $(LOCAL_PATH) $(NODE_MODULES_DIR)/navigation-react-native/cpp
LOCAL_SRC_FILES := $(wildcard $(LOCAL_PATH)/*.cpp) $(wildcard $(NODE_MODULES_DIR)/navigation-react-native/cpp/react/renderer/components/navigation-react-native/*.cpp)
LOCAL_EXPORT_C_INCLUDES := $(LOCAL_PATH)

Also had to include mapbuffer and imagemanager in LOCAL_SHARED_LIBRARIES

libreact_render_mapbuffer \
libreact_render_imagemanager \

And in MainComponentsRegistry.cpp had to include the hand-written ComponentDescriptors because they’re not part of the codegen’ned descriptors anymore.

#include <react/renderer/components/navigation-react-native/NVBarButtonComponentDescriptor.h>
#include <react/renderer/components/navigation-react-native/NVTabBarItemComponentDescriptor.h>

Copied structure from 'native-nodesize' branch and details of cpp/h from reactwg/react-native-new-architecture#31
Copied from native-nodesize branch
Copied over from native-nodesize branch. Different from code samples at reactwg/react-native-new-architecture#31 because using interfaceOnly so the props and event emitters are still generated by codegen
This is all copied from reactwg/react-native-new-architecture#31 and it works! It was missing the self.imageCoordinator = nullptr; in prepareForRecycle - without this then reloading (typing 'r' in console) doesn't work, throws when selecting another color from the list in the zoom sample
Got rid of private bridge import!!
The imports have to be manually included in the MainComponentsRegistry.cpp on Android. So want the includes to look like the others, for example, #include <react/renderer/components/navigation-react-native/NVBarButtonComponentDescriptor.h>
They're mostly related to the React Native slider component where this was copied from
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.

1 participant