1

I've been going at it for 2 days and cannot figure it out :(

I have a tree-like conversation as you can see in the screenshot. When a person types something in an empty input field, the Message is then added to the reducer array "conversationsData.messages". When that happens, the Replies component of each Message is listening to changes to only the ~3 replies of that message. If the replies change, then the Replies should rerender. Buuut... The problem is that every single Reply component, and thus every single message is being re-rendered which is causing lag.

Can you please help me get the memo to work properly?

enter image description here

ConversationManager/Conversation/Message.tsx

import React, { FunctionComponent, ReactElement, useRef, useState, useEffect, useMemo } from 'react'
import { useDispatch, useStore, useSelector } from 'react-redux'
import Autosuggest, { OnSuggestionSelected, ChangeEvent } from 'react-autosuggest'
import colors from '@common/colors'
import { updateMessage, removeMessage } from '@reducers/conversationsData'
import usePrevious from 'react-hooks-use-previous'
import MessageInterface, { MessageInitState } from '@interfaces/message'
import { RootState } from '@reducers/rootReducer'
import NewReply from './NewReply'
import { StyleSheet } from '@interfaces/common'

interface IMessageProps {
  origMessage: MessageInterface,
  isSubClone: boolean,
  firstRender: boolean, // It's firstRender=true if we're rendering the message for the first time, "false" if it's a dynamic render
  isStarter?: boolean
}

const MessageFunc = ({ origMessage, isSubClone, firstRender }: IMessageProps): ReactElement | null => {
  if(!origMessage.id){
    return null
  }

  const dispatch = useDispatch()
  const store = useStore()
  const state: RootState = store.getState()

  const [inputSuggestions, setInputSuggestions] = useState<MessageInterface[]>([])
  const [inputWidth, setInputWidth] = useState(0)
  const $invisibleInput = useRef<HTMLInputElement>(null)
  const isFirstRun = useRef(true)
  const [localMessage, setLocalMessage] = useState<MessageInterface>(MessageInitState)
  const previousLocalMessage = usePrevious<MessageInterface>(localMessage, MessageInitState)

  useEffect(() => {
    isFirstRun.current = true
    setLocalMessage(origMessage)
  }, [origMessage])

  useEffect(() => {
    if(!localMessage.id) return

    if(isFirstRun.current == true){
      setupInputWidth()
      isFirstRun.current = false
    }

    if(previousLocalMessage.text != localMessage.text){
      setupInputWidth()
    }
    if(previousLocalMessage.cloneId != localMessage.cloneId){
      setupIfMessageClone()
    }
  }, [localMessage])


  const characterMessages = state.conversationsData.messages.filter((m) => {
    return m.characterId == origMessage.characterId
  })
  const parent: MessageInterface = characterMessages.find((m) => {
    return m.id == origMessage.parentId
  }) || MessageInitState


  const setupIfMessageClone = () => { // This function is only relevant if this message is a clone of another one
    if(!localMessage.cloneId) return

    const cloneOf = characterMessages.find((m) => {
      return m.id == localMessage.cloneId
    }) || MessageInitState

    setLocalMessage({
      ...localMessage,
      text: cloneOf.text
    })
  }

  const setupInputWidth = () => {
    let width = $invisibleInput.current ? $invisibleInput.current.offsetWidth : 0
    width = width + 30 // Let's make the input width a bit bigger
    setInputWidth(width)
  }

  const _onFocus = () => {
    // if(!localMessage.text){ // No message text, create a new one
    //   dispatch(updateMessage(localMessage))
    // }
  }

  const _onBlur = () => {
    if(localMessage.text){
      dispatch(updateMessage(localMessage))
    }

    // No message, delete it from reducer
    else {
      dispatch(removeMessage(localMessage))
    }
  }

  const _onChange = (event: React.FormEvent, { newValue }: ChangeEvent): void => {
    setLocalMessage({
      ...localMessage,
      cloneId: '',
      text: newValue
    })
  }

  const _suggestionSelected: OnSuggestionSelected<MessageInterface> = (event, { suggestion }) => {
    setLocalMessage({
      ...localMessage,
      cloneId: suggestion.id
    })
  }

  const getSuggestions = (value: string): MessageInterface[] => {
    const inputVal = value.trim().toLowerCase()
    const inputLen = inputVal.length

    return inputLen === 0 ? [] : characterMessages.filter(message =>
      message.text.toLowerCase().slice(0, inputLen) === inputVal
    )
  }

  if(!localMessage.id){
    return null
  }
  else {
    return (
      <>
        <li>
          <div>
            <Autosuggest
              suggestions={inputSuggestions}
              onSuggestionsFetchRequested={({ value }) => setInputSuggestions(getSuggestions(value))}
              onSuggestionsClearRequested={() => setInputSuggestions([])}
              getSuggestionValue={(suggestion) => suggestion.text}
              onSuggestionSelected={_suggestionSelected}
              renderSuggestion={(suggestion) => (
                <div>
                  {suggestion.text}
                </div>
              )}
              theme={{ ...autoSuggestTheme, input: {
                ...styles.input,
                width: inputWidth,
                borderBottomColor: localMessage.cloneId ? colors.purple : 'default',
                borderBottomWidth: localMessage.cloneId ? 2 : 1
              } }}
              inputProps={{
                value: localMessage.text,
                onChange: _onChange,
                onBlur: _onBlur,
                onFocus: _onFocus,
                className: 'form-control',
                disabled: isSubClone
              }}
            />
            <a href="#"></a>
            <span style={styles.invisibleSpan} ref={$invisibleInput}>{localMessage.text}</span>
          </div>
          <ul className="layer">
            <Replies parentMessage={localMessage} isSubClone={isSubClone} />
          </ul>
        </li>
      </>
    )
  }
}

