From fd299509df0c1c6991faecc165c64bd9862dda6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Wed, 8 May 2024 12:36:06 -0700 Subject: [PATCH] Implement potential fix for mounting errors during synchronous state updates (v2) Summary: Changelog: [internal] This is a new attempt at fixing mounting errors during synchronous state updates after what we tried in https://github.com/facebook/react-native/pull/44015. That fix didn't work because `dispatchMountItems` actually makes a copy of the mount items that it's going to process, so when we added the mount items to the list they were actually not being picked up by the current processing. This changes the fix to call `dispatchMountItems` as many times as needed, while there are mount items to process in the list. Reviewed By: sammy-SC Differential Revision: D57107212 fbshipit-source-id: 46988a71daae15d70399258f850653046d0790ff --- .../fabric/mounting/MountItemDispatcher.java | 73 +++++++++++-------- 1 file changed, 43 insertions(+), 30 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java index 67e28885cea..a158825274a 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/MountItemDispatcher.java @@ -105,40 +105,53 @@ public class MountItemDispatcher { if (ReactNativeFeatureFlags.forceBatchingMountItemsOnAndroid()) { mInDispatch = true; - } - final boolean didDispatchItems; - try { - didDispatchItems = dispatchMountItems(); - } catch (Throwable e) { - mReDispatchCounter = 0; - throw e; - } finally { - // Clean up after running dispatchMountItems - even if an exception was thrown - mInDispatch = false; - } - - // We call didDispatchMountItems regardless of whether we actually dispatched anything, since - // NativeAnimatedModule relies on this for executing any animations that may have been scheduled - mItemDispatchListener.didDispatchMountItems(); - - // Decide if we want to try reentering - if (mReDispatchCounter < 10 && didDispatchItems) { - // Executing twice in a row is normal. Only log after that point. - if (mReDispatchCounter > 2) { - ReactSoftExceptionLogger.logSoftException( - TAG, - new ReactNoCrashSoftException( - "Re-dispatched " - + mReDispatchCounter - + " times. This indicates setState (?) is likely being called too many times" - + " during mounting.")); + try { + boolean didDispatchItems = true; + // Dispatch as many mount items as we find. Some mount items might + // trigger state updates that trigger more mount items. This will + // process them correctly. + while (didDispatchItems) { + didDispatchItems = dispatchMountItems(); + } + } finally { + mInDispatch = false; + } + } else { + final boolean didDispatchItems; + try { + didDispatchItems = dispatchMountItems(); + } catch (Throwable e) { + mReDispatchCounter = 0; + throw e; + } finally { + // Clean up after running dispatchMountItems - even if an exception was thrown + mInDispatch = false; } - mReDispatchCounter++; - tryDispatchMountItems(); + // We call didDispatchMountItems regardless of whether we actually dispatched anything, since + // NativeAnimatedModule relies on this for executing any animations that may have been + // scheduled + mItemDispatchListener.didDispatchMountItems(); + + // Decide if we want to try reentering + if (mReDispatchCounter < 10 && didDispatchItems) { + // Executing twice in a row is normal. Only log after that point. + if (mReDispatchCounter > 2) { + ReactSoftExceptionLogger.logSoftException( + TAG, + new ReactNoCrashSoftException( + "Re-dispatched " + + mReDispatchCounter + + " times. This indicates setState (?) is likely being called too many times" + + " during mounting.")); + } + + mReDispatchCounter++; + tryDispatchMountItems(); + } + mReDispatchCounter = 0; } - mReDispatchCounter = 0; } @UiThread