Skip to content

Commit 710e08c

Browse files
zeyapfacebook-github-bot
authored andcommitted
Add Fantom test for layout props & fix an issue in c++ animated (#52110)
Summary: Pull Request resolved: #52110 ## Changelog: [General] [Internal] - Add Fantom test for layout props With this test it turns out `layoutStyleUpdated_` on PropsAnimatedNode actually can change after animation update, because its connected StyleAnimatedNodes might be changing. This bug was introduced since D74602321 Reviewed By: rshest Differential Revision: D76753864 fbshipit-source-id: 5bebb11340086390df20c89adf80abaa63cadc90
1 parent cc442eb commit 710e08c

File tree

5 files changed

+96
-14
lines changed

5 files changed

+96
-14
lines changed

packages/react-native/Libraries/Animated/__tests__/Animated-itest.js

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import ensureInstance from '../../../src/private/__tests__/utilities/ensureInsta
1717
import * as Fantom from '@react-native/fantom';
1818
import {createRef} from 'react';
1919
import {Animated, View, useAnimatedValue} from 'react-native';
20+
import {allowStyleProp} from 'react-native/Libraries/Animated/NativeAnimatedAllowlist';
2021
import * as ReactNativeFeatureFlags from 'react-native/src/private/featureflags/ReactNativeFeatureFlags';
2122
import ReactNativeElement from 'react-native/src/private/webapis/dom/nodes/ReactNativeElement';
2223

@@ -152,6 +153,7 @@ test('animation driven by onScroll event', () => {
152153

153154
test('animated opacity', () => {
154155
let _opacity;
156+
let _opacityAnimation;
155157
const viewRef = createRef<HostInstance>();
156158

157159
function MyApp() {
@@ -182,7 +184,7 @@ test('animated opacity', () => {
182184
expect(viewElement.getBoundingClientRect().x).toBe(0);
183185

184186
Fantom.runTask(() => {
185-
Animated.timing(_opacity, {
187+
_opacityAnimation = Animated.timing(_opacity, {
186188
toValue: 0,
187189
duration: 30,
188190
useNativeDriver: true,
@@ -194,8 +196,10 @@ test('animated opacity', () => {
194196
0,
195197
);
196198

197-
// TODO(T223344928): this shouldn't be neccessary with cxxNativeAnimatedRemoveJsSync:true
198-
Fantom.runWorkLoop();
199+
// TODO: this shouldn't be neccessary since animation should be stopped after duration
200+
Fantom.runTask(() => {
201+
_opacityAnimation?.stop();
202+
});
199203

200204
expect(root.getRenderedOutput({props: ['opacity']}).toJSX()).toEqual(
201205
<rn-view opacity="0" />,
@@ -475,3 +479,61 @@ describe('Value.extractOffset', () => {
475479
Fantom.runWorkLoop();
476480
});
477481
});
482+
483+
test('animate layout props', () => {
484+
const viewRef = createRef<HostInstance>();
485+
allowStyleProp('height');
486+
487+
let _animatedHeight;
488+
let _heightAnimation;
489+
490+
function MyApp() {
491+
const animatedHeight = useAnimatedValue(0);
492+
_animatedHeight = animatedHeight;
493+
return (
494+
<Animated.View
495+
ref={viewRef}
496+
style={[
497+
{
498+
width: 100,
499+
height: animatedHeight,
500+
},
501+
]}
502+
/>
503+
);
504+
}
505+
506+
const root = Fantom.createRoot();
507+
508+
Fantom.runTask(() => {
509+
root.render(<MyApp />);
510+
});
511+
512+
const viewElement = ensureInstance(viewRef.current, ReactNativeElement);
513+
514+
Fantom.runTask(() => {
515+
_heightAnimation = Animated.timing(_animatedHeight, {
516+
toValue: 100,
517+
duration: 10,
518+
useNativeDriver: true,
519+
}).start();
520+
});
521+
522+
Fantom.unstable_produceFramesForDuration(10);
523+
524+
// TODO: this shouldn't be neccessary since animation should be stopped after duration
525+
Fantom.runTask(() => {
526+
_heightAnimation?.stop();
527+
});
528+
529+
// $FlowFixMe[incompatible-use]
530+
expect(Fantom.unstable_getDirectManipulationProps(viewElement).height).toBe(
531+
100,
532+
);
533+
534+
expect(Fantom.unstable_getFabricUpdateProps(viewElement).height).toBe(100);
535+
536+
expect(root.getRenderedOutput({props: ['height']}).toJSX()).toEqual(
537+
<rn-view height="100.000000" />,
538+
);
539+
});

packages/react-native/ReactCxxPlatform/react/renderer/animated/nodes/PropsAnimatedNode.cpp

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
#include "PropsAnimatedNode.h"
1313

1414
#include <react/debug/react_native_assert.h>
15-
#include <react/renderer/animated/NativeAnimatedAllowlist.h>
1615
#include <react/renderer/animated/NativeAnimatedNodesManager.h>
1716
#include <react/renderer/animated/nodes/ColorAnimatedNode.h>
1817
#include <react/renderer/animated/nodes/StyleAnimatedNode.h>
@@ -31,12 +30,8 @@ bool isLayoutStyleUpdated(
3130
if (node->type() == AnimatedNodeType::Style) {
3231
if (const auto& styleNode =
3332
manager.getAnimatedNode<StyleAnimatedNode>(nodeTag)) {
34-
auto& styleNodeProps = styleNode->getProps();
35-
for (const auto& styleNodeProp : styleNodeProps.items()) {
36-
if (getDirectManipulationAllowlist().count(
37-
styleNodeProp.first.asString()) == 0u) {
38-
return true;
39-
}
33+
if (styleNode->isLayoutStyleUpdated()) {
34+
return true;
4035
}
4136
}
4237
}
@@ -53,9 +48,7 @@ PropsAnimatedNode::PropsAnimatedNode(
5348
const folly::dynamic& config,
5449
NativeAnimatedNodesManager& manager)
5550
: AnimatedNode(tag, config, manager, AnimatedNodeType::Props),
56-
props_(folly::dynamic::object()),
57-
layoutStyleUpdated_(isLayoutStyleUpdated(getConfig()["props"], manager)) {
58-
}
51+
props_(folly::dynamic::object()) {}
5952

6053
void PropsAnimatedNode::connectToView(Tag viewTag) {
6154
react_native_assert(
@@ -145,6 +138,8 @@ void PropsAnimatedNode::update(bool forceFabricCommit) {
145138
}
146139
}
147140

141+
layoutStyleUpdated_ = isLayoutStyleUpdated(getConfig()["props"], *manager_);
142+
148143
manager_->schedulePropsCommit(
149144
connectedViewTag_, props_, layoutStyleUpdated_, forceFabricCommit);
150145
}

packages/react-native/ReactCxxPlatform/react/renderer/animated/nodes/PropsAnimatedNode.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ class PropsAnimatedNode final : public AnimatedNode {
4343
private:
4444
std::mutex propsMutex_;
4545
folly::dynamic props_;
46-
const bool layoutStyleUpdated_;
46+
bool layoutStyleUpdated_{false};
4747

4848
Tag connectedViewTag_{animated::undefinedAnimatedNodeIdentifier};
4949
};

packages/react-native/ReactCxxPlatform/react/renderer/animated/nodes/StyleAnimatedNode.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,29 @@
1111

1212
#include "StyleAnimatedNode.h"
1313

14+
#include <react/renderer/animated/NativeAnimatedAllowlist.h>
1415
#include <react/renderer/animated/NativeAnimatedNodesManager.h>
1516
#include <react/renderer/animated/nodes/ColorAnimatedNode.h>
1617
#include <react/renderer/animated/nodes/TransformAnimatedNode.h>
1718
#include <react/renderer/animated/nodes/ValueAnimatedNode.h>
1819

1920
namespace facebook::react {
21+
22+
namespace {
23+
24+
bool isLayoutPropsUpdated(const folly::dynamic& props) {
25+
for (const auto& styleNodeProp : props.items()) {
26+
if (getDirectManipulationAllowlist().count(
27+
styleNodeProp.first.asString()) == 0u) {
28+
return true;
29+
}
30+
}
31+
32+
return false;
33+
}
34+
35+
} // namespace
36+
2037
StyleAnimatedNode::StyleAnimatedNode(
2138
Tag tag,
2239
const folly::dynamic& config,
@@ -77,5 +94,8 @@ void StyleAnimatedNode::update() {
7794
}
7895
}
7996
}
97+
98+
layoutStyleUpdated_ = isLayoutPropsUpdated(props_);
8099
}
100+
81101
} // namespace facebook::react

packages/react-native/ReactCxxPlatform/react/renderer/animated/nodes/StyleAnimatedNode.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,12 @@ class StyleAnimatedNode final : public AnimatedNode {
2828
return props_;
2929
}
3030

31+
bool isLayoutStyleUpdated() const noexcept {
32+
return layoutStyleUpdated_;
33+
}
34+
3135
private:
3236
folly::dynamic props_;
37+
bool layoutStyleUpdated_;
3338
};
3439
} // namespace facebook::react

0 commit comments

Comments
 (0)