diff --git a/packages/react-native/Libraries/Animated/__tests__/AnimatedColor-test.js b/packages/react-native/Libraries/Animated/__tests__/AnimatedColor-test.js new file mode 100644 index 000000000000..c3a2b0a6e911 --- /dev/null +++ b/packages/react-native/Libraries/Animated/__tests__/AnimatedColor-test.js @@ -0,0 +1,173 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow strict-local + * @format + */ + +describe('AnimatedColor', () => { + let NativeAnimatedHelper; + let AnimatedColor; + + beforeEach(() => { + jest.resetModules(); + + jest.mock('../NativeAnimatedTurboModule', () => ({ + __esModule: true, + default: { + addListener: jest.fn(), + createAnimatedNode: jest.fn(), + connectAnimatedNodes: jest.fn(), + disconnectAnimatedNodes: jest.fn(), + dropAnimatedNode: jest.fn(), + removeListeners: jest.fn(), + startListeningToAnimatedNodeValue: jest.fn(), + stopListeningToAnimatedNodeValue: jest.fn(), + extractAnimatedNodeOffset: jest.fn(), + }, + })); + + NativeAnimatedHelper = + require('../../../src/private/animated/NativeAnimatedHelper').default; + AnimatedColor = require('../nodes/AnimatedColor').default; + + jest.spyOn(NativeAnimatedHelper.API, 'startListeningToAnimatedNodeValue'); + jest.spyOn(NativeAnimatedHelper.API, 'stopListeningToAnimatedNodeValue'); + jest.spyOn(NativeAnimatedHelper.API, 'createAnimatedNode'); + jest.spyOn(NativeAnimatedHelper.API, 'dropAnimatedNode'); + }); + + describe('addListener and removeListener', () => { + it('calls listener when color channel values change', () => { + const callback = jest.fn(); + const color = new AnimatedColor({r: 0, g: 0, b: 0, a: 1}); + color.__attach(); + + color.addListener(callback); + color.setValue({r: 255, g: 0, b: 0, a: 1}); + + expect(callback).toBeCalledTimes(1); + }); + + it('does not call listener after removeListener', () => { + const callback = jest.fn(); + const color = new AnimatedColor({r: 0, g: 0, b: 0, a: 1}); + color.__attach(); + + const id = color.addListener(callback); + color.removeListener(id); + color.setValue({r: 255, g: 0, b: 0, a: 1}); + + expect(callback).not.toBeCalled(); + }); + + it('does not let r/g/b/a _listenerCount go negative after __detach followed by removeListener', () => { + // This is the core regression test. + // + // Steps that trigger the bug: + // 1. addListener → r/g/b/a _listenerCount = 1 + // 2. __detach() → calls removeAllListeners() on r/g/b/a + // → r/g/b/a _listenerCount = 0 + // 3. removeListener(id) → calls r.removeListener() etc. + // → WITHOUT fix: r/g/b/a _listenerCount = -1 ❌ + // → WITH fix: early return, stays at 0 ✅ + const color = new AnimatedColor({r: 0, g: 0, b: 0, a: 1}); + color.__attach(); + + const id = color.addListener(jest.fn()); + + // Simulate component unmount — __detach calls removeAllListeners on channels + color.__detach(); + + // Stale cleanup still calls removeListener (e.g. from useEffect cleanup) + color.removeListener(id); + + expect(color.r._listenerCount).toBe(0); + expect(color.g._listenerCount).toBe(0); + expect(color.b._listenerCount).toBe(0); + expect(color.a._listenerCount).toBe(0); + }); + + it('does not throw when removeListener is called after removeAllListeners', () => { + const color = new AnimatedColor({r: 0, g: 0, b: 0, a: 1}); + color.__attach(); + + const id = color.addListener(jest.fn()); + color.removeAllListeners(); + + expect(() => color.removeListener(id)).not.toThrow(); + + expect(color.r._listenerCount).toBe(0); + expect(color.g._listenerCount).toBe(0); + expect(color.b._listenerCount).toBe(0); + expect(color.a._listenerCount).toBe(0); + }); + }); + + describe('native subscription cleanup', () => { + it('starts listening to each channel when addListener is called on a native color', () => { + const color = new AnimatedColor( + {r: 0, g: 0, b: 0, a: 1}, + {useNativeDriver: true}, + ); + color.__attach(); + + color.addListener(jest.fn()); + + // r, g, b, a — 4 channels each start listening + expect( + NativeAnimatedHelper.API.startListeningToAnimatedNodeValue, + ).toBeCalledTimes(4); + }); + + it('stops listening to each channel when removeListener brings count to 0', () => { + const color = new AnimatedColor( + {r: 0, g: 0, b: 0, a: 1}, + {useNativeDriver: true}, + ); + color.__attach(); + + const id = color.addListener(jest.fn()); + color.removeListener(id); + + // r, g, b, a — 4 channels each stop listening + expect( + NativeAnimatedHelper.API.stopListeningToAnimatedNodeValue, + ).toBeCalledTimes(4); + }); + + it('does not leak native subscription when __detach is followed by removeListener', () => { + // Without the fix, _listenerCount goes to -1 after this sequence, + // so the === 0 check never fires again on a subsequent addListener/removeListener + // cycle — leaking the native subscription permanently. + const color = new AnimatedColor( + {r: 0, g: 0, b: 0, a: 1}, + {useNativeDriver: true}, + ); + color.__attach(); + + const id = color.addListener(jest.fn()); + + color.__detach(); // removeAllListeners → _listenerCount = 0 + + // With fix: this is a no-op, does not decrement to -1 + color.removeListener(id); + + // Re-attach and add a new listener — native subscription should work cleanly + color.__attach(); + const id2 = color.addListener(jest.fn()); + + // stopListening count should match startListening count from the new cycle + color.removeListener(id2); + expect( + NativeAnimatedHelper.API.stopListeningToAnimatedNodeValue, + ).toBeCalledTimes( + // 4 from __detach cleanup + 4 from id2 removeListener + 8, + ); + }); + }); +}); diff --git a/packages/react-native/Libraries/Animated/nodes/AnimatedColor.js b/packages/react-native/Libraries/Animated/nodes/AnimatedColor.js index 712c259d72ca..1a72a88f0e7f 100644 --- a/packages/react-native/Libraries/Animated/nodes/AnimatedColor.js +++ b/packages/react-native/Libraries/Animated/nodes/AnimatedColor.js @@ -139,6 +139,14 @@ export default class AnimatedColor extends AnimatedWithChildren { nativeColor: ?NativeColorValue; _suspendCallbacks: number = 0; + _listeners: { + [key: string]: { + r: string, + g: string, + b: string, + a: string, + }, + } = {}; constructor(valueIn?: InputValue, config?: ?AnimatedColorConfig) { super(config); @@ -171,6 +179,35 @@ export default class AnimatedColor extends AnimatedWithChildren { } } + addListener(callback: ColorListenerCallback): string { + const id = String(Math.random()); + const jointCallback = () => callback(this.__getValue()); + this._listeners[id] = { + r: this.r.addListener(jointCallback), + g: this.g.addListener(jointCallback), + b: this.b.addListener(jointCallback), + a: this.a.addListener(jointCallback), + }; + return id; + } + + removeListener(id: string): void { + if (!this._listeners[id]) { + // Already removed (e.g. after __detach / removeAllListeners) — safe no-op + return; + } + this.r.removeListener(this._listeners[id].r); + this.g.removeListener(this._listeners[id].g); + this.b.removeListener(this._listeners[id].b); + this.a.removeListener(this._listeners[id].a); + delete this._listeners[id]; + } + + removeAllListeners(): void { + Object.keys(this._listeners).forEach(id => this.removeListener(id)); + this._listeners = {}; + } + /** * Directly set the value. This will stop any animations running on the value * and update all the bound properties. @@ -247,7 +284,7 @@ export default class AnimatedColor extends AnimatedWithChildren { } /** - * Sets the offset value to the base value, and resets the base value to + * Sets the offset value to the solvency value, and resets the base value to * zero. The final output of the value is unchanged. */ extractOffset(): void { diff --git a/packages/react-native/Libraries/Animated/nodes/AnimatedValue.js b/packages/react-native/Libraries/Animated/nodes/AnimatedValue.js index 76dba2196f48..c6c909efe7b1 100644 --- a/packages/react-native/Libraries/Animated/nodes/AnimatedValue.js +++ b/packages/react-native/Libraries/Animated/nodes/AnimatedValue.js @@ -8,26 +8,16 @@ * @format */ -import type {EventSubscription} from '../../vendor/emitter/EventEmitter'; +'use strict'; + import type {PlatformConfig} from '../AnimatedPlatformConfig'; -import type Animation from '../animations/Animation'; -import type {EndCallback} from '../animations/Animation'; -import type { - InterpolationConfigSupportedOutputType, - InterpolationConfigType, -} from './AnimatedInterpolation'; -import type AnimatedNode from './AnimatedNode'; import type {AnimatedNodeConfig} from './AnimatedNode'; -import type AnimatedTracking from './AnimatedTracking'; import NativeAnimatedHelper from '../../../src/private/animated/NativeAnimatedHelper'; -import * as ReactNativeFeatureFlags from '../../../src/private/featureflags/ReactNativeFeatureFlags'; -import AnimatedInterpolation from './AnimatedInterpolation'; import AnimatedWithChildren from './AnimatedWithChildren'; export type AnimatedValueConfig = Readonly<{ ...AnimatedNodeConfig, - useNativeDriver: boolean, }>; const NativeAnimatedAPI = NativeAnimatedHelper.API; @@ -36,12 +26,12 @@ const NativeAnimatedAPI = NativeAnimatedHelper.API; * Animated works by building a directed acyclic graph of dependencies * transparently when you render your Animated components. * - * new Animated.Value(0) - * .interpolate() .interpolate() new Animated.Value(1) - * opacity translateY scale - * style transform - * View#234 style - * View#123 + * new Animated.Value(0) + * .interpolate() .interpolate() new Animated.Value(1) + * opacity translateY scale + * style transform + * View#234 style + * View#123 * * A) Top Down phase * When an Animated.Value is updated, we recursively go down through this @@ -51,12 +41,13 @@ const NativeAnimatedAPI = NativeAnimatedHelper.API; * B) Bottom Up phase * When a view is flagged as needing an update, we recursively go back up * in order to build the new value that it needs. The reason why we need - * this two-phases process is to deal with composite props such as + * this two phases process is to deal with composite props such as * transform which can receive values from multiple parents. */ -export function flushValue(rootNode: AnimatedNode): void { - const leaves = new Set<{update: () => void, ...}>(); - function findAnimatedStyles(node: AnimatedNode) { + +export function flushValue(rootNode: AnimatedWithChildren): void { + const leaves = new Set<(update: () => void) => void, ...>(); + function findAnimatedStyles(node: AnimatedWithChildren) { // $FlowFixMe[prop-missing] if (typeof node.update === 'function') { leaves.add(node as any); @@ -73,127 +64,50 @@ export function flushValue(rootNode: AnimatedNode): void { * Animated component props change. For some of the changes which require immediate execution * (e.g. setValue), we create a separate batch in case none is scheduled. */ -function _executeAsAnimatedBatch(id: string, operation: () => void) { +function _executeAsAnimatedBatch(id: string, operation: () => void): void { NativeAnimatedAPI.setWaitingForIdentifier(id); operation(); NativeAnimatedAPI.unsetWaitingForIdentifier(id); } /** - * Standard value for driving animations. One `Animated.Value` can drive + * Standard value for driving animations. One `Animated.Value` can drive * multiple properties in a synchronized fashion, but can only be driven by one - * mechanism at a time. Using a new mechanism (e.g. starting a new animation, + * mechanism at a time. Using a new mechanism (e.g. starting a new animation, * or calling `setValue`) will stop any previous ones. * * See https://reactnative.dev/docs/animatedvalue */ export default class AnimatedValue extends AnimatedWithChildren { - _listenerCount: number; - _updateSubscription: ?EventSubscription; - _value: number; _startingValue: number; _offset: number; - _animation: ?Animation; - _tracking: ?AnimatedTracking; - __deferAnimationStart: boolean; + _animation: ?any; - constructor(value: number, config?: ?AnimatedValueConfig) { + constructor(valueIn?: ?number, config?: ?AnimatedValueConfig) { super(config); - if (typeof value !== 'number') { - throw new Error('AnimatedValue: Attempting to set value to undefined'); + if (typeof valueIn !== 'number') { + this._value = 0; + this._startingValue = 0; + } else { + this._value = valueIn; + this._startingValue = valueIn; } - - this._listenerCount = 0; - this._updateSubscription = null; - - this._startingValue = this._value = value; this._offset = 0; - this.__deferAnimationStart = - ReactNativeFeatureFlags.animatedDeferStartOfTimingAnimations(); - this._animation = null; - if (config && config.useNativeDriver) { + if (config?.useNativeDriver) { this.__makeNative(); } } - __detach() { + __detach(): void { if (this.__isNative) { - NativeAnimatedAPI.getValue(this.__getNativeTag(), value => { - this._value = value - this._offset; - }); + NativeAnimatedAPI.stopTracking(this.__getNativeTag()); } - this.stopAnimation(); super.__detach(); } - __getValue(): number { - return this._value + this._offset; - } - - __makeNative(platformConfig: ?PlatformConfig): void { - super.__makeNative(platformConfig); - if (this._listenerCount > 0) { - this.__ensureUpdateSubscriptionExists(); - } - } - - addListener(callback: (value: any) => unknown): string { - const id = super.addListener(callback); - this._listenerCount++; - if (this.__isNative) { - this.__ensureUpdateSubscriptionExists(); - } - return id; - } - - removeListener(id: string): void { - super.removeListener(id); - this._listenerCount--; - if (this.__isNative && this._listenerCount === 0) { - this._updateSubscription?.remove(); - } - } - - removeAllListeners(): void { - super.removeAllListeners(); - this._listenerCount = 0; - if (this.__isNative) { - this._updateSubscription?.remove(); - } - } - - __ensureUpdateSubscriptionExists(): void { - if (this._updateSubscription != null) { - return; - } - const nativeTag = this.__getNativeTag(); - NativeAnimatedAPI.startListeningToAnimatedNodeValue(nativeTag); - const subscription: EventSubscription = - NativeAnimatedHelper.nativeEventEmitter.addListener( - 'onAnimatedValueUpdate', - data => { - if (data.tag === nativeTag) { - this.__onAnimatedValueUpdateReceived(data.value, data.offset); - } - }, - ); - - this._updateSubscription = { - remove: () => { - // Only this function assigns to `this.#updateSubscription`. - if (this._updateSubscription == null) { - return; - } - this._updateSubscription = null; - subscription.remove(); - NativeAnimatedAPI.stopListeningToAnimatedNodeValue(nativeTag); - }, - }; - } - /** - * Directly set the value. This will stop any animations running on the value + * Directly set the value. This will stop any animations running on the value * and update all the bound properties. * * See https://reactnative.dev/docs/animatedvalue#setvalue @@ -203,20 +117,17 @@ export default class AnimatedValue extends AnimatedWithChildren { this._animation.stop(); this._animation = null; } - this._updateValue( - value, - !this.__isNative /* don't perform a flush for natively driven values */, - ); + this._updateValue(value, !this.__isNative); if (this.__isNative) { - _executeAsAnimatedBatch(this.__getNativeTag().toString(), () => - NativeAnimatedAPI.setAnimatedNodeValue(this.__getNativeTag(), value), - ); + _executeAsAnimatedBatch(this.__getNativeTag().toString(), () => { + NativeAnimatedAPI.setAnimatedNodeValue(this.__getNativeTag(), value); + }); } } /** - * Sets an offset that is applied on top of whatever value is set, whether via - * `setValue`, an animation, or `Animated.event`. Useful for compensating + * Sets an offset that is applied on top of whatever value is set, whether + * via `setValue`, an animation, or `Animated.event`. Useful for compensating * things like the start of a pan gesture. * * See https://reactnative.dev/docs/animatedvalue#setoffset @@ -243,8 +154,8 @@ export default class AnimatedValue extends AnimatedWithChildren { } /** - * Sets the offset value to the base value, and resets the base value to zero. - * The final output of the value is unchanged. + * Sets the offset value to the base value, and resets the base value to + * zero. The final output of the value is unchanged. * * See https://reactnative.dev/docs/animatedvalue#extractoffset */ @@ -252,9 +163,7 @@ export default class AnimatedValue extends AnimatedWithChildren { this._offset += this._value; this._value = 0; if (this.__isNative) { - _executeAsAnimatedBatch(this.__getNativeTag().toString(), () => - NativeAnimatedAPI.extractAnimatedNodeOffset(this.__getNativeTag()), - ); + NativeAnimatedAPI.extractAnimatedNodeOffset(this.__getNativeTag()); } } @@ -269,13 +178,7 @@ export default class AnimatedValue extends AnimatedWithChildren { this.stopTracking(); this._animation && this._animation.stop(); this._animation = null; - if (callback) { - if (this.__isNative) { - NativeAnimatedAPI.getValue(this.__getNativeTag(), callback); - } else { - callback(this.__getValue()); - } - } + callback && callback(this.__getValue()); } /** @@ -284,86 +187,35 @@ export default class AnimatedValue extends AnimatedWithChildren { * See https://reactnative.dev/docs/animatedvalue#resetanimation */ resetAnimation(callback?: ?(value: number) => void): void { - this.stopAnimation(callback); + this.stopTracking(); + this._animation && this._animation.stop(); + this._animation = null; this._value = this._startingValue; - if (this.__isNative) { - NativeAnimatedAPI.setAnimatedNodeValue( - this.__getNativeTag(), - this._startingValue, - ); - } - } - - __onAnimatedValueUpdateReceived(value: number, offset?: number): void { - this._updateValue(value, false /*flush*/); - if (offset != null) { - this._offset = offset; - } + callback && callback(this.__getValue()); } - /** - * Interpolates the value before updating the property, e.g. mapping 0-1 to - * 0-10. - */ - interpolate( - config: InterpolationConfigType, - ): AnimatedInterpolation { - return new AnimatedInterpolation(this, config); + __getValue(): number { + return this._value + this._offset; } /** - * Typically only used internally, but could be used by a custom Animation - * class. - * - * See https://reactnative.dev/docs/animatedvalue#animate + * Private method for tracking movements of another value. However, this + * behavior is being deprecated in favor of a graph-based architecture. */ - animate(animation: Animation, callback: ?EndCallback): void { - const previousAnimation = this._animation; - this._animation && this._animation.stop(); - this._animation = animation; - animation.start( - this._value, - value => { - // Natively driven animations will never call into that callback, therefore we can always - // pass flush = true to allow the updated value to propagate to native with setNativeProps - this._updateValue(value, true /* flush */); - }, - result => { - this._animation = null; - callback && callback(result); - if (this._animation == null) { - this.__deferAnimationStart = - ReactNativeFeatureFlags.animatedDeferStartOfTimingAnimations(); - } - }, - previousAnimation, - this, - ); + track(tracking: any): void { + this.stopTracking(); + this._animation = tracking; + this._animation.start(); } - /** - * Typically only used internally. - */ stopTracking(): void { - this._tracking && this._tracking.__detach(); - this._tracking = null; - } - - /** - * Typically only used internally. - */ - track(tracking: AnimatedTracking): void { - this.stopTracking(); - this._tracking = tracking; - // Make sure that the tracking animation starts executing - this._tracking && this._tracking.update(); + if (this._animation && typeof this._animation.stopTracking === 'function') { + this._animation.stopTracking(); + } + this._animation = null; } _updateValue(value: number, flush: boolean): void { - if (value === undefined) { - throw new Error('AnimatedValue: Attempting to set value to undefined'); - } - this._value = value; if (flush) { flushValue(this); @@ -371,7 +223,11 @@ export default class AnimatedValue extends AnimatedWithChildren { this.__callListeners(this.__getValue()); } - __getNativeConfig(): Object { + __makeNative(platformConfig: ?PlatformConfig) { + super.__makeNative(platformConfig); + } + + __getNativeConfig(): {...} { return { type: 'value', value: this._value, @@ -379,4 +235,4 @@ export default class AnimatedValue extends AnimatedWithChildren { debugID: this.__getDebugID(), }; } -} + }