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..8692f0f8 --- /dev/null +++ b/openspec/changes/editor-destroy/design.md @@ -0,0 +1,138 @@ +## 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 | 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 | +| `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 | 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 | +| `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). +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, `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. + +### D4. Destroy during a pending `initialize()` +`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. + +`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 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. + +### 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, by whoever owns it, when the editor stops using it. + +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: +- `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 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 + +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..38bb4f60 --- /dev/null +++ b/openspec/changes/editor-destroy/proposal.md @@ -0,0 +1,37 @@ +## 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. 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. + +## 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. +- `editorjs-bundle`: `EditorJS` exposes `destroy()`. + +## Impact + +- **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/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..cf09fa95 --- /dev/null +++ b/openspec/changes/editor-destroy/specs/core/spec.md @@ -0,0 +1,86 @@ +## 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: 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 +- **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`, 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 +- **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..e641e2b2 --- /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, 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` +- **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** 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 +- **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..82a302b6 --- /dev/null +++ b/openspec/changes/editor-destroy/specs/sdk/spec.md @@ -0,0 +1,56 @@ +## 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. 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` +- **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/tasks.md b/openspec/changes/editor-destroy/tasks.md new file mode 100644 index 00000000..b2bf3705 --- /dev/null +++ b/openspec/changes/editor-destroy/tasks.md @@ -0,0 +1,59 @@ +## 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 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()` + +## 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 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() + +- [ ] 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()` +- [ ] 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 + +- [ ] 7.1 UI plugin teardown is covered by #174: no UI plugin implementation in this change +- [ ] 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 + +- [ ] 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