editor-js / editor-js/document-model
BlocksUI doesn't remove its blocksHolder DOM event listeners (or the holder itself) on destroy()
- Dominant language
- TypeScript
- Stars
- 12
- Forks
- 3
- PR merge metrics
- No merged PRs in 30d
Description
`BlocksUI.#prepareBlocksHolder()` ([packages/ui/src/Blocks/Blocks.ts](https://github.com/editor-js/document-model/blob/main/packages/ui/src/Blocks/Blocks.ts)) registers three DOM listeners directly on `blocksHolder`:
- `beforeinput` (line 89)
- `keydown` (line 118)
- `copy` (line 140)
`BlocksUI.destroy()` only cleans up rendered block elements:
```ts
public destroy(): void {
this.#blocks.forEach(block => block.remove());
this.#blocks = [];
}
```
None of the three listeners above are ever removed, and `#blocksHolder` itself is never detached. The holder is handed off via `BlocksHolderRenderedUIEvent` to `EditorUI`, which appends it into `#editorWrapper` (`packages/ui/src/index.ts`) — but `EditorUI.destroy()` is currently a no-op (`// Cleanup if needed`), so nothing ever detaches it either.
## Impact
If a `blocksHolder` element is reused, or a new `BlocksUI`/`Core` instance is created against a holder from a previous instance, the old listeners stay attached and continue firing alongside the new instance's listeners. Concretely this means duplicate `BeforeInputUIEvent`/`CopyUIEvent` dispatches per user action, and stale `keydown` (undo/redo) handling from a destroyed instance.
## Where this came from
Flagged by review on #129, specifically for the new `copy` listener: https://github.com/editor-js/document-model/pull/129#discussion_r3616676234. Investigation showed the same gap already existed for `beforeinput`/`keydown` before this PR — this issue is to track fixing all three (and the missing holder detachment) rather than just the newest one.
## Suggested fix
- Store references to the three listener functions (or use an `AbortController`/`AbortSignal` passed to `addEventListener` for the whole group) so `destroy()` can call `removeEventListener` for each, matching the pattern already used in `ClipboardPlugin`/`ShortcutsPlugin` elsewhere in this codebase.
- Consider detaching `#blocksHolder` from the DOM in `destroy()`, and implementing `EditorUI.destroy()` (currently a no-op) to tear down `#editorWrapper`.
- Add test coverage — there's currently no `Blocks.spec.ts` at all, so `destroy()` behavior is completely unverified.
Contributor guide
Research direction
Read packages/ui/src/Blocks/Blocks.ts and packages/ui/src/index.ts to trace blocksHolder creation, listener registration, handoff, and destruction. Compare teardown patterns in ClipboardPlugin and ShortcutsPlugin, then add a new Blocks.spec.ts covering listener cleanup and holder detachment; done means destroyed instances no longer dispatch duplicate or stale events.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- frontend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 58/100