17
3
Fork
You've already forked split-bill
2

fix: prevent memory leaks when setting up yjs listeners #2

Merged
nonetoohappy merged 4 commits from ac/fix-yjs-observe-memory-leak into main 2025年01月03日 20:21:00 +01:00
Collaborator
Copy link

Prior to this change, yjs listeners were being set up within the render body of components without any cleanup being done. This is prone to memory leaks because:

  1. The listeners are being set up every time the component renders (due to state or prop changes)
  2. There's no cleanup of the listeners that were set up prior when (1) occurs.

This PR wraps any call to y's observe() to ensure that listeners are appropriately set up and cleaned up when needed.


While this PR does not introduce any logic changes, my observation is that most of this state can - and probably should - live as app state that is shared between all of the relevant pages, as opposed to the current implementation that has each page setting up their own listeners. That recommendation is out of the scope of this PR as it's not fundamentally crucial to functionality, but would potentially simplify the implementation a bit. Can be explored in a follow-up

Prior to this change, yjs listeners were being set up within the render body of components without any cleanup being done. This is prone to memory leaks because: 1. The listeners are being set up _every time the component renders_ (due to state or prop changes) 2. There's no cleanup of the listeners that were set up prior when (1) occurs. This PR wraps any call to y's `observe()` to ensure that listeners are appropriately set up and cleaned up when needed. --- While this PR does not introduce any logic changes, my observation is that most of this state can - and probably should - live as app state that is shared between all of the relevant pages, as opposed to the current implementation that has each page setting up their own listeners. That recommendation is out of the scope of this PR as it's not fundamentally crucial to functionality, but would potentially simplify the implementation a bit. Can be explored in a follow-up
nonetoohappy deleted branch ac/fix-yjs-observe-memory-leak 2025年01月03日 20:21:01 +01:00
Sign in to join this conversation.
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
webxdc/split-bill!2
Reference in a new issue
webxdc/split-bill
No description provided.
Delete branch "ac/fix-yjs-observe-memory-leak"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?