Skip to content

Commit

Permalink
Fix native animated style and transform node to call __makeNative onc…
Browse files Browse the repository at this point in the history
…e all the children are converted to native nodes (facebook#20658)

Summary:
This PR fixes an issue I have found while playing with native animated driver nodes.

I discovered the bug when using animated views that have both animated props and styles. See this snack to see an example: https://snack.expo.io/B17SFXy8Q

In that example we set `opacity` and `style` props which both contain animated props. This is not an usual way to do that, as normally you would place `opacity` inside the `styles` in which case the bug won't surface. But this is only done for demo purposes and in practice the problem will occur if you have a custom native view that exposes props that are not styles and can be animated.

In the above example you get this error:

> Invariant Violation: Attempt to get native tag from node not marked as "native"

When `opacity` is moved into `styles` container the problem no longer occurs.

The problem turned out to be related to the initialization code responsible for creating native animated nodes. In all subclasses of `AnimatedWithChildren` (like `AimatedAddition`) we only call `super.__makeNative` after we call `__makeNative` method on the child nodes. This order was reversed in `AnimatedStyle` and `AnimatedTransform`. As a result when `super.__makeNative` is called in `AnimatedStyle`, we try to call `__getNativeTag` on children nodes not yet marked as native which results in the error described above ("Attempt to get native tag...").

We should instead follow the order of calling `super.__makeNative` that is used in the remaining subclasses of `AnimatedWithChildren`. Such that all the children nodes are first converted to native prior to calling superclass method. This is what this PR is changing.
Pull Request resolved: facebook#20658

Differential Revision: D9297191

Pulled By: hramos

fbshipit-source-id: f5e394fb259ff514c7c1433edcb5fc89203f55e2
  • Loading branch information
kmagiera authored and facebook-github-bot committed Aug 13, 2018
1 parent a5ce1ad commit c5cbd0f
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion Libraries/Animated/src/nodes/AnimatedStyle.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,13 @@ class AnimatedStyle extends AnimatedWithChildren {
}

__makeNative() {
super.__makeNative();
for (const key in this._style) {
const value = this._style[key];
if (value instanceof AnimatedNode) {
value.__makeNative();
}
}
super.__makeNative();
}

__getNativeConfig(): Object {
Expand Down
2 changes: 1 addition & 1 deletion Libraries/Animated/src/nodes/AnimatedTransform.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ class AnimatedTransform extends AnimatedWithChildren {
}

__makeNative() {
super.__makeNative();
this._transforms.forEach(transform => {
for (const key in transform) {
const value = transform[key];
Expand All @@ -31,6 +30,7 @@ class AnimatedTransform extends AnimatedWithChildren {
}
}
});
super.__makeNative();
}

__getValue(): $ReadOnlyArray<Object> {
Expand Down

0 comments on commit c5cbd0f

Please sign in to comment.