New implementation of handling didUpdateReactSubviews and `didUpdat…

…eReactSubviews` events Summary: Motivation: * Current implementation of `didUpdateReactSubviews` relies on `processUpdatedProperties:parentProperties:` method of RCTShadowView, which we plan to remove. * The existing implementation does not call handlers on unmounted nodes (because they are not part of traversing tree), which is not correct. * The current implementation is tight with RCTComponentData, which is conceptually wrong, it should be a UIManager thing. * The new implementation must be much more performant because of simplicity. (We can measure it only after removing `processUpdatedProperties`.) Reviewed By: mmmulani Differential Revision: D6595780 fbshipit-source-id: a517207c17b5d5db839c9ce99a37136292acf78c
facebook · Dec 20, 2017 · 7d1deda · 7d1deda
1 parent 4996b9a
commit 7d1deda
Show file tree

Hide file tree

Showing 6 changed files with 102 additions and 26 deletions.
diff --git a/React/Modules/RCTUIManager.m b/React/Modules/RCTUIManager.m
@@ -66,6 +66,9 @@ @implementation RCTUIManager
   NSMutableDictionary<NSNumber *, RCTShadowView *> *_shadowViewRegistry; // RCT thread only
   NSMutableDictionary<NSNumber *, UIView *> *_viewRegistry; // Main thread only
 
+  NSMapTable<RCTShadowView *, NSArray<NSString *> *> *_shadowViewsWithUpdatedProps; // UIManager queue only.
+  NSHashTable<RCTShadowView *> *_shadowViewsWithUpdatedChildren; // UIManager queue only.
+
   // Keyed by viewName
   NSDictionary *_componentDataByName;
 }
@@ -138,6 +141,9 @@ - (void)setBridge:(RCTBridge *)bridge
   _shadowViewRegistry = [NSMutableDictionary new];
   _viewRegistry = [NSMutableDictionary new];
 
+  _shadowViewsWithUpdatedProps = [NSMapTable weakToStrongObjectsMapTable];
+  _shadowViewsWithUpdatedChildren = [NSHashTable weakObjectsHashTable];
+
   // Internal resources
   _pendingUIBlocks = [NSMutableArray new];
   _rootViewTags = [NSMutableSet new];
@@ -839,6 +845,8 @@ - (void)_removeChildren:(NSArray<UIView *> *)children
     RCTSetChildren(containerTag, reactTags,
                    (NSDictionary<NSNumber *, id<RCTComponent>> *)viewRegistry);
   }];
+
+  [self _shadowViewDidReceiveUpdatedChildren:_shadowViewRegistry[containerTag]];
 }
 
 static void RCTSetChildren(NSNumber *containerTag,
@@ -879,6 +887,8 @@ static void RCTSetChildren(NSNumber *containerTag,
                removeAtIndices:removeAtIndices
                       registry:(NSMutableDictionary<NSNumber *, id<RCTComponent>> *)viewRegistry];
   }];
+
+  [self _shadowViewDidReceiveUpdatedChildren:_shadowViewRegistry[containerTag]];
 }
 
 - (void)_manageChildren:(NSNumber *)containerTag
@@ -972,6 +982,13 @@ - (void)_manageChildren:(NSNumber *)containerTag
 #endif
     }
   });
+
+  [self addUIBlock:^(__unused RCTUIManager *uiManager, NSDictionary<NSNumber *, UIView *> *viewRegistry) {
+    UIView *view = viewRegistry[reactTag];
+    [componentData setProps:props forView:view];
+  }];
+
+  [self _shadowView:shadowView didReceiveUpdatedProps:[props allKeys]];
 }
 
 RCT_EXPORT_METHOD(updateView:(nonnull NSNumber *)reactTag
@@ -986,6 +1003,8 @@ - (void)_manageChildren:(NSNumber *)containerTag
     UIView *view = viewRegistry[reactTag];
     [componentData setProps:props forView:view];
   }];
+
+  [self _shadowView:shadowView didReceiveUpdatedProps:[props allKeys]];
 }
 
 - (void)synchronouslyUpdateViewOnUIThread:(NSNumber *)reactTag
