From a93a15aca3e380f2f8158465b1b315ac3fab8307 Mon Sep 17 00:00:00 2001 From: Vladyslav Krasnolutskyi Date: Tue, 28 May 2024 07:33:44 -0700 Subject: [PATCH] Fix Animate.sequence restart issues (#44031) Summary: Currently, if the `Animated.sequence` animation is finished, and then the `start()` method is called without a `reset()` method being invoked beforehand, the failure happens: ```undefined is not an object (evaluating 'animations[current].start')```. Use cases: - sequence animation started, finished, and then started again - sequence animation is used in `Animation.loop` with `resetBeforeIteration` set to `false`, which essentially does the same as the previous case Related issues: - https://github.com/facebook/react-native/issues/43120 - https://github.com/facebook/react-native/issues/37611 ## Changelog: [General] [Fixed] - Fix sequence restart failure Pull Request resolved: https://github.com/facebook/react-native/pull/44031 Test Plan: Test cases are included: 1 for regression and 2 for mentioned use cases in the summary Reviewed By: cipolleschi Differential Revision: D56015346 Pulled By: dmytrorykun fbshipit-source-id: 8b0f46c8a33397fece807634463ce630c89d28af --- .../Animated/AnimatedImplementation.js | 2 + .../Animated/__tests__/Animated-test.js | 66 +++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/packages/react-native/Libraries/Animated/AnimatedImplementation.js b/packages/react-native/Libraries/Animated/AnimatedImplementation.js index e516115733360b..a11574478a35ce 100644 --- a/packages/react-native/Libraries/Animated/AnimatedImplementation.js +++ b/packages/react-native/Libraries/Animated/AnimatedImplementation.js @@ -315,6 +315,8 @@ const sequence = function ( current++; if (current === animations.length) { + // if the start is called, even without a reset, it should start from the beginning + current = 0; callback && callback(result); return; } diff --git a/packages/react-native/Libraries/Animated/__tests__/Animated-test.js b/packages/react-native/Libraries/Animated/__tests__/Animated-test.js index b58175ac46e275..29f5bc7ceda8ce 100644 --- a/packages/react-native/Libraries/Animated/__tests__/Animated-test.js +++ b/packages/react-native/Libraries/Animated/__tests__/Animated-test.js @@ -264,6 +264,50 @@ describe('Animated tests', () => { expect(cb).toBeCalledWith({finished: false}); }); + + it('supports restarting sequence after it was stopped during execution', () => { + const anim1 = {start: jest.fn(), stop: jest.fn()}; + const anim2 = {start: jest.fn(), stop: jest.fn()}; + const cb = jest.fn(); + + const seq = Animated.sequence([anim1, anim2]); + + seq.start(cb); + + anim1.start.mock.calls[0][0]({finished: true}); + seq.stop(); + + // anim1 should be finished so anim2 should also start + expect(anim1.start).toHaveBeenCalledTimes(1); + expect(anim2.start).toHaveBeenCalledTimes(1); + + seq.start(cb); + + // after restart the sequence should resume from the anim2 + expect(anim1.start).toHaveBeenCalledTimes(1); + expect(anim2.start).toHaveBeenCalledTimes(2); + }); + + it('supports restarting sequence after it was finished without a reset', () => { + const anim1 = {start: jest.fn(), stop: jest.fn()}; + const anim2 = {start: jest.fn(), stop: jest.fn()}; + const cb = jest.fn(); + + const seq = Animated.sequence([anim1, anim2]); + + seq.start(cb); + anim1.start.mock.calls[0][0]({finished: true}); + anim2.start.mock.calls[0][0]({finished: true}); + + // sequence should be finished + expect(cb).toBeCalledWith({finished: true}); + + seq.start(cb); + + // sequence should successfully restart from the anim1 + expect(anim1.start).toHaveBeenCalledTimes(2); + expect(anim2.start).toHaveBeenCalledTimes(1); + }); }); describe('Animated Loop', () => { @@ -506,6 +550,28 @@ describe('Animated tests', () => { expect(cb).not.toBeCalled(); }); + it('restarts sequence normally in a loop if resetBeforeIteration is false', () => { + const anim1 = {start: jest.fn(), stop: jest.fn()}; + const anim2 = {start: jest.fn(), stop: jest.fn()}; + const seq = Animated.sequence([anim1, anim2]); + + const loop = Animated.loop(seq, {resetBeforeIteration: false}); + + loop.start(); + + expect(anim1.start).toHaveBeenCalledTimes(1); + + anim1.start.mock.calls[0][0]({finished: true}); + + expect(anim2.start).toHaveBeenCalledTimes(1); + + anim2.start.mock.calls[0][0]({finished: true}); + + // after anim2 is finished, the sequence is finished, + // hence the loop iteration is finished, so the next iteration starts + expect(anim1.start).toHaveBeenCalledTimes(2); + }); + describe('Animated Parallel', () => { it('works with an empty parallel', () => { const cb = jest.fn();