Enable overrideBySynchronousMountPropsAtMountingAndroid by default on Android#56652
Closed
jingjing2222 wants to merge 1 commit intofacebook:mainfrom
Closed
Enable overrideBySynchronousMountPropsAtMountingAndroid by default on Android#56652jingjing2222 wants to merge 1 commit intofacebook:mainfrom
overrideBySynchronousMountPropsAtMountingAndroid by default on Android#56652jingjing2222 wants to merge 1 commit intofacebook:mainfrom
Conversation
…Animated synchronous prop updates.
overrideBySynchronousMountPropsAtMountingAndroid by default on Android.overrideBySynchronousMountPropsAtMountingAndroid by default on Android
Contributor
|
cc @zeyap |
Contributor
Author
|
rerun Test All / test_ios_rntester (Release, false) (pull_request) please 🙏 |
Contributor
Author
Test All / test_ios_rntester (Release, false) (pull_request) too please 🙏 |
Contributor
|
Yes from previous experiments this flag was pretty low risk, glad it fixed your problem🫡 |
|
@zeyap has imported this pull request. If you are a Meta employee, you can view this in D103070058. |
Collaborator
|
This pull request was successfully merged by @jingjing2222 in eba0bfb When will my fix make it into a release? | How to file a pick request? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Enable
overrideBySynchronousMountPropsAtMountingAndroidby default on Android.This flag was added to prevent regular Fabric mount updates from overriding newer props that were already applied synchronously by Native Animated direct manipulation. Without this path enabled, Android can still hit a race where:
transformprops synchronously on the UI thread.SurfaceMountingManager.updateProps()later and overwrites the newer synchronous animated value.This is not just theoretical. I spent about a week debugging this from a real Android flickering issue, and the final repro is intentionally very small. The visual impact is clear in the README videos, and I am attaching the same videos here as well.
Minimal repro:
https://github.com/jingjing2222/fix-react-native-android-flickering
The repro uses React Native 0.85.2 with Android Fabric. The JS only runs
Animated.timing(..., {useNativeDriver: true})fortranslateXandtranslateY.As-is
as-is.mov
With the current default, flickering is visible during the animation.
To-be
to-be.mov
With
overrideBySynchronousMountPropsAtMountingAndroidenabled, the flickering is gone.The existing mounting-layer flag fixes this by storing the latest synchronous mount props and reapplying them when regular mount props are processed. This makes the final mounted props preserve the latest Native Animated direct manipulation value.
Because this can happen with a simple native-driver transform animation, enabling this default should prevent other Android users from spending time debugging the same race condition.
Why Android only?
iOS Fabric already tracks props managed by NativeAnimated on the component view. When NativeAnimated synchronously updates props such as
transformoropacity, iOS records those prop keys and prevents later normal Fabric updates from re-applying stale values for those props.Android Fabric has a similar mechanism through
tagToSynchronousMountProps, but it was gated behindoverrideBySynchronousMountPropsAtMountingAndroid, which defaults to false. Therefore, NativeAnimated could synchronously apply the final transform to the view, and a later normal Fabric props update could re-apply stale transform props, causing a transient flicker.This change makes Android preserve synchronously mounted NativeAnimated props across later Fabric prop updates, matching the ownership behavior already present on iOS.
Changelog:
[ANDROID] [FIXED] - Prevent stale Fabric mount updates from overriding newer Native Animated synchronous prop updates.
Test Plan:
yarn featureflags --updategit diff --checktranslateX/translateYnative-driver animation