Repository navigation
fix(mobile): long turns no longer freeze the app - #15780
nekohasekai wants to merge 1 commit into
Conversation
Reanimated's DISABLE_COMMIT_PAUSING_MECHANISM, set for keyboard-controller, lets per-frame animation commits overtake a React commit that takes longer than a frame, and React Native retries that commit forever. Reanimated documents the flag as safe only with React Native's preventShadowTreeCommitExhaustion, which the prebuilt core ships off. A local Expo module now turns it on at launch on iOS and Android. Fixes pingdotgg#15641
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — This PR adds a new cross-platform native module that force-enables a React Native commit-scheduling flag for every mobile app launch. Because it changes global runtime behavior and the effective default of the rendering pipeline, the change should receive human review. You can add or adjust custom eligibility rules. Learn more. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (10)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughA new Expo native module overrides React Native’s ChangesNative feature-flag override
Priority: ⬆️ High Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: High Sequence Diagram(s)sequenceDiagram
participant IOSAppLaunch
participant T3ReactNativeFlagsAppDelegateSubscriber
participant T3ReactNativeFeatureFlags
participant AndroidAppLaunch
participant T3ReactNativeFlagsPackage
participant FeatureFlagOverrides
participant ReactNative
IOSAppLaunch->>T3ReactNativeFlagsAppDelegateSubscriber: didFinishLaunchingWithOptions
T3ReactNativeFlagsAppDelegateSubscriber->>T3ReactNativeFeatureFlags: applyOverrides
T3ReactNativeFeatureFlags->>ReactNative: set preventShadowTreeCommitExhaustion to true
AndroidAppLaunch->>T3ReactNativeFlagsPackage: create lifecycle listeners
T3ReactNativeFlagsPackage->>FeatureFlagOverrides: register listener
FeatureFlagOverrides->>ReactNative: set preventShadowTreeCommitExhaustion to true during onCreate
Merge Risk: ⚪ Minimal · up to No identified issue remains that would prevent merging after normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change enables a fixed rendering safeguard rather than exposing a new user-controlled capability. Its main risk is dependence on native startup ordering and matching React Native defaults. Those assumptions are checked during development, but their behavior across all production startup and recovery paths remains unverified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. (7 skipped: 7 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Problem
Opening a long thread while its turn is running freezes the mobile app. The thread never renders. While the turn runs, the "Working" row's shimmer makes Reanimated commit the shadow tree on every frame, so a React commit whose layout takes longer than a frame keeps failing React Native's revision check and starts over. Debug builds abort after 1,024 attempts; Release builds keep retrying.
Reproduce with the harness from the issue: start a turn with 30 units of history followed by 60 s of streaming, then open the thread about 8 s in.
Fixes #15641.
Change
#5451 turned off Reanimated's commit pausing (
DISABLE_COMMIT_PAUSING_MECHANISM) for keyboard-controller. Reanimated documents that flag as safe only together with React Native'spreventShadowTreeCommitExhaustion: with it, React Native takes a lock after three failed attempts and the commit lands. React Native 0.88 ships that flag off, both in the prebuilt iOS core and in the Android artifacts. Reanimated's suggested way to turn it on is to patch React Native and build it from source.Instead, a local Expo module,
t3-react-native-flags, turns the flag on at launch on iOS and Android. The React Native factory installs the stable release level's flags first. The module then swaps in the same set with this one flag on, before React Native starts, using React Native'sdangerouslyForceOverride. Debug builds assert that nothing readpreventShadowTreeCommitExhaustionbefore the swap. If an Expo template change ever starts React Native earlier, the debug app fails at launch instead of silently racing. On Android debug builds, Expo's dev launcher creates the React host before the module runs, and that reads two other flags; they keep their values because the module changes only this one.Why keep the Reanimated flag. The thread feed still renders keyboard-controller's
KeyboardChatScrollViewthrough LegendList'sKeyboardAwareLegendList, and the composer sits in aKeyboardStickyView. I built the variant with commit pausing back on to check.docs/internals/mobile-development.mdrecords why the two flags go together and when the module can be removed.Scope and approval
The triage of #15641 confirmed the starvation and listed enabling
preventShadowTreeCommitExhaustionas the first fix option. It expected that to need React Native built from source; the forced override keeps iOS on the prebuilt core. It also asked for a keyboard recording before turning commit pausing back on; that comparison is above.This PR fixes only the commit starvation. The markdown measurement cost tracked in #14010 still makes a long thread's first render slow. The change is mobile only. It adds native code, so it changes the native fingerprint and reaches users with the next store build, not an OTA update. The fingerprint check can't label a fork PR, so it has no
📱 Native Changelabel.Verification
All runs used an iPhone 17 Pro Simulator on iOS 26.5, with the Debug dev client serving production JavaScript. The isolated backend held only synthetic data, plus the fake streaming provider from the issue. main is
4ee6bfd50e. The branch is rebased ontoefecd3cf8b, which adds four commits that don't touch these dependencies or files.Long-turn scenario, with the app sampled every 5 s:
ShadowTree::commitattempts < 1024(at 80.6 s and about 77 s)An earlier run, made with a build that differed only in the debug check, also finished without a crash. In one other earlier run, the thread took about 45 s to appear while the main thread mounted its views. That is the rendering cost #14010 tracks, not commit retries.
Long-turn video, main vs. this fix (48 s, 2x) · Keyboard video, main / this fix / commit pausing on (4x slow motion) · Sampling tables and measurements
Also checked:
preventShadowTreeCommitExhaustion()as1.enableBridgelessArchitectureandperfMonitorV2Enabled. That is why the check now looks only at the overridden flag.vp fmtis clean.Not tested: Release builds, physical devices, iPad, and the keyboard on screens other than the thread (new task, terminal). I couldn't read the flag from the running Android app, because JDWP disconnected.
Evidence is uploaded as GitHub release attachments on the contributor fork, not committed to the repository.
Model: Claude Opus 5.5. Harness: Claude Code in T3 Code.