Fix: Preserve native animated value after animated component unmount

[TMaszko]

Summary

After animation has been finished using Native driver there is no final value passed from the native to JS side. This causes a bug from #28114.

This PR solves this problem in the same way as react-native-reanimated library. When detaching it is calling native side to get the last value from Animated node and stores it on the JS side.

Preserving animated value even if animation was using useNativeDriver: true
Fixes #28114

Changelog

[Internal] [Fixed] - Save native Animated node value on JS side in detach phase

Test Plan

Unit tests for added getValue method passed. Green CI

[facebook-github-bot]

Hi @TMaszko!

Thank you for your pull request and welcome to our community.We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, 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.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	6,741,871	-22,121
android	hermes	armeabi-v7a	6,406,923	-19,301
android	hermes	x86	7,124,014	-25,229
android	hermes	x86_64	7,014,570	-25,175
android	jsc	arm64-v8a	8,909,209	-23,286
android	jsc	armeabi-v7a	8,566,658	-20,444
android	jsc	x86	8,734,494	-26,371
android	jsc	x86_64	9,310,667	-26,342

Base commit: ee39945

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	809,568	0

Base commit: ee39945

[thymikee]

cc @wcandillon @shergin

[zamotany]

Hey @janicduplessis, we would really appreciate if you could take a look at it

[JoshuaGross]

Hi @TMaszko, this looks reasonable to me at first glance, though I need to do a deeper code-review.

Is it possible to create an RNTester example for this, maybe to one of the existing Animation examples? That would make it a lot easier to test and to prevent regressions in the future.

[TMaszko]

Hi @JoshuaGross , I added this example to the Animated examples as you have asked for :)

[zamotany]

@JoshuaGross any update/feedback??

[facebook-github-bot]

@JoshuaGross has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[JoshuaGross]

I'll test this internally.

[JoshuaGross]

I have minor concerns about perf implications of this, since it adds a native module/TM call to every animated view detach - and those calls aren't free. However, this seems pretty reasonable overall; and as for perf implications, users should strive to re-render or detach Animated values/Views as little as possible anyway, so it doesn't really change any constraints. Given that caveat I think it's reasonable to merge and it seems like there are no ill effects within the FB app. :)

[facebook-github-bot]

@JoshuaGross has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[react-native-bot]

This pull request was successfully merged by @TMaszko in d922842.

^{When will my fix make it into a release? | Upcoming Releases}