From dac590e9c5524d8961af2bad32b6c6cfb1069ec0 Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Fri, 14 Nov 2025 05:34:12 -0800 Subject: [PATCH] Restructure core dependencies to avoid cxxreact and fabric (#54446) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/54446 Changelog: [Internal] Differential Revision: D86515326 --- packages/react-native/Package.swift | 43 +++--- .../react/uimanager/TransformHelper.kt | 4 + .../src/main/jni/react/jni/OnLoad-common.cpp | 2 - .../src/main/jni/react/jni/OnLoad.cpp | 2 + .../ReactCommon/ReactCommon.podspec | 21 +-- .../ReactCommon/cxxreact/ErrorUtils.h | 34 +---- .../ReactCommon/cxxreact/Instance.cpp | 6 +- .../ReactCommon/cxxreact/MoveWrapper.h | 123 +--------------- .../ReactCommon/cxxreact/NativeToJsBridge.cpp | 4 +- .../cxxreact/React-cxxreact.podspec | 1 + .../ReactCommon/jserrorhandler/ErrorUtils.cpp | 42 ++++++ .../ReactCommon/jserrorhandler/ErrorUtils.h | 16 +++ .../jserrorhandler/JsErrorHandler.cpp | 4 +- .../React-jserrorhandler.podspec | 1 - .../jsiexecutor/jsireact/JSIExecutor.cpp | 2 +- .../android/ReactCommon/JavaTurboModule.cpp | 2 +- .../React-runtimescheduler.podspec | 1 + .../runtimescheduler/RuntimeScheduler.cpp | 2 +- .../react/runtime/ReactInstance.cpp | 3 +- .../ReactCommon/react/utils/MoveWrapper.h | 131 ++++++++++++++++++ 20 files changed, 244 insertions(+), 200 deletions(-) create mode 100644 packages/react-native/ReactCommon/jserrorhandler/ErrorUtils.cpp create mode 100644 packages/react-native/ReactCommon/jserrorhandler/ErrorUtils.h create mode 100644 packages/react-native/ReactCommon/react/utils/MoveWrapper.h diff --git a/packages/react-native/Package.swift b/packages/react-native/Package.swift index d95d1c6a1d3c5a..34e561f42722c6 100644 --- a/packages/react-native/Package.swift +++ b/packages/react-native/Package.swift @@ -96,6 +96,7 @@ let reactDebug = RNTarget( path: "ReactCommon/react/debug", dependencies: [.reactNativeDependencies] ) + /// React-jsi.podspec let jsi = RNTarget( name: .jsi, @@ -192,13 +193,30 @@ let reactJsInspector = RNTarget( ] ) +/// ReactCommon.podspec +/// This target represent the ReactCommon/turbomodule/bridging subspec +let reactTurboModuleBridging = RNTarget( + name: .reactTurboModuleBridging, + path: "ReactCommon/react/bridging", + excludedPaths: ["tests"], + dependencies: [.reactNativeDependencies, .reactPerfLogger, .jsi] +) + +/// React-jserrorhandler.podspec +let reactJsErrorHandler = RNTarget( + name: .reactJsErrorHandler, + path: "ReactCommon/jserrorhandler", + excludedPaths: ["tests"], + dependencies: [.reactNativeDependencies, .jsi, .reactFeatureFlags, .reactDebug, .reactTurboModuleBridging] +) + /// React-cxxreact.podspec let reactCxxReact = RNTarget( name: .reactCxxReact, path: "ReactCommon/cxxreact", searchPaths: [CallInvokerPath], excludedPaths: ["tests"], - dependencies: [.reactNativeDependencies, .jsi, .reactPerfLogger, .logger, .reactDebug, .reactJsInspector] + dependencies: [.reactNativeDependencies, .jsi, .reactJsErrorHandler, .reactPerfLogger, .logger, .reactDebug, .reactJsInspector] ) /// React-jsitooling.podspec @@ -243,7 +261,7 @@ let reactPerformanceCdpMetrics = RNTarget( name: .reactPerformanceCdpMetrics, path: "ReactCommon/react/performance/cdpmetrics", excludedPaths: ["tests"], - dependencies: [.reactNativeDependencies, .reactCxxReact, .jsi, .reactPerformanceTimeline, .reactRuntimeExecutor] + dependencies: [.reactNativeDependencies, .jsi, .reactPerformanceTimeline, .reactRuntimeExecutor] ) /// React-performancetimeline.podspec @@ -251,7 +269,7 @@ let reactPerformanceTimeline = RNTarget( name: .reactPerformanceTimeline, path: "ReactCommon/react/performance/timeline", excludedPaths: ["tests"], - dependencies: [.reactNativeDependencies, .reactFeatureFlags, .reactJsInspectorTracing, .reactCxxReact, .reactPerfLogger] + dependencies: [.reactNativeDependencies, .reactFeatureFlags, .reactJsInspectorTracing, .reactPerfLogger] ) /// React-runtimescheduler.podspec @@ -259,24 +277,7 @@ let reactRuntimeScheduler = RNTarget( name: .reactRuntimeScheduler, path: "ReactCommon/react/renderer/runtimescheduler", excludedPaths: ["tests"], - dependencies: [.reactNativeDependencies, .reactFeatureFlags, .reactCxxReact, .reactPerfLogger, .reactPerformanceTimeline, .reactRendererConsistency, .reactUtils, .reactRuntimeExecutor] -) - -/// ReactCommon.podspec -/// This target represent the ReactCommon/turbomodule/bridging subspec -let reactTurboModuleBridging = RNTarget( - name: .reactTurboModuleBridging, - path: "ReactCommon/react/bridging", - excludedPaths: ["tests"], - dependencies: [.reactNativeDependencies, .reactPerfLogger, .reactCxxReact, .jsi, .logger] -) - -/// React-jserrorhandler.podspec -let reactJsErrorHandler = RNTarget( - name: .reactJsErrorHandler, - path: "ReactCommon/jserrorhandler", - excludedPaths: ["tests"], - dependencies: [.reactNativeDependencies, .jsi, .reactCxxReact, .reactFeatureFlags, .reactDebug, .reactTurboModuleBridging] + dependencies: [.reactNativeDependencies, .reactJsErrorHandler, .reactFeatureFlags, .reactCxxReact, .reactPerfLogger, .reactPerformanceTimeline, .reactRendererConsistency, .reactUtils, .reactRuntimeExecutor] ) /// React-graphicsApple diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/TransformHelper.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/TransformHelper.kt index 554f405168026b..980110cd7b18a7 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/TransformHelper.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/TransformHelper.kt @@ -9,6 +9,7 @@ package com.facebook.react.uimanager import com.facebook.common.logging.FLog import com.facebook.react.bridge.NativeArray +import com.facebook.react.bridge.ReactNativeJNISoLoader import com.facebook.react.bridge.ReadableArray import com.facebook.react.bridge.ReadableMap import com.facebook.react.bridge.ReadableType @@ -16,6 +17,9 @@ import com.facebook.react.common.ReactConstants import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags public object TransformHelper { + init { + ReactNativeJNISoLoader.staticInit() + } private val helperMatrix: ThreadLocal = object : ThreadLocal() { diff --git a/packages/react-native/ReactAndroid/src/main/jni/react/jni/OnLoad-common.cpp b/packages/react-native/ReactAndroid/src/main/jni/react/jni/OnLoad-common.cpp index ef069ed7e41d51..293d2ffc8abf5f 100644 --- a/packages/react-native/ReactAndroid/src/main/jni/react/jni/OnLoad-common.cpp +++ b/packages/react-native/ReactAndroid/src/main/jni/react/jni/OnLoad-common.cpp @@ -11,7 +11,6 @@ #include "JReactMarker.h" #include "NativeArray.h" #include "NativeMap.h" -#include "TransformHelper.h" #include "WritableNativeArray.h" #include "WritableNativeMap.h" @@ -28,7 +27,6 @@ extern "C" JNIEXPORT jint JNI_OnLoad(JavaVM* vm, void* reserved) { ReadableNativeMap::registerNatives(); WritableNativeArray::registerNatives(); WritableNativeMap::registerNatives(); - TransformHelper::registerNatives(); }); } 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 b1a66674505ea5..e0972505205b4c 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 @@ -14,6 +14,7 @@ #include "InspectorNetworkRequestListener.h" #include "JavaScriptExecutorHolder.h" #include "ReactInstanceManagerInspectorTarget.h" +#include "TransformHelper.h" #ifndef WITH_GLOGINIT #define WITH_GLOGINIT 1 @@ -43,6 +44,7 @@ extern "C" JNIEXPORT jint JNI_OnLoad(JavaVM* vm, void* reserved) { #endif ReactInstanceManagerInspectorTarget::registerNatives(); InspectorNetworkRequestListener::registerNatives(); + TransformHelper::registerNatives(); }); } diff --git a/packages/react-native/ReactCommon/ReactCommon.podspec b/packages/react-native/ReactCommon/ReactCommon.podspec index 83e81864640d54..635791f59b75a6 100644 --- a/packages/react-native/ReactCommon/ReactCommon.podspec +++ b/packages/react-native/ReactCommon/ReactCommon.podspec @@ -41,21 +41,15 @@ Pod::Spec.new do |s| # TODO (T48588859): Restructure this target to align with dir structure: "react/nativemodule/..." # Note: Update this only when ready to minimize breaking changes. s.subspec "turbomodule" do |ss| - ss.dependency "React-callinvoker", version - ss.dependency "React-perflogger", version - ss.dependency "React-cxxreact", version - ss.dependency "React-jsi", version - ss.dependency "React-logger", version - if use_hermes() - ss.dependency "hermes-engine" - end - ss.subspec "bridging" do |sss| - sss.dependency "React-jsi", version sss.source_files = podspec_sources("react/bridging/**/*.{cpp,h}", "react/bridging/**/*.h") sss.exclude_files = "react/bridging/tests" sss.header_dir = "react/bridging" sss.pod_target_xcconfig = { "HEADER_SEARCH_PATHS" => "\"$(PODS_TARGET_SRCROOT)/ReactCommon\"" } + + sss.dependency "React-jsi", version + sss.dependency "React-callinvoker", version + sss.dependency "React-perflogger", version if use_hermes() sss.dependency "hermes-engine" end @@ -64,9 +58,16 @@ Pod::Spec.new do |s| ss.subspec "core" do |sss| sss.source_files = podspec_sources("react/nativemodule/core/ReactCommon/**/*.{cpp,h}", "react/nativemodule/core/ReactCommon/**/*.h") sss.pod_target_xcconfig = { "HEADER_SEARCH_PATHS" => "\"$(PODS_TARGET_SRCROOT)/ReactCommon\" \"$(PODS_CONFIGURATION_BUILD_DIR)/React-debug/React_debug.framework/Headers\" \"$(PODS_CONFIGURATION_BUILD_DIR)/React-debug/React_featureflags.framework/Headers\" \"$(PODS_CONFIGURATION_BUILD_DIR)/React-utils/React_utils.framework/Headers\"" } + + sss.dependency "React-cxxreact", version sss.dependency "React-debug", version sss.dependency "React-featureflags", version + sss.dependency "React-logger", version + sss.dependency "React-jsi", version sss.dependency "React-utils", version + if use_hermes() + sss.dependency "hermes-engine" + end end end end diff --git a/packages/react-native/ReactCommon/cxxreact/ErrorUtils.h b/packages/react-native/ReactCommon/cxxreact/ErrorUtils.h index 5d9d5db3682718..35f44511507408 100644 --- a/packages/react-native/ReactCommon/cxxreact/ErrorUtils.h +++ b/packages/react-native/ReactCommon/cxxreact/ErrorUtils.h @@ -7,36 +7,6 @@ #pragma once -#include +#warning Deprecated: use instead. -namespace facebook::react { - -inline static void handleJSError(jsi::Runtime &runtime, const jsi::JSError &error, bool isFatal) -{ - auto errorUtils = runtime.global().getProperty(runtime, "ErrorUtils"); - if (errorUtils.isUndefined() || !errorUtils.isObject() || - !errorUtils.getObject(runtime).hasProperty(runtime, "reportFatalError") || - !errorUtils.getObject(runtime).hasProperty(runtime, "reportError")) { - // ErrorUtils was not set up. This probably means the bundle didn't - // load properly. - throw jsi::JSError( - runtime, - "ErrorUtils is not set up properly. Something probably went wrong trying to load the JS bundle. Trying to report error " + - error.getMessage(), - error.getStack()); - } - - // TODO(janzer): Rewrite this function to return the processed error - // instead of just reporting it through the native module - if (isFatal) { - auto func = errorUtils.asObject(runtime).getPropertyAsFunction(runtime, "reportFatalError"); - - func.call(runtime, error.value()); - } else { - auto func = errorUtils.asObject(runtime).getPropertyAsFunction(runtime, "reportError"); - - func.call(runtime, error.value()); - } -} - -} // namespace facebook::react +#include diff --git a/packages/react-native/ReactCommon/cxxreact/Instance.cpp b/packages/react-native/ReactCommon/cxxreact/Instance.cpp index d61f049076eb92..166ba6e2fd7ce0 100644 --- a/packages/react-native/ReactCommon/cxxreact/Instance.cpp +++ b/packages/react-native/ReactCommon/cxxreact/Instance.cpp @@ -9,15 +9,12 @@ #ifndef RCT_REMOVE_LEGACY_ARCH -#include "ErrorUtils.h" +#include #include "JSBigString.h" -#include "JSBundleType.h" #include "JSExecutor.h" #include "MessageQueueThread.h" -#include "MethodCall.h" #include "NativeToJsBridge.h" #include "RAMBundleRegistry.h" -#include "RecoverableError.h" #include "TraceSection.h" #include @@ -27,7 +24,6 @@ #include #include -#include #include #include #include diff --git a/packages/react-native/ReactCommon/cxxreact/MoveWrapper.h b/packages/react-native/ReactCommon/cxxreact/MoveWrapper.h index 22a9a0f238f467..217f5e4d89d823 100644 --- a/packages/react-native/ReactCommon/cxxreact/MoveWrapper.h +++ b/packages/react-native/ReactCommon/cxxreact/MoveWrapper.h @@ -7,125 +7,6 @@ #pragma once -#include +#warning Deprecated. Use instead. -namespace facebook::react { - -/* -NOTE: we keep this internal copy of folly/MoveWrapper.h to unblock -the the workstream of dropping the dependency on folly in RN! - -For a technical explanation on why we still need this we defer -to the doc in folly/Function.h: - -"There are some limitations in std::function that folly::Function tries to -avoid. std::function is copy-constructible and requires that the callable that -it wraps is copy-constructible as well, which is a constraint that is often -inconvenient. In most cases when using a std::function you don't make use of -its copy-constructibility, so you might sometimes feel like you get back very -little in return for a noticeable restriction. This restriction becomes -apparent when trying to use a lambda capturing a unique_ptr (or any -non-copyable type) as a callback for a folly::Future. - -std::unique_ptr foo_ptr = new Foo; - -some_future.then( - [foo_ptr = std::move(foo_ptr)] mutable - (int x) - { foo_ptr->setX(x); } -); - -This piece of code did not compile before folly::Future started using -folly::Function instead of std::function to store the callback. Because the -lambda captures something non-copyable (the unique_ptr), it is not copyable -itself. And std::function can only store copyable callables. - -The implementation of folly::Future did not make use of the -copy-constructibility of std::function at any point. There was no benefit from -the fact that the std::function is copy-constructible, but the fact that it can -only wrap copy-constructible callables posed a restriction. - -A workaround was available: folly::MoveWrapper, which wraps an object that may -be non-copyable and implements copy operations by moving the embedded object. -Using a folly::MoveWrapper, you can capture non-copyable objects in a lambda, -and the lambda itself is still copyable and may be wrapped in a std::function. -It is a pragmatic solution for the above problem, but you have to be a little -careful. The problem is that you can’t use a MoveWrapper anywhere where copy -operations are assumed to behave like actual copy operations. Also, a -folly::MoveWrapper> essentially behaves like auto_ptr. Ask -yourself whether you’d want to use lots of auto_ptrs in your codebase. And the -original question still persists: we very often don’t benefit from -copy-constructibility of std::function, so why do we have to live with this -restriction? I.e. why do we have to use MoveWrapper?" -*/ - -/** C++11 closures don't support move-in capture. Nor does std::bind. - facepalm. - - http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3610.html - - "[...] a work-around that should make people's stomach crawl: - write a wrapper that performs move-on-copy, much like the deprecated - auto_ptr" - - Unlike auto_ptr, this doesn't require a heap allocation. - */ -template -class MoveWrapper { - public: - /** If value can be default-constructed, why not? - Then we don't have to move it in */ - MoveWrapper() = default; - - /// Move a value in. - explicit MoveWrapper(T &&t) : value(std::move(t)) {} - - /// copy is move - MoveWrapper(const MoveWrapper &other) : value(std::move(other.value)) {} - - /// move is also move - MoveWrapper(MoveWrapper &&other) noexcept : value(std::move(other.value)) {} - - const T &operator*() const - { - return value; - } - T &operator*() - { - return value; - } - - const T *operator->() const - { - return &value; - } - T *operator->() - { - return &value; - } - - /// move the value out (sugar for std::move(*moveWrapper)) - T &&move() - { - return std::move(value); - } - - // If you want these you're probably doing it wrong, though they'd be - // easy enough to implement - MoveWrapper &operator=(const MoveWrapper &) = delete; - MoveWrapper &operator=(MoveWrapper &&) = delete; - - private: - mutable T value; -}; - -/// Make a MoveWrapper from the argument. Because the name "makeMoveWrapper" -/// is already quite transparent in its intent, this will work for lvalues as -/// if you had wrapped them in std::move. -template ::type> -MoveWrapper makeMoveWrapper(T &&t) -{ - return MoveWrapper(std::forward(t)); -} - -} // namespace facebook::react +#include diff --git a/packages/react-native/ReactCommon/cxxreact/NativeToJsBridge.cpp b/packages/react-native/ReactCommon/cxxreact/NativeToJsBridge.cpp index 4f39102bb4edd4..2ff380df4f4e76 100644 --- a/packages/react-native/ReactCommon/cxxreact/NativeToJsBridge.cpp +++ b/packages/react-native/ReactCommon/cxxreact/NativeToJsBridge.cpp @@ -12,16 +12,16 @@ #include #include #include +#include #include +#include #include -#include "ErrorUtils.h" #include "Instance.h" #include "JSBigString.h" #include "MessageQueueThread.h" #include "MethodCall.h" #include "ModuleRegistry.h" -#include "MoveWrapper.h" #include "RAMBundleRegistry.h" #include "TraceSection.h" diff --git a/packages/react-native/ReactCommon/cxxreact/React-cxxreact.podspec b/packages/react-native/ReactCommon/cxxreact/React-cxxreact.podspec index 59fbcf3c593e03..ee899bc320ccb6 100644 --- a/packages/react-native/ReactCommon/cxxreact/React-cxxreact.podspec +++ b/packages/react-native/ReactCommon/cxxreact/React-cxxreact.podspec @@ -40,6 +40,7 @@ Pod::Spec.new do |s| s.dependency "React-callinvoker", version add_dependency(s, "React-runtimeexecutor", :additional_framework_paths => ["platform/ios"]) s.dependency "React-perflogger", version + s.dependency "React-jserrorhandler", version s.dependency "React-jsi", version s.dependency "React-logger", version s.dependency "React-debug", version diff --git a/packages/react-native/ReactCommon/jserrorhandler/ErrorUtils.cpp b/packages/react-native/ReactCommon/jserrorhandler/ErrorUtils.cpp new file mode 100644 index 00000000000000..91f951e2121046 --- /dev/null +++ b/packages/react-native/ReactCommon/jserrorhandler/ErrorUtils.cpp @@ -0,0 +1,42 @@ +/* + * 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 "ErrorUtils.h" + +namespace facebook::react { + +void handleJSError( + jsi::Runtime& runtime, + const jsi::JSError& error, + bool isFatal) { + auto errorUtils = runtime.global().getProperty(runtime, "ErrorUtils"); + if (errorUtils.isUndefined() || !errorUtils.isObject() || + !errorUtils.getObject(runtime).hasProperty(runtime, "reportFatalError") || + !errorUtils.getObject(runtime).hasProperty(runtime, "reportError")) { + // ErrorUtils was not set up. This probably means the bundle didn't + // load properly. + throw jsi::JSError( + runtime, + "ErrorUtils is not set up properly. Something probably went wrong trying to load the JS bundle. Trying to report error " + + error.getMessage(), + error.getStack()); + } + + // TODO(janzer): Rewrite this function to return the processed error + // instead of just reporting it through the native module + if (isFatal) { + auto func = errorUtils.asObject(runtime).getPropertyAsFunction( + runtime, "reportFatalError"); + func.call(runtime, error.value()); + } else { + auto func = errorUtils.asObject(runtime).getPropertyAsFunction( + runtime, "reportError"); + func.call(runtime, error.value()); + } +} + +} // namespace facebook::react diff --git a/packages/react-native/ReactCommon/jserrorhandler/ErrorUtils.h b/packages/react-native/ReactCommon/jserrorhandler/ErrorUtils.h new file mode 100644 index 00000000000000..f71377e3a9a6f1 --- /dev/null +++ b/packages/react-native/ReactCommon/jserrorhandler/ErrorUtils.h @@ -0,0 +1,16 @@ +/* + * 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 + +namespace facebook::react { + +void handleJSError(jsi::Runtime &runtime, const jsi::JSError &error, bool isFatal); + +} // namespace facebook::react diff --git a/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp b/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp index 3ed51ff06c46cd..cf022c2d3516a6 100644 --- a/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp +++ b/packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp @@ -6,11 +6,13 @@ */ #include "JsErrorHandler.h" -#include + #include #include #include #include + +#include "ErrorUtils.h" #include "StackTraceParser.h" using namespace facebook; diff --git a/packages/react-native/ReactCommon/jserrorhandler/React-jserrorhandler.podspec b/packages/react-native/ReactCommon/jserrorhandler/React-jserrorhandler.podspec index a5cc33ecc99ac2..ba07c528b8ba01 100644 --- a/packages/react-native/ReactCommon/jserrorhandler/React-jserrorhandler.podspec +++ b/packages/react-native/ReactCommon/jserrorhandler/React-jserrorhandler.podspec @@ -37,7 +37,6 @@ Pod::Spec.new do |s| resolve_use_frameworks(s, header_mappings_dir: '../', module_name: "React_jserrorhandler") s.dependency "React-jsi" - s.dependency "React-cxxreact" s.dependency "ReactCommon/turbomodule/bridging" add_dependency(s, "React-featureflags") add_dependency(s, "React-debug") diff --git a/packages/react-native/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp b/packages/react-native/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp index 5c16909b002ca1..5975b468a6d508 100644 --- a/packages/react-native/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp +++ b/packages/react-native/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp @@ -7,13 +7,13 @@ #include "jsireact/JSIExecutor.h" -#include #include #include #include #include #include #include +#include #include #include #include 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 5c9b464a8c3a04..6b5c024a09230e 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 @@ -8,11 +8,11 @@ #include #include -#include #include #include #include #include +#include #include #include diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/React-runtimescheduler.podspec b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/React-runtimescheduler.podspec index 56823e61d623cd..864bd1919ee25c 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/React-runtimescheduler.podspec +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/React-runtimescheduler.podspec @@ -47,6 +47,7 @@ Pod::Spec.new do |s| s.dependency "React-utils" s.dependency "React-featureflags" s.dependency "React-timing" + s.dependency "React-jserrorhandler" s.dependency "React-jsi" s.dependency "React-performancetimeline" s.dependency "React-rendererconsistency" diff --git a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp index f7393dbdd520a9..0ae806bae8d496 100644 --- a/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp +++ b/packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler.cpp @@ -9,8 +9,8 @@ #include "RuntimeScheduler_Legacy.h" #include "RuntimeScheduler_Modern.h" -#include #include +#include #include #include diff --git a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp index 8e1dfd2fa0eea0..96ade0c64e3e27 100644 --- a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp +++ b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp @@ -8,12 +8,11 @@ #include "ReactInstance.h" #include -#include #include -#include #include #include #include +#include #include #include #include diff --git a/packages/react-native/ReactCommon/react/utils/MoveWrapper.h b/packages/react-native/ReactCommon/react/utils/MoveWrapper.h new file mode 100644 index 00000000000000..22a9a0f238f467 --- /dev/null +++ b/packages/react-native/ReactCommon/react/utils/MoveWrapper.h @@ -0,0 +1,131 @@ +/* + * 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 + +namespace facebook::react { + +/* +NOTE: we keep this internal copy of folly/MoveWrapper.h to unblock +the the workstream of dropping the dependency on folly in RN! + +For a technical explanation on why we still need this we defer +to the doc in folly/Function.h: + +"There are some limitations in std::function that folly::Function tries to +avoid. std::function is copy-constructible and requires that the callable that +it wraps is copy-constructible as well, which is a constraint that is often +inconvenient. In most cases when using a std::function you don't make use of +its copy-constructibility, so you might sometimes feel like you get back very +little in return for a noticeable restriction. This restriction becomes +apparent when trying to use a lambda capturing a unique_ptr (or any +non-copyable type) as a callback for a folly::Future. + +std::unique_ptr foo_ptr = new Foo; + +some_future.then( + [foo_ptr = std::move(foo_ptr)] mutable + (int x) + { foo_ptr->setX(x); } +); + +This piece of code did not compile before folly::Future started using +folly::Function instead of std::function to store the callback. Because the +lambda captures something non-copyable (the unique_ptr), it is not copyable +itself. And std::function can only store copyable callables. + +The implementation of folly::Future did not make use of the +copy-constructibility of std::function at any point. There was no benefit from +the fact that the std::function is copy-constructible, but the fact that it can +only wrap copy-constructible callables posed a restriction. + +A workaround was available: folly::MoveWrapper, which wraps an object that may +be non-copyable and implements copy operations by moving the embedded object. +Using a folly::MoveWrapper, you can capture non-copyable objects in a lambda, +and the lambda itself is still copyable and may be wrapped in a std::function. +It is a pragmatic solution for the above problem, but you have to be a little +careful. The problem is that you can’t use a MoveWrapper anywhere where copy +operations are assumed to behave like actual copy operations. Also, a +folly::MoveWrapper> essentially behaves like auto_ptr. Ask +yourself whether you’d want to use lots of auto_ptrs in your codebase. And the +original question still persists: we very often don’t benefit from +copy-constructibility of std::function, so why do we have to live with this +restriction? I.e. why do we have to use MoveWrapper?" +*/ + +/** C++11 closures don't support move-in capture. Nor does std::bind. + facepalm. + + http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2013/n3610.html + + "[...] a work-around that should make people's stomach crawl: + write a wrapper that performs move-on-copy, much like the deprecated + auto_ptr" + + Unlike auto_ptr, this doesn't require a heap allocation. + */ +template +class MoveWrapper { + public: + /** If value can be default-constructed, why not? + Then we don't have to move it in */ + MoveWrapper() = default; + + /// Move a value in. + explicit MoveWrapper(T &&t) : value(std::move(t)) {} + + /// copy is move + MoveWrapper(const MoveWrapper &other) : value(std::move(other.value)) {} + + /// move is also move + MoveWrapper(MoveWrapper &&other) noexcept : value(std::move(other.value)) {} + + const T &operator*() const + { + return value; + } + T &operator*() + { + return value; + } + + const T *operator->() const + { + return &value; + } + T *operator->() + { + return &value; + } + + /// move the value out (sugar for std::move(*moveWrapper)) + T &&move() + { + return std::move(value); + } + + // If you want these you're probably doing it wrong, though they'd be + // easy enough to implement + MoveWrapper &operator=(const MoveWrapper &) = delete; + MoveWrapper &operator=(MoveWrapper &&) = delete; + + private: + mutable T value; +}; + +/// Make a MoveWrapper from the argument. Because the name "makeMoveWrapper" +/// is already quite transparent in its intent, this will work for lvalues as +/// if you had wrapped them in std::move. +template ::type> +MoveWrapper makeMoveWrapper(T &&t) +{ + return MoveWrapper(std::forward(t)); +} + +} // namespace facebook::react