Conversation
This reverts commit f1075fe. Review found two defects in the overlay: a stale entry could override a newer committed value when a mixed batch routed the synchronous keys through the shadow tree, and the eviction relay took the registry lock and the proxy lock in an order that a custom layout-animation config calling setNativeProps could invert. The next commits fix the erasure at its source instead: commits carry the synchronous values, so the light tree receives correct props without a patch layer. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With synchronous prop updates on, a Reanimated commit for a view clones props from a shadow tree that never saw the values the synchronous path applied. iOS diffs the incoming props against the view's own props and writes the stale value back to the layer for several frames. A layout-only Update mutation replaced the light-tree props that the synchronous path had merged, so a shared element transition started from the untransformed position. Append each registry's current props to the families already in a Reanimated commit, once per commit under the registry lock. Keep the light-tree props when an Update mutation carries the same Props object.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (11)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe change removes evicted-tag reporting from settled animated-property updates. It adds registry-property reinsertion for committed property maps. Synchronous layout-animation updates no longer use overlay-tag exclusions or drop operations. The experimental proxy restores captured properties when a commit has no new props. Merge Risk: ⚪ Minimal · up to The change preserves synchronous transforms and light-tree props without adding commit families or sync-only commits. No actionable production risk is established, so it is mergeable with normal checks. 🚥 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 |
pawicao
marked this pull request as ready for review
September 10, 2026 13:45
pawicao
merged commit Sep 10, 2026
04e588f
into
@pawicao/lighttree-sync-update-props
24 of 26 checks passed
1 task
pawicao
added a commit
that referenced
this pull request
Sep 10, 2026
…10503) > [!NOTE] > This pull request was authored by AI on behalf of @pawicao. Supersedes #10480 with a 23-line change that uses only existing state. With the synchronous prop updates on, a Reanimated commit for a view clones props from a shadow tree that never saw the values the synchronous path applied. On iOS, `RCTViewComponentView` diffs the incoming props against the view's own `_props`, so the commit writes the stale transform back to the layer for several frames. A layout-only Update mutation for the same reason replaced the light-tree props that `applySynchronousProps` had merged, so a shared element transition started from the untransformed position. The fix: 1. `UpdatesRegistryManager::addRegistryProps` appends each registry's current props to the families already in a Reanimated commit. The registry holds a view's latest values until React has them (`UpdatesRegistry::flush` stores the batch before it is partitioned; the settled-props tick erases an entry only after React received it), so no second store is needed. 2. `commitUpdates` calls it once per commit, under the registry lock, before the commit loop, only in the branch that commits a batch. The flush branch already carries every registry value through `collectProps`. 3. `updateLightTree` keeps the light node's props when an Update mutation carries the same Props object. Mounting writes props to a view only when that object changed, so such an Update cannot have changed the native view. No view enters a commit because of this change, no sync-only commit is added, and the commit hook is untouched. Compared with #10480, this drops the pending store, per-key versions, commit receipts, root identity checks, descendant scans, the non-React hook path and the layout-animation retargeting; every one of them rested on a review hypothesis, none on a device reproduction. Set `IOS_SYNCHRONOUSLY_UPDATE_UI_PROPS: true` in `apps/fabric-example/package.json` (`ENABLE_SHARED_ELEMENT_TRANSITIONS` stays on), run `pod install`, build `FabricExample` for the iOS simulator. Overwrite case. Mount this screen and press the button several times, 1-2 s apart. Expected: the box keeps its new position when its height changes. On the base, the box jumps back to the old position for 3-8 frames after each press. ```tsx import { Button, StyleSheet, View } from 'react-native'; import Animated, { useAnimatedStyle, useSharedValue } from 'react-native-reanimated'; export default function Overwrite() { const offset = useSharedValue(0); const height = useSharedValue(100); const transformStyle = useAnimatedStyle(() => ({ transform: [{ translateX: offset.value }] })); const heightStyle = useAnimatedStyle(() => ({ height: height.value })); return ( <View style={{ padding: 20, gap: 20 }}> <Button title="Move, then resize" onPress={() => { offset.value = offset.value === 0 ? 100 : 0; setTimeout(() => { height.value = height.value === 100 ? 150 : 100; }, 100); }} /> <Animated.View style={[{ width: 100, backgroundColor: 'purple' }, transformStyle, heightStyle]} /> </View> ); } ``` Shared transition case. In the `[SET] Light Tree Erasure Repro` example on the base branch, press `shift, erase and go (regression)` on a fresh mount: the transition must start from the green frame. On the base it starts from the red frame in most fresh-mount runs; the runs that pass are the ones where the 500 ms settled-props tick lands inside the 250 ms window before navigation. Measured (matched Debug builds, 30 fps recordings, per-frame pixel analysis): overwrite base 4/4 presses stale, patched 0/4; erasure base 4/6 fresh-mount runs wrong, patched 0/6 (two iterations); sticky header and a React parent-layout change unchanged; Android Pixel 9a clean with the flag off and on. Cost: one extra props parse per family that commits a layout prop in a frame; on 300 such views per frame, `mergeProps` +49% and 3 percentage points more repeated frames on a fixture that is already over budget on the base. - [ ] I added an entry to the `Unpublished` section of each changed package's `CHANGELOG.md`, or this PR does not change `react-native-reanimated` or `react-native-worklets`. No separate entry, as on #10480: the behavior ships under the #10416 entry that this branch is stacked on. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Note
This pull request was authored by AI on behalf of @pawicao.
Summary
Supersedes #10480 with a 23-line change that uses only existing state.
With the synchronous prop updates on, a Reanimated commit for a view clones props from a shadow tree that never saw the values the synchronous path applied. On iOS,
RCTViewComponentViewdiffs the incoming props against the view's own_props, so the commit writes the stale transform back to the layer for several frames. A layout-only Update mutation for the same reason replaced the light-tree props thatapplySynchronousPropshad merged, so a shared element transition started from the untransformed position.The fix:
UpdatesRegistryManager::addRegistryPropsappends each registry's current props to the families already in a Reanimated commit. The registry holds a view's latest values until React has them (UpdatesRegistry::flushstores the batch before it is partitioned; the settled-props tick erases an entry only after React received it), so no second store is needed.commitUpdatescalls it once per commit, under the registry lock, before the commit loop, only in the branch that commits a batch. The flush branch already carries every registry value throughcollectProps.updateLightTreekeeps the light node's props when an Update mutation carries the same Props object. Mounting writes props to a view only when that object changed, so such an Update cannot have changed the native view.No view enters a commit because of this change, no sync-only commit is added, and the commit hook is untouched. Compared with #10480, this drops the pending store, per-key versions, commit receipts, root identity checks, descendant scans, the non-React hook path and the layout-animation retargeting; every one of them rested on a review hypothesis, none on a device reproduction.
Test plan
Set
IOS_SYNCHRONOUSLY_UPDATE_UI_PROPS: trueinapps/fabric-example/package.json(ENABLE_SHARED_ELEMENT_TRANSITIONSstays on), runpod install, buildFabricExamplefor the iOS simulator.Overwrite case. Mount this screen and press the button several times, 1-2 s apart. Expected: the box keeps its new position when its height changes. On the base, the box jumps back to the old position for 3-8 frames after each press.
Shared transition case. In the
[SET] Light Tree Erasure Reproexample on the base branch, pressshift, erase and go (regression)on a fresh mount: the transition must start from the green frame. On the base it starts from the red frame in most fresh-mount runs; the runs that pass are the ones where the 500 ms settled-props tick lands inside the 250 ms window before navigation.Measured (matched Debug builds, 30 fps recordings, per-frame pixel analysis): overwrite base 4/4 presses stale, patched 0/4; erasure base 4/6 fresh-mount runs wrong, patched 0/6 (two iterations); sticky header and a React parent-layout change unchanged; Android Pixel 9a clean with the flag off and on. Cost: one extra props parse per family that commits a layout prop in a frame; on 300 such views per frame,
mergeProps+49% and 3 percentage points more repeated frames on a fixture that is already over budget on the base.Changelog
Unpublishedsection of each changed package'sCHANGELOG.md, or this PR does not changereact-native-reanimatedorreact-native-worklets.No separate entry, as on #10480: the behavior ships under the #10416 entry that this branch is stacked on.