Complete Omission of block.destroy() in Blocks Collection

Open Beginner friendly
#3,029 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
84/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
typescript
Domain
frontend

Research direction

Read src/components/blocks.ts at insert, replace, remove, and removeAll, then compare their lifecycle calls with Block.destroy() in src/components/block/index.ts. Done means each path that removes or replaces a block performs the required cleanup while preserving the existing removal hook behavior.

Written by the indexing model from the issue text.

Description

bug

1. Complete Omission of block.destroy() in Blocks Collection

📍 Affected Locations
  • src/components/blocks.ts (Lines 200–205, 230–240, 285–295, 300–306)
  • src/components/block/index.ts (Lines 688–698)
🔍 Deep Technical Diagnosis

The Block class defines an essential cleanup method in src/components/block/index.ts:

// src/components/block/index.ts: Lines 688-698
public destroy(): void {
  this.holder.removeEventListener('change', this.handleSelectChange);
  this.unwatchBlockMutations();
  this.removeInputEvents();

  super.destroy();

  if (_.isFunction(this.toolInstance.destroy)) {
    this.toolInstance.destroy();
  }
}

However, in the core collection manager src/components/blocks.ts, block.destroy() is never called anywhere:

1. When removing a block:
// src/components/blocks.ts: Lines 285-295
public remove(index: number): void {
  if (isNaN(index)) {
    index = this.length - 1;
  }

  this.blocks[index].holder.remove();
  this.blocks[index].call(BlockToolAPI.REMOVED); // <-- Calls removed hook, BUT NEVER destroy()!
  this.blocks.splice(index, 1);
}
2. When clearing all blocks (removeAll / blocks.clear()):
// src/components/blocks.ts: Lines 300-306
public removeAll(): void {
  this.workingArea.innerHTML = '';
  this.blocks.forEach((block) => block.call(BlockToolAPI.REMOVED)); // <-- NEVER destroy()!
  this.blocks.length = 0;
}
3. When updating a block (replace / blocks.update()):
// src/components/blocks.ts: Lines 230-240
public replace(index: number, block: Block): void {
  const prevBlock = this.blocks[index];
  prevBlock.holder.replaceWith(block.holder);
  this.blocks[index] = block; // <-- Neither removed NOR destroy() is called!
}
4. When inserting with replace (insert(..., replace = true)):
// src/components/blocks.ts: Lines 200-204
if (replace) {
  this.blocks[index].holder.remove();
  this.blocks[index].call(BlockToolAPI.REMOVED); // <-- destroy() skipped!
}
💥 Real-World Impact
  1. Leaked MutationObservers: this.unwatchBlockMutations() is never executed. Each deleted block leaves an active MutationObserver observing an orphaned DOM node.
  2. Leaked Input Event Listeners: this.removeInputEvents() is never executed; all keydown, paste, and input handlers remain bound to inputs.
  3. Broken Tool Teardown: Third-party plugins that implement destroy() (e.g., stopping video streams, tearing down WebGL/Three.js canvases, clearing setInterval timers, closing WebSocket channels) never have their destroy() hook fired when a block is deleted or updated!
  4. Over a long editing session or dynamic updates in collaborative editors (e.g., Yjs), hundreds of orphaned blocks accumulate in memory.
💡 Proposed Solution & Patch
src/components/blocks.ts
@@ -201,6 +201,7 @@ export default class Blocks {
     if (replace) {
       this.blocks[index].holder.remove();
       this.blocks[index].call(BlockToolAPI.REMOVED);
+      this.blocks[index].destroy();
     }

     const deleteCount = replace ? 1 : 0;
@@ -236,6 +237,8 @@ export default class Blocks {
     const prevBlock = this.blocks[index];

     prevBlock.holder.replaceWith(block.holder);
+    prevBlock.call(BlockToolAPI.REMOVED);
+    prevBlock.destroy();

     this.blocks[index] = block;
   }
@@ -290,6 +293,7 @@ export default class Blocks {
     this.blocks[index].holder.remove();

     this.blocks[index].call(BlockToolAPI.REMOVED);
+    this.blocks[index].destroy();

     this.blocks.splice(index, 1);
   }
@@ -301,6 +305,9 @@ export default class Blocks {
     this.workingArea.innerHTML = '';

-    this.blocks.forEach((block) => block.call(BlockToolAPI.REMOVED));
+    this.blocks.forEach((block) => {
+      block.call(BlockToolAPI.REMOVED);
+      block.destroy();
+    });

     this.blocks.length = 0;
   }

Dominant language
TypeScript
Stars
32k
Forks
2.2k
Avg merge
1d 13h
Merged PRs (30d)
2

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from codex-team/editor.js

All issues in codex-team/editor.js

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.