Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply Animated initialProps handling to Fabric only #34927

Closed
wants to merge 1 commit into from

Conversation

javache
Copy link
Member

@javache javache commented Oct 10, 2022

Summary:
The changes made in D36902220 (a041951) and D36958882 (d8c25ca) attempted to reduce flickering and consistency issues when using Animated.

In the old renderer, we explicitly reset all animated props, and wait for the subsequent React commit to set the props to the right state, but if initialProps are used, the React reconciliation may not be able to identify the prop-update is required and will leave out the value. This behaviour is different in the new renderer, where we do not explicitly restoreDefaultValues on detaching the animated node, and instead rely on the latest state being correct(?).

Changelog:
[General][Fixed] Stop styles from being reset when detaching Animated.Values in old renderer

Fixes #34665

Differential Revision: D40194072

Summary:
The changes made in D36902220 (facebook@a041951) and D36958882 (facebook@d8c25ca) attempted to reduce flickering and consistency issues when using Animated.

In the old renderer, we explicitly reset all animated props, and wait for the subsequent React commit to set the props to the right state, but if `initialProps` are used, the React reconciliation may not be able to identify the prop-update is required and will leave out the value. This behaviour is different in the new renderer, where we do not explicitly `restoreDefaultValues` on detaching the animated node, and instead rely on the latest state being correct(?).

Changelog:
[General][Fixed] Stop styles from being reset when detaching Animated.Values in old renderer

Fixes facebook#34665

Differential Revision: D40194072

fbshipit-source-id: b2a3d43a1d9cfd0581957dd215edd50da080967a
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner fb-exported labels Oct 10, 2022
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D40194072

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,768,501 -6
android hermes armeabi-v7a 7,169,441 -22
android hermes x86 8,081,641 -6
android hermes x86_64 8,053,130 -20
android jsc arm64-v8a 9,629,236 +10
android jsc armeabi-v7a 8,393,667 +11
android jsc x86 9,578,654 +12
android jsc x86_64 10,171,751 +0

Base commit: cb3a5cc
Branch: main

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: e4dff28
Branch: main

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @javache in 2f58e52.

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

@react-native-bot react-native-bot added the Merged This PR has been merged. label Oct 11, 2022
kelset pushed a commit that referenced this pull request Oct 11, 2022
Summary:
Pull Request resolved: #34927

The changes made in D36902220 (a041951) and D36958882 (d8c25ca) attempted to reduce flickering and consistency issues when using Animated.

In the old renderer, we explicitly reset all animated props, and wait for the subsequent React commit to set the props to the right state, but if `initialProps` are used, the React reconciliation may not be able to identify the prop-update is required and will leave out the value. This behaviour is different in the new renderer, where we do not explicitly `restoreDefaultValues` on detaching the animated node, and instead rely on the latest state being correct(?).

Changelog:
[General][Fixed] Stop styles from being reset when detaching Animated.Values in old renderer

Fixes #34665

Reviewed By: rshest

Differential Revision: D40194072

fbshipit-source-id: 1b3fb1d1f4a39036a501a8a21e57002035dd5659

# Conflicts:
#	Libraries/Animated/createAnimatedComponent.js
@javache javache deleted the export-D40194072 branch October 11, 2022 13:37
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
Summary:
Pull Request resolved: facebook#34927

The changes made in D36902220 (facebook@a041951) and D36958882 (facebook@d8c25ca) attempted to reduce flickering and consistency issues when using Animated.

In the old renderer, we explicitly reset all animated props, and wait for the subsequent React commit to set the props to the right state, but if `initialProps` are used, the React reconciliation may not be able to identify the prop-update is required and will leave out the value. This behaviour is different in the new renderer, where we do not explicitly `restoreDefaultValues` on detaching the animated node, and instead rely on the latest state being correct(?).

Changelog:
[General][Fixed] Stop styles from being reset when detaching Animated.Values in old renderer

Fixes facebook#34665

Reviewed By: rshest

Differential Revision: D40194072

fbshipit-source-id: 1b3fb1d1f4a39036a501a8a21e57002035dd5659
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Animated.Value resets styles when detached on RN 0.70.0
5 participants