From 71b17e2ebb73637276f47fa51502391263e1d885 Mon Sep 17 00:00:00 2001 From: Nicola Corti Date: Mon, 12 Aug 2024 12:56:38 -0700 Subject: [PATCH] Add support for handling `com.facebook.react.bridge.Dynamic` as parameter type in TurboModules (#45944) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/45944 This diff adds support having (Legacy) Native Modules with functions with parameters of type `Dynamic`. This is currently blocking some libraries making it harder for them to migrate to New Architecture. I've implemented it by adding a `DynamicNative` implementation of `Dynamic` which holds a reference of the payload as a `folly::dynamic`. Changelog: [Android] [Added] - Add support for handling `com.facebook.react.bridge.Dynamic` as parameter type in TurboModules Reviewed By: mdvacca, cipolleschi Differential Revision: D60966684 fbshipit-source-id: 2e63bc53ede5277a9c12f1b19f05f6099f5f35f9 --- .../facebook/react/bridge/DynamicNative.kt | 49 ++++++++++++++++ .../core/TurboModuleInteropUtils.java | 7 +-- .../src/main/jni/react/jni/JDynamicNative.cpp | 46 +++++++++++++++ .../src/main/jni/react/jni/JDynamicNative.h | 56 +++++++++++++++++++ .../src/main/jni/react/jni/OnLoad.cpp | 2 + .../android/ReactCommon/JavaTurboModule.cpp | 9 ++- .../platform/android/SampleLegacyModule.java | 40 +++++++++++++ .../TurboModule/SampleLegacyModuleExample.js | 11 ++++ 8 files changed, 214 insertions(+), 6 deletions(-) create mode 100644 packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/DynamicNative.kt create mode 100644 packages/react-native/ReactAndroid/src/main/jni/react/jni/JDynamicNative.cpp create mode 100644 packages/react-native/ReactAndroid/src/main/jni/react/jni/JDynamicNative.h diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/DynamicNative.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/DynamicNative.kt new file mode 100644 index 00000000000000..8e2c1fe9ea47c3 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/DynamicNative.kt @@ -0,0 +1,49 @@ +/* + * 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.bridge + +import com.facebook.jni.HybridData +import com.facebook.proguard.annotations.DoNotStrip +import com.facebook.proguard.annotations.DoNotStripAny + +/** + * An implementation of [Dynamic] that has a C++ implementation. + * + * This is used to support Legacy Native Modules that have not been migrated to the new architecture + * and are using [Dynamic] as a parameter type. + */ +@DoNotStripAny +private class DynamicNative( + @Suppress("NoHungarianNotation") @field:DoNotStrip private val mHybridData: HybridData? +) : Dynamic { + + override fun getType(): ReadableType = getTypeNative() + + override fun isNull(): Boolean = isNullNative() + + private external fun getTypeNative(): ReadableType + + private external fun isNullNative(): Boolean + + external override fun asBoolean(): Boolean + + // The native representation is holding the value as Double. We do the Int conversion here. + override fun asInt(): Int = asDouble().toInt() + + external override fun asDouble(): Double + + external override fun asString(): String + + external override fun asArray(): ReadableArray + + external override fun asMap(): ReadableMap + + override fun recycle() { + // Noop - nothing to recycle since there is no pooling + } +} diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/turbomodule/core/TurboModuleInteropUtils.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/turbomodule/core/TurboModuleInteropUtils.java index 496cf39a313ba9..66991341ea2075 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/turbomodule/core/TurboModuleInteropUtils.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/internal/turbomodule/core/TurboModuleInteropUtils.java @@ -161,14 +161,11 @@ private static String convertParamClassToJniType( || paramClass == Callback.class || paramClass == Promise.class || paramClass == ReadableMap.class - || paramClass == ReadableArray.class) { + || paramClass == ReadableArray.class + || paramClass == Dynamic.class) { return convertClassToJniType(paramClass); } - if (paramClass == Dynamic.class) { - // TODO(T145105887): Output warnings that TurboModules doesn't yet support Dynamic arguments - } - throw new ParsingException( moduleName, methodName, diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/jni/JDynamicNative.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/jni/JDynamicNative.cpp new file mode 100644 index 00000000000000..477b933744c201 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/main/jni/react/jni/JDynamicNative.cpp @@ -0,0 +1,46 @@ +/* + * 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. + */ + +#include "JDynamicNative.h" +#include "ReadableNativeArray.h" +#include "ReadableNativeMap.h" + +using namespace facebook::jni; + +namespace facebook::react { + +jboolean JDynamicNative::isNullNative() { + return payload_.isNull(); +} + +jni::local_ref JDynamicNative::getTypeNative() { + return ReadableType::getType(payload_.type()); +} + +jni::local_ref JDynamicNative::asString() { + return jni::make_jstring(payload_.asString()); +} + +jboolean JDynamicNative::asBoolean() { + return payload_.asBool(); +} + +jdouble JDynamicNative::asDouble() { + return payload_.asDouble(); +} + +jni::local_ref JDynamicNative::asArray() { + return jni::adopt_local(reinterpret_cast( + ReadableNativeArray::newObjectCxxArgs(payload_).release())); +} + +jni::local_ref JDynamicNative::asMap() { + return jni::adopt_local(reinterpret_cast( + ReadableNativeMap::createWithContents(std::move(payload_)).release())); +} + +} // namespace facebook::react diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/jni/JDynamicNative.h b/packages/react-native/ReactAndroid/src/main/jni/react/jni/JDynamicNative.h new file mode 100644 index 00000000000000..8b05243fc227c8 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/main/jni/react/jni/JDynamicNative.h @@ -0,0 +1,56 @@ +/* + * 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. + */ + +#pragma once + +#include "NativeCommon.h" +#include "ReadableNativeArray.h" +#include "ReadableNativeMap.h" + +#include +#include +#include + +namespace facebook::react { + +struct JDynamic : public jni::JavaClass { + constexpr static auto kJavaDescriptor = "Lcom/facebook/react/bridge/Dynamic;"; +}; + +class JDynamicNative : public jni::HybridClass { + public: + constexpr static auto kJavaDescriptor = + "Lcom/facebook/react/bridge/DynamicNative;"; + + JDynamicNative(folly::dynamic payload) : payload_(std::move(payload)) {} + + static void registerNatives() { + javaClassStatic()->registerNatives( + {makeNativeMethod("isNullNative", JDynamicNative::isNullNative), + makeNativeMethod("getTypeNative", JDynamicNative::getTypeNative), + makeNativeMethod("asDouble", JDynamicNative::asDouble), + makeNativeMethod("asBoolean", JDynamicNative::asBoolean), + makeNativeMethod("asString", JDynamicNative::asString), + makeNativeMethod("asArray", JDynamicNative::asArray), + makeNativeMethod("asMap", JDynamicNative::asMap)}); + } + + private: + friend HybridBase; + + jni::local_ref getTypeNative(); + jni::local_ref asString(); + jboolean asBoolean(); + jdouble asDouble(); + jboolean isNullNative(); + jni::local_ref asArray(); + jni::local_ref asMap(); + + folly::dynamic payload_; +}; + +} // namespace facebook::react diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp index c8832b1f5042de..6d7b0eec906e2e 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/jni/OnLoad.cpp @@ -15,6 +15,7 @@ #include "CatalystInstanceImpl.h" #include "CxxModuleWrapperBase.h" #include "JCallback.h" +#include "JDynamicNative.h" #include "JInspector.h" #include "JReactMarker.h" #include "JavaScriptExecutorHolder.h" @@ -88,6 +89,7 @@ extern "C" JNIEXPORT jint JNI_OnLoad(JavaVM* vm, void* reserved) { NativeMap::registerNatives(); ReadableNativeMap::registerNatives(); WritableNativeMap::registerNatives(); + JDynamicNative::registerNatives(); JReactMarker::registerNatives(); JInspector::registerNatives(); }); diff --git a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp index 6c35e6b91bb579..8ece2391e64cbd 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp +++ b/packages/react-native/ReactCommon/react/nativemodule/core/platform/android/ReactCommon/JavaTurboModule.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -370,7 +371,9 @@ JNIArgs convertJSIArgsToJNIArgs( continue; } - if (arg->isNull() || arg->isUndefined()) { + // Dynamic encapsulates the Null type so we don't want to return null here. + if ((arg->isNull() && type != "Lcom/facebook/react/bridge/Dynamic;") || + arg->isUndefined()) { jarg->l = nullptr; } else if (type == "Ljava/lang/Double;") { if (!arg->isNumber()) { @@ -433,6 +436,10 @@ JNIArgs convertJSIArgsToJNIArgs( auto jParams = ReadableNativeMap::createWithContents(std::move(dynamicFromValue)); jarg->l = makeGlobalIfNecessary(jParams.release()); + } else if (type == "Lcom/facebook/react/bridge/Dynamic;") { + auto dynamicFromValue = jsi::dynamicFromValue(rt, *arg); + auto jParams = JDynamicNative::newObjectCxxArgs(dynamicFromValue); + jarg->l = makeGlobalIfNecessary(jParams.release()); } else { throw JavaTurboModuleInvalidArgumentTypeException( type, argIndex, methodName); diff --git a/packages/react-native/ReactCommon/react/nativemodule/samples/platform/android/SampleLegacyModule.java b/packages/react-native/ReactCommon/react/nativemodule/samples/platform/android/SampleLegacyModule.java index 461be38ad8a71d..92e75ee27d2465 100644 --- a/packages/react-native/ReactCommon/react/nativemodule/samples/platform/android/SampleLegacyModule.java +++ b/packages/react-native/ReactCommon/react/nativemodule/samples/platform/android/SampleLegacyModule.java @@ -13,12 +13,14 @@ import com.facebook.proguard.annotations.DoNotStrip; import com.facebook.react.bridge.Arguments; import com.facebook.react.bridge.Callback; +import com.facebook.react.bridge.Dynamic; import com.facebook.react.bridge.Promise; import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.bridge.ReactContextBaseJavaModule; import com.facebook.react.bridge.ReactMethod; import com.facebook.react.bridge.ReadableArray; import com.facebook.react.bridge.ReadableMap; +import com.facebook.react.bridge.ReadableType; import com.facebook.react.bridge.WritableArray; import com.facebook.react.bridge.WritableMap; import com.facebook.react.bridge.WritableNativeArray; @@ -155,6 +157,44 @@ public WritableMap getUnsafeObject(ReadableMap arg) { return map; } + @SuppressWarnings("unused") + @ReactMethod(isBlockingSynchronousMethod = true) + public WritableMap getDynamic(Dynamic dynamic) { + WritableNativeMap resultMap = new WritableNativeMap(); + ReadableType type = dynamic.getType(); + if (type == ReadableType.Null) { + log("getDynamic as Null", dynamic, dynamic); + resultMap.putString("type", "Null"); + resultMap.putNull("value"); + } else if (type == ReadableType.Boolean) { + boolean result = dynamic.asBoolean(); + log("getDynamic as Boolean", dynamic, result); + resultMap.putString("type", "Boolean"); + resultMap.putBoolean("value", result); + } else if (type == ReadableType.Number) { + int result = dynamic.asInt(); + log("getDynamic as Number", dynamic, result); + resultMap.putString("type", "Number"); + resultMap.putInt("value", result); + } else if (type == ReadableType.String) { + String result = dynamic.asString(); + log("getDynamic as String", dynamic, result); + resultMap.putString("type", "String"); + resultMap.putString("value", result); + } else if (type == ReadableType.Array) { + ReadableArray result = dynamic.asArray(); + log("getDynamic as Array", dynamic, result); + resultMap.putString("type", "Array"); + resultMap.putArray("value", result); + } else if (type == ReadableType.Map) { + ReadableMap result = dynamic.asMap(); + log("getDynamic as Map", dynamic, result); + resultMap.putString("type", "Map"); + resultMap.putMap("value", result); + } + return resultMap; + } + @DoNotStrip @SuppressWarnings("unused") @ReactMethod(isBlockingSynchronousMethod = true) diff --git a/packages/rn-tester/js/examples/TurboModule/SampleLegacyModuleExample.js b/packages/rn-tester/js/examples/TurboModule/SampleLegacyModuleExample.js index 79ca96d27c518f..f3d9a99468e374 100644 --- a/packages/rn-tester/js/examples/TurboModule/SampleLegacyModuleExample.js +++ b/packages/rn-tester/js/examples/TurboModule/SampleLegacyModuleExample.js @@ -141,6 +141,17 @@ class SampleLegacyModuleExample extends React.Component<{||}, State> { getSampleLegacyModule()?.getObject({a: 1, b: 'foo', c: null}), getValue: () => getSampleLegacyModule()?.getValue(5, 'test', {a: 1, b: 'foo'}), + getDynamicWithNull: () => getSampleLegacyModule()?.getDynamic(null), + getDynamicWithBoolean: () => + getSampleLegacyModule()?.getDynamic(true), + getDynamicWithNumber: () => + getSampleLegacyModule()?.getDynamic(42.24), + getDynamicWithString: () => + getSampleLegacyModule()?.getDynamic('The answer is 42'), + getDynamicWithArray: () => + getSampleLegacyModule()?.getDynamic(['the', 'answer', 'is', '42']), + getDynamicWithMap: () => + getSampleLegacyModule()?.getDynamic({answer: '42'}), callback: () => getSampleLegacyModule()?.getValueWithCallback(callbackValue => this._setResult('callback', callbackValue),