react-native-navigation icon indicating copy to clipboard operation
react-native-navigation copied to clipboard

React-native 0.68.* is not compatible with RNN right now - exposing createReactActivityDelegate - and support for new architecture (Fabric)

Open SudoPlz opened this issue 2 years ago • 21 comments

🐛 Bug Report

Hey there,

I'm opening that issue to keep track of react-native 0.68 support. Right now it seems that react-native navigation doesn't work with 0.68 and the new architecture.

Specifically the part where we're supposed to inject ReactRootViews seems to be the pain point, since RNN doesn't export any way to do that currently.

https://github.com/react-native-community/rn-diff-purge/blob/release/0.68.0-rc.1/RnDiffApp/android/app/src/main/java/com/rndiffapp/MainActivity.java#L23

cc'ing @yogevbd on this to hear some thoughts.

SudoPlz avatar Feb 22 '22 19:02 SudoPlz

Hey @SudoPlz, We currently don't have an Android developer working on RNN, we will have to somehow support it soon though.

yogevbd avatar Feb 23 '22 10:02 yogevbd

@SudoPlz as far as I can tell this only affects 0.68 when trying to enable the new architecture, right? 0.68 should work when keeping the old way, right?

danilobuerger avatar Feb 23 '22 17:02 danilobuerger

@danilobuerger not really sure to be honest, this requires testing.

SudoPlz avatar Feb 23 '22 18:02 SudoPlz

Alright, I will take a look at that once 0.68 rc2 is out.

danilobuerger avatar Feb 23 '22 18:02 danilobuerger

So, I investigated this using the old architecture for now.

iOS

There is a breaking change in the app template, that we should communicate in the docs:

