Skip to content

Commit

Permalink
Removing JSI Module (facebook#39545)
Browse files Browse the repository at this point in the history
Summary:
X-link: facebookresearch/playtorch#231


1. Refactoring `FabricJSIModuleProvider` to implement the newly added `UIManagerProvider` interface instead of the `JSIModuleProvider` in order to get rid of JSI Module thereby renaming it to `FabricUIManagerProviderImpl`

2. Adding  `setTurboModuleRegistry(TurboModuleRegistry turboModuleRegistry)`, `setFabricUIManager(UIManager fabricUIManager)`, `getFabricUIManager()` instead of `setTurboModuleManager(JSIModule getter)`

3. Replacing `mJSIModuleRegistry.notifyJSInstanceDestroy()` with `mTurboModuleRegistry.invalidate();`

Changelog:
[Internal] internal

Differential Revision: D49259735
  • Loading branch information
arushikesarwani94 authored and facebook-github-bot committed Oct 16, 2023
1 parent 4466001 commit c2d4a67
Show file tree
Hide file tree
Showing 11 changed files with 128 additions and 156 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
import android.view.View;
import android.view.ViewGroup;
import androidx.annotation.Nullable;
import androidx.core.view.ViewCompat;
import com.facebook.common.logging.FLog;
import com.facebook.debug.holder.PrinterHolder;
import com.facebook.debug.tags.ReactDebugOverlayTags;
Expand All @@ -55,8 +54,6 @@
import com.facebook.react.bridge.CatalystInstanceImpl;
import com.facebook.react.bridge.JSBundleLoader;
import com.facebook.react.bridge.JSExceptionHandler;
import com.facebook.react.bridge.JSIModulePackage;
import com.facebook.react.bridge.JSIModuleType;
import com.facebook.react.bridge.JavaJSExecutor;
import com.facebook.react.bridge.JavaScriptExecutor;
import com.facebook.react.bridge.JavaScriptExecutorFactory;
Expand All @@ -71,13 +68,13 @@
import com.facebook.react.bridge.ReactNoCrashSoftException;
import com.facebook.react.bridge.ReactSoftExceptionLogger;
import com.facebook.react.bridge.UIManager;
import com.facebook.react.bridge.UIManagerProvider;
import com.facebook.react.bridge.UiThreadUtil;
import com.facebook.react.bridge.WritableNativeMap;
import com.facebook.react.bridge.queue.ReactQueueConfigurationSpec;
import com.facebook.react.common.LifecycleState;
import com.facebook.react.common.ReactConstants;
import com.facebook.react.common.SurfaceDelegateFactory;
import com.facebook.react.common.annotations.VisibleForTesting;
import com.facebook.react.config.ReactFeatureFlags;
import com.facebook.react.devsupport.DevSupportManagerFactory;
import com.facebook.react.devsupport.ReactInstanceDevHelper;
Expand All @@ -88,7 +85,6 @@
import com.facebook.react.devsupport.interfaces.RedBoxHandler;
import com.facebook.react.internal.turbomodule.core.TurboModuleManager;
import com.facebook.react.internal.turbomodule.core.TurboModuleManagerDelegate;
import com.facebook.react.internal.turbomodule.core.interfaces.TurboModuleRegistry;
import com.facebook.react.modules.appearance.AppearanceModule;
import com.facebook.react.modules.appregistry.AppRegistry;
import com.facebook.react.modules.core.DefaultHardwareBackBtnHandler;
Expand Down Expand Up @@ -182,7 +178,7 @@ public interface ReactInstanceEventListener
private volatile Boolean mHasStartedDestroying = false;
private final MemoryPressureRouter mMemoryPressureRouter;
private final @Nullable JSExceptionHandler mJSExceptionHandler;
private final @Nullable JSIModulePackage mJSIModulePackage;
private final @Nullable UIManagerProvider mUIManagerProvider;
private final @Nullable ReactPackageTurboModuleManagerDelegate.Builder mTMMDelegateBuilder;
private List<ViewManager> mViewManagers;
private boolean mUseFallbackBundle = true;
Expand Down Expand Up @@ -230,7 +226,7 @@ public static ReactInstanceManagerBuilder builder() {
@Nullable DevBundleDownloadListener devBundleDownloadListener,
int minNumShakes,
int minTimeLeftInFrameForNonBatchedOperationMs,
@Nullable JSIModulePackage jsiModulePackage,
@Nullable UIManagerProvider uIManagerProvider,
@Nullable Map<String, RequestHandler> customPackagerCommandHandlers,
@Nullable ReactPackageTurboModuleManagerDelegate.Builder tmmDelegateBuilder,
@Nullable SurfaceDelegateFactory surfaceDelegateFactory,
Expand Down Expand Up @@ -284,7 +280,7 @@ public static ReactInstanceManagerBuilder builder() {
}
mPackages.addAll(packages);
}
mJSIModulePackage = jsiModulePackage;
mUIManagerProvider = uIManagerProvider;

// Instantiate ReactChoreographer in UI thread.
ReactChoreographer.initialize();
Expand Down Expand Up @@ -1343,7 +1339,7 @@ private ReactApplicationContext createReactContext(
ReactMarker.logMarker(CREATE_CATALYST_INSTANCE_START);
// CREATE_CATALYST_INSTANCE_END is in JSCExecutor.cpp
Systrace.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "createCatalystInstance");
final CatalystInstance catalystInstance;
final CatalystInstanceImpl catalystInstance;
try {
catalystInstance = catalystInstanceBuilder.build();
} finally {
Expand Down Expand Up @@ -1373,26 +1369,19 @@ private ReactApplicationContext createReactContext(
new TurboModuleManager(
catalystInstance.getRuntimeExecutor(),
tmmDelegate,
catalystInstance.getJSCallInvokerHolder(),
catalystInstance.getNativeMethodCallInvokerHolder());
((CatalystInstance) catalystInstance).getJSCallInvokerHolder(),
((CatalystInstance) catalystInstance).getNativeMethodCallInvokerHolder());

catalystInstance.setTurboModuleManager(turboModuleManager);

TurboModuleRegistry registry = (TurboModuleRegistry) turboModuleManager;
catalystInstance.setTurboModuleRegistry(turboModuleManager);

// Eagerly initialize TurboModules
for (String moduleName : registry.getEagerInitModuleNames()) {
registry.getModule(moduleName);
for (String moduleName : turboModuleManager.getEagerInitModuleNames()) {
turboModuleManager.getModule(moduleName);
}
}

if (mJSIModulePackage != null) {
catalystInstance.addJSIModules(
mJSIModulePackage.getJSIModules(
reactContext, catalystInstance.getJavaScriptContextHolder()));
}
if (ReactFeatureFlags.enableFabricRenderer) {
catalystInstance.getJSIModule(JSIModuleType.UIManager);
if (mUIManagerProvider != null && ReactFeatureFlags.enableFabricRenderer) {
catalystInstance.setFabricUIManager(mUIManagerProvider.createUIManager(reactContext));
}
if (mBridgeIdleDebugListener != null) {
catalystInstance.addBridgeIdleDebugListener(mBridgeIdleDebugListener);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
import com.facebook.infer.annotation.Assertions;
import com.facebook.react.bridge.JSBundleLoader;
import com.facebook.react.bridge.JSExceptionHandler;
import com.facebook.react.bridge.JSIModulePackage;
import com.facebook.react.bridge.JavaScriptExecutorFactory;
import com.facebook.react.bridge.NotThreadSafeBridgeIdleDebugListener;
import com.facebook.react.bridge.UIManagerProvider;
import com.facebook.react.common.LifecycleState;
import com.facebook.react.common.SurfaceDelegateFactory;
import com.facebook.react.devsupport.DefaultDevSupportManagerFactory;
Expand Down Expand Up @@ -64,18 +64,18 @@ public class ReactInstanceManagerBuilder {
private @Nullable JavaScriptExecutorFactory mJavaScriptExecutorFactory;
private int mMinNumShakes = 1;
private int mMinTimeLeftInFrameForNonBatchedOperationMs = -1;
private @Nullable JSIModulePackage mJSIModulesPackage;
private @Nullable Map<String, RequestHandler> mCustomPackagerCommandHandlers;
private @Nullable ReactPackageTurboModuleManagerDelegate.Builder mTMMDelegateBuilder;
private @Nullable SurfaceDelegateFactory mSurfaceDelegateFactory;
private @Nullable DevLoadingViewManager mDevLoadingViewManager;
private @Nullable JSEngineResolutionAlgorithm mJSEngineResolutionAlgorithm = null;
private @Nullable JSEngineResolutionAlgorithm mJSEngineResolutionAlgorithm;
private @Nullable UIManagerProvider mUIManagerProvider;

/* package protected */ ReactInstanceManagerBuilder() {}

public ReactInstanceManagerBuilder setJSIModulesPackage(
@Nullable JSIModulePackage jsiModulePackage) {
mJSIModulesPackage = jsiModulePackage;
public ReactInstanceManagerBuilder setUIManagerProvider(
@Nullable UIManagerProvider uIManagerProvider) {
mUIManagerProvider = uIManagerProvider;
return this;
}

Expand Down Expand Up @@ -344,7 +344,7 @@ public ReactInstanceManager build() {
mDevBundleDownloadListener,
mMinNumShakes,
mMinTimeLeftInFrameForNonBatchedOperationMs,
mJSIModulesPackage,
mUIManagerProvider,
mCustomPackagerCommandHandlers,
mTMMDelegateBuilder,
mSurfaceDelegateFactory,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
import com.facebook.react.common.annotations.VisibleForTesting;
import com.facebook.react.internal.turbomodule.core.interfaces.CallInvokerHolder;
import com.facebook.react.internal.turbomodule.core.interfaces.NativeMethodCallInvokerHolder;
import com.facebook.react.internal.turbomodule.core.interfaces.TurboModuleRegistry;
import java.util.Collection;
import java.util.List;

/**
* A higher level API on top of the asynchronous JSC bridge. This provides an environment allowing
Expand Down Expand Up @@ -69,8 +69,6 @@ public interface CatalystInstance
@Nullable
NativeModule getNativeModule(String moduleName);

JSIModule getJSIModule(JSIModuleType moduleType);

Collection<NativeModule> getNativeModules();

/**
Expand Down Expand Up @@ -114,8 +112,6 @@ public interface CatalystInstance

RuntimeScheduler getRuntimeScheduler();

void addJSIModules(List<JSIModuleSpec> jsiModules);

/**
* Returns a hybrid object that contains a pointer to a JS CallInvoker, which is used to schedule
* work on the JS Thread. Required for TurboModuleManager initialization.
Expand All @@ -129,9 +125,23 @@ public interface CatalystInstance
NativeMethodCallInvokerHolder getNativeMethodCallInvokerHolder();

/**
* For the time being, we want code relying on the old infra to also work with TurboModules.
* Hence, we must provide the TurboModuleRegistry to CatalystInstance so that getNativeModule,
* hasNativeModule, and getNativeModules can also return TurboModules.
* @deprecated since this would be deprecated later as part of Stable APIs with bridge removal and
* not encouraged usage.
*/
@Deprecated
void setTurboModuleRegistry(TurboModuleRegistry turboModuleRegistry);

/**
* @deprecated since this would be deprecated later as part of Stable APIs with bridge removal and
* not encouraged usage.
*/
void setTurboModuleManager(JSIModule getter);
@Deprecated
void setFabricUIManager(UIManager fabricUIManager);

/**
* @deprecated since this would be deprecated later as part of Stable APIs with bridge removal and
* not encouraged usage.
*/
@Deprecated
UIManager getFabricUIManager();
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import java.lang.ref.WeakReference;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.atomic.AtomicInteger;

Expand Down Expand Up @@ -92,7 +91,6 @@ public String toString() {
private final Object mJSCallsPendingInitLock = new Object();

private final NativeModuleRegistry mNativeModuleRegistry;
private final JSIModuleRegistry mJSIModuleRegistry = new JSIModuleRegistry();
private final JSExceptionHandler mJSExceptionHandler;
private final MessageQueueThread mNativeModulesQueueThread;
private boolean mInitialized = false;
Expand All @@ -102,7 +100,8 @@ public String toString() {
private @Nullable String mSourceURL;

private JavaScriptContextHolder mJavaScriptContextHolder;
private volatile @Nullable TurboModuleRegistry mTurboModuleRegistry = null;
private @Nullable TurboModuleRegistry mTurboModuleRegistry;
private @Nullable UIManager mFabricUIManager;

// C++ parts
private final HybridData mHybridData;
Expand Down Expand Up @@ -352,7 +351,7 @@ public void destroy() {
mNativeModulesQueueThread.runOnQueue(
() -> {
mNativeModuleRegistry.notifyJSInstanceDestroy();
mJSIModuleRegistry.notifyJSInstanceDestroy();
mTurboModuleRegistry.invalidate();
boolean wasIdle = (mPendingJSCalls.getAndSet(0) == 0);
if (!mBridgeIdleListeners.isEmpty()) {
for (NotThreadSafeBridgeIdleDebugListener listener : mBridgeIdleListeners) {
Expand Down Expand Up @@ -541,16 +540,6 @@ public JavaScriptContextHolder getJavaScriptContextHolder() {

public native RuntimeScheduler getRuntimeScheduler();

@Override
public void addJSIModules(List<JSIModuleSpec> jsiModules) {
mJSIModuleRegistry.registerModules(jsiModules);
}

@Override
public JSIModule getJSIModule(JSIModuleType moduleType) {
return mJSIModuleRegistry.getModule(moduleType);
}

private native long getJavaScriptContext();

private void incrementPendingJSCalls() {
Expand All @@ -568,8 +557,19 @@ private void incrementPendingJSCalls() {
}
}

public void setTurboModuleManager(JSIModule module) {
mTurboModuleRegistry = (TurboModuleRegistry) module;
@Override
public void setTurboModuleRegistry(TurboModuleRegistry turboModuleRegistry) {
mTurboModuleRegistry = turboModuleRegistry;
}

@Override
public void setFabricUIManager(UIManager fabricUIManager) {
mFabricUIManager = fabricUIManager;
}

@Override
public UIManager getFabricUIManager() {
return mFabricUIManager;
}

private void decrementPendingJSCalls() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -546,12 +546,12 @@ public boolean isBridgeless() {
return null;
}

public @Nullable JSIModule getJSIModule(JSIModuleType moduleType) {
public @Nullable UIManager getFabricUIManager() {
if (!hasActiveReactInstance()) {
throw new IllegalStateException(
"Unable to retrieve a JSIModule if CatalystInstance is not active.");
"Unable to retrieve a UIManager if CatalystInstance is not active.");
}
return mCatalystInstance.getJSIModule(moduleType);
return mCatalystInstance.getFabricUIManager();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import com.facebook.react.bridge.JavaScriptContextHolder
import com.facebook.react.bridge.ReactApplicationContext
import com.facebook.react.bridge.UIManager
import com.facebook.react.fabric.ComponentFactory
import com.facebook.react.fabric.FabricJSIModuleProvider
import com.facebook.react.fabric.FabricUIManagerProviderImpl
import com.facebook.react.fabric.ReactNativeConfig
import com.facebook.react.uimanager.ViewManagerRegistry

Expand Down Expand Up @@ -49,11 +49,8 @@ class DefaultJSIModulePackage(private val reactNativeHost: ReactNativeHost) : JS
val viewManagers =
reactNativeHost.reactInstanceManager.getOrCreateViewManagers(reactApplicationContext)
val viewManagerRegistry = ViewManagerRegistry(viewManagers)
return FabricJSIModuleProvider(
reactApplicationContext,
componentFactory,
ReactNativeConfig.DEFAULT_CONFIG,
viewManagerRegistry)
return FabricUIManagerProviderImpl(
componentFactory, ReactNativeConfig.DEFAULT_CONFIG, viewManagerRegistry)
}
}
}

This file was deleted.

Loading

0 comments on commit c2d4a67

Please sign in to comment.