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

Animated: Defer onAnimatedValueUpdate on Attach + Native #48715

Closed
wants to merge 1 commit into from

Conversation

yungsters
Copy link
Contributor

Summary:
{D68154908} fixed a problem with the onAnimatedValueUpdate listener not being correctly attached if __attach were called before __makeNative (which sets __isNative to true).

We're potentially seeing production symptoms of stuttering interactions and user responsiveness, after queuing up many operations. Our hypothesis is that in scenarios where ensureUpdateSubscriptionExists is being called during __makeNative (instead of during __attach), a backup of operations occurs leading to these symptoms.

This diff attempts to validate and mitigate this hypothesis by deferring ensureUpdateSubscriptionExists to when an AnimatedValue instance has had both __attach and __makeNative invoked.

Changelog:
[Internal]

Differential Revision: D68236594

Summary:
{D68154908} fixed a problem with the `onAnimatedValueUpdate` listener not being correctly attached if `__attach` were called before `__makeNative` (which sets `__isNative` to true).

We're potentially seeing production symptoms of stuttering interactions and user responsiveness, after queuing up many operations. Our hypothesis is that in scenarios where `ensureUpdateSubscriptionExists` is being called during `__makeNative` (instead of during `__attach`), a backup of operations occurs leading to these symptoms.

This diff attempts to validate and mitigate this hypothesis by deferring `ensureUpdateSubscriptionExists` to when an `AnimatedValue` instance has had both `__attach` and `__makeNative` invoked.

Changelog:
[Internal]

Differential Revision: D68236594
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 15, 2025
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 843582b.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Jan 16, 2025
@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @yungsters in 843582b

When will my fix make it into a release? | How to file a pick request?

@facebook-github-bot
Copy link
Contributor

This pull request has been reverted by b059050.

fabriziocucci pushed a commit that referenced this pull request Jan 20, 2025
Summary:
Pull Request resolved: #48715

{D68154908} fixed a problem with the `onAnimatedValueUpdate` listener not being correctly attached if `__attach` were called before `__makeNative` (which sets `__isNative` to true).

We're potentially seeing production symptoms of stuttering interactions and user responsiveness, after queuing up many operations. Our hypothesis is that in scenarios where `ensureUpdateSubscriptionExists` is being called during `__makeNative` (instead of during `__attach`), a backup of operations occurs leading to these symptoms.

This diff attempts to validate and mitigate this hypothesis by deferring `ensureUpdateSubscriptionExists` to when an `AnimatedValue` instance has had both `__attach` and `__makeNative` invoked.

Changelog:
[Internal]

Differential Revision: D68236594

fbshipit-source-id: 2089100a773ebfc161fb5b567123eb58a893939f
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 Reverted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants