From 960663857a359af9aa991b8c6f3e9ef5a91263fd Mon Sep 17 00:00:00 2001 From: gohabereg Date: Tue, 6 Oct 2026 00:37:02 +0100 Subject: [PATCH 1/4] docs(openspec): propose editor-destroy Add the editor-destroy change: Core/EditorJS destroy() lifecycle, optional destroy() on tools, required destroy() on the adapter plugin, and a repo-wide rule that listeners are released through AbortController (owned by SDK base classes where they exist). Co-Authored-By: Claude Opus 5.5 --- .../changes/editor-destroy/.openspec.yaml | 2 + openspec/changes/editor-destroy/design.md | 126 ++++++++++++++++++ openspec/changes/editor-destroy/proposal.md | 38 ++++++ .../editor-destroy/specs/architecture/spec.md | 21 +++ .../changes/editor-destroy/specs/core/spec.md | 81 +++++++++++ .../editor-destroy/specs/dom-adapters/spec.md | 24 ++++ .../specs/editorjs-bundle/spec.md | 29 ++++ .../changes/editor-destroy/specs/sdk/spec.md | 51 +++++++ .../changes/editor-destroy/specs/ui/spec.md | 19 +++ openspec/changes/editor-destroy/tasks.md | 60 +++++++++ 10 files changed, 451 insertions(+) create mode 100644 openspec/changes/editor-destroy/.openspec.yaml create mode 100644 openspec/changes/editor-destroy/design.md create mode 100644 openspec/changes/editor-destroy/proposal.md create mode 100644 openspec/changes/editor-destroy/specs/architecture/spec.md create mode 100644 openspec/changes/editor-destroy/specs/core/spec.md create mode 100644 openspec/changes/editor-destroy/specs/dom-adapters/spec.md create mode 100644 openspec/changes/editor-destroy/specs/editorjs-bundle/spec.md create mode 100644 openspec/changes/editor-destroy/specs/sdk/spec.md create mode 100644 openspec/changes/editor-destroy/specs/ui/spec.md create mode 100644 openspec/changes/editor-destroy/tasks.md diff --git a/openspec/changes/editor-destroy/.openspec.yaml b/openspec/changes/editor-destroy/.openspec.yaml new file mode 100644 index 00000000..e3966d7a --- /dev/null +++ b/openspec/changes/editor-destroy/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-10-05 diff --git a/openspec/changes/editor-destroy/design.md b/openspec/changes/editor-destroy/design.md new file mode 100644 index 00000000..628aa45a --- /dev/null +++ b/openspec/changes/editor-destroy/design.md @@ -0,0 +1,126 @@ +## Context + +`Core.initialize()` builds the editor in this order: adapter binding → `SelectionManager`, `BlocksManager`, `BlockRenderer` → plugins (in `use()` order) → tools → `UndoRedoManager` → `model.initializeDocument()` → `core:ready`. `#initializePlugin` creates each plugin instance and then drops the reference, keeping only its `publicApi`. That is why no teardown path exists today. + +Resources that outlive an editor if nobody releases them: + +| Owner | Resource | Today | +|---|---|---| +| `EditorjsUI` | wrapper `div` appended to the integrator's holder | `destroy()` is a no-op | +| `BlocksUI` | blocks holder DOM listeners (`beforeinput`, `keydown`, `copy`), block wrappers | fixed in #174 (open), out of scope here | +| `ToolbarUI` / `ToolboxUI` / `InlineToolbarUI` | EventBus listeners, popovers | `destroy()` removes the holder but keeps the listeners | +| `CaretAdapter` | subscription to the singleton `useSelectionChange` watcher (`document` `selectionchange`) and `api.selection.onCaretUpdate` | `@todo Unsubscribe on adapter destruction` | +| `FormattingAdapter` | `core:ToolLoaded` and `api.document.onUpdate` listeners, inline tool instances | never released | +| `DOMAdapters` | `DOMBlockToolAdapter` per block (model and `ui:beforeinput` listeners) | released only on block removal | +| `BlockRenderer` | model listener, block tool instances | instances aren't tracked (`@todo clear block tool adapter memory`) | +| `SelectionManager` | model `CaretManagerUpdated` listener | never released | +| `UndoRedoManager` | model and EventBus listeners, debounce timer | `destroy()` exists, never called | +| `CollaborationManager` | `OTClient` WebSocket, debounce timer, EventBus/model listeners | `destroy()` exists, never called | +| `ClipboardPlugin`, `ShortcutsPlugin` | EventBus listeners | `destroy()` exists, never called | + +The model and the EventBus belong to one `Core` instance. Once nothing outside holds a reference to them, the garbage collector reclaims them along with their listeners. The real leaks are therefore things reachable from outside the editor: `document` and holder listeners, DOM nodes under the integrator's holder, sockets, and timers. We still unsubscribe model and EventBus listeners explicitly, so a destroyed editor stops reacting to anything. That matters when an integrator keeps a reference to `api` or the model, and when an async render settles after teardown. + +## Goals / Non-Goals + +**Goals:** +- `Core#destroy()` / `EditorJS#destroy()` leave nothing behind that can be reached from outside the editor: no DOM, no `document`/holder listeners, no open socket, no pending timer. +- Every editor-owned component stops reacting to events after teardown. +- Tools get a hook (`destroy()`) to release their own resources. +- Several editors on one page can be created and destroyed independently. + +**Non-Goals:** +- Re-initializing a destroyed instance. Integrators construct a new one. +- Async teardown, such as waiting for the socket to finish closing or flushing pending collaboration ops. See Open Questions. +- Detecting listeners that third-party plugins or tools forget to remove. Each plugin and tool is responsible for its own subscriptions. +- Block Tunes teardown. Tunes aren't instantiated per block yet (see the tunes-as-plugins work). + +## Decisions + +### D1. `destroy()` is synchronous +Every existing `destroy()` in the repo is synchronous, and `EditorjsPlugin.destroy?(): void` is already the contract. `OTClient.close()` is fire-and-forget: it closes the socket once its open promise resolves. A synchronous `destroy()` also fits unmount hooks in UI frameworks, which can't await anything. +*Alternative*: `destroy(): Promise` that awaits socket close. Rejected for now because it would change the plugin contract and isn't needed to stop leaks. + +### D2. Core keeps plugin instances and tears down in reverse construction order +`#initializePlugin` pushes each instance onto a `#pluginInstances` array, paired with its `name`. `destroy()` walks the array backwards, calls `destroy?.()` inside a try/catch with `console.error`, and calls `PluginRegistry.unregister(name)`. Reverse order means a plugin that depends on another one's `publicApi` or DOM, like `EditorjsUI` hosting the elements other UI plugins render, is torn down after its dependents. + +### D3. Teardown order inside `Core.destroy()` +1. Set `#destroyed = true`, so `use()` and `initialize()` throw from now on. Plugin instances and services are cleared as they're destroyed, so a second `destroy()` finds nothing left and is a no-op. +2. Plugins, in reverse order (D2). UI disappears first, collaboration closes its socket and stops sending ops, and keyboard and clipboard plugins detach. +3. `UndoRedoManager.destroy()`, then `SelectionManager.destroy()`. +4. `BlockRenderer.destroy()`: for each tracked block, call the tool instance's `destroy?.()`, then `adapter.destroyBlockToolAdapter(id)`. Remove the model listener. No `BlockRemovedCoreEvent` is dispatched, because the UI is already gone. +5. `adapter.destroy()` (D5). + +`destroy()` never resolves a service from the container, because that could construct it. `initialize()` stores each service it resolves (`SelectionManager`, `BlockRenderer`, `UndoRedoManager`, the adapter) in a `#services` record, and teardown only destroys what that record holds. Each step runs through a helper that logs a throwing step and continues. + +The model is never mutated. If `destroy()` cleared blocks through the model, `CollaborationManager` would broadcast the removals to other clients and `UndoRedoManager` would record them. + +### D4. Destroy during a pending `initialize()` +`initialize()` awaits `#initializeTools()`, which awaits each tool's `prepare()`. After that `await`, `initialize()` checks `#destroyed` and stops before tools are constructed, the document is initialized, or `core:ready` is dispatched. `destroy()` tears down whatever stage was reached. + +`initialize()` then **rejects** with a `DOMException` named `AbortError`, following the platform convention (`fetch` rejects the same way when its signal aborts). Code awaiting readiness (loading data, focusing, subscribing) must not run against a destroyed editor, and resolving would let it run. Callers that unmount during boot ignore the error by name: `if (error.name === 'AbortError') return`. This matters in practice: React StrictMode mounts, unmounts and remounts every component in development, so every React integration hits this path. + +`EditorJS` creates its ready promise in the constructor, so an `AbortError` would be reported as an unhandled rejection whenever nobody awaits `isReady`. The bundle attaches a handler to its own ready promise that ignores `AbortError` and logs any other error with `console.error`. Neither is reported as an unhandled rejection, a real initialization failure isn't lost when nobody awaits `isReady`, and anyone awaiting `isReady` sees the rejection either way. + +*Alternative*: rethrow non-abort errors from the handler so Node reports them as unhandled. Rejected because the rethrown copy is a separate promise: a caller who awaits and handles `isReady` would still get an unhandled rejection reported, and Jest or Node with `--unhandled-rejections=throw` would crash on it. The cost of logging is one extra `console.error` for callers who also handle the error themselves. + +*Alternative*: resolve on interruption. Rejected because `isReady` resolving would no longer mean the editor is usable, and the caller couldn't tell "ready" from "cancelled". + +### D5. Adapter plugin gets a required `destroy()` +`EditorJSAdapterPlugin` narrows `destroy?()` to `destroy()`. Core can't release the rendering layer without it, and `DOMAdapters` is the only implementation. `DOMAdapters.destroy()`: +- destroys each remaining entry in `#adapters` through the existing `destroyBlockToolAdapter` path +- calls `CaretAdapter.destroy()`, which aborts its controller. That controller's `abort` event is tied to `off(holder)` on the selection watcher and to the `api.selection.onCaretUpdate` unsubscribe function +- calls `FormattingAdapter.destroy()`, which aborts its controller (covering the `core:ToolLoaded` and `onUpdate` listeners) and calls `destroy?.()` on each attached inline tool + +Singletons are fetched from its private container, where they already exist, so nothing new gets constructed. + +### D6. Reference-count the `selectionchange` watcher +`useSelectionChange` is a module-level singleton shared by every editor on the page, so its `destroy()` would break the other editors. Instead, `off()` removes the `document` listener when the subscriber map becomes empty, and `on()` adds it back when it adds the first subscriber. This lets "no `document` listener left after every editor is destroyed" pass, and it is safe when several editors run at once. + +### D7. Tool instance lifetime rule +Add `destroy?(): void` to `InlineTool`. `BlockTool` already gets it from the v2 type, and we spell it out in the SDK spec. The rule is that every instance the editor creates gets `destroy()` called once, when the editor stops using it: +- `BlockRenderer`: the per-block instance, on block removal or teardown. +- `FormattingAdapter`: the long-lived inline instance, on teardown. +- `InlineToolbarUI`: instances created when building the toolbar, when the toolbar is rebuilt or destroyed. +- `SelectionManager.applyInlineTool` and `ToolsManager` validation create throwaway instances. They call `destroy?.()` right after use, in a `finally`. + +*Alternative*: call `destroy()` only on long-lived instances. Rejected because tool authors would then have to know which instances are which, while a uniform rule works without that knowledge. + +### D8. Listeners are released through `AbortController` across the repo +Every first-party component that registers listeners owns one `AbortController`. It passes the controller's `signal` to every registration and calls `abort()` in its teardown. This covers: +- `EventTarget` registrations: the EventBus, the model, DOM nodes, `document`. +- Subscriptions that return an unsubscribe function: `api.document.onUpdate`, `api.selection.onCaretUpdate`, and the `selectionchange` watcher (which gets `off(holder)` wrapped in a closure). The component registers that function as an `abort` listener on its own signal (a signal aborts only once, so no `{ once: true }` is needed), as in `signal.addEventListener('abort', api.document.onUpdate(cb))`, so nothing is stored in a field. + +No component keeps handler fields or unsubscribe functions just so it can remove them later. `UndoRedoManager`, `ClipboardPlugin`, `ShortcutsPlugin`, `CollaborationManager`, and `DOMBlockToolAdapter` move to this pattern. Non-listener resources (timers, the socket, DOM nodes, popovers, tool instances) are still released explicitly in `destroy()`. + +Needed plumbing: +- `EditorJSModel`'s typed `addEventListener` overloads take an `options?: AddEventListenerOptions` argument and forward it. +- Core's `DocumentAPI.onUpdate` and `SelectionAPI.onCaretUpdate` create a controller per subscription, pass its signal to `model.addEventListener`, and return `() => controller.abort()`. Their public signatures don't change. + +*Alternative*: add an optional `{ signal }` argument to `onUpdate`/`onCaretUpdate`. Rejected because it makes the public API bigger for something one line at the call site already handles. + +*Alternative*: store handlers and remove them by hand, the way `UndoRedoManager` does today. Rejected because a forgotten removal leaks without any warning, and the UI alone would need about 15 handler fields. + +### D8a. SDK base classes own the controller +When the SDK provides a base class that registers listeners, the teardown belongs to the base class, not to its subclasses. Today `BlockToolAdapter` is the only such class. It creates the controller, ties the `onUpdate` unsubscribe function to the controller's `abort` event, exposes `protected readonly signal: AbortSignal`, and aborts it in `destroy()`. `DOMBlockToolAdapter` passes `this.signal` to its `ui:beforeinput` registration and drops its `destroy()` override and its `#beforeInputListener` field. A subclass overrides `destroy()` only to release resources that aren't listeners, and it still calls `super.destroy()`. + +Plugins and tools are interfaces, not base classes, so they own their own controllers. *Alternative*: Core passes each plugin a `signal` in `EditorjsPluginParams` and aborts it after the plugin's `destroy()`. Rejected because it adds to the plugin API. + +### D9. The `EditorJS` bundle delegates +`EditorJS#destroy()` calls `this.#core.destroy()`. It owns nothing else. + +## Risks / Trade-offs + +- [A plugin's `destroy()` throws partway through] → Each plugin call is wrapped in its own try/catch and logged, and teardown continues. The editor may still be left partly torn down, which beats aborting and leaking everything. +- [An async `render()` settles after teardown and dispatches `BlockAddedCoreEvent` into a dead EventBus] → `BlockRenderer` checks a `#destroyed` flag after the `await`, then destroys the late instance and its adapter instead of dispatching. +- [Ops not yet sent in `CollaborationManager`'s debounced batch are lost on destroy] → This already happens when a tab closes. Flushing would need async teardown (Non-Goal) and is listed in Open Questions. +- [The adapter contract narrows `destroy?()` to `destroy()`, which is a type-level break for third-party adapters] → No third-party adapters exist yet, and the package is pre-1.0. +- [#174 overlaps with the UI teardown] → This change doesn't touch `BlocksUI`. It shares two pieces with #174: the UI package's Jest setup (kept byte-identical, so either merge order folds cleanly) and the `EditorjsUI.destroy()` wrapper removal (same line, plus the controller abort here). Whichever lands second resolves that one hunk. + +## Migration Plan + +Additive for integrators: call `editor.destroy()` on unmount. No data migration and nothing to roll back beyond reverting the PR. + +## Open Questions + +- Should `destroy()` become async later, to flush the pending collaboration batch and await socket close? If so, `EditorjsPlugin.destroy` would need to return `void | Promise`. +- Should the API expose a `core:destroyed` event for third-party code that subscribed through `api` without being a plugin? It isn't needed for the built-ins. diff --git a/openspec/changes/editor-destroy/proposal.md b/openspec/changes/editor-destroy/proposal.md new file mode 100644 index 00000000..468eedc5 --- /dev/null +++ b/openspec/changes/editor-destroy/proposal.md @@ -0,0 +1,38 @@ +## Why + +There is no way to tear an editor down. Neither `Core` nor `EditorJS` has a `destroy()`, so an integrator who unmounts an editor (SPA route change, React/Vue component unmount, several editors on one page) leaks the editor DOM, a document-wide `selectionchange` listener, the collaboration WebSocket, timers, and every tool and plugin instance. Several plugins already implement `destroy()` and the `core` and `sdk` specs already describe it being called on teardown ("Registry is cleared on teardown", "Destroying a plugin"), but nothing calls it. This change makes those requirements true. + +## What Changes + +- Add `destroy(): void` to `Core`. It is synchronous, safe to call more than once, and safe to call while `initialize()` is still pending, in which case the pending `initialize()` rejects with an `AbortError`. After it returns, `use()` and `initialize()` throw. +- `Core` keeps the plugin instances it creates. `destroy()` calls `destroy()` on each of them in reverse construction order and removes each one's entry from the plugin registry. A plugin whose `destroy()` throws is logged and doesn't stop the rest of the teardown. +- `Core` tears down its own services: `UndoRedoManager` (already has `destroy()`), `BlockRenderer` and `SelectionManager` (new), each removing its model and EventBus listeners. +- `BlockRenderer` keeps the block tool instance for each rendered block. When a block is removed, and when the editor is destroyed, it calls the tool's `destroy()` and destroys the block's adapter. Teardown does not touch the model, so no removal operations reach undo history or collaboration. +- Add an optional `destroy()` to the `InlineTool` contract in `@editorjs/sdk`. `BlockTool` already inherits an optional `destroy()` from the v2 type; the SDK spec now names both. The editor calls `destroy()` once on each tool instance it creates, when it stops using that instance. +- Add `destroy()` to the adapter plugin. `DOMAdapters.destroy()` destroys the remaining block adapters, stops `CaretAdapter` watching selection changes and caret updates, and stops `FormattingAdapter` listening to tool and model events (destroying its inline tool instances). The shared `selectionchange` watcher removes its `document` listener once its last subscriber is gone. +- UI plugins other than `BlocksUI` remove their EventBus listeners in `destroy()`, and `EditorjsUI.destroy()` removes the editor wrapper from the holder. The holder element itself belongs to the integrator and stays in place. `BlocksUI` teardown is covered by #174 (fixes #170), so this change has no `BlocksUI` implementation. +- Listeners are released through `AbortController` across the repo. Each component owns one controller, passes its `signal` to every listener it registers, and calls `abort()` on teardown. When an SDK base class registers listeners (`BlockToolAdapter`), the base class owns the controller, gives subclasses a protected `signal`, and aborts it in its own `destroy()`. Subclasses don't override `destroy()` to remove listeners. +- The `EditorAPI` subscription methods keep their signatures. Internally `onUpdate` and `onCaretUpdate` subscribe through their own controller and return `() => controller.abort()`. Components tie that function to their own controller's `abort` event. +- Add `destroy()` to the `EditorJS` bundle, which delegates to `Core.destroy()`. Because `CollaborationManager.destroy()` already closes the `OTClient` socket, the bundle closes its collaboration connection too. + +## Capabilities + +### New Capabilities + +None. + +### Modified Capabilities + +- `architecture`: adds the repo-wide rule that listeners are released through `AbortController`, owned by the SDK base class where one exists. +- `core`: adds an editor teardown requirement to `Core` (idempotency, ordering, mid-initialization destroy, post-destroy guards) and adds per-block tool destruction to the block rendering lifecycle. +- `sdk`: `BlockToolAdapter` owns its controller and exposes `signal` to subclasses, the tool contracts gain an optional `destroy()` on block and inline tools, and the adapter plugin contract gains a teardown that covers adapters not destroyed yet. +- `dom-adapters`: `DOMAdapters` releases the caret, selection, and formatting subscriptions and the block adapters on destroy, and the shared selection watcher detaches from `document` when it has no subscribers left. +- `ui`: shell assembly gains teardown: the wrapper is removed and UI plugins unsubscribe from the EventBus. +- `editorjs-bundle`: `EditorJS` exposes `destroy()`. + +## Impact + +- **Code**: `packages/core/src/index.ts`, `packages/core/src/components/{BlockRenderer,SelectionManager}.ts`, `packages/sdk/src/entities/{InlineTool,EditorjsAdapterPlugin,BlockToolAdapter}.ts`, `packages/core/src/api/{DocumentAPI/DocumentAPI,SelectionAPI}.ts`, `packages/core/src/components/UndoRedoManager.ts`, `packages/model/src/EditorJSModel.ts` (`addEventListener` overloads gain `options`), `packages/plugins/{clipboard-plugin,shortcuts-plugin}`, `packages/collaboration-manager/src/CollaborationManager.ts`, `packages/dom-adapters/src/{index.ts,CaretAdapter,FormattingAdapter,utils/useSelectionChange.ts}`, `packages/ui/src/**` (each plugin's `destroy()`), `packages/editorjs/src/index.ts`. +- **Public API**: adds `Core#destroy()`, `EditorJS#destroy()`, optional `InlineTool#destroy()`, and `EditorJSAdapterPlugin#destroy()`. Nothing breaks: the tool hooks are optional, and `DOMAdapters` is the only adapter implementation in the repo. +- **Docs**: `docs/plugins.md` ("Lifecycle boundary") says "`Core` currently does not expose a global `destroy()` lifecycle hook". This change replaces that sentence. `docs/README.md` ("Lifecycle") gains a teardown step. +- **Related work**: #174 (BlocksUI holder listeners), card "Clean up" on the DocumentModel board. diff --git a/openspec/changes/editor-destroy/specs/architecture/spec.md b/openspec/changes/editor-destroy/specs/architecture/spec.md new file mode 100644 index 00000000..e8633ab1 --- /dev/null +++ b/openspec/changes/editor-destroy/specs/architecture/spec.md @@ -0,0 +1,21 @@ +## ADDED Requirements + +### Requirement: Listener teardown through AbortController +Every first-party component that registers event listeners (core services, adapters, plugins, UI, and SDK base classes) SHALL own an `AbortController`, pass its `signal` to each listener registration it makes, and release all of those listeners by calling `abort()` in its teardown. This applies to `EventTarget` registrations (EventBus, the model, DOM nodes, `document`, `window`). For subscriptions that return an unsubscribe function (`api.document.onUpdate`, `api.selection.onCaretUpdate`, the `selectionchange` watcher), the component SHALL register that function as an `abort` listener on its controller's `signal`. A component SHALL NOT store handler references or unsubscribe functions in fields only to remove listeners. Resources that aren't listeners, such as timers, sockets, DOM nodes, popovers, and tool instances, are released explicitly. + +When an SDK base class registers listeners, the base class SHALL own the controller, expose its `signal` to subclasses as a protected member, and abort it in its own `destroy()`. Subclasses SHALL pass that `signal` to their own registrations rather than creating a second controller or overriding `destroy()` to remove listeners. + +#### Scenario: Teardown removes every listener at once +- **GIVEN** a component that registered listeners on the EventBus, the model, and a DOM node using its controller's `signal` +- **WHEN** its teardown calls `abort()` +- **THEN** none of those listeners fire for events dispatched afterwards + +#### Scenario: Subscription that returns an unsubscribe function +- **GIVEN** a component subscribed through `api.document.onUpdate` and registered the returned function as an `abort` listener on its controller's `signal` +- **WHEN** the component's teardown calls `abort()` +- **THEN** the subscription is removed, and later model changes don't reach its callback + +#### Scenario: Subclass of an SDK base class +- **GIVEN** a class extending an SDK base class registers an extra EventBus listener with the inherited `signal` +- **WHEN** the base class's `destroy()` runs +- **THEN** both the base class's listeners and the subclass's listener are removed, and the subclass doesn't need to override `destroy()` to do it diff --git a/openspec/changes/editor-destroy/specs/core/spec.md b/openspec/changes/editor-destroy/specs/core/spec.md new file mode 100644 index 00000000..f4fb9620 --- /dev/null +++ b/openspec/changes/editor-destroy/specs/core/spec.md @@ -0,0 +1,81 @@ +## ADDED Requirements + +### Requirement: Editor teardown +`Core` SHALL expose a synchronous `destroy()` that releases everything the editor created: plugin instances, the adapter plugin, block and inline tool instances, block adapters, and every model, EventBus, DOM, timer, and network subscription that editor-owned code registered. Teardown SHALL NOT mutate the document model, so no removal events reach undo history, collaboration, or `onModelUpdate`. The holder element passed in the configuration belongs to the integrator and SHALL be left in the DOM. + +#### Scenario: Plugins are destroyed in reverse construction order +- **GIVEN** plugins A, B, and C were constructed in that order during `initialize()` +- **WHEN** `destroy()` is called +- **THEN** `destroy()` is called on C, then B, then A, and plugins that don't implement `destroy()` are skipped + +#### Scenario: Plugin registry entries are removed +- **GIVEN** a plugin registered a `publicApi` under its `name` +- **WHEN** `destroy()` is called +- **THEN** that name is no longer present on `api.plugins` + +#### Scenario: A failing plugin does not abort teardown +- **GIVEN** one plugin's `destroy()` throws +- **WHEN** `destroy()` is called +- **THEN** the error is logged with `console.error`, and every other plugin, the core services, the rendered blocks, and the adapter plugin are still torn down + +#### Scenario: Core services stop listening +- **GIVEN** an initialized editor +- **WHEN** `destroy()` is called +- **THEN** `UndoRedoManager`, `BlockRenderer`, and `SelectionManager` remove their model and EventBus listeners, so model changes or `core:undo`/`core:redo` events dispatched afterwards have no effect on them + +#### Scenario: Rendered blocks are released without touching the model +- **GIVEN** an initialized editor with rendered blocks +- **WHEN** `destroy()` is called +- **THEN** each rendered block tool's `destroy()` is called, each block's adapter is destroyed, the adapter plugin's `destroy()` is called, and the model emits no events + +#### Scenario: Destroying twice is a no-op +- **GIVEN** `destroy()` has already been called +- **WHEN** `destroy()` is called again +- **THEN** it returns without calling any plugin, tool, or adapter `destroy()` a second time + +#### Scenario: Destroying before initialization +- **GIVEN** `use()` has been called but `initialize()` has not +- **WHEN** `destroy()` is called +- **THEN** it completes without error and no plugin or tool is constructed + +#### Scenario: Destroying while initialization is pending +- **GIVEN** `initialize()` has been called and its promise has not settled +- **WHEN** `destroy()` is called +- **THEN** whatever was constructed so far is torn down, the pending `initialize()` rejects with a `DOMException` named `AbortError` without initializing the document or dispatching `CoreEventType.Ready`, and any tool or plugin it would have constructed afterwards is not constructed + +#### Scenario: Using a destroyed editor +- **GIVEN** `destroy()` has been called +- **WHEN** `use()` or `initialize()` is called +- **THEN** it throws an error stating that the editor has been destroyed + +## MODIFIED Requirements + +### Requirement: Block rendering lifecycle +The system SHALL provide `BlockRenderer`, which listens for model `BlockAddedEvent`/`BlockRemovedEvent`, creates a `BlockToolAdapter` per block, instantiates and renders the corresponding tool, and dispatches `BlockAddedCoreEvent`/`BlockRemovedCoreEvent`. `BlockRenderer` SHALL keep the tool instance of each rendered block, keyed by block id, and SHALL call that instance's optional `destroy()` exactly once, either when the block is removed or when the editor is destroyed. + +#### Scenario: Malformed or unknown block events +- **GIVEN** a block event with an undefined index, or referencing a tool name that isn't registered +- **WHEN** `BlockRenderer` processes the event +- **THEN** it throws `[BlockRenderer] Block index should be defined...` or `[BlockRenderer] Block Tool not found` respectively + +#### Scenario: Tool render failure is logged, not thrown +- **GIVEN** a tool's `render()` method returns a rejected promise +- **WHEN** `BlockRenderer` renders that block +- **THEN** the rejection is logged rather than propagated as an uncaught error + +#### Scenario: Removing a block destroys its tool instance +- **GIVEN** a rendered block whose tool instance implements `destroy()` +- **WHEN** the model emits `BlockRemovedEvent` for that block +- **THEN** the tool instance's `destroy()` is called, the block's adapter is destroyed, and `BlockRemovedCoreEvent` is dispatched + +#### Scenario: Destroying the renderer releases every remaining block +- **GIVEN** several rendered blocks +- **WHEN** `BlockRenderer.destroy()` is called +- **THEN** each remaining tool instance's `destroy()` is called, each remaining block's adapter is destroyed, no `BlockRemovedCoreEvent` is dispatched, and the renderer stops listening to model events + +#### Scenario: A block that finishes rendering after teardown +- **GIVEN** a block tool's `render()` is still pending when `BlockRenderer.destroy()` is called +- **WHEN** `render()` settles +- **THEN** no `BlockAddedCoreEvent` is dispatched for it, and the tool instance and its adapter are still destroyed + +Implemented in `src/components/BlockRenderer.ts`, validated by its co-located `.spec.ts`. diff --git a/openspec/changes/editor-destroy/specs/dom-adapters/spec.md b/openspec/changes/editor-destroy/specs/dom-adapters/spec.md new file mode 100644 index 00000000..e7c2ff50 --- /dev/null +++ b/openspec/changes/editor-destroy/specs/dom-adapters/spec.md @@ -0,0 +1,24 @@ +## ADDED Requirements + +### Requirement: Adapter teardown +`DOMAdapters` SHALL implement `destroy()`. It SHALL destroy every `DOMBlockToolAdapter` still alive and clear the `InputsRegistry`, stop `CaretAdapter` watching DOM selection changes and model caret updates, stop `FormattingAdapter` listening to `core:ToolLoaded` and model updates, and call `destroy()` on the inline tool instances `FormattingAdapter` holds. The shared `selectionchange` watcher SHALL remove its `document` listener once its last subscriber unsubscribes, and add it back on the next subscription. + +#### Scenario: Caret adapter stops tracking the holder +- **GIVEN** an initialized `DOMAdapters` +- **WHEN** `destroy()` is called and a `selectionchange` event then fires inside the holder +- **THEN** the model caret isn't updated, and later model caret updates don't change the DOM selection + +#### Scenario: Remaining block adapters are destroyed +- **GIVEN** block adapters created for several blocks that haven't been removed +- **WHEN** `destroy()` is called +- **THEN** each adapter's `destroy()` runs, removing its model and `ui:beforeinput` listeners, and the inputs registry is empty + +#### Scenario: Formatting adapter releases inline tools +- **GIVEN** inline tools attached to `FormattingAdapter` through `core:ToolLoaded` +- **WHEN** `destroy()` is called +- **THEN** each attached inline tool instance's `destroy()` is called, and later model formatting events don't touch the DOM + +#### Scenario: Two editors on one page +- **GIVEN** two editors are initialized on the same page and share the `selectionchange` watcher +- **WHEN** the first editor is destroyed +- **THEN** the second editor still receives selection changes, and the `document` listener is removed only after the second editor is destroyed too diff --git a/openspec/changes/editor-destroy/specs/editorjs-bundle/spec.md b/openspec/changes/editor-destroy/specs/editorjs-bundle/spec.md new file mode 100644 index 00000000..886d6a5e --- /dev/null +++ b/openspec/changes/editor-destroy/specs/editorjs-bundle/spec.md @@ -0,0 +1,29 @@ +## ADDED Requirements + +### Requirement: Editor teardown +The `EditorJS` class SHALL expose a synchronous `destroy()` that delegates to the underlying `Core.destroy()`. This tears down the default UI, plugins, tools, DOM adapter, and the collaboration connection. + +#### Scenario: Destroying a ready editor +- **GIVEN** `editor.isReady` has resolved +- **WHEN** `editor.destroy()` is called +- **THEN** the editor's DOM is removed from the holder, its collaboration socket is closed, and no listener it added to `document` or the holder remains + +#### Scenario: Destroying before ready +- **GIVEN** `editor.isReady` has not settled +- **WHEN** `editor.destroy()` is called +- **THEN** `editor.isReady` rejects with a `DOMException` named `AbortError`, the editor never becomes interactive, and nothing is left in the holder + +#### Scenario: Destroying before ready without awaiting readiness +- **GIVEN** nobody awaits or handles `editor.isReady` +- **WHEN** `editor.destroy()` is called before it settles +- **THEN** the `AbortError` rejection is not reported as an unhandled promise rejection + +#### Scenario: Initialization failure without awaiting readiness +- **GIVEN** nobody awaits or handles `editor.isReady` +- **WHEN** initialization fails with an error other than `AbortError` +- **THEN** that error is logged with `console.error`, is not reported as an unhandled promise rejection, and `editor.isReady` still rejects with it for anyone who awaits it + +#### Scenario: Several editors on one page +- **GIVEN** several `EditorJS` instances are mounted on one page +- **WHEN** each of them is destroyed +- **THEN** each destroy leaves the others working, and once all are destroyed no editor DOM nodes or editor-registered `document`/holder listeners remain diff --git a/openspec/changes/editor-destroy/specs/sdk/spec.md b/openspec/changes/editor-destroy/specs/sdk/spec.md new file mode 100644 index 00000000..ddd7a26f --- /dev/null +++ b/openspec/changes/editor-destroy/specs/sdk/spec.md @@ -0,0 +1,51 @@ +## ADDED Requirements + +### Requirement: Tool instance teardown hook +The `BlockTool` and `InlineTool` instance contracts SHALL each declare an optional `destroy(): void`. The editor SHALL call `destroy()` exactly once on every tool instance it creates, when it stops using that instance: for long-lived instances, when the owning block is removed, the owning component is rebuilt, or the editor is destroyed; for instances created for a single operation, right after that operation completes. A tool that doesn't implement `destroy()` SHALL keep working unchanged. + +#### Scenario: Inline tool declares destroy +- **GIVEN** an inline tool class implements `destroy()` +- **WHEN** it is type-checked against `InlineToolConstructor` +- **THEN** it compiles, and an inline tool without `destroy()` also compiles + +#### Scenario: Block tool declares destroy +- **GIVEN** a block tool class implements `destroy()` +- **WHEN** it is type-checked against `BlockToolConstructor` +- **THEN** it compiles, and a block tool without `destroy()` also compiles + +## MODIFIED Requirements + +### Requirement: BlockToolAdapter bridges model events to per-block DOM state +The system SHALL provide an abstract `BlockToolAdapter` (extending `EventTarget`) that subscribes to `api.document.onUpdate`, translates model-level `DataNodeAddedEvent`/`DataNodeRemovedEvent`/`ValueModifiedEvent` into adapter-level `KeyAddedEvent`/`KeyRemovedEvent`/`ValueNodeChangedEvent` dispatched on itself, and exposes `registerTextInputKey`, `registerValueKey`, `removeKey`, and `destroy()`. `BlockToolAdapter` SHALL own an `AbortController`, tie its `api.document.onUpdate` unsubscribe function to that controller's `abort` event, expose its `signal` to subclasses as a protected member, and abort it in `destroy()`. + +#### Scenario: Cleaning up an adapter +- **GIVEN** a `BlockToolAdapter` instance is subscribed to model updates +- **WHEN** `destroy()` is called +- **THEN** its controller aborts, so its model listener is removed and no further translated events are dispatched + +#### Scenario: Subclass listeners are removed by the base class +- **GIVEN** a `BlockToolAdapter` subclass registered a listener with the inherited `signal` +- **WHEN** `destroy()` is called on the subclass instance +- **THEN** that listener is removed too, even though the subclass doesn't override `destroy()` + +Implemented in `src/entities/BlockToolAdapter.ts`. + +### Requirement: Plugin contracts +The system SHALL define `EditorjsPlugin`/`EditorjsPluginConstructor` (generic UI-plugin contract with optional `destroy()`, an optional `publicApi` member, a static `type`, and a static `name`) and `EditorJSAdapterPlugin`/`EditorjsAdapterPluginConstructor` (singleton adapter plugin contract with `createBlockToolAdapter`/`destroyBlockToolAdapter` and a required `destroy()` that releases every block adapter not destroyed yet, along with the adapter's own subscriptions). + +#### Scenario: Destroying a plugin +- **GIVEN** a registered `EditorjsPlugin` implements an optional `destroy()` method +- **WHEN** the editor tears down +- **THEN** `destroy()` is called on the plugin instance to release its resources + +#### Scenario: Destroying the adapter plugin +- **GIVEN** an adapter plugin with block adapters created through `createBlockToolAdapter` +- **WHEN** the editor tears down +- **THEN** the adapter plugin's `destroy()` is called after every rendered block has been released, and it destroys any block adapter still alive + +#### Scenario: Plugin declares an id and a public API +- **GIVEN** a plugin class declares a static `name` and an instance `publicApi` member +- **WHEN** its type is checked against `EditorjsPluginConstructor` +- **THEN** the `name` literal and the `publicApi` type are both inferred, and a mismatch with the plugin's `EditorjsPluginApiMap` augmentation is a compile error + +Implemented in `src/entities/EditorjsPlugin.ts`, `src/entities/EditorjsAdapterPlugin.ts`, `src/entities/EntityType.ts`. diff --git a/openspec/changes/editor-destroy/specs/ui/spec.md b/openspec/changes/editor-destroy/specs/ui/spec.md new file mode 100644 index 00000000..972d41a1 --- /dev/null +++ b/openspec/changes/editor-destroy/specs/ui/spec.md @@ -0,0 +1,19 @@ +## ADDED Requirements + +### Requirement: UI teardown +Every UI plugin other than `BlocksUI` (`EditorjsUI`, `ToolbarUI`, `ToolboxUI`, `InlineToolbarUI`) SHALL remove each EventBus listener it registered and every DOM node it created when its `destroy()` is called. `EditorjsUI.destroy()` SHALL remove the editor wrapper from the holder and leave the holder element and any content the integrator put in it untouched. `BlocksUI` teardown is covered by #174 (the `remove-blocks-ui-listeners-on-destroy` change), so this change has no `BlocksUI` implementation. + +#### Scenario: The editor wrapper is removed +- **GIVEN** `EditorjsUI` appended its wrapper to the holder +- **WHEN** `destroy()` is called +- **THEN** the wrapper is no longer in the DOM, and the holder stays in the DOM with any of its other children + +#### Scenario: UI plugins stop reacting to events +- **GIVEN** `EditorjsUI`, `ToolbarUI`, `ToolboxUI`, and `InlineToolbarUI` are destroyed +- **WHEN** `ui:*` or `core:*` events they used to handle are dispatched on the EventBus +- **THEN** no UI plugin renders, appends, or dispatches anything in response + +#### Scenario: Popovers are released +- **GIVEN** the toolbox or inline toolbar created a popover +- **WHEN** the owning plugin is destroyed +- **THEN** the popover is destroyed and its DOM removed diff --git a/openspec/changes/editor-destroy/tasks.md b/openspec/changes/editor-destroy/tasks.md new file mode 100644 index 00000000..1b333d40 --- /dev/null +++ b/openspec/changes/editor-destroy/tasks.md @@ -0,0 +1,60 @@ +## 1. SDK contracts + +- [ ] 1.1 Add optional `destroy?(): void` to `InlineTool` and declare it explicitly on `BlockTool` in `packages/sdk/src/entities/{InlineTool,BlockTool}.ts`, with type tests that tools with and without `destroy()` both satisfy their constructor types +- [ ] 1.2 Narrow `EditorJSAdapterPlugin` to a required `destroy(): void` in `packages/sdk/src/entities/EditorjsAdapterPlugin.ts`, and update the test adapter mocks in `packages/core` to implement it + +## 2. Signal plumbing (AbortController rule, design D8/D8a) + +- [ ] 2.1 Add an `options?: AddEventListenerOptions` argument to `EditorJSModel`'s typed `addEventListener` overloads and forward it +- [ ] 2.2 Keep the core `DocumentAPI.onUpdate` and `SelectionAPI.onCaretUpdate` tests green (`should stop calling the callback after the returned function is called`) while switching the implementation to a controller per subscription that returns `() => controller.abort()` +- [ ] 2.3 Write failing `BlockToolAdapter` tests: `should stop dispatching translated events after destroy`, and `should remove a subclass listener registered with the inherited signal on destroy`. Give the base class its own controller and a `protected readonly signal`, and abort it in `destroy()` +- [ ] 2.4 Make `DOMBlockToolAdapter` pass `this.signal` to its `ui:beforeinput` registration, and remove its `destroy()` override and `#beforeInputListener` field +- [ ] 2.5 Move `UndoRedoManager`, `ClipboardPlugin`, `ShortcutsPlugin`, and `CollaborationManager` listener cleanup to one controller each. Keep their existing destroy tests green, and keep explicit cleanup only for timers and the socket + +## 3. Selection watcher reference counting (dom-adapters) + +- [ ] 3.1 Write failing tests in `useSelectionChange.spec.ts`: `should remove the document listener when the last subscriber unsubscribes`, and `should re-add the document listener when a subscriber is added after all were removed` +- [ ] 3.2 Make `off()` detach the `document` listener when the subscriber map empties, and `on()` attach it when the first subscriber is added + +## 4. DOMAdapters teardown + +- [ ] 4.1 Write failing `CaretAdapter` tests: `should stop updating the model caret on selectionchange after destroy`, and `should not touch the DOM selection on model caret updates after destroy` +- [ ] 4.2 Implement `CaretAdapter.destroy()` by aborting its controller, whose `abort` event is tied to `off(holder)` and the `onCaretUpdate` unsubscribe function. Remove the `@todo` +- [ ] 4.3 Write failing `FormattingAdapter` tests: `should call destroy on attached inline tools`, `should ignore core:ToolLoaded after destroy`, and `should ignore model updates after destroy` +- [ ] 4.4 Implement `FormattingAdapter.destroy()`: abort its controller, then destroy the inline tool instances +- [ ] 4.5 Write failing `DOMAdapters` tests: `should destroy every remaining block adapter on destroy`, `should leave the inputs registry empty after destroy`, and `should destroy caret and formatting adapters on destroy` +- [ ] 4.6 Implement `DOMAdapters.destroy()` + +## 5. Core services teardown + +- [ ] 5.1 Write failing `BlockRenderer` tests: `should call tool destroy when its block is removed`, `should destroy every remaining tool and block adapter on destroy without dispatching BlockRemovedCoreEvent`, `should stop handling model events after destroy`, and `should not dispatch BlockAddedCoreEvent for a render that settles after destroy` +- [ ] 5.2 Track tool instances per block id in `BlockRenderer`, subscribe to the model with a controller's signal, add `destroy()`, and remove the `@todo clear block tool adapter memory` +- [ ] 5.3 Write a failing `SelectionManager` test, `should stop handling caret updates after destroy`. Implement `SelectionManager.destroy()` by aborting its controller +- [ ] 5.4 Write a failing test, `should destroy the throwaway inline tool instance after applying it`, for `SelectionManager.applyInlineTool`, and do the same for the instance created during `ToolsManager` validation. Call `destroy?.()` in a `finally` + +## 6. Core.destroy() + +- [ ] 6.1 Write failing `Core` tests (`packages/core/src/index.spec.ts`): `should call plugin destroy in reverse construction order`, `should unregister plugin public APIs`, `should continue teardown and log when a plugin destroy throws`, `should destroy UndoRedoManager, SelectionManager, BlockRenderer and then the adapter`, `should not emit model events during destroy`, and `should be a no-op when called twice` +- [ ] 6.2 Keep plugin instances in `#initializePlugin`, track the initialization stage, and implement `destroy()` in the order from design D3 +- [ ] 6.3 Write failing tests: `should not construct anything when destroyed before initialize`, `should reject initialize with an AbortError without dispatching ready when destroyed mid-initialization`, and `should throw from use and initialize after destroy` +- [ ] 6.4 Implement the `#destroyed` checks after each `await` in `initialize()`, and the post-destroy guards in `use()` and `initialize()` + +## 7. UI teardown + +- [ ] 7.1 Leave `BlocksUI` to #174, which covers its teardown, so this change has no `BlocksUI` implementation. Reuse #174's UI Jest setup byte-for-byte so the two merge cleanly +- [ ] 7.2 Give each UI plugin except `BlocksUI` (`EditorjsUI`, `ToolbarUI`, `ToolboxUI`, `InlineToolbarUI`) an `AbortController` and pass its `signal` to every EventBus and DOM `addEventListener`. Call `abort()` in `destroy()` +- [ ] 7.3 `EditorjsUI.destroy()` removes the wrapper and leaves the holder in place. `ToolboxUI` and `InlineToolbarUI` destroy their popovers. `InlineToolbarUI` calls `destroy?.()` on the inline tool instances it created, both on rebuild and on destroy +- [ ] 7.4 Add the first UI specs (`packages/ui`): `should remove the editor wrapper but keep the holder on destroy`, and `should not react to EventBus events after destroy` for each plugin + +## 8. Bundle and integration + +- [ ] 8.1 Write a failing `EditorJS` test, `should delegate destroy to Core`, and implement `EditorJS#destroy()`. Add tests that an `AbortError` from `isReady` isn't reported as an unhandled rejection while any other initialization error is logged, and handle that on the bundle's ready promise +- [ ] 8.2 Write the integration test in `packages/editorjs`, `should leave no DOM nodes or document/holder listeners after creating and destroying several editors on one page`, spying on `document`/holder `add/removeEventListener` and checking that the counts balance and the holders are empty +- [ ] 8.3 Add an integration test that destroys the editor before `isReady` settles and checks that `isReady` rejects with an `AbortError` and nothing is rendered +- [ ] 8.4 Check that `CollaborationManager.destroy()` closes the `OTClient` socket when it's reached through `EditorJS#destroy()` (mock WebSocket) + +## 9. Docs and wrap-up + +- [ ] 9.1 Replace the "`Core` currently does not expose a global `destroy()`" line in `docs/plugins.md` with the teardown contract (order, idempotency, tools' `destroy()`), and add a teardown step to the lifecycle in `docs/README.md`. Update `docs/diagrams/plugin-lifecycle-flow.mmd` +- [ ] 9.2 Run `yarn lint` and the test suites for every touched package +- [ ] 9.3 Run `openspec validate editor-destroy --type change` and make sure it passes From 65f6c062ed10dcbdcc065f0873ffa504b2ec704b Mon Sep 17 00:00:00 2001 From: gohabereg Date: Tue, 6 Oct 2026 18:30:07 +0100 Subject: [PATCH 2/4] docs(openspec): leave UI plugin teardown to #174 in editor-destroy #174 now covers teardown for every UI plugin (listeners, holders, popovers, the EditorjsUI wrapper, pending inline toolbar config). Drop the ui delta and the UI tasks; keep only InlineToolbarUI destroying its inline tool instances, done after #174 lands. Co-Authored-By: Claude Opus 5.5 --- openspec/changes/editor-destroy/design.md | 8 ++++---- openspec/changes/editor-destroy/proposal.md | 7 +++---- .../changes/editor-destroy/specs/ui/spec.md | 19 ------------------- openspec/changes/editor-destroy/tasks.md | 8 +++----- 4 files changed, 10 insertions(+), 32 deletions(-) delete mode 100644 openspec/changes/editor-destroy/specs/ui/spec.md diff --git a/openspec/changes/editor-destroy/design.md b/openspec/changes/editor-destroy/design.md index 628aa45a..9ba5672e 100644 --- a/openspec/changes/editor-destroy/design.md +++ b/openspec/changes/editor-destroy/design.md @@ -6,9 +6,9 @@ Resources that outlive an editor if nobody releases them: | Owner | Resource | Today | |---|---|---| -| `EditorjsUI` | wrapper `div` appended to the integrator's holder | `destroy()` is a no-op | -| `BlocksUI` | blocks holder DOM listeners (`beforeinput`, `keydown`, `copy`), block wrappers | fixed in #174 (open), out of scope here | -| `ToolbarUI` / `ToolboxUI` / `InlineToolbarUI` | EventBus listeners, popovers | `destroy()` removes the holder but keeps the listeners | +| `EditorjsUI` | wrapper `div` appended to the integrator's holder | covered by #174, out of scope here | +| `BlocksUI` | blocks holder DOM listeners (`beforeinput`, `keydown`, `copy`), block wrappers | covered by #174, out of scope here | +| `ToolbarUI` / `ToolboxUI` / `InlineToolbarUI` | EventBus listeners, popovers | covered by #174, out of scope here; `InlineToolbarUI`'s inline tool instances are handled here after #174 lands (D7) | | `CaretAdapter` | subscription to the singleton `useSelectionChange` watcher (`document` `selectionchange`) and `api.selection.onCaretUpdate` | `@todo Unsubscribe on adapter destruction` | | `FormattingAdapter` | `core:ToolLoaded` and `api.document.onUpdate` listeners, inline tool instances | never released | | `DOMAdapters` | `DOMBlockToolAdapter` per block (model and `ui:beforeinput` listeners) | released only on block removal | @@ -114,7 +114,7 @@ Plugins and tools are interfaces, not base classes, so they own their own contro - [An async `render()` settles after teardown and dispatches `BlockAddedCoreEvent` into a dead EventBus] → `BlockRenderer` checks a `#destroyed` flag after the `await`, then destroys the late instance and its adapter instead of dispatching. - [Ops not yet sent in `CollaborationManager`'s debounced batch are lost on destroy] → This already happens when a tab closes. Flushing would need async teardown (Non-Goal) and is listed in Open Questions. - [The adapter contract narrows `destroy?()` to `destroy()`, which is a type-level break for third-party adapters] → No third-party adapters exist yet, and the package is pre-1.0. -- [#174 overlaps with the UI teardown] → This change doesn't touch `BlocksUI`. It shares two pieces with #174: the UI package's Jest setup (kept byte-identical, so either merge order folds cleanly) and the `EditorjsUI.destroy()` wrapper removal (same line, plus the controller abort here). Whichever lands second resolves that one hunk. +- [#174 overlaps with UI teardown] → #174 covers every UI plugin's teardown, so this change has no UI plugin implementation. The only UI code here is `InlineToolbarUI` destroying its inline tool instances, which is done after #174 lands because #174 rewrites that file. Both changes edit the same `docs/plugins.md` lifecycle line, so whichever lands second merges that one line. ## Migration Plan diff --git a/openspec/changes/editor-destroy/proposal.md b/openspec/changes/editor-destroy/proposal.md index 468eedc5..a4debfaa 100644 --- a/openspec/changes/editor-destroy/proposal.md +++ b/openspec/changes/editor-destroy/proposal.md @@ -10,7 +10,7 @@ There is no way to tear an editor down. Neither `Core` nor `EditorJS` has a `des - `BlockRenderer` keeps the block tool instance for each rendered block. When a block is removed, and when the editor is destroyed, it calls the tool's `destroy()` and destroys the block's adapter. Teardown does not touch the model, so no removal operations reach undo history or collaboration. - Add an optional `destroy()` to the `InlineTool` contract in `@editorjs/sdk`. `BlockTool` already inherits an optional `destroy()` from the v2 type; the SDK spec now names both. The editor calls `destroy()` once on each tool instance it creates, when it stops using that instance. - Add `destroy()` to the adapter plugin. `DOMAdapters.destroy()` destroys the remaining block adapters, stops `CaretAdapter` watching selection changes and caret updates, and stops `FormattingAdapter` listening to tool and model events (destroying its inline tool instances). The shared `selectionchange` watcher removes its `document` listener once its last subscriber is gone. -- UI plugins other than `BlocksUI` remove their EventBus listeners in `destroy()`, and `EditorjsUI.destroy()` removes the editor wrapper from the holder. The holder element itself belongs to the integrator and stays in place. `BlocksUI` teardown is covered by #174 (fixes #170), so this change has no `BlocksUI` implementation. +- UI plugin teardown (every UI plugin unsubscribing, removing its DOM and popovers, and `EditorjsUI` removing its wrapper while leaving the holder) is covered by #174, so this change has no UI plugin implementation. The one UI piece left here is `InlineToolbarUI` destroying the inline tool instances it creates (the tool `destroy()` contract below); it's done after #174 lands, since #174 rewrites that file. - Listeners are released through `AbortController` across the repo. Each component owns one controller, passes its `signal` to every listener it registers, and calls `abort()` on teardown. When an SDK base class registers listeners (`BlockToolAdapter`), the base class owns the controller, gives subclasses a protected `signal`, and aborts it in its own `destroy()`. Subclasses don't override `destroy()` to remove listeners. - The `EditorAPI` subscription methods keep their signatures. Internally `onUpdate` and `onCaretUpdate` subscribe through their own controller and return `() => controller.abort()`. Components tie that function to their own controller's `abort` event. - Add `destroy()` to the `EditorJS` bundle, which delegates to `Core.destroy()`. Because `CollaborationManager.destroy()` already closes the `OTClient` socket, the bundle closes its collaboration connection too. @@ -27,12 +27,11 @@ None. - `core`: adds an editor teardown requirement to `Core` (idempotency, ordering, mid-initialization destroy, post-destroy guards) and adds per-block tool destruction to the block rendering lifecycle. - `sdk`: `BlockToolAdapter` owns its controller and exposes `signal` to subclasses, the tool contracts gain an optional `destroy()` on block and inline tools, and the adapter plugin contract gains a teardown that covers adapters not destroyed yet. - `dom-adapters`: `DOMAdapters` releases the caret, selection, and formatting subscriptions and the block adapters on destroy, and the shared selection watcher detaches from `document` when it has no subscribers left. -- `ui`: shell assembly gains teardown: the wrapper is removed and UI plugins unsubscribe from the EventBus. - `editorjs-bundle`: `EditorJS` exposes `destroy()`. ## Impact -- **Code**: `packages/core/src/index.ts`, `packages/core/src/components/{BlockRenderer,SelectionManager}.ts`, `packages/sdk/src/entities/{InlineTool,EditorjsAdapterPlugin,BlockToolAdapter}.ts`, `packages/core/src/api/{DocumentAPI/DocumentAPI,SelectionAPI}.ts`, `packages/core/src/components/UndoRedoManager.ts`, `packages/model/src/EditorJSModel.ts` (`addEventListener` overloads gain `options`), `packages/plugins/{clipboard-plugin,shortcuts-plugin}`, `packages/collaboration-manager/src/CollaborationManager.ts`, `packages/dom-adapters/src/{index.ts,CaretAdapter,FormattingAdapter,utils/useSelectionChange.ts}`, `packages/ui/src/**` (each plugin's `destroy()`), `packages/editorjs/src/index.ts`. +- **Code**: `packages/core/src/index.ts`, `packages/core/src/components/{BlockRenderer,SelectionManager}.ts`, `packages/sdk/src/entities/{InlineTool,EditorjsAdapterPlugin,BlockToolAdapter}.ts`, `packages/core/src/api/{DocumentAPI/DocumentAPI,SelectionAPI}.ts`, `packages/core/src/components/UndoRedoManager.ts`, `packages/model/src/EditorJSModel.ts` (`addEventListener` overloads gain `options`), `packages/plugins/{clipboard-plugin,shortcuts-plugin}`, `packages/collaboration-manager/src/CollaborationManager.ts`, `packages/dom-adapters/src/{index.ts,CaretAdapter,FormattingAdapter,utils/useSelectionChange.ts}`, `packages/ui/src/InlineToolbar/InlineToolbar.ts` (tool instances, after #174), `packages/editorjs/src/index.ts`. - **Public API**: adds `Core#destroy()`, `EditorJS#destroy()`, optional `InlineTool#destroy()`, and `EditorJSAdapterPlugin#destroy()`. Nothing breaks: the tool hooks are optional, and `DOMAdapters` is the only adapter implementation in the repo. - **Docs**: `docs/plugins.md` ("Lifecycle boundary") says "`Core` currently does not expose a global `destroy()` lifecycle hook". This change replaces that sentence. `docs/README.md` ("Lifecycle") gains a teardown step. -- **Related work**: #174 (BlocksUI holder listeners), card "Clean up" on the DocumentModel board. +- **Related work**: #174 (UI plugin teardown), card "Clean up" on the DocumentModel board. diff --git a/openspec/changes/editor-destroy/specs/ui/spec.md b/openspec/changes/editor-destroy/specs/ui/spec.md deleted file mode 100644 index 972d41a1..00000000 --- a/openspec/changes/editor-destroy/specs/ui/spec.md +++ /dev/null @@ -1,19 +0,0 @@ -## ADDED Requirements - -### Requirement: UI teardown -Every UI plugin other than `BlocksUI` (`EditorjsUI`, `ToolbarUI`, `ToolboxUI`, `InlineToolbarUI`) SHALL remove each EventBus listener it registered and every DOM node it created when its `destroy()` is called. `EditorjsUI.destroy()` SHALL remove the editor wrapper from the holder and leave the holder element and any content the integrator put in it untouched. `BlocksUI` teardown is covered by #174 (the `remove-blocks-ui-listeners-on-destroy` change), so this change has no `BlocksUI` implementation. - -#### Scenario: The editor wrapper is removed -- **GIVEN** `EditorjsUI` appended its wrapper to the holder -- **WHEN** `destroy()` is called -- **THEN** the wrapper is no longer in the DOM, and the holder stays in the DOM with any of its other children - -#### Scenario: UI plugins stop reacting to events -- **GIVEN** `EditorjsUI`, `ToolbarUI`, `ToolboxUI`, and `InlineToolbarUI` are destroyed -- **WHEN** `ui:*` or `core:*` events they used to handle are dispatched on the EventBus -- **THEN** no UI plugin renders, appends, or dispatches anything in response - -#### Scenario: Popovers are released -- **GIVEN** the toolbox or inline toolbar created a popover -- **WHEN** the owning plugin is destroyed -- **THEN** the popover is destroyed and its DOM removed diff --git a/openspec/changes/editor-destroy/tasks.md b/openspec/changes/editor-destroy/tasks.md index 1b333d40..07344f24 100644 --- a/openspec/changes/editor-destroy/tasks.md +++ b/openspec/changes/editor-destroy/tasks.md @@ -39,12 +39,10 @@ - [ ] 6.3 Write failing tests: `should not construct anything when destroyed before initialize`, `should reject initialize with an AbortError without dispatching ready when destroyed mid-initialization`, and `should throw from use and initialize after destroy` - [ ] 6.4 Implement the `#destroyed` checks after each `await` in `initialize()`, and the post-destroy guards in `use()` and `initialize()` -## 7. UI teardown +## 7. UI -- [ ] 7.1 Leave `BlocksUI` to #174, which covers its teardown, so this change has no `BlocksUI` implementation. Reuse #174's UI Jest setup byte-for-byte so the two merge cleanly -- [ ] 7.2 Give each UI plugin except `BlocksUI` (`EditorjsUI`, `ToolbarUI`, `ToolboxUI`, `InlineToolbarUI`) an `AbortController` and pass its `signal` to every EventBus and DOM `addEventListener`. Call `abort()` in `destroy()` -- [ ] 7.3 `EditorjsUI.destroy()` removes the wrapper and leaves the holder in place. `ToolboxUI` and `InlineToolbarUI` destroy their popovers. `InlineToolbarUI` calls `destroy?.()` on the inline tool instances it created, both on rebuild and on destroy -- [ ] 7.4 Add the first UI specs (`packages/ui`): `should remove the editor wrapper but keep the holder on destroy`, and `should not react to EventBus events after destroy` for each plugin +- [ ] 7.1 UI plugin teardown is covered by #174: no UI plugin implementation in this change +- [ ] 7.2 After #174 lands, make `InlineToolbarUI` call `destroy?.()` on the inline tool instances it creates, both when the popover is rebuilt and on destroy, with tests in #174's UI test setup ## 8. Bundle and integration From 73441291164c3e748d1c64848ae5b7d0d808cd62 Mon Sep 17 00:00:00 2001 From: gohabereg Date: Wed, 4 Nov 2026 23:59:20 +0000 Subject: [PATCH 3/4] docs(openspec): tool preparation stops once the editor is destroyed Review on #196 found that the tool preparation queue kept running after destroy(). Tools still queued were prepared, and a tool whose prepare() settled after teardown was created and announced. D4 now describes the fix: ToolsManager owns an AbortController that destroy() aborts, and each queued job checks it before and after prepare(). A prepare() already running can't be interrupted, so its result is dropped. The core scenario for a destroy during a pending initialize() covers it, and task 6.5 tracks the implementation. Co-Authored-By: Claude Opus 5.5 --- openspec/changes/editor-destroy/design.md | 8 +++++++- openspec/changes/editor-destroy/specs/core/spec.md | 2 +- openspec/changes/editor-destroy/tasks.md | 1 + 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/openspec/changes/editor-destroy/design.md b/openspec/changes/editor-destroy/design.md index 9ba5672e..eb4ac28b 100644 --- a/openspec/changes/editor-destroy/design.md +++ b/openspec/changes/editor-destroy/design.md @@ -55,7 +55,13 @@ Every existing `destroy()` in the repo is synchronous, and `EditorjsPlugin.destr The model is never mutated. If `destroy()` cleared blocks through the model, `CollaborationManager` would broadcast the removals to other clients and `UndoRedoManager` would record them. ### D4. Destroy during a pending `initialize()` -`initialize()` awaits `#initializeTools()`, which awaits each tool's `prepare()`. After that `await`, `initialize()` checks `#destroyed` and stops before tools are constructed, the document is initialized, or `core:ready` is dispatched. `destroy()` tears down whatever stage was reached. +`initialize()` awaits `#initializeTools()`, which runs each tool's static `prepare()` one after another through a queue. `destroy()` can land while one of them is pending. + +`ToolsManager` owns an `AbortController` that its `destroy()` aborts. Each queued job checks the signal before calling `prepare()`, so tools still waiting are never prepared. It checks again once `prepare()` settles, so a tool whose preparation finishes after destroy is not created, validated or registered, and no `ToolLoaded` is dispatched for it. An inline instance created then would never be destroyed, because `ToolsManager.destroy()` has already run. + +A `prepare()` that is already running can't be interrupted, because it takes no signal, and passing one would change the tool API. Its result is discarded instead. `initialize()` therefore settles once that `prepare()` settles, not immediately. Racing the queue against the signal would end the wait sooner without stopping the tool's work, so it isn't worth the extra code. + +After the queue settles, `initialize()` checks `#destroyed` and stops before the document is initialized or `core:ready` is dispatched. `destroy()` tears down whatever stage was reached. `initialize()` then **rejects** with a `DOMException` named `AbortError`, following the platform convention (`fetch` rejects the same way when its signal aborts). Code awaiting readiness (loading data, focusing, subscribing) must not run against a destroyed editor, and resolving would let it run. Callers that unmount during boot ignore the error by name: `if (error.name === 'AbortError') return`. This matters in practice: React StrictMode mounts, unmounts and remounts every component in development, so every React integration hits this path. diff --git a/openspec/changes/editor-destroy/specs/core/spec.md b/openspec/changes/editor-destroy/specs/core/spec.md index f4fb9620..8d3c87c5 100644 --- a/openspec/changes/editor-destroy/specs/core/spec.md +++ b/openspec/changes/editor-destroy/specs/core/spec.md @@ -41,7 +41,7 @@ #### Scenario: Destroying while initialization is pending - **GIVEN** `initialize()` has been called and its promise has not settled - **WHEN** `destroy()` is called -- **THEN** whatever was constructed so far is torn down, the pending `initialize()` rejects with a `DOMException` named `AbortError` without initializing the document or dispatching `CoreEventType.Ready`, and any tool or plugin it would have constructed afterwards is not constructed +- **THEN** whatever was constructed so far is torn down, the pending `initialize()` rejects with a `DOMException` named `AbortError` without initializing the document or dispatching `CoreEventType.Ready`, any tool or plugin it would have constructed afterwards is not constructed, tools still queued for preparation are not prepared, and a tool whose `prepare()` settles after `destroy()` is neither created nor registered and dispatches no `CoreEventType.ToolLoaded` #### Scenario: Using a destroyed editor - **GIVEN** `destroy()` has been called diff --git a/openspec/changes/editor-destroy/tasks.md b/openspec/changes/editor-destroy/tasks.md index 07344f24..5c07d7e5 100644 --- a/openspec/changes/editor-destroy/tasks.md +++ b/openspec/changes/editor-destroy/tasks.md @@ -38,6 +38,7 @@ - [ ] 6.2 Keep plugin instances in `#initializePlugin`, track the initialization stage, and implement `destroy()` in the order from design D3 - [ ] 6.3 Write failing tests: `should not construct anything when destroyed before initialize`, `should reject initialize with an AbortError without dispatching ready when destroyed mid-initialization`, and `should throw from use and initialize after destroy` - [ ] 6.4 Implement the `#destroyed` checks after each `await` in `initialize()`, and the post-destroy guards in `use()` and `initialize()` +- [ ] 6.5 Write failing `ToolsManager` tests, `should not prepare the tools still queued` and `should neither create nor announce a tool whose prepare() settles after destroy`. Give `ToolsManager` an `AbortController` that `destroy()` aborts, and check its signal before and after each `prepare()` (design D4) ## 7. UI From eea60ace8fbd2891bde9404fc6c7457f4184fbcc Mon Sep 17 00:00:00 2001 From: gohabereg Date: Wed, 4 Nov 2026 23:59:38 +0000 Subject: [PATCH 4/4] docs(openspec): the inline tool facade owns its shared instance Review on #195 pointed out that InlineToolFacade.create() returns one cached instance per tool. Formatting, selection, validation and the inline toolbar all share it, so the cleanup D7 proposed would destroy tools still in use, several times over. - Design: D7 is rewritten. Each inline tool's facade owns its shared instance, and ToolsManager destroys it once, as the last step of teardown. Both rejected alternatives are explained. D3 gains the ToolsManager step and D5 the FormattingAdapter change. The context table and the #174 risk are updated. - Specs: the SDK teardown hook requirement states the rule, with a facade scenario. Core gains a scenario for shared inline instances destroyed last, once each. The dom-adapters formatting scenario no longer destroys them. - The proposal and tasks 4.3, 4.4, 5.4 and 7.2 are updated to match. Co-Authored-By: Claude Opus 5.5 --- openspec/changes/editor-destroy/design.md | 28 +++++++++++-------- openspec/changes/editor-destroy/proposal.md | 8 +++--- .../changes/editor-destroy/specs/core/spec.md | 5 ++++ .../editor-destroy/specs/dom-adapters/spec.md | 4 +-- .../changes/editor-destroy/specs/sdk/spec.md | 7 ++++- openspec/changes/editor-destroy/tasks.md | 8 +++--- 6 files changed, 38 insertions(+), 22 deletions(-) diff --git a/openspec/changes/editor-destroy/design.md b/openspec/changes/editor-destroy/design.md index eb4ac28b..8692f0f8 100644 --- a/openspec/changes/editor-destroy/design.md +++ b/openspec/changes/editor-destroy/design.md @@ -8,9 +8,10 @@ Resources that outlive an editor if nobody releases them: |---|---|---| | `EditorjsUI` | wrapper `div` appended to the integrator's holder | covered by #174, out of scope here | | `BlocksUI` | blocks holder DOM listeners (`beforeinput`, `keydown`, `copy`), block wrappers | covered by #174, out of scope here | -| `ToolbarUI` / `ToolboxUI` / `InlineToolbarUI` | EventBus listeners, popovers | covered by #174, out of scope here; `InlineToolbarUI`'s inline tool instances are handled here after #174 lands (D7) | +| `ToolbarUI` / `ToolboxUI` / `InlineToolbarUI` | EventBus listeners, popovers | covered by #174, out of scope here | | `CaretAdapter` | subscription to the singleton `useSelectionChange` watcher (`document` `selectionchange`) and `api.selection.onCaretUpdate` | `@todo Unsubscribe on adapter destruction` | -| `FormattingAdapter` | `core:ToolLoaded` and `api.document.onUpdate` listeners, inline tool instances | never released | +| `FormattingAdapter` | `core:ToolLoaded` and `api.document.onUpdate` listeners | never released | +| `ToolsManager` | inline tool facades, each holding the one instance its consumers share | never destroyed | | `DOMAdapters` | `DOMBlockToolAdapter` per block (model and `ui:beforeinput` listeners) | released only on block removal | | `BlockRenderer` | model listener, block tool instances | instances aren't tracked (`@todo clear block tool adapter memory`) | | `SelectionManager` | model `CaretManagerUpdated` listener | never released | @@ -49,8 +50,9 @@ Every existing `destroy()` in the repo is synchronous, and `EditorjsPlugin.destr 3. `UndoRedoManager.destroy()`, then `SelectionManager.destroy()`. 4. `BlockRenderer.destroy()`: for each tracked block, call the tool instance's `destroy?.()`, then `adapter.destroyBlockToolAdapter(id)`. Remove the model listener. No `BlockRemovedCoreEvent` is dispatched, because the UI is already gone. 5. `adapter.destroy()` (D5). +6. `ToolsManager.destroy()`: destroy each inline tool's shared instance once (D7), and stop any tool preparation still running (D4). It runs last because the adapter, the selection manager and the inline toolbar all use those instances until they're gone. -`destroy()` never resolves a service from the container, because that could construct it. `initialize()` stores each service it resolves (`SelectionManager`, `BlockRenderer`, `UndoRedoManager`, the adapter) in a `#services` record, and teardown only destroys what that record holds. Each step runs through a helper that logs a throwing step and continues. +`destroy()` never resolves a service from the container, because that could construct it. `initialize()` stores each service it resolves (`SelectionManager`, `BlockRenderer`, `UndoRedoManager`, the adapter, `ToolsManager`) in a `#services` record, and teardown only destroys what that record holds. Each step runs through a helper that logs a throwing step and continues. The model is never mutated. If `destroy()` cleared blocks through the model, `CollaborationManager` would broadcast the removals to other clients and `UndoRedoManager` would record them. @@ -75,7 +77,7 @@ After the queue settles, `initialize()` checks `#destroyed` and stops before the `EditorJSAdapterPlugin` narrows `destroy?()` to `destroy()`. Core can't release the rendering layer without it, and `DOMAdapters` is the only implementation. `DOMAdapters.destroy()`: - destroys each remaining entry in `#adapters` through the existing `destroyBlockToolAdapter` path - calls `CaretAdapter.destroy()`, which aborts its controller. That controller's `abort` event is tied to `off(holder)` on the selection watcher and to the `api.selection.onCaretUpdate` unsubscribe function -- calls `FormattingAdapter.destroy()`, which aborts its controller (covering the `core:ToolLoaded` and `onUpdate` listeners) and calls `destroy?.()` on each attached inline tool +- calls `FormattingAdapter.destroy()`, which aborts its controller (covering the `core:ToolLoaded` and `onUpdate` listeners) and drops its references to the inline tools. It doesn't destroy them: they're shared (D7) Singletons are fetched from its private container, where they already exist, so nothing new gets constructed. @@ -83,13 +85,17 @@ Singletons are fetched from its private container, where they already exist, so `useSelectionChange` is a module-level singleton shared by every editor on the page, so its `destroy()` would break the other editors. Instead, `off()` removes the `document` listener when the subscriber map becomes empty, and `on()` adds it back when it adds the first subscriber. This lets "no `document` listener left after every editor is destroyed" pass, and it is safe when several editors run at once. ### D7. Tool instance lifetime rule -Add `destroy?(): void` to `InlineTool`. `BlockTool` already gets it from the v2 type, and we spell it out in the SDK spec. The rule is that every instance the editor creates gets `destroy()` called once, when the editor stops using it: -- `BlockRenderer`: the per-block instance, on block removal or teardown. -- `FormattingAdapter`: the long-lived inline instance, on teardown. -- `InlineToolbarUI`: instances created when building the toolbar, when the toolbar is rebuilt or destroyed. -- `SelectionManager.applyInlineTool` and `ToolsManager` validation create throwaway instances. They call `destroy?.()` right after use, in a `finally`. +Add `destroy?(): void` to `InlineTool`. `BlockTool` already gets it from the v2 type, and we spell it out in the SDK spec. The rule is that every instance the editor creates gets `destroy()` called once, by whoever owns it, when the editor stops using it. -*Alternative*: call `destroy()` only on long-lived instances. Rejected because tool authors would then have to know which instances are which, while a uniform rule works without that knowledge. +The owner differs by tool type, because the facades create instances differently: +- **Block tools**: `BlockToolFacade.create()` builds a new instance per block, and `BlockRenderer` owns it. It calls `destroy()` when the block is removed or on teardown. +- **Inline tools**: `InlineToolFacade.create()` returns one cached instance per tool (since #127), so `FormattingAdapter`, `SelectionManager.applyInlineTool`, `ToolsManager` validation and `InlineToolbarUI` all receive the same object. None of them owns it, and none of them destroys it. The facade does: `InlineToolFacade.destroy()` destroys the instance once (only if one was ever created) and drops it. `ToolsManager.destroy()` calls it for every inline tool as the last step of teardown (D3), logging a throwing hook and moving on to the next tool. + +So an inline tool's `destroy()` runs exactly once, when the editor is destroyed. It doesn't run when the inline toolbar is rebuilt or hidden, or after a format is applied. + +*Alternative*: each consumer destroys the instance it was handed. Rejected because the instance is shared. The first consumer to finish would destroy it under the others, and teardown would call the hook once per consumer. That's how this change was first written, and review caught it. + +*Alternative*: make `InlineToolFacade.create()` return a new instance each time, like block tools. Rejected because tools may rely on keeping state between toolbar renders and applications, and that would be a behaviour change outside this work. ### D8. Listeners are released through `AbortController` across the repo Every first-party component that registers listeners owns one `AbortController`. It passes the controller's `signal` to every registration and calls `abort()` in its teardown. This covers: @@ -120,7 +126,7 @@ Plugins and tools are interfaces, not base classes, so they own their own contro - [An async `render()` settles after teardown and dispatches `BlockAddedCoreEvent` into a dead EventBus] → `BlockRenderer` checks a `#destroyed` flag after the `await`, then destroys the late instance and its adapter instead of dispatching. - [Ops not yet sent in `CollaborationManager`'s debounced batch are lost on destroy] → This already happens when a tab closes. Flushing would need async teardown (Non-Goal) and is listed in Open Questions. - [The adapter contract narrows `destroy?()` to `destroy()`, which is a type-level break for third-party adapters] → No third-party adapters exist yet, and the package is pre-1.0. -- [#174 overlaps with UI teardown] → #174 covers every UI plugin's teardown, so this change has no UI plugin implementation. The only UI code here is `InlineToolbarUI` destroying its inline tool instances, which is done after #174 lands because #174 rewrites that file. Both changes edit the same `docs/plugins.md` lifecycle line, so whichever lands second merges that one line. +- [#174 overlaps with UI teardown] → #174 covers every UI plugin's teardown, so this change has no UI plugin implementation. `InlineToolbarUI` doesn't destroy inline tool instances either, because they're shared (D7). Both changes edit the same `docs/plugins.md` lifecycle line, so whichever lands second merges that one line. ## Migration Plan diff --git a/openspec/changes/editor-destroy/proposal.md b/openspec/changes/editor-destroy/proposal.md index a4debfaa..38bb4f60 100644 --- a/openspec/changes/editor-destroy/proposal.md +++ b/openspec/changes/editor-destroy/proposal.md @@ -8,9 +8,9 @@ There is no way to tear an editor down. Neither `Core` nor `EditorJS` has a `des - `Core` keeps the plugin instances it creates. `destroy()` calls `destroy()` on each of them in reverse construction order and removes each one's entry from the plugin registry. A plugin whose `destroy()` throws is logged and doesn't stop the rest of the teardown. - `Core` tears down its own services: `UndoRedoManager` (already has `destroy()`), `BlockRenderer` and `SelectionManager` (new), each removing its model and EventBus listeners. - `BlockRenderer` keeps the block tool instance for each rendered block. When a block is removed, and when the editor is destroyed, it calls the tool's `destroy()` and destroys the block's adapter. Teardown does not touch the model, so no removal operations reach undo history or collaboration. -- Add an optional `destroy()` to the `InlineTool` contract in `@editorjs/sdk`. `BlockTool` already inherits an optional `destroy()` from the v2 type; the SDK spec now names both. The editor calls `destroy()` once on each tool instance it creates, when it stops using that instance. -- Add `destroy()` to the adapter plugin. `DOMAdapters.destroy()` destroys the remaining block adapters, stops `CaretAdapter` watching selection changes and caret updates, and stops `FormattingAdapter` listening to tool and model events (destroying its inline tool instances). The shared `selectionchange` watcher removes its `document` listener once its last subscriber is gone. -- UI plugin teardown (every UI plugin unsubscribing, removing its DOM and popovers, and `EditorjsUI` removing its wrapper while leaving the holder) is covered by #174, so this change has no UI plugin implementation. The one UI piece left here is `InlineToolbarUI` destroying the inline tool instances it creates (the tool `destroy()` contract below); it's done after #174 lands, since #174 rewrites that file. +- Add an optional `destroy()` to the `InlineTool` contract in `@editorjs/sdk`. `BlockTool` already inherits an optional `destroy()` from the v2 type; the SDK spec now names both. The editor calls `destroy()` once on each tool instance it creates, when it stops using that instance. Inline tools have one instance shared by all consumers, so their facade destroys it, through `ToolsManager`, as the last step of teardown. +- Add `destroy()` to the adapter plugin. `DOMAdapters.destroy()` destroys the remaining block adapters, stops `CaretAdapter` watching selection changes and caret updates, and stops `FormattingAdapter` listening to tool and model events. The shared `selectionchange` watcher removes its `document` listener once its last subscriber is gone. +- UI plugin teardown (every UI plugin unsubscribing, removing its DOM and popovers, and `EditorjsUI` removing its wrapper while leaving the holder) is covered by #174, so this change has no UI plugin implementation. - Listeners are released through `AbortController` across the repo. Each component owns one controller, passes its `signal` to every listener it registers, and calls `abort()` on teardown. When an SDK base class registers listeners (`BlockToolAdapter`), the base class owns the controller, gives subclasses a protected `signal`, and aborts it in its own `destroy()`. Subclasses don't override `destroy()` to remove listeners. - The `EditorAPI` subscription methods keep their signatures. Internally `onUpdate` and `onCaretUpdate` subscribe through their own controller and return `() => controller.abort()`. Components tie that function to their own controller's `abort` event. - Add `destroy()` to the `EditorJS` bundle, which delegates to `Core.destroy()`. Because `CollaborationManager.destroy()` already closes the `OTClient` socket, the bundle closes its collaboration connection too. @@ -31,7 +31,7 @@ None. ## Impact -- **Code**: `packages/core/src/index.ts`, `packages/core/src/components/{BlockRenderer,SelectionManager}.ts`, `packages/sdk/src/entities/{InlineTool,EditorjsAdapterPlugin,BlockToolAdapter}.ts`, `packages/core/src/api/{DocumentAPI/DocumentAPI,SelectionAPI}.ts`, `packages/core/src/components/UndoRedoManager.ts`, `packages/model/src/EditorJSModel.ts` (`addEventListener` overloads gain `options`), `packages/plugins/{clipboard-plugin,shortcuts-plugin}`, `packages/collaboration-manager/src/CollaborationManager.ts`, `packages/dom-adapters/src/{index.ts,CaretAdapter,FormattingAdapter,utils/useSelectionChange.ts}`, `packages/ui/src/InlineToolbar/InlineToolbar.ts` (tool instances, after #174), `packages/editorjs/src/index.ts`. +- **Code**: `packages/core/src/index.ts`, `packages/core/src/components/{BlockRenderer,SelectionManager}.ts`, `packages/core/src/tools/ToolsManager.ts`, `packages/sdk/src/entities/{InlineTool,EditorjsAdapterPlugin,BlockToolAdapter}.ts`, `packages/sdk/src/tools/facades/InlineToolFacade.ts`, `packages/core/src/api/{DocumentAPI/DocumentAPI,SelectionAPI}.ts`, `packages/core/src/components/UndoRedoManager.ts`, `packages/model/src/EditorJSModel.ts` (`addEventListener` overloads gain `options`), `packages/plugins/{clipboard-plugin,shortcuts-plugin}`, `packages/collaboration-manager/src/CollaborationManager.ts`, `packages/dom-adapters/src/{index.ts,CaretAdapter,FormattingAdapter,utils/useSelectionChange.ts}`, `packages/editorjs/src/index.ts`. - **Public API**: adds `Core#destroy()`, `EditorJS#destroy()`, optional `InlineTool#destroy()`, and `EditorJSAdapterPlugin#destroy()`. Nothing breaks: the tool hooks are optional, and `DOMAdapters` is the only adapter implementation in the repo. - **Docs**: `docs/plugins.md` ("Lifecycle boundary") says "`Core` currently does not expose a global `destroy()` lifecycle hook". This change replaces that sentence. `docs/README.md` ("Lifecycle") gains a teardown step. - **Related work**: #174 (UI plugin teardown), card "Clean up" on the DocumentModel board. diff --git a/openspec/changes/editor-destroy/specs/core/spec.md b/openspec/changes/editor-destroy/specs/core/spec.md index 8d3c87c5..cf09fa95 100644 --- a/openspec/changes/editor-destroy/specs/core/spec.md +++ b/openspec/changes/editor-destroy/specs/core/spec.md @@ -28,6 +28,11 @@ - **WHEN** `destroy()` is called - **THEN** each rendered block tool's `destroy()` is called, each block's adapter is destroyed, the adapter plugin's `destroy()` is called, and the model emits no events +#### Scenario: Shared inline tool instances are destroyed last, once each +- **GIVEN** an initialized editor whose inline tools have been used by the formatting adapter, the selection manager and the inline toolbar +- **WHEN** `destroy()` is called +- **THEN** each inline tool instance's `destroy()` is called once, after the adapter plugin's `destroy()`; a throwing hook is logged with `console.error` and the other inline tools are still destroyed; inline tools that were never instantiated are not constructed + #### Scenario: Destroying twice is a no-op - **GIVEN** `destroy()` has already been called - **WHEN** `destroy()` is called again diff --git a/openspec/changes/editor-destroy/specs/dom-adapters/spec.md b/openspec/changes/editor-destroy/specs/dom-adapters/spec.md index e7c2ff50..e641e2b2 100644 --- a/openspec/changes/editor-destroy/specs/dom-adapters/spec.md +++ b/openspec/changes/editor-destroy/specs/dom-adapters/spec.md @@ -1,7 +1,7 @@ ## ADDED Requirements ### Requirement: Adapter teardown -`DOMAdapters` SHALL implement `destroy()`. It SHALL destroy every `DOMBlockToolAdapter` still alive and clear the `InputsRegistry`, stop `CaretAdapter` watching DOM selection changes and model caret updates, stop `FormattingAdapter` listening to `core:ToolLoaded` and model updates, and call `destroy()` on the inline tool instances `FormattingAdapter` holds. The shared `selectionchange` watcher SHALL remove its `document` listener once its last subscriber unsubscribes, and add it back on the next subscription. +`DOMAdapters` SHALL implement `destroy()`. It SHALL destroy every `DOMBlockToolAdapter` still alive and clear the `InputsRegistry`, stop `CaretAdapter` watching DOM selection changes and model caret updates, and stop `FormattingAdapter` listening to `core:ToolLoaded` and model updates. `FormattingAdapter` SHALL drop its inline tools without destroying them, because their instances are shared and their facades destroy them. The shared `selectionchange` watcher SHALL remove its `document` listener once its last subscriber unsubscribes, and add it back on the next subscription. #### Scenario: Caret adapter stops tracking the holder - **GIVEN** an initialized `DOMAdapters` @@ -16,7 +16,7 @@ #### Scenario: Formatting adapter releases inline tools - **GIVEN** inline tools attached to `FormattingAdapter` through `core:ToolLoaded` - **WHEN** `destroy()` is called -- **THEN** each attached inline tool instance's `destroy()` is called, and later model formatting events don't touch the DOM +- **THEN** no inline tool instance's `destroy()` is called, the adapter holds none of them anymore, and later model formatting events don't touch the DOM #### Scenario: Two editors on one page - **GIVEN** two editors are initialized on the same page and share the `selectionchange` watcher diff --git a/openspec/changes/editor-destroy/specs/sdk/spec.md b/openspec/changes/editor-destroy/specs/sdk/spec.md index ddd7a26f..82a302b6 100644 --- a/openspec/changes/editor-destroy/specs/sdk/spec.md +++ b/openspec/changes/editor-destroy/specs/sdk/spec.md @@ -1,13 +1,18 @@ ## ADDED Requirements ### Requirement: Tool instance teardown hook -The `BlockTool` and `InlineTool` instance contracts SHALL each declare an optional `destroy(): void`. The editor SHALL call `destroy()` exactly once on every tool instance it creates, when it stops using that instance: for long-lived instances, when the owning block is removed, the owning component is rebuilt, or the editor is destroyed; for instances created for a single operation, right after that operation completes. A tool that doesn't implement `destroy()` SHALL keep working unchanged. +The `BlockTool` and `InlineTool` instance contracts SHALL each declare an optional `destroy(): void`. The editor SHALL call `destroy()` exactly once on every tool instance it creates, when it stops using that instance. A block tool instance belongs to one block and SHALL be destroyed when that block is removed or the editor is destroyed. An inline tool facade SHALL hand the same instance to every consumer. Consumers SHALL NOT destroy it. `InlineToolFacade.destroy()` SHALL destroy it once, and only if it was created. A tool that doesn't implement `destroy()` SHALL keep working unchanged. #### Scenario: Inline tool declares destroy - **GIVEN** an inline tool class implements `destroy()` - **WHEN** it is type-checked against `InlineToolConstructor` - **THEN** it compiles, and an inline tool without `destroy()` also compiles +#### Scenario: The inline tool facade destroys its shared instance once +- **GIVEN** an inline tool facade whose `create()` has been called several times +- **WHEN** `destroy()` is called on the facade, once or more +- **THEN** every `create()` returned the same instance, its `destroy()` is called once, and a facade whose `create()` was never called constructs nothing + #### Scenario: Block tool declares destroy - **GIVEN** a block tool class implements `destroy()` - **WHEN** it is type-checked against `BlockToolConstructor` diff --git a/openspec/changes/editor-destroy/tasks.md b/openspec/changes/editor-destroy/tasks.md index 5c07d7e5..b2bf3705 100644 --- a/openspec/changes/editor-destroy/tasks.md +++ b/openspec/changes/editor-destroy/tasks.md @@ -20,8 +20,8 @@ - [ ] 4.1 Write failing `CaretAdapter` tests: `should stop updating the model caret on selectionchange after destroy`, and `should not touch the DOM selection on model caret updates after destroy` - [ ] 4.2 Implement `CaretAdapter.destroy()` by aborting its controller, whose `abort` event is tied to `off(holder)` and the `onCaretUpdate` unsubscribe function. Remove the `@todo` -- [ ] 4.3 Write failing `FormattingAdapter` tests: `should call destroy on attached inline tools`, `should ignore core:ToolLoaded after destroy`, and `should ignore model updates after destroy` -- [ ] 4.4 Implement `FormattingAdapter.destroy()`: abort its controller, then destroy the inline tool instances +- [ ] 4.3 Write failing `FormattingAdapter` tests: `should not destroy the shared inline tool instances it uses`, `should ignore core:ToolLoaded after destroy`, and `should ignore model updates after destroy` +- [ ] 4.4 Implement `FormattingAdapter.destroy()`: abort its controller, then drop the inline tools without destroying them (design D7) - [ ] 4.5 Write failing `DOMAdapters` tests: `should destroy every remaining block adapter on destroy`, `should leave the inputs registry empty after destroy`, and `should destroy caret and formatting adapters on destroy` - [ ] 4.6 Implement `DOMAdapters.destroy()` @@ -30,7 +30,7 @@ - [ ] 5.1 Write failing `BlockRenderer` tests: `should call tool destroy when its block is removed`, `should destroy every remaining tool and block adapter on destroy without dispatching BlockRemovedCoreEvent`, `should stop handling model events after destroy`, and `should not dispatch BlockAddedCoreEvent for a render that settles after destroy` - [ ] 5.2 Track tool instances per block id in `BlockRenderer`, subscribe to the model with a controller's signal, add `destroy()`, and remove the `@todo clear block tool adapter memory` - [ ] 5.3 Write a failing `SelectionManager` test, `should stop handling caret updates after destroy`. Implement `SelectionManager.destroy()` by aborting its controller -- [ ] 5.4 Write a failing test, `should destroy the throwaway inline tool instance after applying it`, for `SelectionManager.applyInlineTool`, and do the same for the instance created during `ToolsManager` validation. Call `destroy?.()` in a `finally` +- [ ] 5.4 Give `InlineToolFacade` a `destroy()` that destroys its shared instance once, and `ToolsManager` a `destroy()` that calls it for every inline tool, logging a throwing hook; `Core.destroy()` calls it last (design D3, D7). Test that `SelectionManager.applyInlineTool` and `ToolsManager` validation don't destroy the shared instance ## 6. Core.destroy() @@ -43,7 +43,7 @@ ## 7. UI - [ ] 7.1 UI plugin teardown is covered by #174: no UI plugin implementation in this change -- [ ] 7.2 After #174 lands, make `InlineToolbarUI` call `destroy?.()` on the inline tool instances it creates, both when the popover is rebuilt and on destroy, with tests in #174's UI test setup +- [ ] 7.2 After #174 lands, test that `InlineToolbarUI` never destroys the shared inline tool instance, through a real `InlineToolFacade`, across rebuild, hide, a superseded render and destroy ## 8. Bundle and integration