const Message = React.memo(MessageFunc)
// const Message = MessageFunc

interface IRepliesProps {
  parentMessage: MessageInterface,
  isSubClone: boolean
}

const RepliesFunc: FunctionComponent<IRepliesProps> = ({
  parentMessage, isSubClone
}: IRepliesProps): ReactElement | null => {
  const previousParentMessage = usePrevious<MessageInterface>(parentMessage, MessageInitState)
  const isFirstRun = useRef(true)

  const replies: MessageInterface[] = useSelector((state: RootState) => state.conversationsData.messages.filter((m) => {
    // If parent is regular message
    if(!parentMessage.cloneId){
      return m.parentId == parentMessage.id && m.characterId == parentMessage.characterId
    }

    // If parent is a clone, then replies need to come from the main clone
    // else {
    //   return m.parentId == parentMessage.cloneId
    // }
  }))

  if(replies.length){
    return (
      <>
        {console.log('rendering Replies...')}
        {replies.map((reply) => {
          return (
            <Message
              origMessage={reply}
              key={reply.id}
              isSubClone={parentMessage.cloneId ? true : isSubClone}
              firstRender={true}
            />
          )
        })}
        {parentMessage.text && !parentMessage.cloneId && !isSubClone && (
          <NewReply
            parentMessage={parentMessage}
          />
        )}
      </>
    )
  }
  else {
    return null
  }
}

// const Replies = React.memo(RepliesFunc)
const Replies = RepliesFunc

export default Message

const styles: StyleSheet = {
  input: {
    width: 0,
    padding: 0,
    paddingLeft: 10,
    lineHeight: 25,
    height: 25,
    fontSize: 11,
    boxShadow: 'none',
    minWidth: 22
  },
  clone: {
    borderBottomWidth: 2,
    borderBottomColor: colors.purple
  },
  invisibleSpan: { // This is used for getting text width of input (for dynamic resizing of input fields)
    opacity: 0,
    position: 'absolute',
    left: -9999,
    top: -9999,
    fontSize: 11
  }
}

const autoSuggestTheme: StyleSheet = {
  container: {
    position: 'relative'
  },
  inputOpen: {
    borderBottomLeftRadius: 0,
    borderBottomRightRadius: 0
  },
  suggestionsContainer: {
    display: 'none'
  },
  suggestionsContainerOpen: {
    display: 'block',
    position: 'absolute',
    top: 25,
    width: '100%',
    minWidth: 400,
    border: '1px solid #aaa',
    backgroundColor: '#fff',
    fontWeight: 300,
    fontSize: 11,
    borderBottomLeftRadius: 4,
    borderBottomRightRadius: 4,
    zIndex: 2
  },
  suggestionsList: {
    margin: 0,
    padding: 0,
    listStyleType: 'none'
  },
  suggestion: {
    cursor: 'pointer',
    padding: '5px 10px'
  },
  suggestionHighlighted: {
    backgroundColor: '#ddd'
  }
}

reducers/ConversationsData.ts

import { createSlice, PayloadAction } from '@reduxjs/toolkit'
import MessageInterface from '@interfaces/message'
import axios, { AxiosRequestConfig } from 'axios'
import conversationsDataJSON from '@data/conversationsData.json'
import { AppThunk } from '@reducers/store'
import _ from 'lodash'

interface IInitialState {
  loaded: boolean,
  messages: MessageInterface[]
}

export const initialState: IInitialState = {
  loaded: false,
  messages: []
}

