Skip to content

Commit 686ab49

Browse files
janicduplessisfacebook-github-bot
authored andcommitted
Don't restore default values when components unmount (#26978)
Summary: There are some cases where restoring default values on component unmount is not desirable. For example in react-native-screens we want to keep the native view displayed after react has unmounted them. Restoring default values causes an issue there because it will change props controlled my native animated back to their default value instead of keeping whatever value they had been animated to. Restoring default values is only needed for updates anyway, where removing a prop controlled by native animated need to be reset to its default value since react no longer tracks its value. This splits restoring default values and disconnecting from views in 2 separate native methods, this way we can restore default values only on component update and not on unmount. This takes care of being backwards compatible for JS running with the older native code. ## Changelog [General] [Fixed] - NativeAnimated - Don't restore default values when components unmount Pull Request resolved: #26978 Test Plan: - Tested in an app using react-native-screens to make sure native views that are kept after their underlying component has been unmount don't change. Also tested in RNTester animated example. - Tested that new JS works with old native code Reviewed By: mmmulani Differential Revision: D18197735 Pulled By: JoshuaGross fbshipit-source-id: 20fa0f31a3edf1bc57ccb03df9d1486aba83edc4
1 parent e9b4928 commit 686ab49

File tree

12 files changed

+72
-10
lines changed

12 files changed

+72
-10
lines changed

Libraries/Animated/src/NativeAnimatedHelper.js

