Chat React state leaking between accounts (`ChatProvider` is not behind `key={accountId}`)

[WofWca]

Related:

• fix: reduce chat React state leaking between accs #5992
• Prevent re-rendering of account sidebar when switching account #3789 (comment)

Even after #5992 there are still some races.

I think one of them is that selectChat gets captured and then executed inside of an async function whose execution might get delayed, and by that time we might have switched the account already. But, since the function has captured the accountId variable, the if (accountId !== nextAccountId) { doesn't really help.

deltachat-desktop/packages/frontend/src/contexts/ChatContext.tsx

Lines 95 to 156 in 4ffc6b0

	const selectChat = useCallback<SelectChat>(
	(nextAccountId: number, nextChatId: number) => {
	if (!accountId) {
	throw new Error('can not select chat when no `accountId` is given')
	}
	if (accountId !== nextAccountId) {
	throw new Error(
	'accountid of ChatProvider context is not equal to nextAccountId'
	)
	}
	// Jump to last message if user clicked chat twice
	// Remember that there might be no messages in the chat.
	// @TODO: We probably want this to be part of the UI logic instead
	if (nextChatId === chatId) {
	const focusMessage = false
	window.__internal_jump_to_message_asap = {
	accountId,
	chatId: nextChatId,
	jumpToMessageArgs: [
	{
	msgId: undefined,
	highlight: false,
	focus: focusMessage,
	addMessageIdToStack: undefined,
	// `scrollIntoViewArg:` doesn't really have effect when
	// jumping to the last message.
	},
	],
	}
	window.__internal_check_jump_to_message?.()
	// Yes, we only run this in the `nextChatId === chatId` case.
	// Otherwise the composer will get focused when the chat loads,
	// in another `useEffect`.
	if (!focusMessage) {
	const composerTextarea = document.getElementsByClassName(
	'create-or-edit-message-input'
	)[0]
	if (composerTextarea instanceof HTMLElement) {
	composerTextarea.focus()
	} else {
	log.warn(
	'Failed to focus composer: not an HTMLElement',
	composerTextarea
	)
	}
	}
	}
	// Already set known state
	setChatId(nextChatId)
	// Clear system notifications and mark chat as seen in backend
	markChatAsSeen(accountId, nextChatId)
	// Remember that user selected this chat to open it again when they come back
	saveLastChatId(accountId, nextChatId)
	},
	[accountId, chatId]
	)

selectLastChat is one place where this could be happening. Just imagine that there is a await new Promise(r => setTimeout(r, 10_000)) after await getLastChatId().

deltachat-desktop/packages/frontend/src/hooks/chat/useSelectLastChat.ts

Lines 21 to 34 in 454925d

	const selectLastChat = useCallback(async () => {
	if (!accountId) {
	return
	}
	const account = await BackendRemote.rpc.getAccountInfo(accountId)
	if (account.kind === 'Configured') {
	const lastChatId = await getLastChatId(accountId)
	if (lastChatId && !smallScreenMode) {
	selectChat(accountId, lastChatId)
	}
	}
	}, [accountId, selectChat, smallScreenMode])

The result is that setChatId gets executed, but that chatId might have been one obtained from the old account's getLastChatId(accountId).

I believe that we after all shoult strive for putting key={accountId} on ChatProvider, as I mentioned in #3789 (comment), instead of trying to re-implement React's key feature inside of it.
Although I might be wrong and ChatProvider is really one special piece that we should implement manually.

Anyways, the question is: how do we handle that a captured selectChat might get executed with a big delay? Or is the question "how do we make sure to never execute it if it's outdated?"?

[WofWca]

Regarding the selectChat() getting called asynchronously: I had a fun idea of selectChat accepting a Promise that resolves to a chatId.