fix(LayoutAnimations): update Android view hierarchy only during JS pulls - #10372
fix(LayoutAnimations): update Android view hierarchy only during JS pulls#10372bartlomiejbloniarz wants to merge 1 commit into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughAdds Android-aware deferral and scheduling for structural layout-animation cleanup. Extends completed-animation cleanup to preserve selected tags. Adds an experimental example that repeatedly replaces animated items, periodically stalls the UI thread, and provides Start and Stop controls. Registers the example in the layout-animation examples map. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
cdd2123 to
c591edb
Compare
c591edb to
e7d893d
Compare
7994a7f to
5bb5ef1
Compare
5bb5ef1 to
d53260a
Compare
b772e2d to
8502c9a
Compare
8502c9a to
82eb4f7
Compare
82eb4f7 to
a057f29
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
8d90c09 to
f42bcc6
Compare
f42bcc6 to
e821045
Compare
e821045 to
d3ee84d
Compare
195223f to
55f1107
Compare
55f1107 to
c0bf038
Compare
65d0c62 to
c81a201
Compare
672d049 to
71a7474
Compare
Android's push model applies React transactions from the JS thread asynchronously on the UI thread. An Experimental completion pull can run on the UI thread and synchronously mount its Remove/Delete mutations first, leaving React's queued transaction with stale child indices. Keep completed exiting nodes and shared-transition containers pending during UI-thread pulls. A debounced JS-thread pull performs those structural removals in React transaction order, while non-structural final updates still reconcile immediately. The gate is Android-only and can be removed with the pull model. Add an Experimental cleanup-ordering stress example. With the gate bypassed, it reproduces addViewAt index/count failures; with the fix it runs through more than 1,800 cycles and clears all exiting views after Stop. fix(Android): reset the cleanup-pull debounce on surface teardown
71a7474 to
32506fd
Compare
| filteredMutations.end(), transaction.teardownMutations.begin(), transaction.teardownMutations.end()); | ||
|
|
||
| flushCompletedRemovals(filteredMutations); | ||
| flushCompletedRemovals(filteredMutations, flushStructuralMutations); |
There was a problem hiding this comment.
hmm wouldn't it be somehow simpler to just do
if (!flushStructuralMutations) {
flushCompletedRemovals(filteredMutations);
}since flushStructuralMutations is just used as an early return in the beginning of that function?
There was a problem hiding this comment.
oh unless the reasoning is to have similar API shape of flushCompletedRemovals as cleanupAnimations that has to use flushStructuralMutations inside, then I understand
2 participants
This issue only exists on Android. Before this change, Reanimated could observe
pullTransactioncalls in one order while Android converted and applied them in another. A React transaction could first pass throughpullTransactionon the JS thread and move to a local pending vector. Before itsexecuteMountstarted, animation completion could cause anotherpullTransactionon the UI thread. The UI path callsscheduleMountItemandtryDispatchMountItemssynchronously, so its cleanup changes the View Hierarchy immediately. The React path callsscheduleMountItemfrom the JS thread, which only queues its MountItem for a later UI-thread dispatch. Remove or Delete mutations could therefore use child indices for a hierarchy that React had not mounted yet.On Android,
shouldFlushStructuralMutationsnow blocks hierarchy cleanup during UI-thread pulls.maybeScheduleCleanupPullkeeps the work pending and requests one later JS-thread pull. This creates a clear Android-only boundary: React queues its transaction first, then Reanimated queues the hierarchy cleanup. Their MountItems now reach the UI thread in the same order. Other platforms keep the existing behavior.Threading analysis
AI analysis, mostly for future reference
Test Plan
ExperimentalCleanupOrderingExampleon Android.