Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GiftedChat not rendering on Android because of race condition #2503

Open
habovh opened this issue Jun 19, 2024 · 3 comments
Open

GiftedChat not rendering on Android because of race condition #2503

habovh opened this issue Jun 19, 2024 · 3 comments

Comments

@habovh
Copy link

habovh commented Jun 19, 2024

I'm encountering an issue where GiftedChat would not render on Android. I figured some kind of race condition prevents the isInitialized state value to be set to true in the onInitialLayoutViewLayout function.

I believe that there are concurrent setState calls that may re-introduce stale state values in the state because of race conditions. In my testing, the culprit seems to be the setState call in the useEffect() callback. Since the loading view's onLayout function is only called once, and the effect firing once before and multiple times after the layout, this leads to the isInitialized value to remain false —until the layout changes of course but in most cases it doesn't.

Here's a patch that extracts the isInitialized flag from the "main" state to its own separate useState hook. This effectively fixes the issue and is quite clean IMO, for anyone encountering the same issue.

diff --git a/node_modules/react-native-gifted-chat/lib/GiftedChat.js b/node_modules/react-native-gifted-chat/lib/GiftedChat.js
index 9951cd8..c629d68 100644
--- a/node_modules/react-native-gifted-chat/lib/GiftedChat.js
+++ b/node_modules/react-native-gifted-chat/lib/GiftedChat.js
@@ -37,8 +37,8 @@ function GiftedChat(props) {
     const isFirstLayoutRef = useRef(true);
     const actionSheetRef = useRef(null);
     let _isTextInputWasFocused = false;
+    const [isInitialized, setIsInitialized] = useState(false)
     const [state, setState] = useState({
-        isInitialized: false,
         composerHeight: minComposerHeight,
         messagesContainerHeight: undefined,
         typingDisabled: false,
@@ -279,9 +279,9 @@ function GiftedChat(props) {
         notifyInputTextReset();
         maxHeightRef.current = layout.height;
         const newMessagesContainerHeight = getMessagesContainerHeightWithKeyboard(minComposerHeight);
+        setIsInitialized(true)
         setState({
             ...state,
-            isInitialized: true,
             text: getTextFromProp(initialText),
             composerHeight: minComposerHeight,
             messagesContainerHeight: newMessagesContainerHeight,
@@ -339,7 +339,7 @@ function GiftedChat(props) {
         actionSheet: actionSheet || (() => { var _a; return (_a = actionSheetRef.current) === null || _a === void 0 ? void 0 : _a.getContext(); }),
         getLocale: () => locale,
     }), [actionSheet, locale]);
-    if (state.isInitialized === true) {
+    if (isInitialized === true) {
         return (<GiftedChatContext.Provider value={contextValues}>
         <View testID={TEST_ID.WRAPPER} style={styles.wrapper}>
           <ActionSheetProvider ref={actionSheetRef}>

I feel like using a single "do-it-all" state is not optimal, requires to always spread any unchanged values and may lead to surprises like this if not being careful. I understand it can also be convenient to be able to update multiple state values in a single call, but I'd then only group values that are closely linked to each other or often updated together. I'd like your feedback on this though, as if the consensus leans towards splitting the state, I'm more than happy to open a PR with individual useState hooks for the various values stored there.

@levepic
Copy link

levepic commented Jun 26, 2024

I had an issue only on emulator that GiftedChat didnt render at all, but only for the first view. For any consquent views (opening the screen) it did work. I tried your solution, but with it, Gifted never renders, not for later views neither. It turned out the error was caused for me because of my "preload" feature (chat is opened with the first message in chatpartners list and the load is initiated immediately when you click on the partner, not when the chatscreen renders). For mey a key attribute for giftedchat that changes with preload fixed the issue (key 1 for preload, key 2 when fully loaded)

@habovh
Copy link
Author

habovh commented Jun 26, 2024

@levepic I believe you were actually benefiting from the multiple state updates that happened internally. Now that the initialised value only changes once when the screen is laid out, you cannot rely on the previous "bug" to render the chat.
From a conceptual standpoint, the previous approach did not always update the isInitialized state value on purpose, while my approach does.

Did using using the extraData prop allow you to render properly using my suggested changes? The key prop should not have any effect on the rendering —except if you're rendering multiple <GiftedChat/> components using Array.map() for example.

@mariomurrent-softwaresolutions

Which version are you talking about?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants