Summary:
"The instance should not stick around after unmount..." - Tim Yung, 2021
I have a hypothesis that, if a component instance of an animated component sticks around after unmount, it could cause memory leaks due to references to Fabric ShadowNodes across the JSI (this would not impact non-Fabric... in theory).
Wild guess. If OOMs disappear then maybe this hypothesis is correct, but it's a long shot. I figure there's ~no harm in doing this cleanup here anyway.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D26650348
fbshipit-source-id: 90633db650b65755cacfb52344e7b53e46c9b125
Summary:
There's logic in Animated JS that prevents flattening of animated views in Fabric. However, we cannot actually detect Fabric vs non-Fabric in the first render pass; in the past we defaulted to assuming non-Fabric. Now we assume Fabric for View flattening purposes.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D26647393
fbshipit-source-id: c91b51aeeb4f352cc502bc018f086e36fd1ffd85
Summary:
The `isFabric` method used in createAnimatedComponent is unreliable (another reason in a long list of reasons why you should not duplicate this code elsewhere, and why we want to delete it ASAP).
In particular, during the first render, the ref component has not been set yet, so we /cannot/ detect if the component is Fabric or non-Fabric and assume it's non-Fabric.
In Fabric, this causes `collapsable` and `nativeID` values to change after the first render.
To reduce this re-rendering, but not eliminate it for all components, I've introduced a flag that indicates if a component will /never/ be flattened. In particular, Image components, ScrollViews, Text cannot ever be flattened,
so we should always pass `collapsable:false` and the same nativeID prop for those components. For Animated <View>s and other components, the re-rendering issue is still a problem in Fabric for now.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25720322
fbshipit-source-id: fe3234d8ae974911a2b5f82e4f6a093216f43d4b
Summary:
In D24521951 (https://github.com/facebook/react-native/commit/2ff1d4c041ee662871a84363a3f85a8bc9e857ce) I refactored the JS-side queueing for NativeAnimated API calls, and used randomized IDs for queueing. This could cause bugs or unexpected behavior, and potentially crashes, if there's ever a collision in random numbers generated or
a collision between random number and one of the deterministic numbers generated in createAnimatedComponent.
In this diff I make both of them namespaced with a string, and deterministic, to eliminate any potential collisions. This could also be slightly faster (but not meaningfully) since we're not relying on Math.random.
Changelog: [Internal]
Reviewed By: yungsters
Differential Revision: D24553557
fbshipit-source-id: 8b765e21597ad4f8e641453c1f9f90bdf1ee022f
Summary:
This small PR fixes few "no-unused-var" issues and and removes two old entries for no longer existing files from the `.eslintignore`.
## Changelog
[Internal] [Fixed] - Lint: fix few "no-unused-var" warnings for imports
Pull Request resolved: https://github.com/facebook/react-native/pull/30157
Test Plan: Successfully run of `yarn lint` script. Warnings count has been reduced from `61` to `58`.
Reviewed By: yungsters
Differential Revision: D24288226
Pulled By: appden
fbshipit-source-id: 06e4ef015a331e3f2eac3b9aa6f757a3764e3ed9
Summary:
I believe the old method of queueing these operations for Fabric is causing crashes because "connectNode" is on a separate JS queue from setting up nodes.
In hindsight, this seems silly. We must ensure that nodes are created before they're connected, and we weren't doing that?
Using a single queue is conceptually simpler, should be easier to reason about, and should fix some crashes.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D24521951
fbshipit-source-id: f6c38ac0023faa28414063d8b96daf32ba95524d
Summary:
Removes the legacy `react-animated` package configuration and collapses the `Animated/src/` directory into `Animated/`.
Also, reconfigures all references to `Animated/src/` to just be `Animated/`.
Changelog:
[Internal]
Reviewed By: cpojer
Differential Revision: D22450848
fbshipit-source-id: 9fd4861e9f357d817d82e9fec71967a2936a3830