@@ -1067,6 +1086,9 @@ - (void)_layoutAndMount
     [self addUIBlock:uiBlock];
   }
 
+  [self _dispatchPropsDidChangeEvents];
+  [self _dispatchChildrenDidChangeEvents];
+
   [_observerCoordinator uiManagerWillPerformLayout:self];
 
   // Perform layout
@@ -1135,6 +1157,75 @@ - (void)setNeedsLayout
   }
 }
 
+- (void)_shadowView:(RCTShadowView *)shadowView didReceiveUpdatedProps:(NSArray<NSString *> *)props
+{
+  // We collect a set with changed `shadowViews` and its changed props,
+  // so we have to maintain this collection properly.
+  NSArray<NSString *> *previousProps;
+  if ((previousProps = [_shadowViewsWithUpdatedProps objectForKey:shadowView])) {
+    // Merging already registred changed props and new ones.
+    NSMutableSet *set = [NSMutableSet setWithArray:previousProps];
+    [set addObjectsFromArray:props];
+    props = [set allObjects];
+  }
+
+  [_shadowViewsWithUpdatedProps setObject:props forKey:shadowView];
+}
+
+- (void)_shadowViewDidReceiveUpdatedChildren:(RCTShadowView *)shadowView
+{
+  [_shadowViewsWithUpdatedChildren addObject:shadowView];
+}
+
+- (void)_dispatchChildrenDidChangeEvents
+{
+  if (_shadowViewsWithUpdatedChildren.count == 0) {
+    return;
+  }
+
+  NSHashTable<RCTShadowView *> *shadowViews = _shadowViewsWithUpdatedChildren;
+  _shadowViewsWithUpdatedChildren = [NSHashTable weakObjectsHashTable];
+
+  NSMutableArray *tags = [NSMutableArray arrayWithCapacity:shadowViews.count];
+
+  for (RCTShadowView *shadowView in shadowViews) {
+    [shadowView didUpdateReactSubviews];
+    [tags addObject:shadowView.reactTag];
+  }
+
+  [self addUIBlock:^(RCTUIManager *uiManager, NSDictionary<NSNumber *, UIView *> *viewRegistry) {
+    for (NSNumber *tag in tags) {
+      UIView<RCTComponent> *view = viewRegistry[tag];
+      [view didUpdateReactSubviews];
+    }
+  }];
+}
+
+- (void)_dispatchPropsDidChangeEvents
+{
+  if (_shadowViewsWithUpdatedProps.count == 0) {
+    return;
+  }
+
+  NSMapTable<RCTShadowView *, NSArray<NSString *> *> *shadowViews = _shadowViewsWithUpdatedProps;
+  _shadowViewsWithUpdatedProps = [NSMapTable weakToStrongObjectsMapTable];
+
+  NSMapTable<NSNumber *, NSArray<NSString *> *> *tags = [NSMapTable strongToStrongObjectsMapTable];
+
+  for (RCTShadowView *shadowView in shadowViews) {
+    NSArray<NSString *> *props = [shadowViews objectForKey:shadowView];
+    [shadowView didSetProps:props];
+    [tags setObject:props forKey:shadowView.reactTag];
+  }
+
+  [self addUIBlock:^(RCTUIManager *uiManager, NSDictionary<NSNumber *, UIView *> *viewRegistry) {
+    for (NSNumber *tag in tags) {
+      UIView<RCTComponent> *view = viewRegistry[tag];
+      [view didSetProps:[tags objectForKey:tag]];
+    }
+  }];
+}
+
 RCT_EXPORT_METHOD(measure:(nonnull NSNumber *)reactTag
                   callback:(RCTResponseSenderBlock)callback)
 {

diff --git a/React/Views/RCTComponent.h b/React/Views/RCTComponent.h
@@ -36,8 +36,6 @@ typedef void (^RCTBubblingEventBlock)(NSDictionary *body);
 // View/ShadowView is a root view
 - (BOOL)isReactRootView;
 
-@optional
-
 /**
  * Called each time props have been set.
  * Not all props have to be set - React can set only changed ones.

diff --git a/React/Views/RCTComponentData.m b/React/Views/RCTComponentData.m
@@ -355,10 +355,6 @@ - (void)setProps:(NSDictionary<NSString *, id> *)props forView:(id<RCTComponent>
   [props enumerateKeysAndObjectsUsingBlock:^(NSString *key, id json, __unused BOOL *stop) {
     [self propBlockForKey:key isShadowView:NO](view, json);
   }];
-
-  if ([view respondsToSelector:@selector(didSetProps:)]) {
-    [view didSetProps:[props allKeys]];
-  }
 }
 
 - (void)setProps:(NSDictionary<NSString *, id> *)props forShadowView:(RCTShadowView *)shadowView
@@ -370,10 +366,6 @@ - (void)setProps:(NSDictionary<NSString *, id> *)props forShadowView:(RCTShadowV
   [props enumerateKeysAndObjectsUsingBlock:^(NSString *key, id json, __unused BOOL *stop) {
     [self propBlockForKey:key isShadowView:YES](shadowView, json);
   }];
-
-  if ([shadowView respondsToSelector:@selector(didSetProps:)]) {
-    [shadowView didSetProps:[props allKeys]];
-  }
 }
 
 - (NSDictionary<NSString *, id> *)viewConfig

diff --git a/React/Views/RCTShadowView.m b/React/Views/RCTShadowView.m
@@ -41,7 +41,6 @@ @implementation RCTShadowView
   BOOL _recomputePadding;
   BOOL _recomputeMargin;
   BOOL _recomputeBorder;
-  BOOL _didUpdateSubviews;
   YGValue _paddingMetaProps[META_PROP_COUNT];
   YGValue _marginMetaProps[META_PROP_COUNT];
   YGValue _borderMetaProps[META_PROP_COUNT];
@@ -263,19 +262,6 @@ - (void)applyLayoutToChildren:(YGNodeRef)node
 - (NSDictionary<NSString *, id> *)processUpdatedProperties:(NSMutableSet<RCTApplierBlock> *)applierBlocks
                                           parentProperties:(NSDictionary<NSString *, id> *)parentProperties
 {
-  // TODO: we always refresh all propagated properties when propagation is
-  // dirtied, but really we should track which properties have changed and
-  // only update those.
-
-  if (_didUpdateSubviews) {
-    _didUpdateSubviews = NO;
-    [self didUpdateReactSubviews];
-    [applierBlocks addObject:^(NSDictionary<NSNumber *, UIView *> *viewRegistry) {
-      UIView *view = viewRegistry[self->_reactTag];
-      [view didUpdateReactSubviews];
-    }];
-  }
-
   return parentProperties;
 }
 
@@ -435,7 +421,6 @@ - (void)insertReactSubview:(RCTShadowView *)subview atIndex:(NSInteger)atIndex
     YGNodeInsertChild(_yogaNode, subview.yogaNode, (uint32_t)atIndex);
   }
   subview->_superview = self;
-  _didUpdateSubviews = YES;
   [self dirtyText];
   [self dirtyPropagation];
 }
@@ -444,7 +429,6 @@ - (void)removeReactSubview:(RCTShadowView *)subview
 {
   [subview dirtyText];
   [subview dirtyPropagation];
-  _didUpdateSubviews = YES;
   subview->_superview = nil;
   [_reactSubviews removeObject:subview];
   if (![self isYogaLeafNode]) {

diff --git a/React/Views/UIView+React.h b/React/Views/UIView+React.h
@@ -60,6 +60,12 @@
  */
 - (void)didUpdateReactSubviews;
 
+/**
+ * Called each time props have been set.
+ * The default implementation does nothing.
+ */
+- (void)didSetProps:(NSArray<NSString *> *)changedProps;
+
 /**
  * Used by the UIIManager to set the view frame.
  * May be overriden to disable animation, etc.

diff --git a/React/Views/UIView+React.m b/React/Views/UIView+React.m
@@ -172,6 +172,11 @@ - (void)didUpdateReactSubviews
   }
 }
 
+- (void)didSetProps:(__unused NSArray<NSString *> *)changedProps
+{
+  // The default implementation does nothing.
+}
+
 - (void)reactSetFrame:(CGRect)frame
 {
   // These frames are in terms of anchorPoint = topLeft, but internally the