Skip to content

feat: implement destroy() in adapters and plugins - #207

Open
gohabereg wants to merge 9 commits into
feat/editor-destroy-lifecyclefrom
feat/editor-destroy-plugins
Open

gohabereg wants to merge 9 commits into
feat/editor-destroy-lifecyclefrom
feat/editor-destroy-plugins

Conversation

@gohabereg

@gohabereg gohabereg commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Why

Last step of the editor-destroy OpenSpec change (spec #195, lifecycle #196). #196 gives Core and EditorJS a destroy(); this PR makes the adapters and plugins release everything they hold, adds the e2e coverage, and archives the change.

What Changes

  • DOMAdapters.destroy(): destroys the remaining block adapters, CaretAdapter and FormattingAdapter. The adapter plugin contract now requires destroy(), and Core calls it last.
  • dom-adapters: CaretAdapter/FormattingAdapter release their subscriptions through an AbortController; FormattingAdapter destroys its inline tool instances. The shared selectionchange watcher adds its document listener with the first subscriber and removes it with the last, so several editors can be destroyed independently. DOMBlockToolAdapter uses the inherited BlockToolAdapter signal instead of overriding destroy().
  • Plugins: ClipboardPlugin, ShortcutsPlugin and CollaborationManager register every listener with one controller each. OTClient does too, including its one-shot handshake and acknowledgement handlers, and close() closes the socket directly, so one still connecting is closed as well.
  • Core: its onModelUpdate model listener goes through a controller that destroy() aborts, so a model kept by the integrator no longer reaches the callback after teardown.
  • UI: UI plugin teardown itself came with fix(ui): remove blocks holder listeners and detach it on destroy #174. On top of it, InlineToolbarUI destroys the inline tool instances behind a popover when it is rebuilt, hidden or destroyed, and those of a render that is superseded.
  • Tests: dom-adapters Jest now runs in ESM mode like the other packages. New e2e/tests/destroy.spec.ts checks document/window listener cleanup (baseline from a page that never had an editor), that other editors keep working, that holders are left empty, the AbortError when destroyed before ready (with no unhandled rejection), and that the collaboration socket closes. The fixture exposes EditorJS on window and accepts ?editors=0.
  • Docs: docs/plugins.md, docs/README.md and the lifecycle diagram describe the teardown contract.
  • OpenSpec: the architecture rule is scoped to components that have a teardown (model internals and a tool's listener on its adapter go away with those objects), and the SDK names its one exception: a failed inline toolbar build's instances. Every task is checked and the change is archived (chore(openspec): archive editor-destroy), folding the deltas into architecture, core, dom-adapters, editorjs-bundle, sdk and ui.

ui-kit

The leftover window scroll listener and .ct element came from ui-kit's hint tooltip. It's fixed upstream in codex-tooltip 1.1.0 (codex-team/codex.tooltips#32) and @editorjs/ui-kit 2.0.1 (editor-js/utils#43), and this PR bumps @editorjs/ui-kit to ^2.0.1. destroy.spec.ts also checks the case where a hint was shown before the editor is destroyed.

Stack

  1. docs(openspec): propose editor-destroy #195 spec
  2. feat(core): add editor destroy lifecycle #196 lifecycle and API changes
  3. This PR (targets feat(core): add editor destroy lifecycle #196's branch)

Verification

  • Workspace build, unit tests and lint pass. Every mutant Stryker generates on lines this PR adds is killed (dom-adapters, plugins, collaboration-manager by hand since its Stryker setup runs no tests for that file, and the InlineToolbarUI lines).
  • e2e: 96 passed on Chromium and WebKit, with no failures beyond the tests the suite already marks as expected to fail.

🤖 Generated with Claude Code

gohabereg and others added 5 commits October 16, 2026 22:17
- DOMAdapters.destroy(): destroys remaining block adapters, CaretAdapter
  and FormattingAdapter; the adapter plugin contract now requires it.
- CaretAdapter/FormattingAdapter release their subscriptions through an
  AbortController; FormattingAdapter destroys its inline tool instances,
  including one replaced by a re-attached tool.
- The shared selectionchange watcher adds its document listener with the
  first subscriber and removes it with the last, so several editors can
  be destroyed independently.
- DOMBlockToolAdapter uses the inherited BlockToolAdapter signal instead
  of overriding destroy().
- ClipboardPlugin, ShortcutsPlugin and CollaborationManager register
  every listener with one AbortController each.
- dom-adapters Jest runs in ESM mode like the other packages.
- Docs describe the teardown contract and lifecycle.

UI plugin teardown is covered by #174.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- destroy.spec.ts: document/window listeners of destroyed editors are
  removed (baseline from a page that never had an editor), other editors
  keep working, isReady rejects with AbortError when destroyed before
  ready without an unhandled rejection, and the collaboration socket is
  closed (mocked with routeWebSocket).
- Holder-emptiness checks are test.fail() until #174 removes the UI DOM
  on destroy.
- Fixture exposes EditorJS and its editors on window and accepts
  ?editors=0; a listener tracker in e2e/support counts active listeners.
- mountDocument waits for the ready flag to be attached, not visible, so
  an empty page works.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#174 removes the UI plugins' DOM and listeners on destroy, so the
holder-emptiness checks in destroy.spec.ts now pass.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
InlineToolbarUI keeps the inline tool instances behind the current
popover and calls their optional destroy() when the popover is rebuilt,
hidden or destroyed. A render superseded before it finishes destroys the
instances it created. Instances of a build whose tool config rejects
are still not released; that is noted on the existing @todo about
failed builds.

The editor-destroy change records #174 as merged and gains a ui delta
for the new scenario.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Unit Tests

Package Coverage Delta
@editorjs/shortcuts-plugin 100% 0% ⚪️
@editorjs/editorjs 100% 0% ⚪️
@editorjs/clipboard-plugin 60% -6.66% 🔴
@editorjs/collaboration-manager 85.51% -0.3% 🔴
@editorjs/ot-server 20% 0% ⚪️
@editorjs/dom-adapters 18.14% -68.81% 🔴
@editorjs/core 76.71% 0% ⚪️
@editorjs/model 98.48% 0% ⚪️
@editorjs/model-types 59.82% 0% ⚪️

Mutation Tests

Package Mutation score Dashboard URL
@editorjs/shortcuts-plugin 96.55% 🟢 Dashboard
@editorjs/clipboard-plugin 95.45% 🟢 Dashboard
@editorjs/collaboration-manager 25.66% 🔴 Dashboard
@editorjs/dom-adapters 13.57% 🔴 Dashboard
@editorjs/core 70.75% 🟡 Dashboard
@editorjs/model No files to mutate found.

gohabereg and others added 3 commits October 16, 2026 22:36
- Core registers its onModelUpdate model listener with its own
  controller and aborts it in destroy(), so a model kept by the
  integrator no longer reaches the callback after teardown.
- The shared selectionchange watcher registers its document listener
  with a controller created for the first subscriber and aborted with
  the last, instead of adding and removing the handler by hand.
- OTClient registers every socket listener, including the one-shot
  handshake and acknowledgement handlers, with one controller that
  close() aborts, and closes the socket directly so one still
  connecting is closed too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- architecture: the AbortController rule covers components that have a
  teardown; listeners on objects a component owns or is handed, and that
  go away with them (model internals, a tool's adapter), are released
  with those objects.
- sdk: name the one exception to "every tool instance is destroyed": a
  failed inline toolbar build's instances.

Applied to both the folded specs and the archived change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ui-kit 2.0.1 bundles codex-tooltip 1.1.0, which only keeps its element
and window scroll listener while a hint is shown, and no longer creates
the tooltip just to hide it on teardown. Destroying every editor now
leaves no tooltip element or listener behind.

- destroy.spec.ts: a test that shows a hint, destroys the editor and
  checks that no tooltip element or document/window listener remains.
- The archived design notes the upstream fix and the dependency.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@gohabereg
gohabereg marked this pull request as ready for review October 19, 2026 17:48
@neSpecc
neSpecc requested a balanced review from Copilot October 21, 2026 22:14

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file could not be rendered

Error rendering embedded code

Parse error on line 36:
...ind api.plugins;<br/>read at access time
-----------------------^
Expecting 'NEWLINE', ',', '()', 'SOLID_OPEN_ARROW', 'DOTTED_OPEN_ARROW', 'SOLID_ARROW', 'SOLID_ARROW_TOP', 'SOLID_ARROW_BOTTOM', 'STICK_ARROW_TOP', 'STICK_ARROW_BOTTOM', 'SOLID_ARROW_TOP_DOTTED', 'SOLID_ARROW_BOTTOM_DOTTED', 'STICK_ARROW_TOP_DOTTED', 'STICK_ARROW_BOTTOM_DOTTED', 'SOLID_ARROW_TOP_REVERSE', 'SOLID_ARROW_BOTTOM_REVERSE', 'STICK_ARROW_TOP_REVERSE', 'STICK_ARROW_BOTTOM_REVERSE', 'SOLID_ARROW_TOP_REVERSE_DOTTED', 'SOLID_ARROW_BOTTOM_REVERSE_DOTTED', 'STICK_ARROW_TOP_REVERSE_DOTTED', 'STICK_ARROW_BOTTOM_REVERSE_DOTTED', 'BIDIRECTIONAL_SOLID_ARROW', 'DOTTED_ARROW', 'BIDIRECTIONAL_DOTTED_ARROW', 'SOLID_CROSS', 'DOTTED_CROSS', 'SOLID_POINT', 'DOTTED_POINT', 'TXT', got 'INVALID'

For more information, see https://docs.github.com/get-started/writing-on-github/working-with-advanced-formatting/creating-diagrams#creating-mermaid-diagrams

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Shared inline-tool instances are disposed while still in use, and cleanup failures can leave editor resources retained.

4 open issues
What changed in this PR

Extends the editor teardown lifecycle from #196 to adapters, plugins and inline-toolbar tools, building on the UI cleanup from #174.

Changes:

  • Adds adapter destruction, abort-based listener cleanup and collaboration socket shutdown.
  • Adds lifecycle tests and browser teardown coverage, and updates UI-kit for tooltip cleanup.
  • Documents teardown contracts and archives the completed OpenSpec change.
File Description
yarn.lock Locks updated UI-kit and tooltip dependencies.
packages/​ui/​src/​lifecycle.spec.ts Tests inline-tool cleanup across toolbar lifecycles.
packages/​ui/​src/​InlineToolbar/​InlineToolbar.ts Tracks and destroys toolbar tool instances.
packages/​ui/​package.json Updates UI-kit dependency.
packages/​sdk/​src/​entities/​EditorjsAdapterPlugin.ts Requires adapter destruction.
packages/​sdk/​package.json Updates UI-kit dependency.
packages/​plugins/​shortcuts-plugin/​src/​index.ts Aborts shortcut listeners on destruction.
packages/​plugins/​shortcuts-plugin/​src/​index.spec.ts Tests shortcut listener teardown.
packages/​plugins/​clipboard-plugin/​src/​index.ts Uses abort-based clipboard listener cleanup.
packages/​plugins/​clipboard-plugin/​src/​index.spec.ts Tests clipboard behavior after destruction.
packages/​editorjs/​e2e/​tests/​destroy.spec.ts Adds browser teardown coverage.
packages/​editorjs/​e2e/​support/​listeners.ts Tracks listeners for teardown assertions.
packages/​editorjs/​e2e/​support/​editor.ts Supports empty-page fixture readiness.
packages/​editorjs/​e2e/​README.md Documents the zero-editor fixture.
packages/​editorjs/​e2e/​fixtures/​main.ts Supports dynamic editor mounting and empty baselines.
packages/​dom-adapters/​src/​utils/​useSelectionChange.ts Manages the shared selection listener lifetime.
packages/​dom-adapters/​src/​utils/​useSelectionChange.spec.ts Tests shared-listener subscription cleanup.
packages/​dom-adapters/​src/​utils/​selectionRangeInInput.spec.ts Converts mocks to ESM-compatible loading.
packages/​dom-adapters/​src/​index.ts Adds DOMAdapters teardown.
packages/​dom-adapters/​src/​index.spec.ts Tests adapter and input-registry cleanup.
packages/​dom-adapters/​src/​FormattingAdapter/​index.ts Releases subscriptions and inline tools.
packages/​dom-adapters/​src/​FormattingAdapter/​FormattingAdapter.spec.ts Tests formatting adapter teardown.
packages/​dom-adapters/​src/​CaretAdapter/​index.ts Releases selection and caret subscriptions.
packages/​dom-adapters/​src/​CaretAdapter/​CaretAdapter.spec.ts Tests caret subscription cleanup.
packages/​dom-adapters/​src/​BlockToolAdapter/​index.ts Uses the inherited listener signal.
packages/​dom-adapters/​package.json Enables ESM-mode test execution.
packages/​core/​src/​index.ts Aborts model callbacks and requires adapter teardown.
packages/​core/​src/​index.spec.ts Tests model callback removal after destruction.
packages/​collaboration-manager/​test/​mocks/​ws.ts Adds signal-aware socket listener tracking.
packages/​collaboration-manager/​src/​CollaborationManager.ts Consolidates subscription cleanup through a controller.
packages/​collaboration-manager/​src/​CollaborationManager.spec.ts Tests collaboration listener teardown.
packages/​collaboration-manager/​src/​client/​OTClient.ts Removes socket listeners and closes connecting sockets.
packages/​collaboration-manager/​src/​client/​OTClient.spec.ts Tests socket and pending-handler cleanup.
openspec/​specs/​ui/​spec.md Specifies toolbar tool disposal.
openspec/​specs/​sdk/​spec.md Defines adapter and tool teardown contracts.
openspec/​specs/​editorjs-bundle/​spec.md Specifies public editor teardown behavior.
openspec/​specs/​dom-adapters/​spec.md Specifies adapter resource cleanup.
openspec/​specs/​core/​spec.md Defines core teardown lifecycle guarantees.
openspec/​specs/​architecture/​spec.md Defines abort-based listener ownership rules.
openspec/​changes/​archive/​2026-10-16-editor-destroy/​tasks.md Marks implementation tasks complete.
openspec/​changes/​archive/​2026-10-16-editor-destroy/​specs/​ui/​spec.md Archives UI lifetime requirements.
openspec/​changes/​archive/​2026-10-16-editor-destroy/​specs/​sdk/​spec.md Records the failed-build cleanup exception.
openspec/​changes/​archive/​2026-10-16-editor-destroy/​specs/​editorjs-bundle/​spec.md Archives bundle teardown requirements.
openspec/​changes/​archive/​2026-10-16-editor-destroy/​specs/​dom-adapters/​spec.md Archives adapter teardown requirements.
openspec/​changes/​archive/​2026-10-16-editor-destroy/​specs/​core/​spec.md Archives core lifecycle requirements.
openspec/​changes/​archive/​2026-10-16-editor-destroy/​specs/​architecture/​spec.md Clarifies listener-rule scope and exceptions.
openspec/​changes/​archive/​2026-10-16-editor-destroy/​proposal.md Updates completed scope and related work.
openspec/​changes/​archive/​2026-10-16-editor-destroy/​design.md Records cleanup limitations and upstream fixes.
openspec/​changes/​archive/​2026-10-16-editor-destroy/​.openspec.yaml Preserves archived change metadata.
docs/​README.md Adds teardown to the lifecycle overview.
docs/​plugins.md Documents plugin and tool teardown contracts.
docs/​diagrams/​plugin-lifecycle-flow.mmd Illustrates teardown order and resource release.

Review effort: Balanced

const shortcut = tool.options.shortcut;
const instance = tool.create();

instances.add(instance);
Comment on lines +136 to +137
this.#tools.forEach(tool => tool.destroy?.());
this.#tools.clear();
Comment on lines +188 to +190
if (inputsWatched.length === 0) {
stopListening();
}
Comment on lines +503 to +504
this.#toolInstances.forEach(instance => instance.destroy?.());
this.#toolInstances.clear();
A ';' inside a Note ends the statement in a Mermaid sequence diagram,
so the registry note (from #184) broke parsing on line 36 and GitHub
couldn't render the file. Use a comma instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants