Tomek Zawadzki
05bb19dd3f
Fixes the fatal Android crash "IllegalStateException: Required value was null" in ReactViewGroup.updateSubviewClipStatus (Sentry APP-T20Q, 14 users in 14 days on 1.128.0/1.129.0). Reentrant child removal during a clipping pass can leave a null slot in allChildren below allChildrenCount; the checkNotNull on that slot then kills the app during animated scrolls. A null slot means the view is already detached, so skip it and count it as clipped instead of throwing. Unfixed upstream as of July 2026; the sibling fix attempt facebook/react-native#57365 was abandoned. Verified against a deterministic state-injection repro: unpatched RN crashes with the exact Sentry stack (ReactViewGroup.kt:469/435/411), patched RN survives with the app fully functional. Note: takes effect on Android only when react-android is built from source; see the caveat in the patch md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
98 lines
6.0 KiB
Markdown
98 lines
6.0 KiB
Markdown
# ***This second part of this patch is load bearing, do not remove.***
|
|
|
|
## RefreshControl Patch - iOS 17.4 Haptic Regression
|
|
|
|
Patching `RCTRefreshControl.mm` temporarily to play an impact haptic on refresh when using iOS 17.4 or higher. Since
|
|
17.4, there has been a regression somewhere causing haptics to not play on iOS on refresh. Should monitor for an update
|
|
in the RN repo: https://github.com/facebook/react-native/issues/43388
|
|
|
|
## RefreshControl Path - ScrollForwarder
|
|
|
|
Patching `RCTRefreshControl.m` and `RCTRefreshControl.h` to add a new `forwarderBeginRefreshing` method to the class.
|
|
This method is used by `ExpoScrollForwarder` to initiate a refresh of the underlying `UIScrollView` from inside that
|
|
module.
|
|
|
|
## RCTPullToRefreshViewComponentView.mm Patch - RefreshControl initial props dropped on New Arch
|
|
|
|
**TODO: Remove after bumping React Native to 0.82+** (fixed upstream by facebook/react-native#52615, #52584
|
|
and #53231).
|
|
|
|
On Fabric, `updateProps` diffs against `_props`, but the initial-layout replay in `layoutSubviews` passes
|
|
`_props` as the new props too, so the diff is a no-op and `tintColor`/`progressViewOffset`/`title` are never
|
|
applied on mount. This hides the pull-to-refresh spinner behind the floating home header (it stays at offset
|
|
0 instead of `headerOffset`). We diff against the `oldProps` argument instead, null-guarded with default
|
|
props for the create-mutation path.
|
|
|
|
Issue: https://github.com/facebook/react-native/issues/56343
|
|
|
|
## RCTEnhancedScrollView.mm / RCTScrollViewComponentView.mm Patch - centerContent insets stale after content resize on New Arch
|
|
|
|
**TODO: Remove after bumping React Native to 0.87+** (fixed upstream by facebook/react-native#56832,
|
|
commit d50c1b5207; first shipped in 0.87.0-rc.0).
|
|
|
|
On Fabric, `centerContent` centers by computing `contentInset` in `centerContentIfNeeded`, but that
|
|
recompute only ran on `setFrame`/`didAddSubview`/`scrollViewDidZoom` - not when a state update assigns a
|
|
new `contentSize` in `updateState`. Any content that resizes after mount inside a `centerContent`
|
|
ScrollView (e.g. the lightbox image crop view getting its real aspect ratio from `onLoad` when the embed
|
|
has no aspectRatio metadata) keeps the old insets: content rests off-center and the excess inset creates
|
|
phantom scroll range, so the image can be dragged and parked off-center and the native scroll steals the
|
|
swipe-down-to-dismiss pan. The old architecture paired every `contentSize` update with re-centering in
|
|
`RCTScrollView.updateContentSizeIfNeeded`; Fabric dropped that link.
|
|
|
|
Backport of the upstream fix: `setContentSize:`/`setCenterContent:` overrides on `RCTEnhancedScrollView`
|
|
that call `centerContentIfNeeded`, plus the `updateProps` guards so the `contentInset` prop does not
|
|
fight the computed centering inset.
|
|
|
|
Issue: https://github.com/facebook/react-native/issues/55090
|
|
|
|
## RCTScrollViewComponentView.mm Patch - ScrollView pinch/pan ignored outside content area on New Arch
|
|
|
|
**TODO: Remove after bumping React Native to 0.87+** (fixed upstream by facebook/react-native#56747,
|
|
commit efcab20908; first shipped in 0.87.0-rc.0).
|
|
|
|
On Fabric, `betterHitTest` in `RCTScrollViewComponentView` deliberately skips the `_containerView`
|
|
and hit-tests its grandchildren, returning `self` (the wrapper component view) when the touch lands
|
|
inside the scroll view bounds but outside any content. UIKit only delivers touches to a gesture
|
|
recognizer when the hit view is the recognizer's view or a descendant of it, and the `UIScrollView`
|
|
is a *child* of the wrapper - so its native pinch/pan recognizers never see those touches. In the
|
|
lightbox this means pinch-to-zoom and pan-while-zoomed only respond when the fingers are over the
|
|
image itself, not over the letterbox bars above/below it. On the old architecture, default UIKit
|
|
hit-testing returns the `UIScrollView` itself for those touches, so everything works.
|
|
|
|
Backport of the upstream one-liner: return `_scrollView` instead of `self` so touches in the
|
|
content-less area are attributed to the `UIScrollView`.
|
|
|
|
Issue: https://github.com/facebook/react-native/issues/54123
|
|
PR: https://github.com/react/react-native/pull/56747
|
|
|
|
## ReactViewGroup.kt Patch - Fatal "Required value was null" during subview clipping on Android
|
|
|
|
Fixes Sentry issue APP-T20Q: `IllegalStateException: Required value was null` thrown by
|
|
`checkNotNull(allChildren?.get(idx))` in `updateSubviewClipStatus`, reached from
|
|
`ReactScrollView.onScrollChanged -> updateClippingRect` during an animated smooth scroll
|
|
(New Architecture, `removeClippedSubviews`).
|
|
|
|
The clipping loop in `updateClippingToRect` captures its bound once, but clipping a view
|
|
(`removeViewsInLayout`) can synchronously trigger reentrant child removal (layout-change
|
|
listeners, animation-end callbacks, Fabric mounting on the UI thread), which compacts
|
|
`allChildren` and nulls the tail mid-loop. Upstream already catches the
|
|
`IndexOutOfBoundsException` variant of this corruption with diagnostics, but the null-child
|
|
variant throws `IllegalStateException` and escapes as a fatal crash. A null entry means the
|
|
view is already detached, so we skip it and count it as clipped to keep index math aligned.
|
|
|
|
Not fixed upstream as of July 2026 (identical `checkNotNull` on `main`); the sibling fix
|
|
attempt facebook/react-native#57365 for the same bookkeeping corruption (different stack)
|
|
was abandoned. Re-check when bumping React Native.
|
|
|
|
**Caveat: this hunk has no effect on the shipped app today.** The Android build consumes the
|
|
prebuilt `com.facebook.react:react-android` artifact from Maven Central, not the Kotlin
|
|
sources in node_modules (unlike the iOS hunks above, which CocoaPods compiles from source).
|
|
It only takes effect if we enable building React Native Android from source
|
|
(`includeBuild` + dependency substitution in `android/settings.gradle`), or once an
|
|
equivalent fix ships upstream. Kept so the fix is ready and documented either way.
|
|
|
|
## RCTTextLayoutManager.mm Patch - Text overflows instead of wrapping on the last line
|
|
|
|
Issue: https://github.com/react/react-native/issues/53450#issuecomment-3298157830
|
|
Bandaid fix taken from: https://github.com/react/react-native/commit/581d643a9e59fd88f93757f80194e1efd11bd0e5
|