[Bridgeless][IOS] `bridge` is `nil` on Swift when NewArch is enabled

lodev09 commented 3 weeks ago

Description

I maintain a native component library and trying to make it work with New Architecture interop. It builds without problem. However, the bridge in the view manager instance is nil and causing the component to crash.

To give context, the library is similar to RN Modal where it uses the view component as a host view of a view controller. It needs the bridge to attach RCTTouchHandler to the container view. The library is written in swift. https://github.com/lodev09/react-native-true-sheet

Any ideas or I'm missing something?

Steps to reproduce

Clone the library
Modify example/app.json and set newArchEnabled: true at line 38
Run the example project
Notice the crash

React Native Version

0.74.2

Affected Platforms

Runtime - iOS

Areas

Bridgeless - The New Initialization Flow

Output of `npx react-native info`

System:
  OS: macOS 14.5
  CPU: (10) arm64 Apple M1 Pro
  Memory: 355.75 MB / 16.00 GB
  Shell:
    version: "5.9"
    path: /bin/zsh
Binaries:
  Node:
    version: 18.20.3
    path: ~/.nvm/versions/node/v18.20.3/bin/node
  Yarn:
    version: 4.2.2
    path: ~/.nvm/versions/node/v18.20.3/bin/yarn
  npm:
    version: 10.8.1
    path: ~/.nvm/versions/node/v18.20.3/bin/npm
  Watchman: Not Found
Managers:
  CocoaPods:
    version: 1.15.2
    path: /opt/homebrew/bin/pod
SDKs:
  iOS SDK:
    Platforms:
      - DriverKit 23.5
      - iOS 17.5
      - macOS 14.5
      - tvOS 17.5
      - visionOS 1.2
      - watchOS 10.5
  Android SDK: Not Found
IDEs:
  Android Studio: 2023.2 AI-232.10300.40.2321.11668458
  Xcode:
    version: 15.4/15F31d
    path: /usr/bin/xcodebuild
Languages:
  Java:
    version: 17.0.10
    path: /usr/bin/javac
  Ruby:
    version: 3.0.6
    path: /Users/lodev09/.rbenv/shims/ruby
npmPackages:
  "@react-native-community/cli": Not Found
  react:
    installed: 18.2.0
    wanted: 18.2.0
  react-native:
    installed: 0.74.2
    wanted: 0.74.2
  react-native-macos: Not Found
npmGlobalPackages:
  "*react-native*": Not Found
Android:
  hermesEnabled: true
  newArchEnabled: true
iOS:
  hermesEnabled: true
  newArchEnabled: true

Stacktrace or Logs

TrueSheet/TrueSheetViewManager.swift:22: Fatal error: Unexpectedly found nil while implicitly unwrapping an Optional value

Reproducer

https://github.com/lodev09/react-native-true-sheet/tree/main/example

Screenshots and Videos

cipolleschi commented 2 weeks ago

Hi @lodev09, thanks for maintaining a library for React Native and thank you for the issue. As the name implies, the Bridgless mode means No Bridge. So it makes sense that the bridge instance is actually nil: no bridge is actually initialized.

There are two questions that I need to ask you:

Are you migrating the component to Fabric?
Are you trying to use the component throught the Interop Layer?

My guess is that nobody is setting the bridge in the ViewManager, after it is created: in bridgeless mode, that's kind of expected, given that there is no bridge. But in theory it should work thanks to the Interop Layer.

I'll have a look at it later today or tomorrow.

lodev09 commented 2 weeks ago

As the name implies, the Bridgless mode means No Bridge. So it makes sense that the bridge instance is actually nil: no bridge is actually initialized.

Hmmm, interesting. I'm wondering how the RN Modal works on bridgeless then? It uses exactly the same code to set the TouchHandler -- the only difference is that my library is written in swift.

Are you migrating the component to Fabric?

Are you trying to use the component throught the Interop Layer?

I'm trying to run it through Interop layer first and then ultimately fabric. As I mentioned, everything works fine aside from the bridge being nil.

I just need the TouchHandler to work on New Arch :/

cipolleschi commented 2 weeks ago

I'm wondering how the RN Modal works on bridgeless then?

RN Modals are actually already migrated to Fabric. When you run the new architecture, the component that is loaded is this one.

cipolleschi commented 2 weeks ago

Ok, I got the issue. I'll work on a fix as there could be other libraries that rely on the bridge property to be set and it is currently not set in the interop layer.

I hope I'll be able to send the fix out before EOW.

lodev09 commented 2 weeks ago

Okay.. thank you so much @cipolleschi 🙏

lodev09 commented 2 weeks ago

Quick question: If I understand correctly, swift is not yet supported for Fabric components right?

cipolleschi commented 2 weeks ago

swift is not yet supported for Fabric components right?

Tricky question! Technically, no, it is not supported. What we mean with that is that you can't create a Fabric component using only Swift.

However, you can create a thin wrapper in objective-c++ and then forward the various calls to Swift. We haven't experimented with that setup a lot, and we are aware that there are some things that are not compatible and that will require some wrapping to make them work in Swift.

