voize-gmbh / reakt-native-toolkit

Combine React Native with Kotlin Multiplatform (KMP)
Apache License 2.0
134 stars 5 forks source link

[Question] Why do we need to override `AppDelegate.createBridgeWithDelegate`? #72

Open rocketraman opened 1 month ago

rocketraman commented 1 month ago

The example app overrides createBridgeWithDelegate to pass self rather than delegate. This seems to be necessary in React 0.74.5, otherwise the extraModulesForBridge method is never called.

Is this an upstream bug that we are working around? Might there be any unintended side-effects of this?

rocketraman commented 1 month ago

And an update... yes, it appears this approach is not compatible with Expo... returning self seems to break Expo's internal loading mechanisms. So I'd love to understand what is going on here.

erksch commented 1 month ago

I updated the code to reflect the latest initial setup of RN 0.74.0 setup (see here).

Before that, we could initialize the bridge ourself and assign the delegate using

RCTBridge *bridge = [[RCTBridge alloc] initWithDelegate:self launchOptions:launchOptions];

see AppDelegate.mm at this point

But the new setup did not include manually initializing the bridge, probably to simplify the setup with the new architecture. Because of this I looked for another way to set the bridge delegate and found that overwriting createBridgeWithDelegate was the best option.

But I am happy to use a different solution. If self is the problem we could also create a separate class for the bridge delegate that contains extraModulesForBridge.

rocketraman commented 1 month ago

But I am happy to use a different solution. If self is the problem we could also create a separate class for the bridge delegate that contains extraModulesForBridge.

This approach seems to work for me. To be specific, I did this:

// CustomBridgeDelegate.h
#import <Foundation/Foundation.h>
#import <React/RCTBridgeDelegate.h>
#import "AppDelegate.h"

@interface CustomBridgeDelegate : NSObject<RCTBridgeDelegate> 

- (instancetype)initWithDelegates:(AppDelegate *)appDelegate
                   originalDelegate:(id<RCTBridgeDelegate>)originalDelegate;

@end

// CustomBridgeDelegate.mm
#import "CustomBridgeDelegate.h"
#import <MobileSdk/MobileSdk.h>
#import <Expo/Expo.h>
#import "AppDelegate.h"
#import <React/RCTLinkingManager.h>
#import <React/RCTBridgeDelegate.h>

@interface CustomBridgeDelegate ()

@property (nonatomic, weak) AppDelegate* appDelegate;
@property (nonatomic, weak) id<RCTBridgeDelegate> originalDelegate;

@end

@implementation CustomBridgeDelegate

- (instancetype)initWithDelegates:(AppDelegate *)appDelegate
                   originalDelegate:(id<RCTBridgeDelegate>)originalDelegate {
    self = [super init];
    if (self) {
        self.appDelegate = appDelegate;
        self.originalDelegate = originalDelegate;
    }
    return self;
}

#pragma mark - RCTBridgeDelegate Methods

- (NSArray<id<RCTBridgeModule>> *)extraModulesForBridge:(RCTBridge *)bridge {
    // NSLog(@"[CustomBridgeDelegate] extraModulesForBridge called");
    NSArray<id<RCTBridgeModule>> *rnNativeModules = [self.appDelegate.mobileSdk createNativeModules];
    NSArray<id<RCTBridgeModule>> *rnViewManagers = @[];
    NSArray *customModules = [rnNativeModules arrayByAddingObjectsFromArray:rnViewManagers];

    // Get modules from the original delegate, if any
    NSArray *originalModules = nil;
    if ([self.originalDelegate respondsToSelector:@selector(extraModulesForBridge:)]) {
        originalModules = [self.originalDelegate extraModulesForBridge:bridge];
    }

    // Combine both arrays
    NSMutableArray *allModules = [NSMutableArray arrayWithArray:customModules];
    if (originalModules) {
        [allModules addObjectsFromArray:originalModules];
    }

    return allModules;
}

// Forward sourceURLForBridge to the original delegate (an Expo class)
- (NSURL *)sourceURLForBridge:(RCTBridge *)bridge {
    if ([self.originalDelegate respondsToSelector:@selector(sourceURLForBridge:)]) {
        return [self.originalDelegate sourceURLForBridge:bridge];
    }
    NSLog(@"[CustomBridgeDelegate] Warning: originalDelegate does not implement sourceURLForBridge");
    return nil;
}

@end

and then in AppDelegate.mm:

- (RCTBridge *)createBridgeWithDelegate:(id<RCTBridgeDelegate>)delegate launchOptions:(NSDictionary *)launchOptions
{
  // this does not hook extraModulesForBridge properly, and passing :this instead of :delegate does, but
  // does not hook the sourceURLForBridge properly -- create a CustomBridgeDelegate that implements both of these
  // return [[RCTBridge alloc] initWithDelegate:delegate launchOptions:launchOptions];

  // Create the custom delegate, passing this and the delegate
  CustomBridgeDelegate *customDelegate = [[CustomBridgeDelegate alloc] initWithDelegates:self
                                                                        originalDelegate:(RCTAppDelegate *)delegate];

  // Initialize the bridge with the custom delegate
  return [[RCTBridge alloc] initWithDelegate:customDelegate launchOptions:launchOptions];
}

It feels like a hack rather than a proper solution though -- I'm not really sure I understand how the React Native guys intended for the RCTBridgeDelegate to be customized. RCTBridgeDelegate is a protocol, but an instance of it is passed to RCTAppDelegate.mm, and I can't find the code in React Native that actually creates this instance -- the closest seems to be a call in RCTRootViewFactory.mm that calls it with self. Expo is customizing this delegate somehow as well.

rocketraman commented 3 weeks ago

It seems that this approach does not support the "reload" function of Expo in iOS.

When the app is initially loaded, the RN modules are loaded as well. However, when "Reload" is executed from the developer menu (or any changes to RN files which in effect does the "Reload" behind the scenes), it appears that the native modules are unloaded, and on reload none of the CustomDelegate functions are called at all. The result is that the app fails with the error that happens when the native modules are not linked.

Accordingly, the DX on iOS is terrible at the moment, and it would be really nice to know how to work around this.

Alternatively, we could try to plug in to the "official" way of loading React Native modules in iOS which would use the RCT_EXPORT_MODULE() macro, however I was not able to figure out how to call this from Kotlin. And even this is deprecated -- the new architecture has a different approach entirely.

rocketraman commented 3 weeks ago

I'm wondering if this commit in Expo will solve it -- it seems they've added some explicit logic for extraModulesForBridge.