Details
Details
I had a repro of ENG-9448 where I simply logged out and back in with my t125 test user. After this diff:
- I no longer see Cannot read properties of undefined errors
- The robotext messages are delivered for all of my Farcaster friends
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- ashoat/bug
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
Good find, and the fix looks good.
However, I think unfortunately we used this antipattern in several places where we have a loop that is running on an old version of callback.
I think now, we should rely on effects and reactivity, I would prefer something like we have here.
Also, I am curious if we shouldn't introduce a generic component like this:
type MessageQueueHook<T> = { +enqueue: (queueItems: $ReadOnlyArray<T>) => void, }; function useActivititesQueue<T>( performActivity: (queueItem: T) => void | Promise<void>, ): MessageQueueHook<T> { const [queue, setQueue] = React.useState<$ReadOnlyArray<T>>(); const [isProcessing, setProcessing] = React.useState(false); const processNextMessage = React.useCallback(async () => { if (isProcessing || queue.length === 0) { return; } setProcessing(true); const nextActivity = queue[0]; await performActivity(nextActivity); setQueue(currentQueue => currentQueue.slice(1)); setProcessing(false); }, [isProcessing, queue, performActivity]); const enqueue = React.useCallback( (queueItems: $ReadOnlyArray<T>) => setQueue(prevQueue => [...prevQueue, ...queueItems]), [], ); React.useEffect(() => { if (isProcessing) { return; } void processNextMessage(); }, [isProcessing, processNextMessage]); return { enqueue }; }
and use it everywhere we use queue like this to avoid similar issues (which are not that easy to debug), and get rid of loops.
Comment Actions
(sorry for the confusing naming in the code above, this is just a draft to describe the idea)