Repository navigation
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 10.17% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 59 functions across 28 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Linked Issues check Issue #4777requires Android Fabric screen adoption to avoid dirtying Yoga when the native frame size is unchanged, including screens with a persistent zero frame. `RNSScreenComponentDescriptor::adopt…Out of Scope Changes check The v4 and v5 size guards address the same state-driven setSizepattern described by the PR. Header-origin invalidation supports the stated regression requirement formeasure()and press-rectangle…Title check The title clearly and concisely describes the primary change: guarding shadow node size updates on iOS and Android so they occur only when the size changes. Description check The description is directly related to the changeset. It explains the size-update optimization, header-origin invalidation, affected components, tests, profiling results, and known coverage limits. Full details: Linked Issues check
-
Explanation
Issue
#4777requires Android Fabric screen adoption to avoid dirtying Yoga when the native frame size is unchanged, including screens with a persistent zero frame.RNSScreenComponentDescriptor::adoptnow callssetSizeIfChangedfor both the reported-frame path and the zero-frame path.SizeFromStateShadowNode::setSizeIfChangedcompares both Yoga dimensions before callingsetSize. TheTest4777screen covers native-stack scrolling, JS-tab scrolling, and per-frame Reanimated commits. These changes satisfy the linked issue's coding requirement.
Full details: Out of Scope Changes check
-
Explanation
The v4 and v5 size guards address the same state-driven
setSizepattern described by the PR. Header-origin invalidation supports the stated regression requirement formeasure()and press-rectangle coordinates after the size guard.Test4777provides focused verification scenarios. No unrelated change is established by the reviewed changes.
-
- Fix all pre-merge checks with AI
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
|
Fyi: I have filed a PR to fix this properly upstream here - react/react-native#59101 |
There was a problem hiding this comment.
🟡 Changes recommended
Size-only guards can skip layout needed to refresh state-driven header coordinates.
3 open findings
What changed in this PR
Reduces unnecessary Fabric layout work on iOS and Android by avoiding redundant state-driven size updates, addressing #4777.
Changes:
- Adds a shared Yoga size comparison helper.
- Guards size updates across legacy and v5 navigation components.
- Adds a profiling reproduction screen.
| File | Description |
|---|---|
| common/cpp/react/renderer/components/rnscreens/utils/YogaSizeUtil.h | Adds size comparison helper. |
| common/cpp/react/renderer/components/rnscreens/RNSTabsBottomAccessoryShadowNode.h | Adds guarded size setter. |
| common/cpp/react/renderer/components/rnscreens/RNSTabsBottomAccessoryComponentDescriptor.h | Uses guarded accessory sizing. |
| common/cpp/react/renderer/components/rnscreens/RNSStackScreenShadowNode.h | Adds guarded size setter. |
| common/cpp/react/renderer/components/rnscreens/RNSStackScreenComponentDescriptor.h | Guards Android screen sizing. |
| common/cpp/react/renderer/components/rnscreens/RNSStackHeaderConfigShadowNode.h | Adds guarded size setter. |
| common/cpp/react/renderer/components/rnscreens/RNSStackHeaderConfigComponentDescriptor.h | Guards header sizing. |
| common/cpp/react/renderer/components/rnscreens/RNSSplitScreenShadowNode.h | Adds guarded size setter. |
| common/cpp/react/renderer/components/rnscreens/RNSSplitScreenComponentDescriptor.h | Guards split-screen sizing. |
| common/cpp/react/renderer/components/rnscreens/RNSFormSheetHostShadowNode.h | Adds guarded size setter. |
| common/cpp/react/renderer/components/rnscreens/RNSFormSheetHostComponentDescriptor.h | Guards form-sheet sizing. |
| common/cpp/react/renderer/components/rnscreens/legacy/RNSScreenStackHeaderConfigShadowNode.h | Adds guarded size setter. |
| common/cpp/react/renderer/components/rnscreens/legacy/RNSScreenStackHeaderConfigComponentDescriptor.h | Guards legacy header sizing. |
| common/cpp/react/renderer/components/rnscreens/legacy/RNSScreenShadowNode.h | Adds guarded size setter. |
| common/cpp/react/renderer/components/rnscreens/legacy/RNSScreenComponentDescriptor.h | Guards sizing and Android undefined resets. |
| common/cpp/react/renderer/components/rnscreens/legacy/RNSModalScreenShadowNode.h | Adds guarded size setter. |
| common/cpp/react/renderer/components/rnscreens/legacy/RNSModalScreenComponentDescriptor.h | Guards modal sizing. |
| apps/src/tests/issue-tests/Test4777.tsx | Adds scroll and animation profiling scenario. |
| apps/src/tests/issue-tests/index.ts | Registers the reproduction screen. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| configShadowNode.setSizeIfChanged( | ||
| Size{stateData.frameSize.width, stateData.frameSize.height}); |
| configShadowNode.setSizeIfChanged( | ||
| {stateData.frameSize.width, YGUndefined}); |
| #else | ||
| if (stateData.frameSize.width != 0 && stateData.frameSize.height != 0) { | ||
| layoutableShadowNode.setSize(stateData.frameSize); | ||
| configShadowNode.setSizeIfChanged(stateData.frameSize); |
kkafar
left a comment
kkafar
left a comment
There was a problem hiding this comment.
Note
This is an automated review generated by Claude Code, not a review written by @kkafar. Kacper asked for it to be posted as-is. Findings were not verified on a device. Treat them as leads to check, not as confirmed bugs.
Summary
The perf diagnosis is right and the guard is cleaner than the one suggested in #4777: it compares against exactly what setSize writes into the Yoga style. But the unconditional dirty in adopt() was load-bearing for the header shadow nodes, so the claim "Behaviour is otherwise unchanged" doesn't hold.
Fabric only calls a child's layout() override when Yoga reports new layout for it (YogaLayoutableShadowNode::layout → getHasNewLayout()). A state-only clone doesn't dirty a node. Several header nodes patch layoutMetrics_.frame.origin from state inside layout():
| Node | layout() correction reads |
|---|---|
v4 RNSScreenStackHeaderSubviewShadowNode (iOS + Android) |
stateData.contentOffset |
v4 RNSScreenStackHeaderConfigShadowNode iOS |
stateData.frameOrigin |
v4 RNSScreenStackHeaderConfigShadowNode Android |
-stateData.frameSize.height (height isn't in the style, which is {width, YGUndefined}) |
v5 RNSStackHeaderConfigShadowNode / RNSStackHeaderItemShadowNode iOS |
stateData.contentOffset |
Before this PR, any state update on these nodes re-cloned the header config, and setSize dirtied it, so layout re-ran and the corrections were re-applied. Even unrelated commits under the Screen did this, which hid ordering issues. After this PR, an origin-only (or Android height-only) state update produces a clean tree, RootShadowNode::layoutIfNeeded returns early, and the shadow-tree origin stays stale until the config's props change.
That is exactly what measure(), Reanimated measure and Pressability's press-rect check read. It risks bringing back the "header button onPress doesn't fire on a physical device" class of bugs (#2219 / #2466, #2855 / #2858, #2714 / #2905 / #2910).
Possible fixes:
- (a) compute the header origins on read via
getContentOriginOffset, as v5 Android already does; - (b) in those descriptors,
dirtyLayout()only when the state-derived origin differs fromgetLayoutMetrics().frame.origin(mirroring the Androidtranslucentearly return).
Separately, the root cause belongs upstream: YogaLayoutableShadowNode::setSize should early-out the way setPadding already does.
Details are in the inline comments, ranked most-severe first. A list of test screens to check for regressions will follow in a separate comment.
Generated by Claude Code
| #else | ||
| if (stateData.frameSize.width != 0 && stateData.frameSize.height != 0) { | ||
| layoutableShadowNode.setSize(stateData.frameSize); | ||
| configShadowNode.setSizeIfChanged(stateData.frameSize); |
There was a problem hiding this comment.
[correctness, high] v4 header subview origin corrections stop being re-applied (iOS + Android).
RNSScreenStackHeaderSubviewShadowNode::layout() writes stateData.contentOffset into the frame origin, but it runs only when the subview gets new layout. That used to happen on every subview state update, because the update re-clones this config and the unconditional setSize dirtied it. Now the config stays clean and the subview's origin goes stale.
Scenario: v4 native stack with a headerRight/headerLeft Pressable. Change headerBackTitle or the title, or make the back button appear. The props commit lays the header out with the old subview offset. UIKit (or the Toolbar) then moves the item and sends a subview state update with a new origin and the same size, and that commit dirties nothing. measure() returns stale coordinates. On a physical device, small finger movement makes Pressability think the touch left the press rect, so onPress never fires.
Generated by Claude Code
|
|
||
| if (stateData.frameSize.width != 0 && stateData.frameSize.height != 0) { | ||
| layoutableShadowNode.setSize( | ||
| configShadowNode.setSizeIfChanged( |
There was a problem hiding this comment.
[correctness, high] v5 iOS header config and header item origins go stale.
RNSStackHeaderConfigShadowNode::layout() and RNSStackHeaderItemShadowNode::layout() (iOS) both set the origin from stateData.contentOffset. Items are children of the config, so their corrections relied on this setSize dirtying the config on every clone.
Scenario: an item moves within the bar but keeps its size (a neighbour item is added or removed, the title changes, iOS 26 regroups items). RNSStackHeaderItemShadowStateProxy sends a new origin, no node is dirty, layout is skipped, and measure / press-rect checks are off by the move distance. The same applies to the config itself when only the navbar origin changes.
Generated by Claude Code
| #ifdef ANDROID | ||
| if (stateData.frameSize.width != 0) { | ||
| layoutableShadowNode.setSize({stateData.frameSize.width, YGUndefined}); | ||
| configShadowNode.setSizeIfChanged( |
There was a problem hiding this comment.
[correctness, medium-high] Android v4: a header height change no longer re-runs layout.
applyFrameCorrections() sets origin.y = -stateData.frameSize.height, but only the width goes into the Yoga style here ({width, YGUndefined}), so yogaStyleHasSize ignores height.
Scenario: a non-translucent header whose toolbar height changes at the same width (top inset / status-bar translucency, font scale). If this config's state commit lands after the Screen's frame commit (they're separate state updates, and the order isn't guaranteed), nothing is dirty. origin.y stays at -oldHeight, and every header subview's measured pageY is off by the height delta.
Generated by Claude Code
| #else | ||
| if (stateData.frameSize.width != 0 && stateData.frameSize.height != 0) { | ||
| layoutableShadowNode.setSize(stateData.frameSize); | ||
| configShadowNode.setSizeIfChanged(stateData.frameSize); |
There was a problem hiding this comment.
[correctness, medium-high] iOS v4: origin-only frameOrigin updates are dropped.
frameOrigin isn't part of the Yoga style, and setPadding already early-outs. So a state update that only moves the navbar (same size, same insets) leaves the config clean, and the new origin never reaches layoutMetrics_.
Scenario: a translucent header on a device where hiding the status bar changes the top safe area. Toggle statusBarHidden: the navbar moves up and keeps its 44pt height. The header config and all its subviews are then measured at the old y.
Generated by Claude Code
| * rounding over the whole tree on every commit, including ones that change | ||
| * nothing layout-related (scroll offset state updates, animated transforms). | ||
| */ | ||
| inline bool yogaStyleHasSize( |
There was a problem hiding this comment.
[altitude] The guard needs a structural partner.
The doc comment presents the guard as safe, but the unconditional dirty was the only thing keeping the state-derived corrections in layout() overrides current. Either move those corrections out of layout() (compute them on read in getContentOriginOffset, as v5 Android does), or dirty explicitly in adopt() when the state origin differs from getLayoutMetrics().frame.origin.
Also consider an RN PR making YogaLayoutableShadowNode::setSize early-out like setPadding. That would fix RN's own Modal/InputAccessory descriptors too.
Generated by Claude Code
| const pop = () => setShowList(false); | ||
|
|
||
| return ( | ||
| <ScreenStack style={{ flex: 1 }}> |
There was a problem hiding this comment.
[test-coverage] The test misses the issue's reported path and the geometry this change can regress.
Only ScreenStack screens are rendered (the frame-known branch). The original #4777 scenario, a Screen in a ScreenContainer / JS bottom tabs (the frameSize == 0 reset branch on Android), isn't exercised. Nothing checks that header subview/item positions stay correct either.
Suggestions: add a JS-tabs variant, plus a headerRight item whose measure() is logged after toggling the back title or title.
Generated by Claude Code
|
|
||
| // Like `setSize`, but leaves the Yoga node clean when the size is unchanged. | ||
| // See `rnscreens::yogaStyleHasSize`. | ||
| void setSizeIfChanged(Size size) const { |
There was a problem hiding this comment.
[simplification] setSizeIfChanged is copy-pasted into 8 shadow nodes.
yogaNode_ is protected, so a free function can't reach it. A small shared base or mixin over ConcreteViewShadowNode would keep this in one place. As it stands, the next change to the guard (e.g. also dirtying on origin changes, per the comments above) has to be made 8 times, and a missed copy reintroduces either the perf bug or a stale-layout bug.
Generated by Claude Code
| @@ -19,16 +19,14 @@ class RNSFormSheetHostComponentDescriptor final | |||
|
|
|||
| react_native_assert( | |||
There was a problem hiding this comment.
[simplification, nit] The layoutableShadowNode casts were removed, but the react_native_assert(dynamic_cast<YogaLayoutableShadowNode *>(...)) that guarded them stays. The same is true in SplitScreen, StackScreen, StackHeaderConfig, ModalScreen and Screen. The static type already guarantees this, so these are now dead RTTI checks in debug adopt().
Generated by Claude Code
| // the whole tree, even when nothing layout-related changed. | ||
| // | ||
| // Both sources of commits below change no layout: scrolling (ScrollView | ||
| // content offset state updates) and a color that changes every frame. Profile |
There was a problem hiding this comment.
[docs, nit] "a color that changes every frame": Math.floor(frame / 10) changes it every 10th frame. The other 9 renders produce no host prop diff, so no Screen clone and no commit to measure. The PR test plan says the same thing. Drop the / 10 or fix the wording.
Generated by Claude Code
| @@ -0,0 +1,31 @@ | |||
| #pragma once | |||
|
|
|||
| #include <react/renderer/components/view/YogaLayoutableShadowNode.h> | |||
There was a problem hiding this comment.
[nit] Only yoga::Style / StyleSizeLength and react::Size are needed. Including the whole YogaLayoutableShadowNode.h adds compile-time coupling to the 8 shadow-node headers that now include this file. <yoga/style/Style.h> should be enough.
Generated by Claude Code
|
Note Generated automatically by Claude Code, not written by @kkafar. Kacper asked for it to be posted as-is. It was compiled from PRs and issues that touched this codev4 Screen (frame size from state, Android frame corrections, orientation commit hook)
v4 header config / header subviews (state-driven origin corrections, the area most at risk here)
v4 FullWindowOverlay / ModalScreen
v5 components (StackScreen, StackHeaderConfig / items, SplitScreen, FormSheetHost, TabsBottomAccessory)
|
There was a problem hiding this comment.
Thanks for the PR. I haven't managed to review today, will do so on Monday.
In general I like the changes, but these touch very fragile part of the code, especially in v4 version. Therefore I've asked LLM to go through git blame and list issues / PRs that touched this code and added regression test cases - we'll have to go through all of them to make sure that this change does not introduce regressions.
In principle I believe this should be fixed upstream, but we can land a temporary patch, before these changes land in core.
|
Thanks @kkafar! I'll point my claude at the feedback, see what it thinks |
|
Note This comment was written by an LLM (Claude Code, working with @mozzius). Samuel asked for it to be posted. The code reading below was checked against the 4.30.0 source, but nothing in it has been verified on a device yet. Thanks for the thorough review! The main finding holds up. Several header shadow nodes overwrite their frame origin from state inside
Plan. I'll go with (b):
I'll also move Upstream. The core fix is filed as react/react-native#59101 (issue react/react-native#59100). If it lands, header origins go stale in exactly the same way, whether or not this PR merges, because screens' For context: Bluesky ships this patch, but we don't use native header items and hide native headers almost everywhere, so we haven't hit it in production. It is a real regression for apps that use header items, though. |
Add scenarios that measure() header items (v4 stack, and v5 stack on iOS) after changes that move them without resizing them, and a long list in a ScreenContainer, which keeps an Android Screen in the "frame unknown" branch of its descriptor. Animate the color with Reanimated: from React, only every 10th frame changed the color, and when a child re-renders React re-appends all children of its parent, which dirties the parent anyway. Stack screens are now positioned absolutely, like ScreenStackItem does, so measured positions are comparable with the native ones. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…mixin Replace the 8 copies of `setSizeIfChanged` with `rnscreens::SizeFromStateShadowNode`, a mixin that goes between a shadow node and its `ConcreteViewShadowNode` base, since it needs the protected Yoga node. It replaces `utils/YogaSizeUtil.h` and only includes `<yoga/style/Style.h>` instead of all of `YogaLayoutableShadowNode.h`. Also drop the `YogaLayoutableShadowNode` RTTI asserts left behind in the descriptors that no longer cast to it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Several header shadow nodes overwrite their frame origin with one from native state in `layout()`, which Fabric only calls when Yoga has new layout for the node. The unconditional `setSize` in `adopt()` used to dirty the header config on every clone, which kept those origins current. With the size guard, a state update that only moves a header node (or, on Android, only changes the toolbar height) left the tree clean, so `measure()` and Pressability's press-rect checks read a stale origin. Each affected node now exposes `correctFrameOrigin()`, which `layout()` and the descriptor share, and its descriptor marks the layout dirty in `adopt()` when that origin differs from the one in the node's layout metrics. Clones inherit the corrected metrics, so a clone whose origin didn't change stays clean: - v4 `RNSScreenStackHeaderSubview` (`contentOffset`), without touching its Yoga style (see software-mansion#2905) - v4 `RNSScreenStackHeaderConfig` (iOS: `frameOrigin`, Android: Y origin from the toolbar height, except for translucent headers) - v5 `RNSStackHeaderConfig` and `RNSStackHeaderItem` on iOS (`contentOffset`) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Note This comment was written by an LLM (Claude Code, working with @mozzius). The results below come from an iPhone 17 simulator (iOS 27.1) and a Pixel 9 emulator (Android 16), not from a physical device. There are three new commits on top of the previous one, and the description now has the results. Going through the inline comments:
With these changes, scrolling and per-frame Reanimated commits still don't run root layout (iOS: 0 |
Description
Several component descriptors call
YogaLayoutableShadowNode::setSize()fromadopt(), with a size taken from native state.adopt()runs on every clone. UnlikesetPadding(),setSize()marks the Yoga node dirty even when the size hasn't changed.Every commit clones the path from the changed node up to the root. When a parent is cloned with new children, React Native also re-clones the children it no longer owns, such as a stack's header config next to the screen content. Screens sit near the root of almost every tree, so every commit under them dirtied the root.
RootShadowNode::layoutIfNeededthen ranlayoutTreeplus Yoga'sroundLayoutResultsToPixelGrid, which walks the whole tree. That includes commits that change nothing layout-related:translateYdriven by scroll)contentOffsetstate updates, on every scroll event (JS thread)There are two paths:
setSize(frameSize)on every clone.Screen'sadopt()resets the size to undefined while native hasn't reported a frame.Screen.onLayoutonly reports frames for screens whose container is aScreenStack. So screens in aScreenContainer, e.g. JS bottom tabs, stay in that branch forever and are dirtied on every clone too.The unconditional dirty was also load-bearing for header origins. Several header shadow nodes overwrite their frame origin with one from native state in
layout(), and Fabric only callslayout()on a node that has new layout from Yoga. The header config used to be dirtied on every clone, so those origins stayed current. With only the size guard, a state update that just moves a header node (or, on Android, just changes the toolbar height) leaves the tree clean, andmeasure()and Pressability's press-rect checks read a stale origin. This PR therefore also dirties those nodes when, and only when, the origin from state changes.Closes #4777. That issue was reported for Android only, but iOS is affected as well.
Changes
rnscreens::SizeFromStateShadowNode(common/cpp/.../rnscreens/utils/LayoutFromState.h): a mixin that goes between a shadow node and itsConcreteViewShadowNodebase, e.g.SizeFromStateShadowNode<ConcreteViewShadowNode<...>>. It providessetSizeIfChanged(Size), which only callssetSizewhen the node's Yoga style doesn't already have that width and height. It needs the protected Yoga node, hence a mixin rather than a free function. It only includes<yoga/style/Style.h>. The descriptors that set a size from state inadopt()now call it instead ofsetSize():legacy/):Screen(both branches, including the Android undefined reset),ModalScreen,ScreenStackHeaderConfigStackScreen(Android),SplitScreen,FormSheetHost,TabsBottomAccessory,StackHeaderConfigcorrectFrameOrigin(Point)method that both itslayout()and its descriptor use.rnscreens::dirtyLayoutIfFrameOriginStale()(same header) marks the layout dirty inadopt()when that origin differs from the one in the node's layout metrics. Clones inherit the corrected metrics, so an unchanged clone stays clean. The corrections are plain assignments, so the comparison is exact. Hidden (display: none) nodes are skipped, sincelayout()isn't called for them.ScreenStackHeaderSubview:contentOffset. The descriptor still never callssetSize(see fix(Fabric,iOS): header subviews do not support dynamic content changes #2905): marking the layout dirty doesn't touch the Yoga style.ScreenStackHeaderConfig:frameOriginon iOS, Y origin from the toolbar height on Android. The Androidtranslucentearly return moved intocorrectFrameOrigin, so a translucent header never dirties.StackHeaderConfigandStackHeaderItemon iOS:contentOffset.Screenheader-height correction (+=), whose inputs only change together with the Screen's padding or size, which already dirty it; and the constantx = 0corrections inTabsBottomAccessoryand v5 AndroidStackHeaderSubview.StyleSizeLength::points(NaN)isundefined()and undefined values compare equal, so only the first reset dirties.RNSScreenShadowNodeCommitHook's orientation-change reset still callssetSizedirectly.YogaLayoutableShadowNodeRTTI asserts left in descriptors that no longer cast to it.Test4777(issue tests) has four scenarios:ScreenContainer), each with an "Animate color" toggle driven by ReanimatedheaderLeft/headerRightitems whosemeasure()is shown on screen and logged, with toggles for the title, the back title, the back button and an extra right itemTest plan
Test4777, performance:RootShadowNode::layout/roundLayoutResultsToPixelGridruns for each of those commits. After it, it doesn't.The color used to be animated from React state. That didn't test this: when a child re-renders, React re-appends all children of its parent, and
YogaLayoutableShadowNode::appendChildalways dirties the parent.Test4777, header item positions:measure()result (also logged with a[Test4777]prefix). Compare it with where the item is drawn, e.g. its accessibility frame.Results
Debug builds of
FabricExample: iPhone 17 simulator (iOS 27.1) and a Pixel 9 emulator (Android 16, arm64). Positions are in points / dp, native positions come from the accessibility tree.Header item positions (
measure()pageX, vs. where the item is drawn):mainheaderRightafter "Add extra right item"headerLeftafter "Hide back button"headerRightafter "Add extra right item"With the previous commit, a tap with two ~1.5pt moves on the moved
headerRightdidn't fireonPresson either platform (a plain tap did). With this PR it does.One thing that isn't related to this PR: on iOS 27, when the back button title collapses or the back button is hidden, UIKit moves
headerLeftwithout native sending a new subview position, someasure()is stale onmaintoo.Root layout during commits that change no layout:
mainlayoutTree/ commits)ScreenContainer), scrolllayout()/ clones, per screen)ScreenContainer), scrolliOS counts are lldb breakpoint hits on
YogaLayoutableShadowNode::layoutTreeandRootShadowNode::layoutIfNeeded. The 4 remaining layouts came from React commits that re-append children (appendChilddirties the parent). On Android, breakpoints over adb were too slow to keep the app responsive, so these counts come from a temporary log line inRNSScreenShadowNode::layout()andRNSScreenComponentDescriptor::adopt().Measured in the Bluesky app, which carries the size guard as a patch on 4.26.2 (bluesky-social/social-app#12000). Each run is a fixed 20-swipe scroll, with baseline and patched builds alternating:
debugOptimized)On the feed, the Android frame times didn't measurably change, because that screen is bound by drawing on the UI thread.
Before this change, roughly 90% of the layout cost was pixel-grid rounding; the actual layout work was small. In patched builds, the only layout left while scrolling comes from React commits that mount new rows.
Builds and checks:
FabricExamplebuilds for the iOS simulator and Android (arm64). On Android,common/cppcompiles with-Wall -Wpedantic -Werror(the app'sreact_codegen_rnscreenstarget). clang-format, Prettier, ESLint andyarn check-typespass. I haven't run the Detox e2e or SPM jobs, the tvOS build, or the regression test screens listed in the review, and I haven't tried a physical device.Not covered by a scenario: an origin-only change of the v4 iOS header config (e.g. hiding the status bar with a translucent header on a device without a notch), and an Android toolbar height change at the same width. Both go through the same check as the cases above.
React Native's own
ModalHostViewComponentDescriptorandInputAccessoryComponentDescriptorhave the same pattern (setSize+setPositionTypeinadopt()). That needs a fix in React Native itself: react/react-native#59101. If it lands, the header origin part of this PR is still needed.Checklist
🤖 Generated with Claude Code