diff --git a/ios/feastr/Classes/AppDelegate.m b/ios/feastr/Classes/AppDelegate.m
index d29d69933..8b67f1d11 100644
--- a/ios/feastr/Classes/AppDelegate.m
+++ b/ios/feastr/Classes/AppDelegate.m
@@ -111,7 +111,7 @@ - (void)applicationWillEnterForeground:(UIApplication *)application {
 
 - (NSURL *)sourceURLForBridge:(RCTBridge *)bridge {
 #if DEBUG
-    return [[RCTBundleURLProvider sharedSettings] jsBundleURLForBundleRoot:@"index" fallbackResource:nil];
+    return [[RCTBundleURLProvider sharedSettings] jsBundleURLForBundleRoot:@"index"];
 #else
     return [[NSBundle mainBundle] URLForResource:@"main" withExtension:@"jsbundle"];
 #endif

Android

We need a new product flavor. The RootView interface added two more methods that we need to implement in ModalContentLayout:

  /**
   * Called when a child starts a native gesture (e.g. a scroll in a ScrollView). Should be called
   * from the child's onTouchIntercepted implementation.
   */
  void onChildStartedNativeGesture(View childView, MotionEvent ev);

  /**
   * Called when a child ends a native gesture. Should be called from the child's onTouchIntercepted
   * implementation.
   */
  void onChildEndedNativeGesture(View childView, MotionEvent ev);
diff --git a/node_modules/react-native-navigation/lib/android/app/src/main/java/com/reactnativenavigation/react/modal/ModalContentLayout.kt b/node_modules/react-native-navigation/lib/android/app/src/main/java/com/reactnativenavigation/react/modal/ModalContentLayout.kt
index da7d5b1..ea8516f 100644
--- a/node_modules/react-native-navigation/lib/android/app/src/main/java/com/reactnativenavigation/react/modal/ModalContentLayout.kt
+++ b/node_modules/react-native-navigation/lib/android/app/src/main/java/com/reactnativenavigation/react/modal/ModalContentLayout.kt
@@ -49,9 +49,15 @@ class ModalContentLayout(context: Context?) : ReactViewGroup(context), RootView{
             updateFirstChildView()
         }
     }
+    override fun onChildStartedNativeGesture(child: View, androidEvent: MotionEvent?) {
+        mJSTouchDispatcher.onChildStartedNativeGesture(androidEvent, this.getEventDispatcher())
+    }
     override fun onChildStartedNativeGesture(androidEvent: MotionEvent?) {
         mJSTouchDispatcher.onChildStartedNativeGesture(androidEvent, this.getEventDispatcher())
     }
+    override fun onChildEndedNativeGesture(child: View, androidEvent: MotionEvent?) {
+        mJSTouchDispatcher.onChildEndedNativeGesture(androidEvent, this.getEventDispatcher())
+    }
     override fun requestDisallowInterceptTouchEvent(disallowIntercept: Boolean) {}
     private fun getEventDispatcher(): EventDispatcher? {
         val reactContext: ReactContext = this.getReactContext()

danilobuerger avatar Feb 26 '22 10:02 danilobuerger

@yogevbd I think we should leave this open until we also support the new architecture?

danilobuerger avatar Feb 28 '22 11:02 danilobuerger

Your'e right.

yogevbd avatar Feb 28 '22 12:02 yogevbd

I started looking at iOS + new arch (rnn 7.26.0, rn 0.68 rc2 with pending patched for rc3):

  • We have to manually enable rnn to use the farbic views (which currently is a good thing, but we should do that automatically) by adding the following pod:
pod 'ReactNativeNavigation/Fabric', :path => "../node_modules/react-native-navigation"

I got this to compile and work. So far only with waitToRender turned off and the views seem misaligned.

danilobuerger avatar Mar 09 '22 11:03 danilobuerger

When can we expect a release for 0.68.0 support since the stable released yesterday ?

YazeedAsaad avatar Mar 31 '22 11:03 YazeedAsaad

@YazeedAsaad rnn does work with rn 0.68 in its default configuration (old architecture).

danilobuerger avatar Mar 31 '22 15:03 danilobuerger

it does indeed, i meant the new Architecture if that’s planned

YazeedAsaad avatar Mar 31 '22 16:03 YazeedAsaad

I am not aware that any one is working on it. But feel free to submit a PR if changes are required to make it work with the new architecture.

danilobuerger avatar Mar 31 '22 16:03 danilobuerger

I would perfer if it was possible to opt into Fabric on a per screen basis as it would enable incremental adoption.

oblador avatar Apr 01 '22 06:04 oblador

@danilobuerger Do you have any pointers on how to use the old architecture in 0.68? Setting up a new RN project with npx react-native init seems to use the new architecture? I'm having a hard time trying to figure out how to setup rnn.

Maybe the run setup guide could be updated for 0.68?

LarsJK avatar Apr 01 '22 07:04 LarsJK

@LarsJK the new architecture is not enabled by default. The default setup with rn 0.68 and current rnn will just work as before.

danilobuerger avatar Apr 01 '22 08:04 danilobuerger

@danilobuerger Ok thanks then I guess I have other issues. Will make a new issue instead of taking over this one..

LarsJK avatar Apr 01 '22 08:04 LarsJK

Same issue. After npx react-native init and npx rnn-link: ios/app/AppDelegate.m was renamed to ios/app/AppDelegate.mm

doublex avatar Apr 01 '22 09:04 doublex

@doublex this issue is about getting rnn to work with the new architecture. So your issue is not related. Feel free to create a new one about the linker, or even better create a PR with a fix.

danilobuerger avatar Apr 01 '22 09:04 danilobuerger

@doublex this issue is about getting rnn to work with the new architecture. So your issue is not related. Feel free to create a new one about the linker, or even better create a PR with a fix.

I get where you're coming from, but both the docs and rnn-link do not work, so the happy path is broken, and this is probably why people are having issues. For those having issues on a freshly initialized 0.68 project, you can run npx rnn-link, then remove the body from MainActivity (or just comment it out):

public class MainActivity extends NavigationActivity {}

Without this, the build fails because NavigationActivity doesn't extend ReactActivity and doesn't have a createReactActivityDelegate method.

bfricka avatar Apr 25 '22 23:04 bfricka

Does anyone know if someone is already working in this? Not that I'm offering, just wanted to see if there is any progress

enahum avatar Jun 23 '22 16:06 enahum

Does anyone know if someone is already working in this? Not that I'm offering, just wanted to see if there is any progress

Please try to add this to settings.gradle: include ':react-native-navigation' project(':react-native-navigation').projectDir = new File(rootProject.projectDir, '../node_modules/react-native-navigation/lib/android/app/')

kentdev92 avatar Jun 27 '22 16:06 kentdev92

@guyca I noticed you worked on https://github.com/wix/react-native-navigation/pull/7518.

  • Does this add support for RN 0.68?
  • Should I close this ticket? Is it official, can we use the new architecture now?
  • If not, are there extra steps the RNN team should take to support that, and is there a place where you track the progress? Right now all I'm aware of is the current issue.

Thanks for all your hard work.

SudoPlz avatar Oct 04 '22 18:10 SudoPlz

@yogevbd @guyca – following up on @SudoPlz question: is it possible to use RNN with React Native 0.68+?

oferRounds avatar Nov 24 '22 18:11 oferRounds

@oferRounds Yes, we use it with RN 0.68.4.

guyca avatar Nov 24 '22 18:11 guyca

Thanks @guyca! so that means RNN use the new Arch?

oferRounds avatar Nov 24 '22 18:11 oferRounds

@guyca I noticed you worked on #7518.

  • Does this add support for RN 0.68?
  • Should I close this ticket? Is it official, can we use the new architecture now?
  • If not, are there extra steps the RNN team should take to support that, and is there a place where you track the progress? Right now all I'm aware of is the current issue.

Thanks for all your hard work.

@guyca any news with that?

Thank you 🙏

SudoPlz avatar Dec 20 '22 15:12 SudoPlz

@SudoPlz We're using RNN and RN 68 at Wix. AFAIK there are no issues with that. Regarding the new architecture - I think we plan on researching the possibility of switching to it sometime in Q1/Q2 2023. @yogevbd correct me if I'm wrong.

guyca avatar Dec 25 '22 07:12 guyca

Any updates?

ivan-khr85 avatar Mar 03 '23 00:03 ivan-khr85

Any updates?

erennyuksell avatar Jun 03 '23 11:06 erennyuksell

This has been implemented and rolled out on 7.36.0.

I managed to run our app using Fabric and RNN 7.36.0 but I did discover one issue: https://github.com/wix/react-native-navigation/issues/7770. I'll chat about it with @yogevbd

SudoPlz avatar Aug 11 '23 16:08 SudoPlz