From 1761aea17551dbcee62fecb210497ec3f9f1b7c3 Mon Sep 17 00:00:00 2001 From: William Bell Date: Mon, 31 Jul 2023 04:04:20 -0700 Subject: [PATCH 1/5] Update new_architecture.rb for React-ImageManager (#38247) Summary: This added React-ImageManager to the use_frameworks! - a lot of rpm modules podspec need this. bypass-github-export-checks [iOS] [FIXED] - Add React-ImageManager path to work with use_frameworks! For more details, see: https://reactnative.dev/contributing/changelogs-in-pull-requests Pull Request resolved: https://github.com/facebook/react-native/pull/38247 Test Plan: Should not be breaking - it will add to the header_search_paths React-ImageManager Reviewed By: dmytrorykun Differential Revision: D47593749 Pulled By: cipolleschi fbshipit-source-id: a66e90707e5fa73573deab1f04e8d8693869a90c --- .../scripts/cocoapods/__tests__/new_architecture-test.rb | 6 ++++-- packages/react-native/scripts/cocoapods/new_architecture.rb | 4 ++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/packages/react-native/scripts/cocoapods/__tests__/new_architecture-test.rb b/packages/react-native/scripts/cocoapods/__tests__/new_architecture-test.rb index a7157c469f7d98..82c5e390f25653 100644 --- a/packages/react-native/scripts/cocoapods/__tests__/new_architecture-test.rb +++ b/packages/react-native/scripts/cocoapods/__tests__/new_architecture-test.rb @@ -129,7 +129,7 @@ def test_installModulesDependencies_whenNewArchEnabledAndNewArchAndNoSearchPaths # Assert assert_equal(spec.compiler_flags, NewArchitectureHelper.folly_compiler_flags) - assert_equal(spec.pod_target_xcconfig["HEADER_SEARCH_PATHS"], "\"$(PODS_ROOT)/boost\" \"$(PODS_ROOT)/DoubleConversion\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers/react/renderer/graphics/platform/ios\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers/react/nativemodule/core\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-RCTFabric/RCTFabric.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-utils/React_utils.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-debug/React_debug.framework/Headers\"") + assert_equal(spec.pod_target_xcconfig["HEADER_SEARCH_PATHS"], "\"$(PODS_ROOT)/boost\" \"$(PODS_ROOT)/Headers/Private/Yoga\" \"$(PODS_ROOT)/DoubleConversion\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers/react/renderer/graphics/platform/ios\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-FabricImage/React_FabricImage.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers/react/nativemodule/core\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-RCTFabric/RCTFabric.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-utils/React_utils.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-debug/React_debug.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-ImageManager/React_ImageManager.framework/Headers\" \"$(PODS_CONFIGURATION_BUILD_DIR)/React-rendererdebug/React_rendererdebug.framework/Headers\"") assert_equal(spec.pod_target_xcconfig["CLANG_CXX_LANGUAGE_STANDARD"], "c++17") assert_equal(spec.pod_target_xcconfig["OTHER_CPLUSPLUSFLAGS"], "$(inherited) -DRCT_NEW_ARCH_ENABLED=1 -DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1") assert_equal( @@ -149,6 +149,8 @@ def test_installModulesDependencies_whenNewArchEnabledAndNewArchAndNoSearchPaths { :dependency_name => "React-graphics" }, { :dependency_name => "React-utils" }, { :dependency_name => "React-debug" }, + { :dependency_name => "React-ImageManager" }, + { :dependency_name => "React-rendererdebug" }, { :dependency_name => "hermes-engine" } ]) end @@ -167,7 +169,7 @@ def test_installModulesDependencies_whenNewArchDisabledAndSearchPathsAndCompiler # Assert assert_equal(spec.compiler_flags, "-Wno-nullability-completeness #{NewArchitectureHelper.folly_compiler_flags}") - assert_equal(spec.pod_target_xcconfig["HEADER_SEARCH_PATHS"], "#{other_flags} \"$(PODS_ROOT)/boost\" \"$(PODS_ROOT)/DoubleConversion\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers/react/renderer/graphics/platform/ios\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers/react/nativemodule/core\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-RCTFabric/RCTFabric.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-utils/React_utils.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-debug/React_debug.framework/Headers\"") + assert_equal(spec.pod_target_xcconfig["HEADER_SEARCH_PATHS"], "#{other_flags} \"$(PODS_ROOT)/boost\" \"$(PODS_ROOT)/Headers/Private/Yoga\" \"$(PODS_ROOT)/DoubleConversion\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers/react/renderer/graphics/platform/ios\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-FabricImage/React_FabricImage.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers/react/nativemodule/core\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-RCTFabric/RCTFabric.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-utils/React_utils.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-debug/React_debug.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-ImageManager/React_ImageManager.framework/Headers\" \"$(PODS_CONFIGURATION_BUILD_DIR)/React-rendererdebug/React_rendererdebug.framework/Headers\"") assert_equal(spec.pod_target_xcconfig["CLANG_CXX_LANGUAGE_STANDARD"], "c++17") assert_equal( spec.dependencies, diff --git a/packages/react-native/scripts/cocoapods/new_architecture.rb b/packages/react-native/scripts/cocoapods/new_architecture.rb index 1eb04c2c7f5298..8df1f93296fdc5 100644 --- a/packages/react-native/scripts/cocoapods/new_architecture.rb +++ b/packages/react-native/scripts/cocoapods/new_architecture.rb @@ -98,6 +98,8 @@ def self.install_modules_dependencies(spec, new_arch_enabled, folly_version) header_search_paths << "\"${PODS_CONFIGURATION_BUILD_DIR}/React-RCTFabric/RCTFabric.framework/Headers\"" header_search_paths << "\"${PODS_CONFIGURATION_BUILD_DIR}/React-utils/React_utils.framework/Headers\"" header_search_paths << "\"${PODS_CONFIGURATION_BUILD_DIR}/React-debug/React_debug.framework/Headers\"" + header_search_paths << "\"${PODS_CONFIGURATION_BUILD_DIR}/React-ImageManager/React_ImageManager.framework/Headers\"" + header_search_paths << "\"$(PODS_CONFIGURATION_BUILD_DIR)/React-rendererdebug/React_rendererdebug.framework/Headers\"" end header_search_paths_string = header_search_paths.join(" ") spec.compiler_flags = compiler_flags.empty? ? @@folly_compiler_flags : "#{compiler_flags} #{@@folly_compiler_flags}" @@ -125,6 +127,8 @@ def self.install_modules_dependencies(spec, new_arch_enabled, folly_version) spec.dependency "React-graphics" spec.dependency "React-utils" spec.dependency "React-debug" + spec.dependency "React-ImageManager" + spec.dependency "React-rendererdebug" if ENV["USE_HERMES"] == nil || ENV["USE_HERMES"] == "1" spec.dependency "hermes-engine" From abedc58477b18c26bdcc0cb40b069f7de7e07d87 Mon Sep 17 00:00:00 2001 From: Luna Wei Date: Tue, 8 Aug 2023 12:03:06 -0700 Subject: [PATCH 2/5] Fix missing Platform in VirtualizedList --- packages/virtualized-lists/Lists/VirtualizedList.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/virtualized-lists/Lists/VirtualizedList.js b/packages/virtualized-lists/Lists/VirtualizedList.js index 29625326ec153c..ef5a3f0a87099b 100644 --- a/packages/virtualized-lists/Lists/VirtualizedList.js +++ b/packages/virtualized-lists/Lists/VirtualizedList.js @@ -25,6 +25,7 @@ import type { } from './VirtualizedListProps'; import { + Platform, RefreshControl, ScrollView, View, From 3386bb4dae22507dd33f22c68fdbd364a40d9d85 Mon Sep 17 00:00:00 2001 From: Nicola Corti Date: Tue, 8 Aug 2023 20:13:49 +0100 Subject: [PATCH 3/5] [LOCAL] Fabric Interop - Properly dispatch integer commands (#38527) (#38835) resolved: https://github.com/facebook/react-native/pull/38527 --- .../react/fabric/FabricUIManager.java | 47 +++++++++++---- .../react/fabric/FabricUIManagerTest.kt | 57 +++++++++++++++++++ .../fakes/FakeBatchEventDispatchedListener.kt | 17 ++++++ .../uiapp/component/MyLegacyViewManager.java | 20 +++++-- 4 files changed, 126 insertions(+), 15 deletions(-) create mode 100644 packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/FabricUIManagerTest.kt create mode 100644 packages/react-native/ReactAndroid/src/test/java/com/facebook/testutils/fakes/FakeBatchEventDispatchedListener.kt diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java index 49acc4c358371a..c0df88da2c6cd1 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -53,7 +53,6 @@ import com.facebook.react.common.build.ReactBuildConfig; import com.facebook.react.common.mapbuffer.ReadableMapBuffer; import com.facebook.react.config.ReactFeatureFlags; -import com.facebook.react.fabric.events.EventBeatManager; import com.facebook.react.fabric.events.EventEmitterWrapper; import com.facebook.react.fabric.events.FabricEventEmitter; import com.facebook.react.fabric.interop.InteropEventEmitter; @@ -61,6 +60,7 @@ import com.facebook.react.fabric.mounting.MountingManager; import com.facebook.react.fabric.mounting.SurfaceMountingManager; import com.facebook.react.fabric.mounting.SurfaceMountingManager.ViewEvent; +import com.facebook.react.fabric.mounting.mountitems.DispatchCommandMountItem; import com.facebook.react.fabric.mounting.mountitems.DispatchIntCommandMountItem; import com.facebook.react.fabric.mounting.mountitems.DispatchStringCommandMountItem; import com.facebook.react.fabric.mounting.mountitems.IntBufferBatchMountItem; @@ -79,6 +79,7 @@ import com.facebook.react.uimanager.UIManagerHelper; import com.facebook.react.uimanager.ViewManagerPropertyUpdater; import com.facebook.react.uimanager.ViewManagerRegistry; +import com.facebook.react.uimanager.events.BatchEventDispatchedListener; import com.facebook.react.uimanager.events.EventCategoryDef; import com.facebook.react.uimanager.events.EventDispatcher; import com.facebook.react.uimanager.events.EventDispatcherImpl; @@ -168,7 +169,7 @@ public void onFabricCommitEnd(DevToolsReactPerfLogger.FabricCommitPoint commitPo @NonNull private final MountItemDispatcher mMountItemDispatcher; @NonNull private final ViewManagerRegistry mViewManagerRegistry; - @NonNull private final EventBeatManager mEventBeatManager; + @NonNull private final BatchEventDispatchedListener mBatchEventDispatchedListener; @NonNull private final CopyOnWriteArrayList mListeners = new CopyOnWriteArrayList<>(); @@ -208,16 +209,16 @@ public void executeItems(Queue items) { }; public FabricUIManager( - ReactApplicationContext reactContext, - ViewManagerRegistry viewManagerRegistry, - EventBeatManager eventBeatManager) { + @NonNull ReactApplicationContext reactContext, + @NonNull ViewManagerRegistry viewManagerRegistry, + @NonNull BatchEventDispatchedListener batchEventDispatchedListener) { mDispatchUIFrameCallback = new DispatchUIFrameCallback(reactContext); mReactApplicationContext = reactContext; mMountingManager = new MountingManager(viewManagerRegistry, mMountItemExecutor); mMountItemDispatcher = new MountItemDispatcher(mMountingManager, new MountItemDispatchListener()); mEventDispatcher = new EventDispatcherImpl(reactContext); - mEventBeatManager = eventBeatManager; + mBatchEventDispatchedListener = batchEventDispatchedListener; mReactApplicationContext.addLifecycleEventListener(this); mViewManagerRegistry = viewManagerRegistry; @@ -385,7 +386,7 @@ public void stopSurface(final int surfaceID) { @Override public void initialize() { mEventDispatcher.registerEventEmitter(FABRIC, new FabricEventEmitter(this)); - mEventDispatcher.addBatchEventDispatchedListener(mEventBeatManager); + mEventDispatcher.addBatchEventDispatchedListener(mBatchEventDispatchedListener); if (ENABLE_FABRIC_PERF_LOGS) { mDevToolsReactPerfLogger = new DevToolsReactPerfLogger(); mDevToolsReactPerfLogger.addDevToolsReactPerfLoggerListener(FABRIC_PERF_LOGGER); @@ -424,7 +425,7 @@ public void onCatalystInstanceDestroy() { // memory immediately. mDispatchUIFrameCallback.stop(); - mEventDispatcher.removeBatchEventDispatchedListener(mEventBeatManager); + mEventDispatcher.removeBatchEventDispatchedListener(mBatchEventDispatchedListener); mEventDispatcher.unregisterEventEmitter(FABRIC); mReactApplicationContext.unregisterComponentCallbacks(mViewManagerRegistry); @@ -1039,8 +1040,16 @@ public void dispatchCommand( final int reactTag, final String commandId, @Nullable final ReadableArray commandArgs) { - mMountItemDispatcher.dispatchCommandMountItem( - new DispatchStringCommandMountItem(surfaceId, reactTag, commandId, commandArgs)); + if (ReactFeatureFlags.unstable_useFabricInterop) { + // For Fabric Interop, we check if the commandId is an integer. If it is, we use the integer + // overload of dispatchCommand. Otherwise, we use the string overload. + // and the events won't be correctly dispatched. + mMountItemDispatcher.dispatchCommandMountItem( + createDispatchCommandMountItemForInterop(surfaceId, reactTag, commandId, commandArgs)); + } else { + mMountItemDispatcher.dispatchCommandMountItem( + new DispatchStringCommandMountItem(surfaceId, reactTag, commandId, commandArgs)); + } } @Override @@ -1200,6 +1209,24 @@ public void didDispatchMountItems() { } } + /** + * Util function that takes care of handling commands for Fabric Interop. If the command is a + * string that represents a number (say "42"), it will be parsed as an integer and the + * corresponding dispatch command mount item will be created. + */ + /* package */ DispatchCommandMountItem createDispatchCommandMountItemForInterop( + final int surfaceId, + final int reactTag, + final String commandId, + @Nullable final ReadableArray commandArgs) { + try { + int commandIdInteger = Integer.parseInt(commandId); + return new DispatchIntCommandMountItem(surfaceId, reactTag, commandIdInteger, commandArgs); + } catch (NumberFormatException e) { + return new DispatchStringCommandMountItem(surfaceId, reactTag, commandId, commandArgs); + } + } + private class DispatchUIFrameCallback extends GuardedFrameCallback { private volatile boolean mIsMountingEnabled = true; diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/FabricUIManagerTest.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/FabricUIManagerTest.kt new file mode 100644 index 00000000000000..f8827eb7225547 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/react/fabric/FabricUIManagerTest.kt @@ -0,0 +1,57 @@ +/* + * 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.fabric + +import com.facebook.react.bridge.ReactApplicationContext +import com.facebook.react.uimanager.ViewManagerRegistry +import com.facebook.react.uimanager.events.BatchEventDispatchedListener +import com.facebook.testutils.fakes.FakeBatchEventDispatchedListener +import com.facebook.testutils.shadows.ShadowSoLoader +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import org.robolectric.annotation.Config + +@RunWith(RobolectricTestRunner::class) +@Config(shadows = [ShadowSoLoader::class]) +class FabricUIManagerTest { + + private lateinit var reactContext: ReactApplicationContext + private lateinit var viewManagerRegistry: ViewManagerRegistry + private lateinit var batchEventDispatchedListener: BatchEventDispatchedListener + private lateinit var underTest: FabricUIManager + + @Before + fun setup() { + reactContext = ReactApplicationContext(RuntimeEnvironment.getApplication()) + viewManagerRegistry = ViewManagerRegistry(emptyList()) + batchEventDispatchedListener = FakeBatchEventDispatchedListener() + underTest = FabricUIManager(reactContext, viewManagerRegistry, batchEventDispatchedListener) + } + + @Test + fun createDispatchCommandMountItemForInterop_withValidString_returnsStringEvent() { + val command = underTest.createDispatchCommandMountItemForInterop(11, 1, "anEvent", null) + + // DispatchStringCommandMountItem is package private so we can `as` check it. + val className = command::class.java.name.substringAfterLast(".") + assertEquals("DispatchStringCommandMountItem", className) + } + + @Test + fun createDispatchCommandMountItemForInterop_withValidInt_returnsIntEvent() { + val command = underTest.createDispatchCommandMountItemForInterop(11, 1, "42", null) + + // DispatchIntCommandMountItem is package private so we can `as` check it. + val className = command::class.java.name.substringAfterLast(".") + assertEquals("DispatchIntCommandMountItem", className) + } +} diff --git a/packages/react-native/ReactAndroid/src/test/java/com/facebook/testutils/fakes/FakeBatchEventDispatchedListener.kt b/packages/react-native/ReactAndroid/src/test/java/com/facebook/testutils/fakes/FakeBatchEventDispatchedListener.kt new file mode 100644 index 00000000000000..9f3dd1d78398a6 --- /dev/null +++ b/packages/react-native/ReactAndroid/src/test/java/com/facebook/testutils/fakes/FakeBatchEventDispatchedListener.kt @@ -0,0 +1,17 @@ +/* + * 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.testutils.fakes + +import com.facebook.react.uimanager.events.BatchEventDispatchedListener + +/** A fake [BatchEventDispatchedListener] for testing that does nothing. */ +class FakeBatchEventDispatchedListener : BatchEventDispatchedListener { + override fun onBatchEventDispatched() { + // do nothing + } +} diff --git a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/component/MyLegacyViewManager.java b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/component/MyLegacyViewManager.java index a192837166ffa1..e2f34691472d88 100644 --- a/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/component/MyLegacyViewManager.java +++ b/packages/rn-tester/android/app/src/main/java/com/facebook/react/uiapp/component/MyLegacyViewManager.java @@ -27,7 +27,7 @@ public class MyLegacyViewManager extends SimpleViewManager { public static final String REACT_CLASS = "RNTMyLegacyNativeView"; - private static final Integer COMMAND_CHANGE_BACKGROUND_COLOR = 42; + private static final int COMMAND_CHANGE_BACKGROUND_COLOR = 42; private final ReactApplicationContext mCallerContext; public MyLegacyViewManager(ReactApplicationContext reactContext) { @@ -71,8 +71,7 @@ public final Map getExportedViewConstants() { @Override public final Map getExportedCustomBubblingEventTypeConstants() { - Map eventTypeConstants = new HashMap(); - eventTypeConstants.putAll( + return new HashMap<>( MapBuilder.builder() .put( "onColorChanged", @@ -81,18 +80,29 @@ public final Map getExportedCustomBubblingEventTypeConstants() { MapBuilder.of( "bubbled", "onColorChanged", "captured", "onColorChangedCapture"))) .build()); - return eventTypeConstants; } @Override public void receiveCommand( @NonNull MyNativeView view, String commandId, @Nullable ReadableArray args) { - if (commandId.contentEquals(COMMAND_CHANGE_BACKGROUND_COLOR.toString())) { + if (commandId.contentEquals("changeBackgroundColor")) { int sentColor = Color.parseColor(args.getString(0)); view.setBackgroundColor(sentColor); } } + @SuppressWarnings("deprecation") // We intentionally want to test against the legacy API here. + @Override + public void receiveCommand( + @NonNull MyNativeView view, int commandId, @Nullable ReadableArray args) { + switch (commandId) { + case COMMAND_CHANGE_BACKGROUND_COLOR: + int sentColor = Color.parseColor(args.getString(0)); + view.setBackgroundColor(sentColor); + break; + } + } + @Nullable @Override public Map getCommandsMap() { From c7f026166f656c60eea55314c714c2c97f477e42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20M=C4=85ka?= Date: Fri, 4 Aug 2023 07:33:37 -0700 Subject: [PATCH 4/5] Restore checking shadow tree commit cancellation after commit hook execution (#38715) Summary: Hello! This PR is a fix for one merged some time ago (https://github.com/facebook/react-native/pull/36216). In the PR check for `nullptr` value of `newRootShadowNode` just after performing commit hooks was overlooked. This PR restores previous behaviour of conditional commit cancellation after commit hook execution. ## Changelog: [INTERNAL] [FIXED] - Restore checking shadow tree commit cancellation after commit hook execution Pull Request resolved: https://github.com/facebook/react-native/pull/38715 Test Plan: Just register a commit hook that return `nullptr`. In that case current code crashes due to `nullptr` dereference. Reviewed By: sammy-SC Differential Revision: D47972245 Pulled By: ryancat fbshipit-source-id: 7599ad11ed4b2dcaf25e53f676ec4530e37410d5 --- .../react/renderer/mounting/ShadowTree.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp b/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp index 0b9106bbb5d2b0..5278e34fc99dd6 100644 --- a/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp +++ b/packages/react-native/ReactCommon/react/renderer/mounting/ShadowTree.cpp @@ -355,6 +355,11 @@ CommitStatus ShadowTree::tryCommit( newRootShadowNode = delegate_.shadowTreeWillCommit( *this, oldRootShadowNode, newRootShadowNode); + if (!newRootShadowNode || + (commitOptions.shouldYield && commitOptions.shouldYield())) { + return CommitStatus::Cancelled; + } + // Layout nodes. std::vector affectedLayoutableNodes{}; affectedLayoutableNodes.reserve(1024); @@ -372,17 +377,16 @@ CommitStatus ShadowTree::tryCommit( // Updating `currentRevision_` in unique manner if it hasn't changed. std::unique_lock lock(commitMutex_); + if (commitOptions.shouldYield && commitOptions.shouldYield()) { + return CommitStatus::Cancelled; + } + if (currentRevision_.number != oldRevision.number) { return CommitStatus::Failed; } auto newRevisionNumber = oldRevision.number + 1; - if (!newRootShadowNode || - (commitOptions.shouldYield && commitOptions.shouldYield())) { - return CommitStatus::Cancelled; - } - { std::lock_guard dispatchLock(EventEmitter::DispatchMutex()); From db81d8ff0e52bfecab3e1df60e2933aab954e041 Mon Sep 17 00:00:00 2001 From: Riccardo Cipolleschi Date: Thu, 10 Aug 2023 10:02:13 +0100 Subject: [PATCH 5/5] [LOCAL] Fix Ruby Tests for 0.72 --- .../scripts/cocoapods/__tests__/new_architecture-test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-native/scripts/cocoapods/__tests__/new_architecture-test.rb b/packages/react-native/scripts/cocoapods/__tests__/new_architecture-test.rb index 82c5e390f25653..53f08d51998948 100644 --- a/packages/react-native/scripts/cocoapods/__tests__/new_architecture-test.rb +++ b/packages/react-native/scripts/cocoapods/__tests__/new_architecture-test.rb @@ -129,7 +129,7 @@ def test_installModulesDependencies_whenNewArchEnabledAndNewArchAndNoSearchPaths # Assert assert_equal(spec.compiler_flags, NewArchitectureHelper.folly_compiler_flags) - assert_equal(spec.pod_target_xcconfig["HEADER_SEARCH_PATHS"], "\"$(PODS_ROOT)/boost\" \"$(PODS_ROOT)/Headers/Private/Yoga\" \"$(PODS_ROOT)/DoubleConversion\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers/react/renderer/graphics/platform/ios\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-FabricImage/React_FabricImage.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers/react/nativemodule/core\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-RCTFabric/RCTFabric.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-utils/React_utils.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-debug/React_debug.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-ImageManager/React_ImageManager.framework/Headers\" \"$(PODS_CONFIGURATION_BUILD_DIR)/React-rendererdebug/React_rendererdebug.framework/Headers\"") + assert_equal(spec.pod_target_xcconfig["HEADER_SEARCH_PATHS"], "\"$(PODS_ROOT)/boost\" \"$(PODS_ROOT)/DoubleConversion\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers/react/renderer/graphics/platform/ios\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers/react/nativemodule/core\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-RCTFabric/RCTFabric.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-utils/React_utils.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-debug/React_debug.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-ImageManager/React_ImageManager.framework/Headers\" \"$(PODS_CONFIGURATION_BUILD_DIR)/React-rendererdebug/React_rendererdebug.framework/Headers\"") assert_equal(spec.pod_target_xcconfig["CLANG_CXX_LANGUAGE_STANDARD"], "c++17") assert_equal(spec.pod_target_xcconfig["OTHER_CPLUSPLUSFLAGS"], "$(inherited) -DRCT_NEW_ARCH_ENABLED=1 -DFOLLY_NO_CONFIG -DFOLLY_MOBILE=1 -DFOLLY_USE_LIBCPP=1") assert_equal( @@ -169,7 +169,7 @@ def test_installModulesDependencies_whenNewArchDisabledAndSearchPathsAndCompiler # Assert assert_equal(spec.compiler_flags, "-Wno-nullability-completeness #{NewArchitectureHelper.folly_compiler_flags}") - assert_equal(spec.pod_target_xcconfig["HEADER_SEARCH_PATHS"], "#{other_flags} \"$(PODS_ROOT)/boost\" \"$(PODS_ROOT)/Headers/Private/Yoga\" \"$(PODS_ROOT)/DoubleConversion\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers/react/renderer/graphics/platform/ios\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-FabricImage/React_FabricImage.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers/react/nativemodule/core\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-RCTFabric/RCTFabric.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-utils/React_utils.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-debug/React_debug.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-ImageManager/React_ImageManager.framework/Headers\" \"$(PODS_CONFIGURATION_BUILD_DIR)/React-rendererdebug/React_rendererdebug.framework/Headers\"") + assert_equal(spec.pod_target_xcconfig["HEADER_SEARCH_PATHS"], "#{other_flags} \"$(PODS_ROOT)/boost\" \"$(PODS_ROOT)/DoubleConversion\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-graphics/React_graphics.framework/Headers/react/renderer/graphics/platform/ios\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-Fabric/React_Fabric.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/ReactCommon/ReactCommon.framework/Headers/react/nativemodule/core\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-RCTFabric/RCTFabric.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-utils/React_utils.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-debug/React_debug.framework/Headers\" \"${PODS_CONFIGURATION_BUILD_DIR}/React-ImageManager/React_ImageManager.framework/Headers\" \"$(PODS_CONFIGURATION_BUILD_DIR)/React-rendererdebug/React_rendererdebug.framework/Headers\"") assert_equal(spec.pod_target_xcconfig["CLANG_CXX_LANGUAGE_STANDARD"], "c++17") assert_equal( spec.dependencies,