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 RN New Arch Fabric support #9

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

muhamad-rizki
Copy link

@muhamad-rizki muhamad-rizki commented Mar 15, 2023

This pull request implement RN New Architecture Fabric for Android

Copy link
Collaborator

@simontreny simontreny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @muhamad-rizki for your work on this, it's really appreciated 🙏

Unfortunately, I was not able to make it work on my end in the example app from your branch. When I run the Android app with Fabric enabled with your changes, here is what I get:

Are there specific instructions to follow to build the app with Fabric enabled, other than setting newArchEnabled=true in gradle.properties?


@ReactModule(name = FastShadowViewManagerImpl.NAME)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only difference between this file and the one in oldarch is this annotation. Is there any way we could use the same file for both archs? Can we use @ReactModule with the non-Fabric arch?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi sorry for late response, I just following guide from here (https://github.com/react-native-community/RNNewArchitectureLibraries/tree/feat/back-fabric-comp), I'll look into it to make same file for both archs

@muhamad-rizki
Copy link
Author

muhamad-rizki commented Mar 26, 2023

Thanks @muhamad-rizki for your work on this, it's really appreciated 🙏

Unfortunately, I was not able to make it work on my end in the example app from your branch. When I run the Android app with Fabric enabled with your changes, here is what I get:

Are there specific instructions to follow to build the app with Fabric enabled, other than setting newArchEnabled=true in gradle.properties?

I test it with my apps directly instead from example folder with RN version 0.71, I tried to build the example folder with newArchEnabled=true but I cannot get it compiled yet with exception

error: package com.swmansion.reanimated does not exist
import com.swmansion.reanimated.ReanimatedPackage;

it looks like RN Reanimated only support fabric with RN version 0.70+, I can make it compiled when removing reanimated example.

And it looks like RN New Arch Auto linking is supported from RN v0.70+, that's why example apps not worked (https://reactnative.dev/blog/2022/09/05/version-070#android-auto-linking-for-new-architecture-libraries), it need some adjustment on example app (193ab4c)
image

@@ -5,6 +5,10 @@ include $(REACT_ANDROID_DIR)/Android-prebuilt.mk
# If you wish to add a custom TurboModule or Fabric component in your app you
# will have to include the following autogenerated makefile.
# include $(GENERATED_SRC_DIR)/codegen/jni/Android.mk

# Includes the MK file for `react-native-fast-shadow`
include $(NODE_MODULES_DIR)/../../android/build/generated/source/codegen/jni/Android.mk
Copy link
Author

@muhamad-rizki muhamad-rizki Mar 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's pointed to android folder directly due to example app folder structure, on end user app it should be something like this $(NODE_MODULES_DIR)/react-native-fast-shadow/android/build/generated/source/codegen/jni/Android.mk

@kunger97
Copy link

is this pull request ready for merge?

@muhamad-rizki
Copy link
Author

Hi @simontreny
I just updated the example app and solving conflicts, can we merge this so this library can support RN new architecture?
image

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.

3 participants