export const charactersDataSlice = createSlice({
  name: 'conversationsData',
  initialState,
  reducers: {
    loadData: (state, action: PayloadAction<MessageInterface[]>) => {
      return state = {
        loaded: true,
        messages:action.payload
      }
    },
    add: (state, { payload }: PayloadAction<{message: MessageInterface}>) => {
      state.messages.push(payload.message)
    },
    edit: (state, { payload }: PayloadAction<{message: MessageInterface}>) => {
      const updatedConversations = state.messages.map(message => {
        if(message.id == payload.message.id && message.characterId == payload.message.characterId){
          return message = {
            ...payload.message,
            text: payload.message.cloneId ? '' : payload.message.text // If there's a cloneId, don't save the text since the text comes from the clone parent
          }
        }
        else {
          return message
        }
      })

      state.messages = updatedConversations
    },
    remove: (state, { payload }: PayloadAction<{message: MessageInterface}>) => {
      _.remove(state.messages, (message) => {
        return message.id == payload.message.id && message.characterId == payload.message.characterId
      })
    }
  }
})

const { actions, reducer } = charactersDataSlice
const { loadData, edit, add, remove } = actions

// Thunk actions
// ---------
const loadConversationsData = (): AppThunk => {
  return dispatch => {
    const conversationsData: MessageInterface[] = conversationsDataJSON
    dispatch(loadData(conversationsData))
  }
}

const updateMessage = (message: MessageInterface): AppThunk => {
  return (dispatch, getState) => {
    const existingMessage: MessageInterface | undefined = getState().conversationsData.messages.find((m: MessageInterface) => {
      return m.id == message.id && m.characterId == message.characterId
    })

    // If message exists, update it
    if(existingMessage){
      dispatch(edit({
        message: message
      }))
    }

    // else create a new message
    else {
      dispatch(add({
        message: message
      }))
    }

    setTimeout(() => {
      dispatch(saveConversationsData())
    }, 10)
  }
}

const removeMessage = (message: MessageInterface): AppThunk => {
  return (dispatch, getState) => {
    const children: MessageInterface[] | [] = getState().conversationsData.messages.filter((m: MessageInterface) => {
      return m.parentId == message.id && m.characterId == message.characterId
    })
    const hasChildren = children.length > 0

    // If message has children, stop
    if(hasChildren){
      alert('This message has children. Will not kill this message. Remove the children first.')
    }

    // Otherwise, go ahead and kill message
    else {
      dispatch(remove({
        message: message
      }))

      setTimeout(() => {
        dispatch(saveConversationsData())
      }, 10)
    }
  }
}

export const saveConversationsData = (): AppThunk => {
  return (dispatch, getState) => {
    const conversationsMessages = getState().conversationsData.messages

    const conversationsMessagesJSON = JSON.stringify(conversationsMessages, null, '\t')

    const options: AxiosRequestConfig = {
      method: 'POST',
      url: 'http://localhost:8888/api/update-conversations.php',
      headers: { 'content-type': 'application/json; charset=UTF-8' },
      data: conversationsMessagesJSON
    }

    axios(options)
      .catch(error => console.error('Saving conversationsData error:', error))
  }
}

// Exporting it all
// ---------
export { loadConversationsData, updateMessage, removeMessage }
export default reducer

interfaces/message.ts

export default interface MessageInterface {
  id: string,
  characterId: string,
  text: string,
  cloneId: string,
  parentId: string
}

export const MessageInitState: MessageInterface = {
  id: '',
  characterId: '',
  text: '',
  cloneId: '',
  parentId: ''
}

Ilya Karnaukhov
  • 3,838
  • 7
  • 31
  • 53
  • Code is confusing, but a prop called `firstRender` is a really bad sign. – Adam Jenkins Feb 18 '21 at 16:17
  • You need to normalize your `messages` state. It shouldn't be just a list. The problem is that you are creating a new list even when just one message changed. Instead your state should be an object mapping a message `id` to a message object. Every message object should know its immediate reply message ids. Then you can simply select any message through its id and update them independently. Additionally you can have a `messageIDs` list that defines the order of the top level messages. – trixn Feb 18 '21 at 16:30
  • `createEntityAdapter` from the redux toolkit also does that and provides helper functions to properly update individual entities. – trixn Feb 18 '21 at 16:37
  • Thanks @trixn, so the problem is how I'm dealing with the data using Redux, and not how I'm selecting it using useSelector? Are you able to create a small example to showcase how all this would work using an entity adapter? – Ilya Karnaukhov Feb 18 '21 at 16:58
  • the second argument of react memo is a function used for comparison, which by default compare instances, always different by definition in redux. ps your own fn checking the ids of items instead – quirimmo Feb 18 '21 at 20:14
  • @IlyaKarnaukhov I've crafted a working example here: https://codesandbox.io/s/eloquent-platform-sf2cm?file=/src/App.js – trixn Feb 19 '21 at 11:32

1 Answers1

1

Because your selector uses Array.prototype.filter you create a new array every time the messages array changes for each component.

