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

New Queue System for ChatMain #2556

Open
taoeffect opened this issue Jan 28, 2025 · 0 comments
Open

New Queue System for ChatMain #2556

taoeffect opened this issue Jan 28, 2025 · 0 comments
Assignees
Labels
App:Frontend Kind:Enhancement Improvements, new features, performance upgrades, etc. Level:Advanced Note:UI/UX Priority:High

Comments

@taoeffect
Copy link
Member

taoeffect commented Jan 28, 2025

Problem

The design of ChatMain.vue is very problematic. It is currently causing tests to fail because of its very async natural and the way it handles switching chatrooms.

Because of its poor design in handling switching chatrooms, all over the code are lines like this after almost every await:

if (!this.checkEventSourceConsistency(chatRoomID)) return

And this from bd0d0d1:

if (this.renderingChatRoomId !== this.summary.chatroomID) // NOTE: this is actually a bug, it should be this.ephemeral.renderingChatRoomId

We are checking this.summary.chatroomID, this.ephemeral.renderingChatRoomId, this.currentChatRoomId all over the place. This is wrong. We should be using only a single renderingChatRoomId for the component, and nothing else.

Solution

Redesign how ChatMain handles switching chatrooms.

Make it so that ChatMain only cares about a single "chatroomID" variable, e.g. renderingChatRoomId or whatever.

Every time a switch in chatroomID is detected, do not mess with any of ChatMain's variables. Do not update anything. Instead, create a request to ChatMain for ChatMain to process in its own time, on a queue. This request will say, "Please update your renderingChatRoomId to this new value", and ChatMain will in the meantime ignore all new messages that come in on other chatroomID contracts.

In other words, ChatMain will process requests from a queue to update the current chatroomID in its ephemeral data. And these requests can stack up. So for example, Cypress can rapidly switch from chatroom to chatroom but ChatMain will only render and show chatroom for its own internal ephemeral.chatroomID (renderingChatRoomId, whatever). It will process any requests to update this variable in sequence, one at a time.

*Note: when pulling events off of this internal queue, ChatMain can also check this size of the queue. For example, it can decide that if there are more than 1 request on the queue, it can pop() and ignore the old ones and just process the most recent one, to speed things up.

With these changes made, you should be able to greatly clean up ChatMain and remove all paranoid and DRY-violating calls to checkEventSourceConsistency etc.

One last note: PR #2555 should probably be merged first as it makes a few changes to ChatMain in an attempt to fix the Cypress tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
App:Frontend Kind:Enhancement Improvements, new features, performance upgrades, etc. Level:Advanced Note:UI/UX Priority:High
Projects
None yet
Development

No branches or pull requests

2 participants