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

Memory leak in EDITOR_TO_FORCE_RENDER #5592

Open
johnwiseheart opened this issue Jan 17, 2024 · 2 comments
Open

Memory leak in EDITOR_TO_FORCE_RENDER #5592

johnwiseheart opened this issue Jan 17, 2024 · 2 comments
Assignees
Labels

Comments

@johnwiseheart
Copy link

johnwiseheart commented Jan 17, 2024

Description
EDITOR_TO_FORCE_RENDER is set here, but never manually cleared. The key is a strong reference to the editor, leading to this map increasing in size with each editor which is mounted. (See gif)

Recording
ScreenCast

Sandbox
Sandbox

Steps
To reproduce the behavior, see gif.

Expectation
When the editor is unmounted, the EDITOR_TO_FORCE_RENDER map should no longer contain any values.

Environment

  • Slate Version: 0.101.5
  • Operating System: Mac OS
  • Browser: Chrome

Context
I found this because I was attempting to upgrade from a very old version of slate, which also contained a memory leak in EDITOR_TO_ELEMENT which has been already fixed. However, the fixed version contains this leak instead, so I can't justify the time it would take to do the upgrade.

Personally I'm not expecting this to just be fixed immediately, but I wanted to make the issue, since if it had already been made it would have saved me quite a lot of time in tracking down this leak (and also the previous, fixed one). If someone reading this is excited to fix this issue, great! If not, I can come back and fix it if/when I make this upgrade.

@dylans dylans self-assigned this Feb 21, 2024
@bufen8x8
Copy link

Any progress on this?

@WindRunnerMax
Copy link
Contributor

Maybe you just forgot to manually trigger garbage collection. The browser doesn't collect memory in real-time, and WeakMap only holds weak references, so it won't cause a memory leak.

A simple example:

const obj = { a: {}, b: {} };
const wp = new WeakMap();
wp.set(obj.a, 1);
wp.set(obj.b, 2);
console.log(wp);
delete obj.a;
console.log(wp);
// Trigger GC
console.log(wp);

20240809200536_rec_-convert

In your sandbox example, you can also perform garbage collection to manually reclaim unused objects.When repeatedly switching and rebuilding the Editor, the WeakMap object does create multiple object references. However, after triggering GC, only the current object reference remains.

20240809200130_rec_-convert

But from the gif above, you can see something interesting. Even after the editor is unmounted, there still remains a reference to the Editor. I suspect that React might have cached the props from the last render here, but I haven't actually verified it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants