From 2f5a1e6124251a7b48ba9298e8d4dafdb749a2d7 Mon Sep 17 00:00:00 2001 From: Oleksandr Melnykov Date: Thu, 21 Apr 2022 16:27:57 -0700 Subject: [PATCH] Back out "Pass mutation list to RCTMountingTransactionObserving callbacks" Summary: https://fb.workplace.com/groups/fbapp.commerce.engsupport/permalink/2074812256012212/ Back out "[react-native][PR] Pass mutation list to RCTMountingTransactionObserving callbacks" Original commit changeset: f40afc512f2c Original Phabricator Diff: D35214478 (https://github.com/facebook/react-native/commit/91fc2c00919af98f248b2544f780d63e1056e1af) Changelog: [Internal] Reviewed By: cipolleschi Differential Revision: D35825832 fbshipit-source-id: b53b616dca39c84b3a8e8e4cbaa4a45834e53fe3 --- .../Modal/RCTModalHostViewComponentView.mm | 3 +- .../ScrollView/RCTScrollViewComponentView.mm | 3 +- .../Mounting/RCTComponentViewFactory.mm | 4 +-- React/Fabric/Mounting/RCTMountingManager.mm | 13 ++++---- ...CTMountingTransactionObserverCoordinator.h | 8 ++--- ...TMountingTransactionObserverCoordinator.mm | 18 +++++------ .../RCTMountingTransactionObserving.h | 8 ++--- .../mounting/MountingTransactionMetadata.cpp | 12 +++++++ .../mounting/MountingTransactionMetadata.h | 32 +++++++++++++++++++ .../renderer/mounting/TelemetryController.cpp | 15 +++++---- .../renderer/mounting/TelemetryController.h | 14 ++++---- 11 files changed, 84 insertions(+), 46 deletions(-) create mode 100644 ReactCommon/react/renderer/mounting/MountingTransactionMetadata.cpp create mode 100644 ReactCommon/react/renderer/mounting/MountingTransactionMetadata.h diff --git a/React/Fabric/Mounting/ComponentViews/Modal/RCTModalHostViewComponentView.mm b/React/Fabric/Mounting/ComponentViews/Modal/RCTModalHostViewComponentView.mm index 28c4bd227b7cc9..6cbe904eec9d2e 100644 --- a/React/Fabric/Mounting/ComponentViews/Modal/RCTModalHostViewComponentView.mm +++ b/React/Fabric/Mounting/ComponentViews/Modal/RCTModalHostViewComponentView.mm @@ -192,8 +192,7 @@ - (void)ensurePresentedOnlyIfNeeded #pragma mark - RCTMountingTransactionObserving -- (void)mountingTransactionWillMount:(MountingTransaction const &)transaction - withSurfaceTelemetry:(facebook::react::SurfaceTelemetry const &)surfaceTelemetry +- (void)mountingTransactionWillMountWithMetadata:(MountingTransactionMetadata const &)metadata { _modalContentsSnapshot = [self.viewController.view snapshotViewAfterScreenUpdates:NO]; } diff --git a/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm b/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm index 2d339aead9dc1a..7752626d2d8363 100644 --- a/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm +++ b/React/Fabric/Mounting/ComponentViews/ScrollView/RCTScrollViewComponentView.mm @@ -147,8 +147,7 @@ - (void)dealloc #pragma mark - RCTMountingTransactionObserving -- (void)mountingTransactionDidMount:(MountingTransaction const &)transaction - withSurfaceTelemetry:(facebook::react::SurfaceTelemetry const &)surfaceTelemetry +- (void)mountingTransactionDidMountWithMetadata:(MountingTransactionMetadata const &)metadata { [self _remountChildren]; } diff --git a/React/Fabric/Mounting/RCTComponentViewFactory.mm b/React/Fabric/Mounting/RCTComponentViewFactory.mm index 1487beaa2a92ce..cf5a9181243b78 100644 --- a/React/Fabric/Mounting/RCTComponentViewFactory.mm +++ b/React/Fabric/Mounting/RCTComponentViewFactory.mm @@ -95,9 +95,9 @@ - (RCTComponentViewClassDescriptor)_componentViewClassDescriptorFromClass:(Class { .viewClass = viewClass, .observesMountingTransactionWillMount = - (bool)class_respondsToSelector(viewClass, @selector(mountingTransactionWillMount:withSurfaceTelemetry:)), + (bool)class_respondsToSelector(viewClass, @selector(mountingTransactionWillMountWithMetadata:)), .observesMountingTransactionDidMount = - (bool)class_respondsToSelector(viewClass, @selector(mountingTransactionDidMount:withSurfaceTelemetry:)), + (bool)class_respondsToSelector(viewClass, @selector(mountingTransactionDidMountWithMetadata:)), }; #pragma clang diagnostic pop } diff --git a/React/Fabric/Mounting/RCTMountingManager.mm b/React/Fabric/Mounting/RCTMountingManager.mm index 23d57bc42fe364..fb18cf4e96d15b 100644 --- a/React/Fabric/Mounting/RCTMountingManager.mm +++ b/React/Fabric/Mounting/RCTMountingManager.mm @@ -266,16 +266,15 @@ - (void)performTransaction:(MountingCoordinator::Shared const &)mountingCoordina auto surfaceId = mountingCoordinator->getSurfaceId(); mountingCoordinator->getTelemetryController().pullTransaction( - [&](MountingTransaction const &transaction, SurfaceTelemetry const &surfaceTelemetry) { + [&](MountingTransactionMetadata metadata) { [self.delegate mountingManager:self willMountComponentsWithRootTag:surfaceId]; - _observerCoordinator.notifyObserversMountingTransactionWillMount(transaction, surfaceTelemetry); + _observerCoordinator.notifyObserversMountingTransactionWillMount(metadata); }, - [&](MountingTransaction const &transaction, SurfaceTelemetry const &surfaceTelemetry) { - RCTPerformMountInstructions( - transaction.getMutations(), _componentViewRegistry, _observerCoordinator, surfaceId); + [&](ShadowViewMutationList const &mutations) { + RCTPerformMountInstructions(mutations, _componentViewRegistry, _observerCoordinator, surfaceId); }, - [&](MountingTransaction const &transaction, SurfaceTelemetry const &surfaceTelemetry) { - _observerCoordinator.notifyObserversMountingTransactionDidMount(transaction, surfaceTelemetry); + [&](MountingTransactionMetadata metadata) { + _observerCoordinator.notifyObserversMountingTransactionDidMount(metadata); [self.delegate mountingManager:self didMountComponentsWithRootTag:surfaceId]; }); } diff --git a/React/Fabric/Mounting/RCTMountingTransactionObserverCoordinator.h b/React/Fabric/Mounting/RCTMountingTransactionObserverCoordinator.h index f8178140491aed..8f0f57234e2db3 100644 --- a/React/Fabric/Mounting/RCTMountingTransactionObserverCoordinator.h +++ b/React/Fabric/Mounting/RCTMountingTransactionObserverCoordinator.h @@ -11,7 +11,7 @@ #import #import -#include +#import class RCTMountingTransactionObserverCoordinator final { public: @@ -31,11 +31,9 @@ class RCTMountingTransactionObserverCoordinator final { * To be called from `RCTMountingManager`. */ void notifyObserversMountingTransactionWillMount( - facebook::react::MountingTransaction const &transaction, - facebook::react::SurfaceTelemetry const &surfaceTelemetry) const; + facebook::react::MountingTransactionMetadata const &metadata) const; void notifyObserversMountingTransactionDidMount( - facebook::react::MountingTransaction const &transaction, - facebook::react::SurfaceTelemetry const &surfaceTelemetry) const; + facebook::react::MountingTransactionMetadata const &metadata) const; private: facebook::butter::map< diff --git a/React/Fabric/Mounting/RCTMountingTransactionObserverCoordinator.mm b/React/Fabric/Mounting/RCTMountingTransactionObserverCoordinator.mm index 52424ac048f6b1..959c14603733a9 100644 --- a/React/Fabric/Mounting/RCTMountingTransactionObserverCoordinator.mm +++ b/React/Fabric/Mounting/RCTMountingTransactionObserverCoordinator.mm @@ -40,10 +40,9 @@ } void RCTMountingTransactionObserverCoordinator::notifyObserversMountingTransactionWillMount( - MountingTransaction const &transaction, - SurfaceTelemetry const &surfaceTelemetry) const + MountingTransactionMetadata const &metadata) const { - auto surfaceId = transaction.getSurfaceId(); + auto surfaceId = metadata.surfaceId; auto surfaceRegistryIterator = registry_.find(surfaceId); if (surfaceRegistryIterator == registry_.end()) { return; @@ -51,17 +50,16 @@ auto &surfaceRegistry = surfaceRegistryIterator->second; for (auto const &componentViewDescriptor : surfaceRegistry) { if (componentViewDescriptor.observesMountingTransactionWillMount) { - [(id)componentViewDescriptor.view mountingTransactionWillMount:transaction - withSurfaceTelemetry:surfaceTelemetry]; + [(id)componentViewDescriptor.view + mountingTransactionWillMountWithMetadata:metadata]; } } } void RCTMountingTransactionObserverCoordinator::notifyObserversMountingTransactionDidMount( - MountingTransaction const &transaction, - SurfaceTelemetry const &surfaceTelemetry) const + MountingTransactionMetadata const &metadata) const { - auto surfaceId = transaction.getSurfaceId(); + auto surfaceId = metadata.surfaceId; auto surfaceRegistryIterator = registry_.find(surfaceId); if (surfaceRegistryIterator == registry_.end()) { return; @@ -69,8 +67,8 @@ auto &surfaceRegistry = surfaceRegistryIterator->second; for (auto const &componentViewDescriptor : surfaceRegistry) { if (componentViewDescriptor.observesMountingTransactionDidMount) { - [(id)componentViewDescriptor.view mountingTransactionDidMount:transaction - withSurfaceTelemetry:surfaceTelemetry]; + [(id)componentViewDescriptor.view + mountingTransactionDidMountWithMetadata:metadata]; } } } diff --git a/React/Fabric/Mounting/RCTMountingTransactionObserving.h b/React/Fabric/Mounting/RCTMountingTransactionObserving.h index c24029a2d81ab1..35594d87ad61e7 100644 --- a/React/Fabric/Mounting/RCTMountingTransactionObserving.h +++ b/React/Fabric/Mounting/RCTMountingTransactionObserving.h @@ -7,7 +7,7 @@ #import -#include +#import NS_ASSUME_NONNULL_BEGIN @@ -50,16 +50,14 @@ NS_ASSUME_NONNULL_BEGIN * Is not being called for a component view which is being mounted as part of the transaction (because the view is not * registered as an observer yet). */ -- (void)mountingTransactionWillMount:(facebook::react::MountingTransaction const &)transaction - withSurfaceTelemetry:(facebook::react::SurfaceTelemetry const &)surfaceTelemetry; +- (void)mountingTransactionWillMountWithMetadata:(facebook::react::MountingTransactionMetadata const &)metadata; /* * Called right after the last mutation instruction is executed. * Is not being called for a component view which was being unmounted as part of the transaction (because the view is * not registered as an observer already). */ -- (void)mountingTransactionDidMount:(facebook::react::MountingTransaction const &)transaction - withSurfaceTelemetry:(facebook::react::SurfaceTelemetry const &)surfaceTelemetry; +- (void)mountingTransactionDidMountWithMetadata:(facebook::react::MountingTransactionMetadata const &)metadata; @end diff --git a/ReactCommon/react/renderer/mounting/MountingTransactionMetadata.cpp b/ReactCommon/react/renderer/mounting/MountingTransactionMetadata.cpp new file mode 100644 index 00000000000000..e6106cda4f0447 --- /dev/null +++ b/ReactCommon/react/renderer/mounting/MountingTransactionMetadata.cpp @@ -0,0 +1,12 @@ +/* + * 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 "MountingTransactionMetadata.h" + +namespace facebook { +namespace react {} // namespace react +} // namespace facebook diff --git a/ReactCommon/react/renderer/mounting/MountingTransactionMetadata.h b/ReactCommon/react/renderer/mounting/MountingTransactionMetadata.h new file mode 100644 index 00000000000000..d0eda81002cf7e --- /dev/null +++ b/ReactCommon/react/renderer/mounting/MountingTransactionMetadata.h @@ -0,0 +1,32 @@ +/* + * 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 +#include + +namespace facebook { +namespace react { + +/* + * Contains all (meta)information related to a MountingTransaction except a list + * of mutation instructions. + * The class is meant to be used when a consumer should not have access to all + * information about the transaction (incapsulation) but still needs to observe + * it to produce some side-effects. + */ +class MountingTransactionMetadata final { + public: + SurfaceId surfaceId; + MountingTransaction::Number number; + TransactionTelemetry telemetry; + SurfaceTelemetry surfaceTelemetry; +}; + +} // namespace react +} // namespace facebook diff --git a/ReactCommon/react/renderer/mounting/TelemetryController.cpp b/ReactCommon/react/renderer/mounting/TelemetryController.cpp index 98d4eecba9cbb1..f660768a5cd7b2 100644 --- a/ReactCommon/react/renderer/mounting/TelemetryController.cpp +++ b/ReactCommon/react/renderer/mounting/TelemetryController.cpp @@ -17,9 +17,10 @@ TelemetryController::TelemetryController( : mountingCoordinator_(mountingCoordinator) {} bool TelemetryController::pullTransaction( - MountingTransactionCallback const &willMount, - MountingTransactionCallback const &doMount, - MountingTransactionCallback const &didMount) const { + std::function const &willMount, + std::function const &doMount, + std::function const &didMount) + const { auto optional = mountingCoordinator_.pullTransaction(); if (!optional.has_value()) { return false; @@ -27,6 +28,8 @@ bool TelemetryController::pullTransaction( auto transaction = std::move(*optional); + auto surfaceId = transaction.getSurfaceId(); + auto number = transaction.getNumber(); auto telemetry = transaction.getTelemetry(); auto numberOfMutations = static_cast(transaction.getMutations().size()); @@ -34,15 +37,15 @@ bool TelemetryController::pullTransaction( auto compoundTelemetry = compoundTelemetry_; mutex_.unlock(); - willMount(transaction, compoundTelemetry); + willMount({surfaceId, number, telemetry, compoundTelemetry}); telemetry.willMount(); - doMount(transaction, compoundTelemetry); + doMount(transaction.getMutations()); telemetry.didMount(); compoundTelemetry.incorporate(telemetry, numberOfMutations); - didMount(transaction, compoundTelemetry); + didMount({surfaceId, number, telemetry, compoundTelemetry}); mutex_.lock(); compoundTelemetry_ = compoundTelemetry; diff --git a/ReactCommon/react/renderer/mounting/TelemetryController.h b/ReactCommon/react/renderer/mounting/TelemetryController.h index 23ecd25c806a54..8ea445e8f0a32d 100644 --- a/ReactCommon/react/renderer/mounting/TelemetryController.h +++ b/ReactCommon/react/renderer/mounting/TelemetryController.h @@ -11,6 +11,7 @@ #include #include +#include #include namespace facebook { @@ -18,10 +19,6 @@ namespace react { class MountingCoordinator; -using MountingTransactionCallback = std::function; - /* * Provides convenient tools for aggregating and accessing telemetry data * associated with running Surface. @@ -46,9 +43,12 @@ class TelemetryController final { * Calls `MountingCoordinator::pullTransaction()` and aggregates telemetry. */ bool pullTransaction( - MountingTransactionCallback const &willMount, - MountingTransactionCallback const &doMount, - MountingTransactionCallback const &didMount) const; + std::function const + &willMount, + std::function const + &doMount, + std::function const &didMount) + const; private: MountingCoordinator const &mountingCoordinator_;