Complete Omission of block.destroy() in Blocks Collection
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
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
- Leaked
MutationObservers:this.unwatchBlockMutations()is never executed. Each deleted block leaves an activeMutationObserverobserving an orphaned DOM node. - Leaked Input Event Listeners:
this.removeInputEvents()is never executed; all keydown, paste, and input handlers remain bound to inputs. - Broken Tool Teardown: Third-party plugins that implement
destroy()(e.g., stopping video streams, tearing down WebGL/Three.js canvases, clearingsetIntervaltimers, closing WebSocket channels) never have theirdestroy()hook fired when a block is deleted or updated! - 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
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from codex-team/editor.js
-
bug good first issue
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
codex-team/editor.js#3032 · 1 comment ·
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
codex-team/editor.js#3030 ·
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
codex-team/editor.js#3005 · 1 comment ·
-
good first issue
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
codex-team/editor.js#2616 · 1 comment · 1 reaction ·
-
bug
Difficulty 3/5 1-2 days Newbie friendliness 72/100
codex-team/editor.js#3031 ·
All issues in codex-team/editor.js
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
Eynzof/Hermes-CN-Desktop#610 ·
-
bug clawsweeper:linked-pr-open clawsweeper:needs-live-repro clawsweeper:no-new-fix-pr impact:message-loss issue-rating: 🐚 platinum hermit P2 regression
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
-
enhancement
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
-
calcite-components needs triage refactor
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
Esri/calcite-design-system#15203 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 90/100
danielmiessler/LifeOS#2218 ·