Skip to content

Commit

Permalink
Pass mutation list to RCTMountingTransactionObserving callbacks (#33510)
Browse files Browse the repository at this point in the history
Summary:
This PR updates `RCTMountingTransactionObserving` protocol to accept full `MountingTransaction` object as an argument to its methods as opposed to just `MountingTransactionMetdata` which contained only some subset of information.

This change makes it possible for components implementing the protocol to analyze the list of mutations and hence only take action when certain mutations are about to happen.

One of the use cases for `RCTMountingTransactionObserving` protocol is to allow for Modal to take view snapshot before it is closed, such that an animated close transition can be performed over the snapshotted content. Note that when modal is removed from the view hierarchy its children are gone too and therefore the snapshot mechanism makes it possible for children to still be visible while the animated closing transition is ongoing. A similar use-case to that can be seen in react-native-screens library, where we use the same snapshot mechanism for views that are removed from navigation stack.

Before this change, we'd use `mountingTransactionDidMountWithMetadata` to take a snapshot. However, making a snapshot of relatively complex view hierarchy can be expensive, and we'd like to make sure that we only perform a snapshot when the given modal is about to be removed. Because the mentioned method does not provide an information about what changes are going to be performed in a given transaction, we'd make the snapshot for every single view transaction that happens while the modal is mounted.

In this PR we're updating `RCTMountingTransactionObserving` protocol's methods, in particular, we rename methods to no longer contain "Metadata" in them and to accept `MountingTransaction` as the only argument instead of `MountingTransactionMetadata` object. With this change we are also deleting `MountingTransactionMetadata` altogether as it has no uses outside the protocol. Finally, we update the two uses of the protocol in `RCTScrollViewComponentView` and `RCTModalHostViewComponentView`.

## Changelog

[iOS][Fabric] - Update RCTMountingTransactionObserving protocol to consume MountingTransaction objects

Pull Request resolved: #33510

Test Plan:
As there are not that many uses of `RCTMountingTransactionObserving` protocol during testing I focused on checking if the updated method is called and if the provided objects contains the proper data. Unfortunately, despite code for the modal protocol being present in OSS version it does seem like some parts of modal implementation are still missing and the component only renders an unimplemented view (checked this with rn-tester). I only managed to verify the use in `RCTScrollViewComponentView` with the following steps:
1. Build for iOS
2. Put a breakpoint in mountingTransactionDidMount method in `RCTScrollViewComponentView.mm`
3. Verify that the program stops on the breakpoint when a scrollview is rendered (use any screen on rn-tester app)
4. Inspect the provided object in the debugger (ensure the list of transactions is not empty)

Outside of that we verified the transactions can be processed in `mountingTransactionDidMount` after the changes from this PR are applied in FabricExample app in [react-native-screens](https://github.com/software-mansion/react-native-screens/tree/main/FabricExample) repo.

Reviewed By: cipolleschi

Differential Revision: D35214478

Pulled By: ShikaSD

fbshipit-source-id: f40afc512f2c8cfa6262d2fb82fb1ccb05aa734c
  • Loading branch information
kmagiera authored and facebook-github-bot committed Apr 21, 2022
1 parent 29c5461 commit 91fc2c0
Show file tree
Hide file tree
Showing 11 changed files with 46 additions and 84 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ - (void)ensurePresentedOnlyIfNeeded

#pragma mark - RCTMountingTransactionObserving

- (void)mountingTransactionWillMountWithMetadata:(MountingTransactionMetadata const &)metadata
- (void)mountingTransactionWillMount:(MountingTransaction const &)transaction
withSurfaceTelemetry:(facebook::react::SurfaceTelemetry const &)surfaceTelemetry
{
_modalContentsSnapshot = [self.viewController.view snapshotViewAfterScreenUpdates:NO];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,8 @@ - (void)dealloc

#pragma mark - RCTMountingTransactionObserving

- (void)mountingTransactionDidMountWithMetadata:(MountingTransactionMetadata const &)metadata
- (void)mountingTransactionDidMount:(MountingTransaction const &)transaction
withSurfaceTelemetry:(facebook::react::SurfaceTelemetry const &)surfaceTelemetry
{
[self _remountChildren];
}
Expand Down
4 changes: 2 additions & 2 deletions React/Fabric/Mounting/RCTComponentViewFactory.mm
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,9 @@ - (RCTComponentViewClassDescriptor)_componentViewClassDescriptorFromClass:(Class
{
.viewClass = viewClass,
.observesMountingTransactionWillMount =
(bool)class_respondsToSelector(viewClass, @selector(mountingTransactionWillMountWithMetadata:)),
(bool)class_respondsToSelector(viewClass, @selector(mountingTransactionWillMount:withSurfaceTelemetry:)),
.observesMountingTransactionDidMount =
(bool)class_respondsToSelector(viewClass, @selector(mountingTransactionDidMountWithMetadata:)),
(bool)class_respondsToSelector(viewClass, @selector(mountingTransactionDidMount:withSurfaceTelemetry:)),
};
#pragma clang diagnostic pop
}
Expand Down
13 changes: 7 additions & 6 deletions React/Fabric/Mounting/RCTMountingManager.mm
Original file line number Diff line number Diff line change
Expand Up @@ -266,15 +266,16 @@ - (void)performTransaction:(MountingCoordinator::Shared const &)mountingCoordina
auto surfaceId = mountingCoordinator->getSurfaceId();

mountingCoordinator->getTelemetryController().pullTransaction(
[&](MountingTransactionMetadata metadata) {
[&](MountingTransaction const &transaction, SurfaceTelemetry const &surfaceTelemetry) {
[self.delegate mountingManager:self willMountComponentsWithRootTag:surfaceId];
_observerCoordinator.notifyObserversMountingTransactionWillMount(metadata);
_observerCoordinator.notifyObserversMountingTransactionWillMount(transaction, surfaceTelemetry);
},
[&](ShadowViewMutationList const &mutations) {
RCTPerformMountInstructions(mutations, _componentViewRegistry, _observerCoordinator, surfaceId);
[&](MountingTransaction const &transaction, SurfaceTelemetry const &surfaceTelemetry) {
RCTPerformMountInstructions(
transaction.getMutations(), _componentViewRegistry, _observerCoordinator, surfaceId);
},
[&](MountingTransactionMetadata metadata) {
_observerCoordinator.notifyObserversMountingTransactionDidMount(metadata);
[&](MountingTransaction const &transaction, SurfaceTelemetry const &surfaceTelemetry) {
_observerCoordinator.notifyObserversMountingTransactionDidMount(transaction, surfaceTelemetry);
[self.delegate mountingManager:self didMountComponentsWithRootTag:surfaceId];
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#import <butter/map.h>
#import <butter/set.h>

#import <react/renderer/mounting/MountingTransactionMetadata.h>
#include <react/renderer/mounting/MountingTransaction.h>

class RCTMountingTransactionObserverCoordinator final {
public:
Expand All @@ -31,9 +31,11 @@ class RCTMountingTransactionObserverCoordinator final {
* To be called from `RCTMountingManager`.
*/
void notifyObserversMountingTransactionWillMount(
facebook::react::MountingTransactionMetadata const &metadata) const;
facebook::react::MountingTransaction const &transaction,
facebook::react::SurfaceTelemetry const &surfaceTelemetry) const;
void notifyObserversMountingTransactionDidMount(
facebook::react::MountingTransactionMetadata const &metadata) const;
facebook::react::MountingTransaction const &transaction,
facebook::react::SurfaceTelemetry const &surfaceTelemetry) const;

private:
facebook::butter::map<
Expand Down
18 changes: 10 additions & 8 deletions React/Fabric/Mounting/RCTMountingTransactionObserverCoordinator.mm
Original file line number Diff line number Diff line change
Expand Up @@ -40,35 +40,37 @@
}

void RCTMountingTransactionObserverCoordinator::notifyObserversMountingTransactionWillMount(
MountingTransactionMetadata const &metadata) const
MountingTransaction const &transaction,
SurfaceTelemetry const &surfaceTelemetry) const
{
auto surfaceId = metadata.surfaceId;
auto surfaceId = transaction.getSurfaceId();
auto surfaceRegistryIterator = registry_.find(surfaceId);
if (surfaceRegistryIterator == registry_.end()) {
return;
}
auto &surfaceRegistry = surfaceRegistryIterator->second;
for (auto const &componentViewDescriptor : surfaceRegistry) {
if (componentViewDescriptor.observesMountingTransactionWillMount) {
[(id<RCTMountingTransactionObserving>)componentViewDescriptor.view
mountingTransactionWillMountWithMetadata:metadata];
[(id<RCTMountingTransactionObserving>)componentViewDescriptor.view mountingTransactionWillMount:transaction
withSurfaceTelemetry:surfaceTelemetry];
}
}
}

void RCTMountingTransactionObserverCoordinator::notifyObserversMountingTransactionDidMount(
MountingTransactionMetadata const &metadata) const
MountingTransaction const &transaction,
SurfaceTelemetry const &surfaceTelemetry) const
{
auto surfaceId = metadata.surfaceId;
auto surfaceId = transaction.getSurfaceId();
auto surfaceRegistryIterator = registry_.find(surfaceId);
if (surfaceRegistryIterator == registry_.end()) {
return;
}
auto &surfaceRegistry = surfaceRegistryIterator->second;
for (auto const &componentViewDescriptor : surfaceRegistry) {
if (componentViewDescriptor.observesMountingTransactionDidMount) {
[(id<RCTMountingTransactionObserving>)componentViewDescriptor.view
mountingTransactionDidMountWithMetadata:metadata];
[(id<RCTMountingTransactionObserving>)componentViewDescriptor.view mountingTransactionDidMount:transaction
withSurfaceTelemetry:surfaceTelemetry];
}
}
}
8 changes: 5 additions & 3 deletions React/Fabric/Mounting/RCTMountingTransactionObserving.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

#import <UIKit/UIKit.h>

#import <react/renderer/mounting/MountingTransactionMetadata.h>
#include <react/renderer/mounting/MountingTransaction.h>

NS_ASSUME_NONNULL_BEGIN

Expand Down Expand Up @@ -50,14 +50,16 @@ 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)mountingTransactionWillMountWithMetadata:(facebook::react::MountingTransactionMetadata const &)metadata;
- (void)mountingTransactionWillMount:(facebook::react::MountingTransaction const &)transaction
withSurfaceTelemetry:(facebook::react::SurfaceTelemetry const &)surfaceTelemetry;

/*
* 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)mountingTransactionDidMountWithMetadata:(facebook::react::MountingTransactionMetadata const &)metadata;
- (void)mountingTransactionDidMount:(facebook::react::MountingTransaction const &)transaction
withSurfaceTelemetry:(facebook::react::SurfaceTelemetry const &)surfaceTelemetry;

@end

Expand Down

This file was deleted.

32 changes: 0 additions & 32 deletions ReactCommon/react/renderer/mounting/MountingTransactionMetadata.h

This file was deleted.

15 changes: 6 additions & 9 deletions ReactCommon/react/renderer/mounting/TelemetryController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,35 +17,32 @@ TelemetryController::TelemetryController(
: mountingCoordinator_(mountingCoordinator) {}

bool TelemetryController::pullTransaction(
std::function<void(MountingTransactionMetadata metadata)> const &willMount,
std::function<void(ShadowViewMutationList const &mutations)> const &doMount,
std::function<void(MountingTransactionMetadata metadata)> const &didMount)
const {
MountingTransactionCallback const &willMount,
MountingTransactionCallback const &doMount,
MountingTransactionCallback const &didMount) const {
auto optional = mountingCoordinator_.pullTransaction();
if (!optional.has_value()) {
return false;
}

auto transaction = std::move(*optional);

auto surfaceId = transaction.getSurfaceId();
auto number = transaction.getNumber();
auto telemetry = transaction.getTelemetry();
auto numberOfMutations = static_cast<int>(transaction.getMutations().size());

mutex_.lock();
auto compoundTelemetry = compoundTelemetry_;
mutex_.unlock();

willMount({surfaceId, number, telemetry, compoundTelemetry});
willMount(transaction, compoundTelemetry);

telemetry.willMount();
doMount(transaction.getMutations());
doMount(transaction, compoundTelemetry);
telemetry.didMount();

compoundTelemetry.incorporate(telemetry, numberOfMutations);

didMount({surfaceId, number, telemetry, compoundTelemetry});
didMount(transaction, compoundTelemetry);

mutex_.lock();
compoundTelemetry_ = compoundTelemetry;
Expand Down
14 changes: 7 additions & 7 deletions ReactCommon/react/renderer/mounting/TelemetryController.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,17 @@
#include <mutex>

#include <react/renderer/mounting/MountingTransaction.h>
#include <react/renderer/mounting/MountingTransactionMetadata.h>
#include <react/renderer/telemetry/TransactionTelemetry.h>

namespace facebook {
namespace react {

class MountingCoordinator;

using MountingTransactionCallback = std::function<void(
MountingTransaction const &transaction,
SurfaceTelemetry const &surfaceTelemetry)>;

/*
* Provides convenient tools for aggregating and accessing telemetry data
* associated with running Surface.
Expand All @@ -43,12 +46,9 @@ class TelemetryController final {
* Calls `MountingCoordinator::pullTransaction()` and aggregates telemetry.
*/
bool pullTransaction(
std::function<void(MountingTransactionMetadata metadata)> const
&willMount,
std::function<void(ShadowViewMutationList const &mutations)> const
&doMount,
std::function<void(MountingTransactionMetadata metadata)> const &didMount)
const;
MountingTransactionCallback const &willMount,
MountingTransactionCallback const &doMount,
MountingTransactionCallback const &didMount) const;

private:
MountingCoordinator const &mountingCoordinator_;
Expand Down

0 comments on commit 91fc2c0

Please sign in to comment.