Skip to content

Commit

Permalink
Use bridgeless flag instead of specific binding in UIManagerBinding
Browse files Browse the repository at this point in the history
Summary:
This fix is still a little hypothetical. We have a few different JS errors that we're seeing with bridgeless mode that seem to be caused by Fabric trying to access `__fbBatchedBridge` from C++. I think what's happening is:

1. User encounters an unrelated JS error very early in rendering a new surface (possibly while the bundle is still loading?)
2. In release builds, BridgelessReactFragment handles the error by stopping the surface and rendering a retry button (actually, the surface is stopped in a bunch of places in BaseFbReactFragment, which might be why this is popping up now - I recently refactored that class to share more of its logic in bridgeless mode)
3. Fabric stops the surface by first checking to see if the custom binding `RN$stopSurface` exists; if not, it falls back to calling the registered callable module `ReactFabric`.

I think #3 is where things are going wrong for bridgeless mode; if you call stopSurface before `RN$stopSurface` is installed (which happens when ReactFabric shim is required) then you'll fall back to the bridge version.

My solution here is to instead rely on a flag set in C++ to determine whether we're in bridgeless mode, and then check to see if the stopSurface binding has been installed. If not, we just noop - if the ReactFabric shim hasn't been required, we probably don't actually have a React surface that needs to be stopped.

At least, that's my current theory. We'll see if this actually works.

Changelog: [Fixed][iOS] Fix an issue calling stopSurface in bridgeless mode before surface is started

Reviewed By: mdvacca

Differential Revision: D25453696

fbshipit-source-id: bff76675c43989101d0ba5ae0aba60089db230bf
  • Loading branch information
Emily Janzer authored and facebook-github-bot committed Dec 11, 2020
1 parent 11cf9ca commit 8109690
Showing 1 changed file with 9 additions and 4 deletions.
13 changes: 9 additions & 4 deletions ReactCommon/react/renderer/uimanager/UIManagerBinding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,15 @@ void UIManagerBinding::startSurface(

void UIManagerBinding::stopSurface(jsi::Runtime &runtime, SurfaceId surfaceId)
const {
if (runtime.global().hasProperty(runtime, "RN$stopSurface")) {
auto method =
runtime.global().getPropertyAsFunction(runtime, "RN$stopSurface");
method.call(runtime, {jsi::Value{surfaceId}});
auto global = runtime.global();
if (global.hasProperty(runtime, "RN$Bridgeless")) {
if (!global.hasProperty(runtime, "RN$stopSurface")) {
// ReactFabric module has not been loaded yet; there's no surface to stop.
return;
}
// Bridgeless mode uses a custom JSI binding instead of callable module.
global.getPropertyAsFunction(runtime, "RN$stopSurface")
.call(runtime, {jsi::Value{surfaceId}});
} else {
auto module = getModule(runtime, "ReactFabric");
auto method =
Expand Down

0 comments on commit 8109690

Please sign in to comment.