[RN73] Support for iOS

[d4vidi]

No description provided.

[SudoPlz]

hey @d4vidi can I help with that?
I could open a PR if you're willing to review, because I'm currently blocked on this.

[gosha212]

Hi @SudoPlz,
I'm currently working on this task. It will be ready soon. This task is hard to estimate because it is a research task.

[SudoPlz]

@gosha212

allright.

By the way I did figure out some of the changes that are required:

diff --git a/node_modules/react-native-navigation/lib/ios/RNNReactView.h b/node_modules/react-native-navigation/lib/ios/RNNReactView.h
index 62850dc..9c83269 100644
--- a/node_modules/react-native-navigation/lib/ios/RNNReactView.h
+++ b/node_modules/react-native-navigation/lib/ios/RNNReactView.h
@@ -1,5 +1,6 @@
 #ifdef RCT_NEW_ARCH_ENABLED
-#import <React/RCTFabricSurfaceHostingProxyRootView.h>
+#import <React/RCTSurfaceHostingProxyRootView.h>
+#import <React/RCTFabricSurface.h>
 #else
 #import <React/RCTRootView.h>
 #endif
@@ -32,7 +33,7 @@ typedef void (^RNNReactViewReadyCompletionBlock)(void);
 
 #ifdef RCT_NEW_ARCH_ENABLED
 @interface RNNReactView
-    : RCTFabricSurfaceHostingProxyRootView <RCTRootViewDelegate, RNNComponentProtocol>
+    : RCTSurfaceHostingProxyRootView <RCTRootViewDelegate, RNNComponentProtocol>
 #else
 @interface RNNReactView : RCTRootView <RCTRootViewDelegate, RNNComponentProtocol>
 #endif
diff --git a/node_modules/react-native-navigation/lib/ios/RNNReactView.m b/node_modules/react-native-navigation/lib/ios/RNNReactView.m
index fec6fc6..4c86f23 100644
--- a/node_modules/react-native-navigation/lib/ios/RNNReactView.m
+++ b/node_modules/react-native-navigation/lib/ios/RNNReactView.m
@@ -1,6 +1,5 @@
 #import "RNNReactView.h"
 #import <React/RCTRootContentView.h>
-
 @implementation RNNReactView {
     BOOL _isAppeared;
 }
@@ -11,10 +10,12 @@ - (instancetype)initWithBridge:(RCTBridge *)bridge
                   eventEmitter:(RNNEventEmitter *)eventEmitter
                sizeMeasureMode:(RCTSurfaceSizeMeasureMode)sizeMeasureMode
            reactViewReadyBlock:(RNNReactViewReadyCompletionBlock)reactViewReadyBlock {
-    self = [super initWithBridge:bridge
-                      moduleName:moduleName
-               initialProperties:initialProperties
-                 sizeMeasureMode:sizeMeasureMode];
+
+    id<RCTSurfaceProtocol> surface = [[RCTFabricSurface alloc] initWithBridge:bridge
+                                                                       moduleName:moduleName
+                                                            initialProperties:initialProperties];
+    self = [super initWithSurface:surface
+                  sizeMeasureMode:sizeMeasureMode];
 #else
 - (instancetype)initWithBridge:(RCTBridge *)bridge
                     moduleName:(NSString *)moduleName

but now I'm stuck on 'iosfwd' file not found and 'memory' file not found errors.
If you find a way to get this library to build, I'll convert your changes to a patchfile so that me and others can use it sooner (before the PR actually lands).

[gosha212]

@SudoPlz I did the Andorid part and currently working on the typescript. Hope to get to it very soon. I will share a link to a draft PR in the beginning of the next week.

[oblador]

@SudoPlz you can get past that by replacing the fabric if statement in the podspec with:

  if fabric_enabled
    install_modules_dependencies(s)
    s.requires_arc    = true
  end

[gosha212]

@SudoPlz #7844 here is the PR

[oblador]

@gosha212 Awesome – thank you, however it only fixes Android? iOS Fabric is still broken for me.

[gosha212]

@oblador I didn't test it with Fabric yet. The tests are working on both platforms

[oblador]

Ah, ok. @SudoPlz's comment is about Fabric on iOS hence the confusion.

[gosha212]

@oblador The code works with Fabric on Android. IOS still WIP

[gosha212]

@oblador the latest commit should work with Fabric. The fix is not merged yet.

[oblador]

@gosha212 Thanks! I tried it out and it does not work with fabric and RN 0.73.4 for me. In the pod spec you are overwriting all the config set by react native. I got everything working with this more minimal setup:

  if fabric_enabled
    s.pod_target_xcconfig = {
      'HEADER_SEARCH_PATHS' => '"$(PODS_ROOT)/Headers/Private/React-Core"',
    }
    s.requires_arc = true
    install_modules_dependencies(s)
  end

[asafkorem]

@gosha212 Thanks! I tried it out and it does not work with fabric and RN 0.73.4 for me. In the pod spec you are overwriting all the config set by react native. I got everything working with this more minimal setup:

  if fabric_enabled
    s.pod_target_xcconfig = {
      'HEADER_SEARCH_PATHS' => '"$(PODS_ROOT)/Headers/Private/React-Core"',
    }
    s.requires_arc = true
    install_modules_dependencies(s)
  end

Hey @oblador, in the podspec we just add React's headers to the search paths and declare some dependencies. What error are you getting? can you please copy it here?

[asafkorem]

@SudoPlz @oblador please try the latest released version (7.38.0)

[oblador]

@asafkorem That version fails for me during install with:

node_modules/react-native-navigation: Command failed.
Exit code: 1
Command: node scripts/postinstall.js

The issue is overwriting search paths and c++ flags causing my build to fail for failing to import memory et al. install_modules_dependencies will append, therefore it should be placed after not before your own configs

[oblador]

@asafkorem have a fix for the postinstall bug here: #7849

[asafkorem]

Thanks @oblador, checking 🙏🏼

[SudoPlz]

Waiting for the merged code to be released. 0.38 fails on my end too because of the post install script.

[asafkorem]

@oblador @SudoPlz please try 7.38.1

[SudoPlz]

I did, everything builds fine. The only problem is, I get a blank screen and no react components show up at all when fabric is enabled.

[oblador]

Yup same for me, it installs and builds now, but I'm seeing an empty screen.

[SudoPlz]

so glad it's not just me.

[asafkorem]

@oblador @SudoPlz
We'll look into this. Does the issue occur on iOS, Android, or both? Also, does this issue persist in the most recent version when Fabric is disabled as well, or only when Fabric is enabled?

[SudoPlz]

@oblador @SudoPlz We'll look into this. Does the issue occur on iOS, Android, or both? Also, does this issue persist in the most recent version when Fabric is disabled as well, or only when Fabric is enabled?

I've only tested iOS so far.

It only happens when fabric is enabled. (when it's disabled everything works fine)

[asafkorem]

OK, thanks for reporting. I'll test that with a clean RN+RNN app.
Also, can you please list the other public dependencies you're using in your app? I wonder if there might be another suspect. Our playground app plays well with the latest version 🤔

[asafkorem]

FYI @gosha212 @d4vidi @SudoPlz @oblador

I'm able to reproduce this issue on iOS with a new RN 0.73.4 + Fabric + RNN 7.38.1 app
When disabling fabric - app works fine.

For some reason it doesn't reproduce on our playground app.

I'll continue the investigation later today. Will update.

[SudoPlz]

@asafkorem super happy you were able to repro this one.
Did you track the root cause of the issue by any chance?

[asafkorem]

Hi @SudoPlz, I made a brief investigation to identify the underlying issue but haven't uncovered anything so far :(
At the moment, we aren't focused on this, but we plan to revisit it shortly. It is prioritized

[SudoPlz]

Hey @asafkorem, no pressure or anything, but I'm curious, what's the blocker?

[SudoPlz]

@asafkorem or is there any way I can help contribute to help move along the migration?
It's important for me to start working towards fabric, and I'm not confident I understand what's blocking it at the moment, so if there's anything I can do to help I'd be happy to.

[asafkorem]

Hey @SudoPlz, this issue requires investigation and we're not working on it at the moment. We plan to get there soon but for the meanwhile feel free to clone the reproduction repo https://github.com/asafkorem/react-native-navigation-demo and investigate why we get this white screen when Fabric is enabled.
Thanks for the willingness to help!

Also, if we'll have any update from our end we'll post it here.

[SudoPlz]

Hey @d4vidi why was this closed since it's still bugging?
Is there another issue responsible for fixing React Native fabric?

[d4vidi]

Hey @d4vidi why was this closed since it's still bugging? Is there another issue responsible for fixing React Native fabric?

Hey @SudoPlz, yes - we will deal with fabric under a different context, in due time. Contribution (upgrading and surfacing the issues) would definitely help speed things up 🙏🏻

[gosha212]

@SudoPlz here is a ticket for tracking: #7836
As @d4vidi mentioned. We will be happy to get some contribution here =)

[TheLonelyAstronaut]

@d4vidi @gosha212 added fix in issue #7865