If you would store the data in the state as nested data you can prevent this from happening. For example: {id:1, message:'hello', replies:[{id:2, message:'world', replies:[]}]}.

A simpler way is to use the memoization of reselect to see if each element in the filtered array is the same as it was last time. This will require more resources than the nested solution as it will perform the filter on every change for every branch but won't re render needlessly.

Here is the simple example:

const { Provider, useDispatch, useSelector } = ReactRedux;
const { createStore, applyMiddleware, compose } = Redux;
const { createSelector, defaultMemoize } = Reselect;

const initialState = { messages: [] };
//action types
const ADD = 'ADD';
//helper crating id for messages
const id = ((id) => () => ++id)(0);
//action creators
const add = (parentId, message) => ({
  type: ADD,
  payload: { parentId, message, id: id() },
});
const reducer = (state, { type, payload }) => {
  if (type === ADD) {
    const { parentId, message, id } = payload;
    return {
      ...state,
      messages: state.messages.concat({
        id,
        parentId,
        message,
      }),
    };
  }
  return state;
};
//selectors
const selectMessages = (state) => state.messages;
//curry creating selector function that closes over message id
//  https://github.com/amsterdamharu/selectors
const createSelectMessageById = (messageId) =>
  createSelector([selectMessages], (messages) =>
    messages.find(({ id }) => id === messageId)
  );
//used to check each item in the array is same as last
//  time the function was called
const createMemoizeArray = (array) => {
  const memArray = defaultMemoize((...array) => array);
  return (array) => memArray.apply(null, array);
};
//curry creating selector function that closes over parentId
//  https://github.com/amsterdamharu/selectors
const createSelectMessagesByParentId = (parentId) => {
  //memoizedArray([1,2,3]) === memoizedArray([1,2,3]) is true
  //https://github.com/reduxjs/reselect/issues/451#issuecomment-637521511
  const memoizedArray = createMemoizeArray();
  return createSelector([selectMessages], (messages) =>
    memoizedArray(
      messages.filter((m) => m.parentId === parentId)
    )
  );
};
//creating store with redux dev tools
const composeEnhancers =
  window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ || compose;
const store = createStore(
  reducer,
  initialState,
  composeEnhancers(
    applyMiddleware(() => (next) => (action) =>
      next(action)
    )
  )
);
const AddMessage = ({ addMessage }) => {
  const [reply, setReply] = React.useState('');
  return (
    <div>
      <label>
        message:
        <input
          type="text"
          onChange={(e) => setReply(e.target.value)}
          value={reply}
        />
      </label>
      <button onClick={() => addMessage(reply)}>Add</button>
    </div>
  );
};
const AddMessageContainer = React.memo(
  function AddMessageContainer({ messageId }) {
    const dispatch = useDispatch();
    const addMessage = React.useCallback(
      (message) => dispatch(add(messageId, message)),
      //dispatch in deps should not be needed but
      //  my linter still complains about it
      [dispatch, messageId]
    );
    return <AddMessage addMessage={addMessage} />;
  }
);
const Message = ({ message, replies }) => {
  console.log('in message render', message && message.message);
  return (
    <div>
      {message ? <h1>{message.message}</h1> : ''}
      {Boolean(replies.length) && (
        <ul>
          {replies.map(({ id }) => (
            <MessageContainer key={id} messageId={id} />
          ))}
        </ul>
      )}
      {/* too bad optional chaining (message?.id) does not work on SO */}
      <AddMessageContainer
        messageId={message && message.id}
      />
    </div>
  );
};
const MessageContainer = React.memo(
  function MessageContainer({ messageId }) {
    const selectMessage = React.useMemo(
      () => createSelectMessageById(messageId),
      [messageId]
    );
    const selectReplies = React.useMemo(
      () => createSelectMessagesByParentId(messageId),
      [messageId]
    );
    const message = useSelector(selectMessage);
    const replies = useSelector(selectReplies);
    return <Message message={message} replies={replies} />;
  }
);

const App = () => {
  return <MessageContainer />;
};

ReactDOM.render(
  <Provider store={store}>
    <App />
  </Provider>,
  document.getElementById('root')
);
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/16.8.4/umd/react.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-dom/16.8.4/umd/react-dom.production.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/redux/4.0.5/redux.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/react-redux/7.2.0/react-redux.min.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/reselect/4.0.0/reselect.min.js"></script>
<div id="root"></div>
HMR
  • 37,593
  • 24
  • 91
  • 160
  • just as other way around to see the issue: why not implementing the second argument of React.memo for having a comparison by ids? – quirimmo Feb 18 '21 at 20:12
  • @quirimmo I prefer to put selector logic in selectors. comparing ids wouldn't work because if a branch has another branch or leaf that doesn't change it's id. – HMR Feb 18 '21 at 21:39