cipolleschi commented 2 weeks ago

So.. https://github.com/facebook/react-native/pull/45329 should fix the crash. But, from what I am seeing, the buttons are not tappable... :/

lodev09 commented 2 weeks ago

Yeah.. I use the bridge to mainly setSize and create an RCTTouchHandler instance to be attached to the container view. I'm hoping this is supported on the Interop layer.

https://github.com/lodev09/react-native-true-sheet/blob/643f4e8f39aabfe1403de74042b25bdb392f18ce/ios/TrueSheetView.swift#L59

cipolleschi commented 2 weeks ago

I need to ask around and investigate... I'll keep you posted!

cipolleschi commented 2 weeks ago

That was quick... xD I found that:

RCTTouchHandler is for the Old Arch
RCTSurfaceTouchHandler is for the New Arch.

I made a quick experiment in TrueSheetView.swift:

// ...
class TrueSheetView: UIView, RCTInvalidating, TrueSheetViewControllerDelegate {
//...
  private var touchHandler: RCTTouchHandler
+  private var surfaceTouchHandler: RCTSurfaceTouchHandler
// ..
init(with bridge: RCTBridge) {
// ...
    touchHandler = RCTTouchHandler(bridge: bridge)
+    surfaceTouchHandler = RCTSurfaceTouchHandler()
//...

override func insertReactSubview(_ subview: UIView!, at index: Int) {
// ...

    containerView = subview
-    touchHandler.attach(to: containerView)
+    touchHandler.attach(to: subview)
+    surfaceTouchHandler.attach(to: subview)
  }
//..
  override func removeReactSubview(_ subview: UIView!) {
 //...

    touchHandler.detach(from: subview)
+    surfaceTouchHandler.detach(from: subview)

//...

And, of course, adding the following in the bridging header

#import <React/RCTTouchHandler.h>
+ #import <React/RCTSurfaceTouchHandler.h>
#import <React/RCTScrollView.h>

https://github.com/facebook/react-native/assets/11162307/83ddc883-9b49-4f17-9909-9d94588777bc

sizings looks a bit off, though... :(

lodev09 commented 2 weeks ago

Wow thank you! :D Just need a new arch condition and should be go to go with the touch handler!

sizings looks a bit off, though... :(

Yeah, looks like bridge.uiManager.setSize isn't working... is there any alternative? 😅 Code is here: https://github.com/lodev09/react-native-true-sheet/blob/643f4e8f39aabfe1403de74042b25bdb392f18ce/ios/TrueSheetView.swift#L147-L152

cipolleschi commented 2 weeks ago

It turns out, that method was not implemented in the interop layer for Bridgeless. I'll try to work on that tomorrow!

lodev09 commented 2 weeks ago

You're awesome @cipolleschi. Thank you so much!

cipolleschi commented 2 weeks ago

Thank you for reporting the issue and for providing a way for me to work on that!

(Anyway... start thinking about migrating it to Fabric! 😅 It will be needed, at some point! Happy to help there as well)

lodev09 commented 2 weeks ago

@cipolleschi yes, I'm really eager to migrate it to fabric -- it's just a bummer that swift isn't supported out of the box. But we'll get there 💪

lodev09 commented 2 weeks ago

This is my first native ios/android project so I'm not 100% familiar with all this yet 😅

cipolleschi commented 2 weeks ago

@lodev09 I looked into this issue a little bit more. The method that is missing is RCTUIManager setSize:forView:.

The reason why it hasn't been implemented in the Interop Layer is architectural. In the Old Architecture, we have a native layer that creates a tree of views. Every view is backed by a shadow view that is used to compute the layout. The JS React tree talks to the platform and creates the Native tree and the Shadow tree at the same time. The old RCTUIManager has a mapping between native view and shadow view. The native layer is the source of truth.

In the New Architecture, we have a shadow tree implementation in C++. In the New Architecture, the Shadow Tree drives everything and it creates the Native tree. The Native views don't have a handle to get the shadow views anymore and they can't really change the layout from the Native layer. The shadow layer is the source of truth.

Due to this, we cannot really port the setSize method back to the Interop Layer. I searched on Github and there are not a lot of libraries that uses that method (< 1% of all the React Native libraries) so it does not really make sense to support this API due to its architectural incompatibility with the New Architecture and its low usage.

My suggestion would be to migrate your library to Fabric properly and support both architectures.

We have some guides on how to do it:

And you can use the New Architecture implementation of the React Native Modal component as base implementation for the Fabric version of it.

I would also love to help with review and suggestions on how to bridge ObjC++ with Swift. I made some experiments with modules in the past, here and here, but I never had the time to try with Componentes.

I hope these pointers can help you move forward with this.

lodev09 commented 2 weeks ago

@cipolleschi I see. I'm going to implement an alternative approach to updating the size as a short term solution for now, maybe in the JS side.

I agree with migrating to Fabric for the long term. I might just convert it all to ObjC 😅

Anyways, thanks again for your help!

facebook / react-native