Skip to content

Commit

Permalink
Merge pull request #2 from SparshaSaha/CherryPickFixesto068stable
Browse files Browse the repository at this point in the history
Cherry pick fixesto068stable
  • Loading branch information
SparshaSaha authored Aug 2, 2022
2 parents 72e1eda + 9df0f69 commit 50cb7ac
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 35 deletions.
18 changes: 18 additions & 0 deletions ReactAndroid/src/main/java/com/facebook/react/JSInterpreter.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright (c) Meta Platforms, Inc. and 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;

/**
* An enum that specifies the JS Engine to be used in the app Old Logic uses the legacy code
* JSC/HERMES loads the respective engine using the revamped logic
*/
public enum JSInterpreter {
OLD_LOGIC,
JSC,
HERMES
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ public class ReactInstanceManagerBuilder {
private @Nullable Map<String, RequestHandler> mCustomPackagerCommandHandlers;
private @Nullable ReactPackageTurboModuleManagerDelegate.Builder mTMMDelegateBuilder;
private @Nullable SurfaceDelegateFactory mSurfaceDelegateFactory;
private JSInterpreter jsInterpreter = JSInterpreter.OLD_LOGIC;

/* package protected */ ReactInstanceManagerBuilder() {}

Expand Down Expand Up @@ -125,6 +126,31 @@ public ReactInstanceManagerBuilder setJSBundleLoader(JSBundleLoader jsBundleLoad
return this;
}

/**
* Sets the jsEngine as JSC or HERMES as per the setJsEngineAsHermes call Uses the enum {@link
* JSInterpreter}
*
* @param jsInterpreter
*/
private void setJSEngine(JSInterpreter jsInterpreter) {
this.jsInterpreter = jsInterpreter;
}

/**
* Utility setter to set the required JSEngine as HERMES or JSC Defaults to OLD_LOGIC if not
* called by the host app
*
* @param hermesEnabled hermesEnabled = true sets the JS Engine as HERMES and JSC otherwise
*/
public ReactInstanceManagerBuilder setJsEngineAsHermes(boolean hermesEnabled) {
if (hermesEnabled) {
setJSEngine(JSInterpreter.HERMES);
} else {
setJSEngine(JSInterpreter.JSC);
}
return this;
}

/**
* Path to your app's main module on Metro. This is used when reloading JS during development. All
* paths are relative to the root folder the packager is serving files from. Examples: {@code
Expand Down Expand Up @@ -345,41 +371,35 @@ public ReactInstanceManager build() {

private JavaScriptExecutorFactory getDefaultJSExecutorFactory(
String appName, String deviceName, Context applicationContext) {
try {
// If JSC is included, use it as normal
initializeSoLoaderIfNecessary(applicationContext);
JSCExecutor.loadLibrary();
return new JSCExecutorFactory(appName, deviceName);
} catch (UnsatisfiedLinkError jscE) {
// https://github.com/facebook/hermes/issues/78 shows that
// people who aren't trying to use Hermes are having issues.
// https://github.com/facebook/react-native/issues/25923#issuecomment-554295179
// includes the actual JSC error in at least one case.
//
// So, if "__cxa_bad_typeid" shows up in the jscE exception
// message, then we will assume that's the failure and just
// throw now.

if (jscE.getMessage().contains("__cxa_bad_typeid")) {
throw jscE;
}

// Otherwise use Hermes
// Relying solely on try catch block and loading jsc even when
// project is using hermes can lead to launch-time crashes especially in
// monorepo architectures and hybrid apps using both native android
// and react native.
// So we can use the value of enableHermes received by the constructor
// to decide which library to load at launch

// if nothing is specified, use old loading method
// else load the required engine
if (jsInterpreter == JSInterpreter.OLD_LOGIC) {
try {
// If JSC is included, use it as normal
initializeSoLoaderIfNecessary(applicationContext);
JSCExecutor.loadLibrary();
return new JSCExecutorFactory(appName, deviceName);
} catch (UnsatisfiedLinkError jscE) {
if (jscE.getMessage().contains("__cxa_bad_typeid")) {
throw jscE;
}
HermesExecutor.loadLibrary();
return new HermesExecutorFactory();
} catch (UnsatisfiedLinkError hermesE) {
// If we get here, either this is a JSC build, and of course
// Hermes failed (since it's not in the APK), or it's a Hermes
// build, and Hermes had a problem.

// We suspect this is a JSC issue (it's the default), so we
// will throw that exception, but we will print hermesE first,
// since it could be a Hermes issue and we don't want to
// swallow that.
hermesE.printStackTrace();
throw jscE;
}
} else if (jsInterpreter == JSInterpreter.HERMES) {
HermesExecutor.loadLibrary();
return new HermesExecutorFactory();
} else {
JSCExecutor.loadLibrary();
return new JSCExecutorFactory(appName, deviceName);
}
}
}
20 changes: 15 additions & 5 deletions ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java
Original file line number Diff line number Diff line change
Expand Up @@ -719,12 +719,22 @@ private void attachToReactInstanceManager() {

// React Native requires that the RootView id be managed entirely by React Native, and will
// crash in addRootView/startSurface if the native View id isn't set to NO_ID.

// This behavior can not be guaranteed in hybrid apps that have a native android layer over
// which reactRootViews are added and the native views need to have ids on them in order to
// work.
// Hence this can cause unnecessary crashes at runtime for hybrid apps.
// So converting this to a soft exception such that pure react-native devs can still see the
// warning while hybrid apps continue to run without crashes

if (getId() != View.NO_ID) {
throw new IllegalViewOperationException(
"Trying to attach a ReactRootView with an explicit id already set to ["
+ getId()
+ "]. React Native uses the id field to track react tags and will overwrite this"
+ " field. If that is fine, explicitly overwrite the id field to View.NO_ID.");
ReactSoftExceptionLogger.logSoftException(
TAG,
new IllegalViewOperationException(
"Trying to attach a ReactRootView with an explicit id already set to ["
+ getId()
+ "]. React Native uses the id field to track react tags and will overwrite this"
+ " field. If that is fine, explicitly overwrite the id field to View.NO_ID."));
}

try {
Expand Down

0 comments on commit 50cb7ac

Please sign in to comment.