Fix Animated listeners on nodes derived from Animated.Value

[dennytosp]

Summary:

Fixes #49719.

addListener stopped firing on any AnimatedNode derived from an Animated.Value — Animated.add/subtract/multiply/divide/modulo/diffClamp and .interpolate() — for natively driven animations. It worked in 0.77 and regressed in 0.78. The issue has since been reported for both Animated.add and .interpolate(), on both architectures, and the only workaround is to listen to the source value and recompute the derived value in JS — which observes intermediate values that never reach the screen.

There are two independent causes.

1. JS: derived nodes never subscribe to native updates

Before 38c46fe ("Animated: Lower onAnimatedValueUpdate to AnimatedValue", #48514), AnimatedNode.addListener called startListeningToAnimatedNodeValue for any node that had been made native. That commit moved the logic down into AnimatedValue, on the premise that:

the startListeningToAnimatedNodeValue native module method only supports native tags for instances of AnimatedValue […] On Android, startListeningToAnimatedNodeValue throws if the node is not an instance of ValueAnimatedNode. On iOS, it does nothing if node is not an instance of RCTValueAnimatedNode.

That holds for props/style/transform/object/tracking/color nodes, but not for the operator and interpolation nodes, which are value nodes natively:

• Android — AdditionAnimatedNode, SubtractionAnimatedNode, MultiplicationAnimatedNode, DivisionAnimatedNode, ModulusAnimatedNode, DiffClampAnimatedNode and InterpolationAnimatedNode all extend ValueAnimatedNode, and NativeAnimatedNodesManager checks node !is ValueAnimatedNode.
• iOS — the corresponding RCT*AnimatedNode classes all inherit RCTValueAnimatedNode, and the manager checks isKindOfClass:[RCTValueAnimatedNode class].

So the subscription was dropped for a set of nodes that natively support it. This restores it on AnimatedNode, gated on a new __isNativeValueNode flag that is true only for nodes backed by a native value node. That keeps the guarantee #48514 was after — never call startListeningToAnimatedNodeValue with a non-value tag — but states it explicitly instead of leaving it implicit in the class hierarchy. AnimatedValue now inherits that machinery rather than duplicating it.

AnimatedNode.__detach also drops the subscription before dropAnimatedNode, so it can never outlive the tag it observes.

2. C++: startListeningToAnimatedNodeValue rejects derived value nodes

Unlike Android and iOS, the C++ backend used by the New Architecture compares the node's exact type tag instead of testing for a ValueAnimatedNode subclass:

iter->second->type() == AnimatedNodeType::Value

so an addition or interpolation node was rejected with "does not exist, or is not a 'value' node", even though static_cast<ValueAnimatedNode*> would have been valid. Replaced with an exhaustive isValueNodeType predicate, so every node type whose C++ class derives from ValueAnimatedNode (including Round) is accepted and the non-value ones are still rejected.

AnimatedNode.__onAnimatedValueUpdateReceived had to accept a missing offset as well: the C++ backend emits onAnimatedValueUpdate without one, which would otherwise produce NaN (value + undefined).

Changelog:

[GENERAL] [FIXED] - addListener fires again for natively driven Animated values derived with add/subtract/multiply/divide/modulo/diffClamp/interpolate

Test Plan:

New tests

Libraries/Animated/__tests__/AnimatedComposition-itest.js — renders a derived node bound to translateX, attaches a listener to the derived node before it is made native, runs a timing animation on the source value, and asserts the listener observed the derived value; then asserts removeListener stops the updates. Parameterised over the five operators plus interpolate, and over both drivers.

ReactCommon/react/renderer/animated/tests/AnimatedNodeTests.cpp — StartListeningToDerivedValueNode asserts the C++ backend delivers updates for an addition node and stops on stopListeningToAnimatedNodeValue. StartListeningToNonValueNodeIsIgnored asserts registering on a transform node is still a no-op.

Results

$ yarn fantom packages/react-native/Libraries/Animated/__tests__/AnimatedComposition-itest.js
  addListener on derived value nodes
    ✓ Animated.add notifies its listeners on the JS driver
    ✓ Animated.subtract notifies its listeners on the JS driver
    ✓ Animated.multiply notifies its listeners on the JS driver
    ✓ Animated.divide notifies its listeners on the JS driver
    ✓ Animated.modulo notifies its listeners on the JS driver
    ✓ interpolate notifies its listeners on the JS driver
    ✓ Animated.add notifies its listeners on the native driver
    ✓ Animated.subtract notifies its listeners on the native driver
    ✓ Animated.multiply notifies its listeners on the native driver
    ✓ Animated.divide notifies its listeners on the native driver
    ✓ Animated.modulo notifies its listeners on the native driver
    ✓ interpolate notifies its listeners on the native driver

Tests: 26 passed, 26 total

Reverting only the fix (keeping the new test) fails exactly the six native driver cases, which is the reported bug; the JS driver cases were never broken:

✕ Animated.add notifies its listeners on the native driver
  Expected 0 to be greater than or equal to 0
  > 469 |         expect(values.length).toBeGreaterThan(0);
…
Tests: 6 failed, 20 passed, 26 total

No regressions in the rest of the Animated suite:

$ yarn fantom packages/react-native/Libraries/Animated/__tests__/
Test Suites: 2 skipped, 16 passed, 16 of 18 total
Tests:       3 skipped, 297 passed, 300 total

$ yarn jest packages/react-native/Libraries/Animated
Test Suites: 3 passed, 3 total
Tests:       66 passed, 66 total

$ yarn flow-check
Found 0 errors

$ yarn lint     # changed files
(no output)

yarn build-types regenerated ReactNativeApi.d.ts; the only substantive change is that AnimatedValue's removeListener/removeAllListeners are now inherited from AnimatedNode rather than redeclared. addListener keeps its narrower ValueListenerCallback signature.

Correction to an earlier revision of this description: the C++ tests were not run locally, and they are not covered by the public CI either. react/renderer/animated/tests is excluded from the iOS build (React-Fabric.podspec: ss.exclude_files = "react/renderer/animated/tests") and is not in the Android CMake glob (react/renderer/animated/CMakeLists.txt globs *.cpp drivers/*.cpp event_drivers/*.cpp internal/*.cpp nodes/*.cpp), so they build only in the internal build reached at import time. What is verified here: NativeAnimatedNodesManager.cpp compiles clean under -Wall -Werror -Wpedantic as part of the Fantom tester build, and the JS-side changes are covered by the Fantom and Jest runs above.

Notes

For a colour interpolate() on the native driver the listener receives the interpolated colour as a number rather than a string, because the value is computed natively. That matches pre-0.78 behaviour and is out of scope here.

[meta-cla]

Hi @dennytosp!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

[github-actions]

Warning

JavaScript API change detected

This PR commits an update to ReactNativeApi.d.ts, indicating a change to React Native's public JavaScript API.

• Please include a clear changelog message.
• This change will be subject to additional review.

This change was flagged as: POTENTIALLY_BREAKING

[dennytosp]

Note on the POTENTIALLY_BREAKING flag for ReactNativeApi.d.ts — I believe it's a false positive, so saving a reviewer the trip.

The only removed declarations are two members of AnimatedValue:

 declare class AnimatedValue_default extends AnimatedWithChildren_default {
   ...
-  removeAllListeners(): void
-  removeListener(id: string): void

Both are still declared on the base class in the same file, and AnimatedValue_default extends AnimatedWithChildren_default extends AnimatedNode_default:

declare class AnimatedNode_default {
  addListener(callback: (value: any) => unknown): string
  constructor(config?: null | Readonly<AnimatedNodeConfig> | undefined)
  hasListeners(): boolean
  removeAllListeners(): void
  removeListener(id: string): void
  toJSON(): unknown
}
declare class AnimatedWithChildren_default extends AnimatedNode_default {}

AnimatedValue previously overrode both only to tear down its own native value subscription. That bookkeeping now lives on AnimatedNode (so that derived value nodes get it too), which made the overrides redundant. The resolved type of AnimatedValue#removeListener and #removeAllListeners is unchanged for TypeScript consumers.

addListener is deliberately kept as an override on AnimatedValue, since it narrows the callback to ValueListenerCallback.

The rest of the diff in that file is content-hash churn on symbols that transitively reference Animated (useAnimatedValue, ScrollView, FlatList, …). No signatures changed.

[meta-cla]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!