codex-team/editor.js

Memory leak in Shortcuts class

Open

#2,631 opened on Feb 21, 2024

View on GitHub
 (6 comments) (2 reactions) (0 assignees)TypeScript (26,602 stars) (1,985 forks)batch import
good first issue

Description

Editor.js Version

v2.29.0

Issue description

Hi!

I've found a memory leak in the Shortcuts class. When you remove the shortcut for an element you don't check if the element's shortcuts are empty and left the element in the registeredShortcuts map (with an empty array):

https://github.com/codex-team/editor.js/blob/b619946e8f23ebc9ccfc41ef653439278e99491e/src/components/utils/shortcuts.ts#L89

If you create a new EditorJS and destroys it, you will see a bunch of detached elements referenced by registeredShortcuts map.

Steps to reproduce:

You can reproduce the issue with this simple html:

<html>
  <body>
    <button id="remove">Remove</button>
    <div id="editor" style="min-width: 700px"></div>
    <script src="https://cdn.jsdelivr.net/npm/@editorjs/editorjs@latest"></script>
    <script>
      let editor = new EditorJS({
        holder: 'editor',
      });

      function remove() {
        document.querySelector('#editor').remove();
        editor.isReady.then(() => {
          editor.destroy();
          editor = undefined;
        });
      }
      document.querySelector('#remove').addEventListener('click', remove);
    </script>
  </body>
</html>

Press "Remove" button and take a snapshot in Chrome dev tools. You will see detached HTMLDivElement's referenced by registeredShortcuts:

screenshot

If you remove the element from the map when the shortcuts array is empty, the problem disappears:

const newShortcuts = shortcuts.filter(el => el !== shortcut);

newShortcuts.length===0?
  this.registeredShortcuts.delete(element):
  this.registeredShortcuts.set(element, newShortcuts);

screenshot2

Contributor guide

Memory leak in Shortcuts class · codex-team/editor.js#2631 | Good First Issue