Skip to content

Commit

Permalink
Animated - Add missing super calls to fix native animated crash
Browse files Browse the repository at this point in the history
Summary:
There was some missing super.__detach calls in some Animated nodes, we rely on the base class being called to drop the node in the native implementation, not doing so will cause some nodes to leak. It also resulted in a crash when removing certain nodes because they would get updated after being detached.

**Test plan**
Reproduced the crash by unmounting a view that uses a DiffClamp node and made sure this fixes it. Also tested that other native animations still worked properly.

Fixes #11317
Closes #12910

Differential Revision: D4718188

Pulled By: javache

fbshipit-source-id: 179ec1334532152c124a9c0f447f488311925d0a
  • Loading branch information
janicduplessis authored and facebook-github-bot committed Mar 29, 2017
1 parent 673093e commit c233191
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 1 deletion.
4 changes: 4 additions & 0 deletions Libraries/Animated/src/AnimatedImplementation.js
Expand Up @@ -1327,6 +1327,7 @@ class AnimatedModulo extends AnimatedWithChildren {

__detach(): void {
this._a.__removeChild(this);
super.__detach();
}

__getNativeConfig(): any {
Expand Down Expand Up @@ -1377,6 +1378,7 @@ class AnimatedDiffClamp extends AnimatedWithChildren {

__detach(): void {
this._a.__removeChild(this);
super.__detach();
}

__getNativeConfig(): any {
Expand Down Expand Up @@ -1460,6 +1462,7 @@ class AnimatedTransform extends AnimatedWithChildren {
}
}
});
super.__detach();
}

__getNativeConfig(): any {
Expand Down Expand Up @@ -1567,6 +1570,7 @@ class AnimatedStyle extends AnimatedWithChildren {
value.__removeChild(this);
}
}
super.__detach();
}

__makeNative() {
Expand Down
2 changes: 1 addition & 1 deletion Libraries/Animated/src/__tests__/AnimatedNative-test.js
Expand Up @@ -281,7 +281,7 @@ describe('Native Animated', () => {
);

expect(nativeAnimatedModule.disconnectAnimatedNodes).toHaveBeenCalledTimes(2);
expect(nativeAnimatedModule.dropAnimatedNode).toHaveBeenCalledTimes(2);
expect(nativeAnimatedModule.dropAnimatedNode).toHaveBeenCalledTimes(3);
});

it('sends a valid description for value, style and props nodes', () => {
Expand Down

0 comments on commit c233191

Please sign in to comment.