Skip to content

Commit

Permalink
Always return an EventDispatcher in bridgeless mode
Browse files Browse the repository at this point in the history
Summary:
This changes how we access the EventDispatcher from the FabricUIManager in bridgeless mode.

Currently, we have implemented a similar API to what we use for Fabric (used in UIManagerHelper): `BridgelessReactContext.getJSIModule(UIManager).getEventDispatcher()`. However, `getJSIModule` does not have a nullable return type, which means that we have to throw an exception if the UIManager can't be found - if, for example, the instance is not initialized yet (or has been destroyed). This is causing crashes when a view tries to access the EventDispatcher before the instance is initialized, which takes longer for Venice because we include JS bundle loading as part of initialization (we may need to revisit that).

Ideally, we'd like to create a first-class API for `getEventDispatcher()`, and make sure that it never crashes if the instance is destroyed, because we don't care if JS events aren't delivered at that point. However, there are some obstacles to making this change for the bridge - mostly related to avoiding circular dependencies between the bridge module and the uimanager module. (Also, this might be a behavior change for the bridge, because I think we currently start queueing events before it's initialized? and product code might be relying on that).

Reviewed By: mdvacca

Differential Revision: D21672949

fbshipit-source-id: a38e96cd40c6f70124b7ca2a5c9722988fe7fcf4
  • Loading branch information
Emily Janzer authored and facebook-github-bot committed May 27, 2020
1 parent fd97e95 commit 0a12f3e
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
import android.app.Activity;
import android.content.Context;
import androidx.annotation.Nullable;
import com.facebook.react.bridge.JSIModule;
import com.facebook.react.bridge.JSIModuleType;
import com.facebook.react.bridge.LifecycleEventListener;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.bridge.ReactContext;
Expand Down Expand Up @@ -71,16 +69,12 @@ public boolean hasCurrentActivity() {
return mSurfaceID;
}

@Override
public boolean isBridgeless() {
return mReactApplicationContext.isBridgeless();
public ReactApplicationContext getReactApplicationContext() {
return mReactApplicationContext;
}

@Override
public JSIModule getJSIModule(JSIModuleType moduleType) {
if (isBridgeless()) {
return mReactApplicationContext.getJSIModule(moduleType);
}
return super.getJSIModule(moduleType);
public boolean isBridgeless() {
return mReactApplicationContext.isBridgeless();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.facebook.react.bridge.UIManager;
import com.facebook.react.uimanager.common.UIManagerType;
import com.facebook.react.uimanager.events.EventDispatcher;
import com.facebook.react.uimanager.events.EventDispatcherProvider;

/** Helper class for {@link UIManager}. */
public class UIManagerHelper {
Expand All @@ -43,32 +44,28 @@ private static UIManager getUIManager(
ReactContext context,
@UIManagerType int uiManagerType,
boolean returnNullIfCatalystIsInactive) {
if (context.isBridgeless()) {
return (UIManager) context.getJSIModule(JSIModuleType.UIManager);
} else {
if (!context.hasCatalystInstance()) {
ReactSoftException.logSoftException(
"UIManagerHelper",
new ReactNoCrashSoftException(
"Cannot get UIManager because the context doesn't contain a CatalystInstance."));
if (!context.hasCatalystInstance()) {
ReactSoftException.logSoftException(
"UIManagerHelper",
new ReactNoCrashSoftException(
"Cannot get UIManager because the context doesn't contain a CatalystInstance."));
return null;
}
// TODO T60461551: add tests to verify emission of events when the ReactContext is being turn
// down.
if (!context.hasActiveCatalystInstance()) {
ReactSoftException.logSoftException(
"UIManagerHelper",
new ReactNoCrashSoftException(
"Cannot get UIManager because the context doesn't contain an active CatalystInstance."));
if (returnNullIfCatalystIsInactive) {
return null;
}
// TODO T60461551: add tests to verify emission of events when the ReactContext is being turn
// down.
if (!context.hasActiveCatalystInstance()) {
ReactSoftException.logSoftException(
"UIManagerHelper",
new ReactNoCrashSoftException(
"Cannot get UIManager because the context doesn't contain an active CatalystInstance."));
if (returnNullIfCatalystIsInactive) {
return null;
}
}
CatalystInstance catalystInstance = context.getCatalystInstance();
return uiManagerType == FABRIC
? (UIManager) catalystInstance.getJSIModule(JSIModuleType.UIManager)
: catalystInstance.getNativeModule(UIManagerModule.class);
}
CatalystInstance catalystInstance = context.getCatalystInstance();
return uiManagerType == FABRIC
? (UIManager) catalystInstance.getJSIModule(JSIModuleType.UIManager)
: catalystInstance.getNativeModule(UIManagerModule.class);
}

/**
Expand All @@ -87,6 +84,13 @@ public static EventDispatcher getEventDispatcherForReactTag(ReactContext context
@Nullable
public static EventDispatcher getEventDispatcher(
ReactContext context, @UIManagerType int uiManagerType) {
// TODO T67518514 Clean this up once we migrate everything over to bridgeless mode
if (context.isBridgeless()) {
if (context instanceof ThemedReactContext) {
context = ((ThemedReactContext) context).getReactApplicationContext();
}
return ((EventDispatcherProvider) context).getEventDispatcher();
}
UIManager uiManager = getUIManager(context, uiManagerType, false);
return uiManager == null ? null : (EventDispatcher) uiManager.getEventDispatcher();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

package com.facebook.react.uimanager.events;

import com.facebook.common.logging.FLog;

/**
* A singleton class that overrides {@link EventDispatcher} with no-op methods, to be used by
* callers that expect an EventDispatcher when the instance doesn't exist.
*/
public class BlackHoleEventDispatcher implements EventDispatcher {

private static final EventDispatcher sEventDispatcher = new BlackHoleEventDispatcher();

public static EventDispatcher get() {
return sEventDispatcher;
}

private BlackHoleEventDispatcher() {}

@Override
public void dispatchEvent(Event event) {
FLog.d(
getClass().getSimpleName(),
"Trying to emit event to JS, but the React instance isn't ready. Event: "
+ event.getEventName());
}

@Override
public void dispatchAllEvents() {}

@Override
public void addListener(EventDispatcherListener listener) {}

@Override
public void removeListener(EventDispatcherListener listener) {}

@Override
public void addBatchEventDispatchedListener(BatchEventDispatchedListener listener) {}

@Override
public void removeBatchEventDispatchedListener(BatchEventDispatchedListener listener) {}

@Override
public void registerEventEmitter(int uiManagerType, RCTEventEmitter eventEmitter) {}

@Override
public void unregisterEventEmitter(int uiManagerType) {}

@Override
public void onCatalystInstanceDestroyed() {}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/*
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

package com.facebook.react.uimanager.events;

/**
* An interface that can be implemented by a {@link com.facebook.react.bridge.ReactContext} to
* provide a first-class API for accessing the {@link EventDispatcher} from the {@link
* com.facebook.react.bridge.UIManager}.
*/
public interface EventDispatcherProvider {

/**
* This method should always return an EventDispatcher, even if the instance doesn't exist; in
* that case it should return the empty {@link BlackHoleEventDispatcher}.
*
* @return An {@link EventDispatcher} to emit events to JS.
*/
EventDispatcher getEventDispatcher();
}

0 comments on commit 0a12f3e

Please sign in to comment.