+7
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,13 @@ const API = {
116116
invariant(NativeAnimatedModule, 'Native animated module is not available');
117117
NativeAnimatedModule.disconnectAnimatedNodeFromView(nodeTag, viewTag);
118118
},
119+
restoreDefaultValues: function(nodeTag: number): void {
120+
invariant(NativeAnimatedModule, 'Native animated module is not available');
121+
// Backwards compat with older native runtimes, can be removed later.
122+
if (NativeAnimatedModule.restoreDefaultValues != null) {
123+
NativeAnimatedModule.restoreDefaultValues(nodeTag);
124+
}
125+
},
119126
dropAnimatedNode: function(tag: number): void {
120127
invariant(NativeAnimatedModule, 'Native animated module is not available');
121128
NativeAnimatedModule.dropAnimatedNode(tag);

Libraries/Animated/src/NativeAnimatedModule.js

+1
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ export interface Spec extends TurboModule {
4545
+extractAnimatedNodeOffset: (nodeTag: number) => void;
4646
+connectAnimatedNodeToView: (nodeTag: number, viewTag: number) => void;
4747
+disconnectAnimatedNodeFromView: (nodeTag: number, viewTag: number) => void;
48+
+restoreDefaultValues: (nodeTag: number) => void;
4849
+dropAnimatedNode: (tag: number) => void;
4950
+addAnimatedEventToView: (
5051
viewTag: number,

Libraries/Animated/src/__tests__/AnimatedNative-test.js

+22
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ describe('Native Animated', () => {
4646
extractAnimatedNodeOffset: jest.fn(),
4747
flattenAnimatedNodeOffset: jest.fn(),
4848
removeAnimatedEventFromView: jest.fn(),
49+
restoreDefaultValues: jest.fn(),
4950
setAnimatedNodeOffset: jest.fn(),
5051
setAnimatedNodeValue: jest.fn(),
5152
startAnimatingNode: jest.fn(),
@@ -837,4 +838,25 @@ describe('Native Animated', () => {
837838
expect(NativeAnimatedModule.stopAnimation).toBeCalledWith(animationId);
838839
});
839840
});
841+
842+
describe('Animated Components', () => {
843+
it('Should restore default values on prop updates only', () => {
844+
const opacity = new Animated.Value(0);
845+
opacity.__makeNative();
846+
847+
const root = TestRenderer.create(<Animated.View style={{opacity}} />);
848+
expect(NativeAnimatedModule.restoreDefaultValues).not.toHaveBeenCalled();
849+
850+
root.update(<Animated.View style={{opacity}} />);
851+
expect(NativeAnimatedModule.restoreDefaultValues).toHaveBeenCalledWith(
852+
expect.any(Number),
853+
);
854+
855+
root.unmount();
856+
// Make sure it doesn't get called on unmount.
857+
expect(NativeAnimatedModule.restoreDefaultValues).toHaveBeenCalledTimes(
858+
1,
859+
);
860+
});
861+
});
840862
});

Libraries/Animated/src/createAnimatedComponent.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,10 @@ function createAnimatedComponent<Props: {+[string]: mixed}, Instance>(
111111
// This way the intermediate state isn't to go to 0 and trigger
112112
// this expensive recursive detaching to then re-attach everything on
113113
// the very next operation.
114-
oldPropsAnimated && oldPropsAnimated.__detach();
114+
if (oldPropsAnimated) {
115+
oldPropsAnimated.__restoreDefaultValues();
116+
oldPropsAnimated.__detach();
117+
}
115118
}
116119

117120
_setComponentRef = setAndForwardRef({

Libraries/Animated/src/nodes/AnimatedProps.js

+10
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,16 @@ class AnimatedProps extends AnimatedNode {
147147
);
148148
}
149149

150+
__restoreDefaultValues(): void {
151+
// When using the native driver, view properties need to be restored to
152+
// their default values manually since react no longer tracks them. This
153+
// is needed to handle cases where a prop driven by native animated is removed
154+
// after having been changed natively by an animation.
155+
if (this.__isNative) {
156+
NativeAnimatedHelper.API.restoreDefaultValues(this.__getNativeTag());
157+
}
158+
}
159+
150160
__getNativeConfig(): Object {
151161
const propsConfig = {};
152162
for (const propKey in this._props) {

Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec-generated.mm

+7
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,10 @@ + (RCTManagedPointer *)JS_NativeAnimatedModule_EventMapping:(id)json
281281
return static_cast<ObjCTurboModule&>(turboModule).invokeObjCMethod(rt, VoidKind, "disconnectAnimatedNodeFromView", @selector(disconnectAnimatedNodeFromView:viewTag:), args, count);
282282
}
283283

284+
static facebook::jsi::Value __hostFunction_NativeAnimatedModuleSpecJSI_restoreDefaultValues(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
285+
return static_cast<ObjCTurboModule&>(turboModule).invokeObjCMethod(rt, VoidKind, "restoreDefaultValues", @selector(restoreDefaultValues:), args, count);
286+
}
287+
284288
static facebook::jsi::Value __hostFunction_NativeAnimatedModuleSpecJSI_dropAnimatedNode(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) {
285289
return static_cast<ObjCTurboModule&>(turboModule).invokeObjCMethod(rt, VoidKind, "dropAnimatedNode", @selector(dropAnimatedNode:), args, count);
286290
}
@@ -344,6 +348,9 @@ + (RCTManagedPointer *)JS_NativeAnimatedModule_EventMapping:(id)json
344348
methodMap_["disconnectAnimatedNodeFromView"] = MethodMetadata {2, __hostFunction_NativeAnimatedModuleSpecJSI_disconnectAnimatedNodeFromView};
345349

346350

351+
methodMap_["restoreDefaultValues"] = MethodMetadata {1, __hostFunction_NativeAnimatedModuleSpecJSI_restoreDefaultValues};
352+
353+
347354
methodMap_["dropAnimatedNode"] = MethodMetadata {1, __hostFunction_NativeAnimatedModuleSpecJSI_dropAnimatedNode};
348355

349356

Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec.h

+1
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,7 @@ namespace JS {
290290
viewTag:(double)viewTag;
291291
- (void)disconnectAnimatedNodeFromView:(double)nodeTag
292292
viewTag:(double)viewTag;
293+
- (void)restoreDefaultValues:(double)nodeTag;
293294
- (void)dropAnimatedNode:(double)tag;
294295
- (void)addAnimatedEventToView:(double)viewTag
295296
eventName:(NSString *)eventName

Libraries/NativeAnimation/RCTNativeAnimatedModule.mm

+7-3
Original file line numberDiff line numberDiff line change
@@ -160,14 +160,18 @@ - (void)setBridge:(RCTBridge *)bridge
160160
RCT_EXPORT_METHOD(disconnectAnimatedNodeFromView:(double)nodeTag
161161
viewTag:(double)viewTag)
162162
{
163-
[self addPreOperationBlock:^(RCTNativeAnimatedNodesManager *nodesManager) {
164-
[nodesManager restoreDefaultValues:[NSNumber numberWithDouble:nodeTag]];
165-
}];
166163
[self addOperationBlock:^(RCTNativeAnimatedNodesManager *nodesManager) {
167164
[nodesManager disconnectAnimatedNodeFromView:[NSNumber numberWithDouble:nodeTag] viewTag:[NSNumber numberWithDouble:viewTag]];
168165
}];
169166
}
170167

168+
RCT_EXPORT_METHOD(restoreDefaultValues:(double)nodeTag)
169+
{
170+
[self addPreOperationBlock:^(RCTNativeAnimatedNodesManager *nodesManager) {
171+
[nodesManager restoreDefaultValues:[NSNumber numberWithDouble:nodeTag]];
172+
}];
173+
}
174+
171175
RCT_EXPORT_METHOD(dropAnimatedNode:(double)tag)
172176
{
173177
[self addOperationBlock:^(RCTNativeAnimatedNodesManager *nodesManager) {

ReactAndroid/src/main/java/com/facebook/fbreact/specs/NativeAnimatedModuleSpec.java

+3
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,9 @@ public abstract void removeAnimatedEventFromView(double viewTag, String eventNam
5959
@ReactMethod
6060
public abstract void setAnimatedNodeOffset(double nodeTag, double offset);
6161

62+
@ReactMethod
63+
public abstract void restoreDefaultValues(double nodeTag);
64+
6265
@ReactMethod
6366
public abstract void startAnimatingNode(double animationId, double nodeTag, ReadableMap config,
6467
Callback endCallback);

ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedModule.java

+8-4
Original file line numberDiff line numberDiff line change
@@ -373,18 +373,22 @@ public void execute(NativeAnimatedNodesManager animatedNodesManager) {
373373

374374
@ReactMethod
375375
public void disconnectAnimatedNodeFromView(final int animatedNodeTag, final int viewTag) {
376-
mPreOperations.add(
376+
mOperations.add(
377377
new UIThreadOperation() {
378378
@Override
379379
public void execute(NativeAnimatedNodesManager animatedNodesManager) {
380-
animatedNodesManager.restoreDefaultValues(animatedNodeTag, viewTag);
380+
animatedNodesManager.disconnectAnimatedNodeFromView(animatedNodeTag, viewTag);
381381
}
382382
});
383-
mOperations.add(
383+
}
384+
385+
@ReactMethod
386+
public void restoreDefaultValues(final int animatedNodeTag) {
387+
mPreOperations.add(
384388
new UIThreadOperation() {
385389
@Override
386390
public void execute(NativeAnimatedNodesManager animatedNodesManager) {
387-
animatedNodesManager.disconnectAnimatedNodeFromView(animatedNodeTag, viewTag);
391+
animatedNodesManager.restoreDefaultValues(animatedNodeTag);
388392
}
389393
});
390394
}

ReactAndroid/src/main/java/com/facebook/react/animated/NativeAnimatedNodesManager.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ public void disconnectAnimatedNodeFromView(int animatedNodeTag, int viewTag) {
318318
propsAnimatedNode.disconnectFromView(viewTag);
319319
}
320320

321-
public void restoreDefaultValues(int animatedNodeTag, int viewTag) {
321+
public void restoreDefaultValues(int animatedNodeTag) {
322322
AnimatedNode node = mAnimatedNodes.get(animatedNodeTag);
323323
// Restoring default values needs to happen before UIManager operations so it is
324324
// possible the node hasn't been created yet if it is being connected and

ReactAndroid/src/test/java/com/facebook/react/animated/NativeAnimatedNodeTraversalTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -981,7 +981,7 @@ public void testRestoreDefaultProps() {
981981
assertThat(stylesCaptor.getValue().getDouble("opacity")).isEqualTo(0);
982982

983983
reset(mUIManagerMock);
984-
mNativeAnimatedNodesManager.restoreDefaultValues(propsNodeTag, viewTag);
984+
mNativeAnimatedNodesManager.restoreDefaultValues(propsNodeTag);
985985
verify(mUIManagerMock).synchronouslyUpdateViewOnUIThread(eq(viewTag), stylesCaptor.capture());
986986
assertThat(stylesCaptor.getValue().isNull("opacity"));
987987
}

0 commit comments

Comments
 (0)