feat(Reanimated): route synchronous prop updates to the light tree - #10416
feat(Reanimated): route synchronous prop updates to the light tree#10416pawicao wants to merge 9 commits into
Conversation
a9587fd to
de78feb
Compare
pawicao
changed the title
|
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 (1)
🚧 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; 7 remain after this review. 📝 WalkthroughWalkthroughThe change forwards synchronous UI prop updates to the experimental layout animation light tree. It tracks, reapplies, skips, and removes synchronous-props overlays. It reports evicted tags and permits synchronous-update feature flags with shared element transitions on Android and iOS. It adds animated transform and light-tree erasure examples with native-stack navigation and shared-transition boundaries, then registers both examples for iOS and Android. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Full details: Description checkExplanation The description clearly explains routing synchronous UI prop updates into the experimental layout animations light tree, the shared-element transition fix, implementation details, removed guards, and verification steps. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 49214958-e0f5-4a14-a49e-b544abf048f3
⛔ Files ignored due to path filters (1)
apps/fabric-example/ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
apps/common-app/src/apps/reanimated/examples/SharedElementTransitions/AnimatedTransform.tsxapps/common-app/src/apps/reanimated/examples/index.tsapps/fabric-example/package.jsonpackages/react-native-reanimated/CHANGELOG.mdpackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxyCommon.hpackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxy_Experimental.cpppackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxy_Experimental.hpackages/react-native-reanimated/Common/cpp/reanimated/NativeModules/ReanimatedModuleProxy.cpppackages/react-native-reanimated/android/build.gradle.ktspackages/react-native-reanimated/scripts/reanimated_utils.rb
💤 Files with no reviewable changes (2)
- packages/react-native-reanimated/android/build.gradle.kts
- packages/react-native-reanimated/scripts/reanimated_utils.rb
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
01a1638 to
d79fc2b
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxy_Experimental.cpp (1)
268-278: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGuard the
Deletelookup before dereferencing the iterator.
handleSubtreeRemovalcan callendAnimationsRecursively, which erases the tag fromlightNodes_beforeupdateLightTreeprocesses a later inputDeletefor the same tag. In release builds,react_native_assertdoes not stop execution, soit->seconddereferenceslightNodes_.end()and can crash the transaction. Add anit == lightNodes_.end()guard before accessingit->second.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fbe012bc-2737-46d1-b344-ddf9676c2483
📒 Files selected for processing (3)
packages/react-native-reanimated/CHANGELOG.mdpackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxy_Experimental.cpppackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxy_Experimental.h
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/react-native-reanimated/CHANGELOG.md
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5841de32-d88e-4883-b532-3a89fb5d44ab
📒 Files selected for processing (9)
apps/common-app/src/apps/reanimated/examples/SharedElementTransitions/LightTreeErasure.tsxapps/common-app/src/apps/reanimated/examples/index.tspackages/react-native-reanimated/Common/cpp/reanimated/Fabric/updates/AnimatedPropsRegistry.cpppackages/react-native-reanimated/Common/cpp/reanimated/Fabric/updates/AnimatedPropsRegistry.hpackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxyCommon.hpackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxy_Experimental.cpppackages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxy_Experimental.hpackages/react-native-reanimated/Common/cpp/reanimated/NativeModules/ReanimatedModuleProxy.cpppackages/react-native-reanimated/Common/cpp/reanimated/NativeModules/ReanimatedModuleProxy.h
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
01088b4 to
ae39ad2
Compare
On iOS, a commit that does not carry the synchronous props replaced the merged light tree props. Keep the last synchronous props for each animated view and put them back after such a commit. Drop them when the settled values reach React state or when the view is deleted. Updates from the CSS registries stay out of this store because no eviction path manages their lifetime.
ae39ad2 to
61e25de
Compare
1 participant
Note
This pull request was authored by AI on behalf of @pawicao.
Summary
The experimental layout animations proxy keeps a light tree as its model of the mounted views. Until now only
pullTransactionfed it. Props applied throughsynchronouslyUpdateUIPropsskip the shadow tree andpullTransaction, so the light tree never saw them. When a shared element held a transform applied through that path, the transition snapshot appeared at the untransformed layout position. That is why we didn't allow having both*_SYNCHRONOUSLY_UPDATE_UI_PROPSandENABLE_SHARED_ELEMENT_TRANSITIONSflags enabled at the same time.Feeding the light tree
applySynchronousProps(no-op in the common LA proxy base), merges each synchronous update into the matchingLightNodeunder the proxy mutex, through the component descriptor.applySynchronousUpdatescalls it afterpartitionUpdates, so both sync paths feed the tree —performOperationsand the Android draw pass — before the native views get the new values.Keeping the merge across commits (iOS)
On iOS the merge alone does not survive. The
Updatebranch ofupdateLightTreereplaces the stored props with the committed ones, and commits do not carry the synchronous values: React receives them only through the settled sync-back, about one second after a value stops changing. Inside that window every commit that touches the view — even a plain relayout — erased the merge.The proxy now keeps the last synchronous props per tag (
synchronousPropsOverlay_) and puts them back after each replacement. This is the same rule mounting and the commit hook already enforce elsewhere: a commit must not clobber props the animated registry owns. The overlay lives exactly as long as that ownership:collectSettledUpdatesreports the tags it evicts andgetSettledUpdatesrelays them todropSynchronousProps.Deletemutation erases the entry.performOperationscollects their tags and the proxy skips and erases them.Android needs no overlay: its
Updatebranch merges raw props. (that, as I understand, can change in the future)Removed guards
The bypass is gone, so the guards that kept the synchronous-update flags and
ENABLE_SHARED_ELEMENT_TRANSITIONSapart are removed:shouldUseSynchronousUpdatesInPerformOperationsconjuncts,pod installassertion,Recordings
Before
before02.mov
After
after02.mov
Test plan
Two new examples in fabric-example, run with
ENABLE_SHARED_ELEMENT_TRANSITIONSand the platform's synchronous-updates flag on:[SET] Animated Transform— a box with a static style transform (translateX: 60) and a box whose transform comes from a non-animatinguseAnimatedStyle(translateX: 80, set once on mount, so it reaches the native view only through the synchronous path). Both transitions must start from the shifted positions.[SET] Light Tree Erasure Repro— both buttons shift a box to a marked frame and navigate 250 ms later; the second also triggers an unrelated commit in between. Without the overlay the second transition started from the layout frame; both must start from the shifted frame.Verified:
applySynchronousPropsfires on mount (pre-mergetransform.matrix[12] = 0), andstartSharedTransitionreceives before-snapshots withmatrix[12] = 60and80; the back transition shows the same values on theafterside.react_native_assertaborts in logcat.Changelog
Unpublishedsection of each changed package'sCHANGELOG.md, or this PR does not changereact-native-reanimatedorreact-native-worklets.