Skip to content

Commit

Permalink
reactBridgeDidFinishTransaction was removed from RCTNavigator
Browse files Browse the repository at this point in the history
Summary:
We are removing `reactBridgeDidFinishTransaction`.
Why?
 * It is a performance drain. Supporting this requires dispatching main-thread block on every single transaction complete;
 * It has "too broad" non-conceptual semantic which encouraged using this as a "band-aid solution" for poorly designed components;
 * It is conceptually incompatible with new approaches that we are trying to implement to optimize the render layer;
 * It was deprecated for very long time.

This diff replaces usage of `reactBridgeDidFinishTransaction` with `uiManagerDidPerformMounting` which has very similar semantic except that fact that `uiManagerDidPerformMounting` is called asynchronously on the next run loop tick. And this should be okay because new React partial rendering does not guarantee synchronous execution anyways.

Reviewed By: mmmulani

Differential Revision: D6549217

fbshipit-source-id: 2649e943e82e6fbe02c7678583a97db3f5800201
  • Loading branch information
shergin authored and facebook-github-bot committed Dec 18, 2017
1 parent b8e60a3 commit 099b280
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 3 deletions.
2 changes: 2 additions & 0 deletions React/Views/RCTNavigator.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,6 @@
*/
- (BOOL)requestSchedulingJavaScriptNavigation;

- (void)uiManagerDidPerformMounting;

@end
4 changes: 2 additions & 2 deletions React/Views/RCTNavigator.m
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ - (void)insertReactSubview:(RCTNavItem *)view atIndex:(NSInteger)atIndex

- (void)didUpdateReactSubviews
{
// Do nothing, as subviews are managed by `reactBridgeDidFinishTransaction`
// Do nothing, as subviews are managed by `uiManagerDidPerformMounting`
}

- (void)layoutSubviews
Expand Down Expand Up @@ -510,7 +510,7 @@ - (UIView *)reactSuperview
return superview ?: self.reactNavSuperviewLink;
}

- (void)reactBridgeDidFinishTransaction
- (void)uiManagerDidPerformMounting
{
// we can't hook up the VC hierarchy in 'init' because the subviews aren't
// hooked up yet, so we do it on demand here
Expand Down
40 changes: 39 additions & 1 deletion React/Views/RCTNavigatorManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,42 @@
#import "RCTConvert.h"
#import "RCTNavigator.h"
#import "RCTUIManager.h"
#import "RCTUIManagerObserverCoordinator.h"
#import "UIView+React.h"

@interface RCTNavigatorManager () <RCTUIManagerObserver>

@end

@implementation RCTNavigatorManager
{
// The main thread only.
NSHashTable<RCTNavigator *> *_viewRegistry;
}

- (void)setBridge:(RCTBridge *)bridge
{
[super setBridge:bridge];

[self.bridge.uiManager.observerCoordinator addObserver:self];
}

- (void)invalidate
{
[self.bridge.uiManager.observerCoordinator removeObserver:self];
}

RCT_EXPORT_MODULE()

- (UIView *)view
{
return [[RCTNavigator alloc] initWithBridge:self.bridge];
if (!_viewRegistry) {
_viewRegistry = [NSHashTable hashTableWithOptions:NSPointerFunctionsWeakMemory];
}

RCTNavigator *view = [[RCTNavigator alloc] initWithBridge:self.bridge];
[_viewRegistry addObject:view];
return view;
}

RCT_EXPORT_VIEW_PROPERTY(requestedTopOfStack, NSInteger)
Expand All @@ -44,4 +71,15 @@ - (UIView *)view
}];
}

#pragma mark - RCTUIManagerObserver

- (void)uiManagerDidPerformMounting:(__unused RCTUIManager *)manager
{
RCTExecuteOnMainQueue(^{
for (RCTNavigator *view in self->_viewRegistry) {
[view uiManagerDidPerformMounting];
}
});
}

@end

0 comments on commit 099b280

Please sign in to comment.