Repository navigation
Conversation
- 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>
This was referenced Oct 16, 2026
Contributor
Unit Tests
Mutation Tests
|
- 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>
neSpecc
reviewed
Oct 21, 2026
Contributor
There was a problem hiding this comment.
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
Contributor
There was a problem hiding this comment.
🟡 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Why
Last step of the
editor-destroyOpenSpec change (spec #195, lifecycle #196). #196 givesCoreandEditorJSadestroy(); 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,CaretAdapterandFormattingAdapter. The adapter plugin contract now requiresdestroy(), andCorecalls it last.CaretAdapter/FormattingAdapterrelease their subscriptions through anAbortController;FormattingAdapterdestroys its inline tool instances. The sharedselectionchangewatcher adds itsdocumentlistener with the first subscriber and removes it with the last, so several editors can be destroyed independently.DOMBlockToolAdapteruses the inheritedBlockToolAdaptersignal instead of overridingdestroy().ClipboardPlugin,ShortcutsPluginandCollaborationManagerregister every listener with one controller each.OTClientdoes too, including its one-shot handshake and acknowledgement handlers, andclose()closes the socket directly, so one still connecting is closed as well.onModelUpdatemodel listener goes through a controller thatdestroy()aborts, so a model kept by the integrator no longer reaches the callback after teardown.InlineToolbarUIdestroys the inline tool instances behind a popover when it is rebuilt, hidden or destroyed, and those of a render that is superseded.e2e/tests/destroy.spec.tschecks document/window listener cleanup (baseline from a page that never had an editor), that other editors keep working, that holders are left empty, theAbortErrorwhen destroyed before ready (with no unhandled rejection), and that the collaboration socket closes. The fixture exposesEditorJSonwindowand accepts?editors=0.docs/plugins.md,docs/README.mdand the lifecycle diagram describe the teardown contract.chore(openspec): archive editor-destroy), folding the deltas intoarchitecture,core,dom-adapters,editorjs-bundle,sdkandui.ui-kit
The leftover
windowscrolllistener and.ctelement came from ui-kit's hint tooltip. It's fixed upstream incodex-tooltip1.1.0 (codex-team/codex.tooltips#32) and@editorjs/ui-kit2.0.1 (editor-js/utils#43), and this PR bumps@editorjs/ui-kitto^2.0.1.destroy.spec.tsalso checks the case where a hint was shown before the editor is destroyed.Stack
Verification
InlineToolbarUIlines).🤖 Generated with Claude Code