Permalink
Browse files

Native Animations - Fix edge case with restore default values

Summary:
There was an edge case where sometimes a view could be added and removed in the same batch so this caused issues because we ran `disconnectAnimatedNodeFromView` before `connectAnimatedNodeToView`. This separates restoring default values from `disconnectAnimatedNodeFromView` so we can run only `restoreDefaultValues` on the pre-operations queue and just do nothing in case the view doesn't exist (it is fine because we know it will be removed immediately).

**Test plan**
Tested that native animations still work properly in RNTester and tested that the edge case crash was fixed.
Closes #14114

Differential Revision: D5128989

Pulled By: javache

fbshipit-source-id: 9f47a2d3aafeb06d8ed1a4dd1800b8af225edb7d
  • Loading branch information...
janicduplessis authored and facebook-github-bot committed May 25, 2017
1 parent 46104a1 commit fc45471af2e42b556d834df203ec35a587e0a733
@@ -20,4 +20,6 @@
- (void)disconnectFromView:(NSNumber *)viewTag;
- (void)restoreDefaultValues;
@end
@@ -43,6 +43,13 @@ - (void)connectToView:(NSNumber *)viewTag
}
- (void)disconnectFromView:(NSNumber *)viewTag
{
_connectedViewTag = nil;
_connectedViewName = nil;
_uiManager = nil;
}
- (void)restoreDefaultValues
{
// Restore the default value for all props that were modified by this node.
for (NSString *key in _propsDictionary.allKeys) {
@@ -54,10 +61,6 @@ - (void)disconnectFromView:(NSNumber *)viewTag
viewName:_connectedViewName
props:_propsDictionary];
}
_connectedViewTag = nil;
_connectedViewName = nil;
_uiManager = nil;
}
- (NSString *)propertyNameForParentTag:(NSNumber *)parentTag
@@ -136,10 +136,10 @@ - (void)setBridge:(RCTBridge *)bridge
RCT_EXPORT_METHOD(disconnectAnimatedNodeFromView:(nonnull NSNumber *)nodeTag
viewTag:(nonnull NSNumber *)viewTag)
{
// Disconnecting a view also restores its default values so we have to make
// sure this happens before views get updated with their new props. This is
// why we enqueue this on the pre-operations queue.
[self addPreOperationBlock:^(RCTNativeAnimatedNodesManager *nodesManager) {
[nodesManager restoreDefaultValues:nodeTag];
}];
[self addOperationBlock:^(RCTNativeAnimatedNodesManager *nodesManager) {
[nodesManager disconnectAnimatedNodeFromView:nodeTag viewTag:viewTag];
}];
}
@@ -36,6 +36,8 @@
viewTag:(nonnull NSNumber *)viewTag
viewName:(nonnull NSString *)viewName;
- (void)restoreDefaultValues:(nonnull NSNumber *)nodeTag;
- (void)disconnectAnimatedNodeFromView:(nonnull NSNumber *)nodeTag
viewTag:(nonnull NSNumber *)viewTag;
@@ -135,6 +135,22 @@ - (void)disconnectAnimatedNodeFromView:(nonnull NSNumber *)nodeTag
}
}
- (void)restoreDefaultValues:(nonnull NSNumber *)nodeTag
{
RCTAnimatedNode *node = _animationNodes[nodeTag];
// Restoring default values needs to happen before UIManager operations so it is
// possible the node hasn't been created yet if it is being connected and
// disconnected in the same batch. In that case we don't need to restore
// default values since it will never actually update the view.
if (node == nil) {
return;
}
if (![node isKindOfClass:[RCTPropsAnimatedNode class]]) {
RCTLogError(@"Not a props node.");
}
[(RCTPropsAnimatedNode *)node restoreDefaultValues];
}
- (void)dropAnimatedNode:(nonnull NSNumber *)tag
{
RCTAnimatedNode *node = _animationNodes[tag];

0 comments on commit fc45471

Please sign in to comment.