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

Prevent iOS bridgeless initialization from getting the wrong bridge #5953

Conversation

gabrieldonadel
Copy link
Contributor

Summary

As I pointed out here, using [RCTBridge currentBridge] may lead to the wrong bridge in apps that have multiple React instances, as is the case for expo-dev-client.

RCTCxxBridge *cxxBridge = (RCTCxxBridge *)[RCTBridge currentBridge];

Fixes #5497

Instead of using [RCTBridge currentBridge] we can just call self.bridge and ensure the correct bridge is always used

Test plan

Run FabricExample app on iOS

Copy link
Member

@tomekzaw tomekzaw left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

@tomekzaw
Copy link
Member

I wonder if there's anything we can do to prevent us from using currentBridge in the future, maybe some CI checks?

@tsapeta
Copy link
Member

tsapeta commented Apr 30, 2024

I wonder if there's anything we can do to prevent us from using currentBridge in the future, maybe some CI checks?

Why would you need that? This API is going to be deprecated and removed anyway, like everything that relates to the bridge. Also, it's always risky to depend on global things like this. See the comment for currentBridge: https://github.com/facebook/react-native/blob/6262158c11709ea0cbc7603732608046296f39e0/packages/react-native/React/Base/RCTBridge.mm#L252-L261

@tomekzaw
Copy link
Member

Why would you need that?

To prevent future us from using this API.

This API is going to be deprecated and removed anyway

Okay, that sounds good

@piaskowyk piaskowyk added this pull request to the merge queue Apr 30, 2024
Merged via the queue into software-mansion:main with commit 2e729a7 Apr 30, 2024
12 checks passed
@gabrieldonadel gabrieldonadel deleted the @gabrieldonadel/fix-dev-client-compatibility branch April 30, 2024 13:46
@mym0404
Copy link

mym0404 commented May 1, 2024

I'm looking for a way to pass a bridge from ViewManager to View in Fabric.

It seems this code is not run in Fabric ViewManager class.

- (UIView*)view {
  RNCNaverMapViewImpl* ret = [[RNCNaverMapViewImpl alloc] init];
  ret.bridge = self.bridge;

  return ret;
}

Is there any example inject Bridge into View from Fabric ViewManager? Any suggestion or example github repo is welcome.

Note

I need RCTBridge in View because I should use RCTImageLoader for fetch some network image for View

@tsapeta
Copy link
Member

tsapeta commented May 1, 2024

@mym0404 As far as I know, there is no way to achieve this. Fabric creates the view instance itself. I think this is intentional as the views shouldn't have any dependencies other than props in order to work properly with the view recycling mechanism.

@mym0404
Copy link

mym0404 commented May 1, 2024

@tsapeta understood, thank you for response.

j-piasecki pushed a commit to software-mansion/react-native-gesture-handler that referenced this pull request May 9, 2024
## Description

This PR fixes getting access to right bridge, because `currentBridge`
method can return invalid pointer when more than one instance of react
exists in the application. Same changes we made in the Reanimated -
software-mansion/react-native-reanimated#5953

## Test plan

I tested FabricExample on iOS with/without bridge, after startup and
after reload, and it seems to work.
WoLewicki pushed a commit to software-mansion/react-native-screens that referenced this pull request May 21, 2024
…2146)


Similar to
software-mansion/react-native-reanimated#5953,
using `[RCTBridge currentBridge]` may lead to the wrong bridge in apps
that have multiple React instances, as is the case for expo-dev-client.
This results in apps sometimes crashing when reloading the bundle after
interacting with the DevMenu.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants