Remove messages from store upon closing a conversation#504
Remove messages from store upon closing a conversation#504MasterK0927 wants to merge 5 commits intodevelopfrom
Conversation
|
I see that this PR currently addresses "Cleanup messages from store upon closing a conversation". I'm not worried about keeping a bunch of messages in the store, that's going to be a tiny amount of data. The issue is going to be from keeping a bunch of messages containing images in the store, which could be a massive amount of data, very quickly. A "workaround" for that actual issue could be what you've done in this PR, where we only keep 20 messages in the store after leaving the conversation page. I'm fine with that as a much simpler refactor. But I want to confirm -- does this resolve #494 for you? You noted you were able to reproduce it? Does this PR resolve it? I'd also request that we split out "Cache store to the localStorage" into a separate PR, as they really are addressing different issues. |
There was a problem hiding this comment.
I'd prefer a more explicit naming like deleteMessagesFromStore
This PR helps mitigate #494, but there are still some edge cases where it might occur. Right now, it removes both text and file messages from the store without explicitly checking message types. A more robust cleanup approach could be based on file size and cache management. Upon testing a version of Volla Messages including #502 and this, on 3 different devices, app surely performs stable. But we should gather feedback from beta users about how it holds up for them in their usage. |
I'm specifically asking this: In #494 you noted that you reproduced the issue. At that time, you had joined a conversation that synced some data, up to the point where your conductor state made the issue arise, consistently. I want to you get your conductor back into that same state: join the conversation, wait for data to sync, until the issue arises consistently. Then, keeping the same conductor data, switch to this new branch. Then confirm that your code changes on this branch make the issue not arise anymore, consistently. This is the only way to be sure we are actually addressing the noted issue. |
Partially Resolves #494
TODO: