From 960663857a359af9aa991b8c6f3e9ef5a91263fd Mon Sep 17 00:00:00 2001 From: gohabereg Date: Tue, 6 Oct 2026 00:37:02 +0100 Subject: [PATCH 01/10] 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 52fc5cc7124c6fe6a2ffa5f1a81f52472fa47563 Mon Sep 17 00:00:00 2001 From: gohabereg Date: Tue, 6 Oct 2026 00:48:28 +0100 Subject: [PATCH 02/10] feat(core): add editor destroy lifecycle - Core#destroy(): destroys plugins in reverse construction order and unregisters their public APIs, then UndoRedoManager, SelectionManager, BlockRenderer and the adapter. Idempotent, safe before and during initialize(), never mutates the model. use()/initialize() throw after it. - EditorJS#destroy() delegates to Core. - BlockRenderer keeps each block's tool instance and destroys it on block removal and on teardown; a render settling after teardown is dropped. - Optional destroy() on BlockTool/InlineTool; throwaway inline tool instances (SelectionManager, ToolsManager validation) are destroyed right after use. - Listeners are released through AbortController: onUpdate/onCaretUpdate subscribe through their own controller, BlockToolAdapter owns one and exposes a protected signal to subclasses, core services abort theirs on destroy. Model addEventListener overloads accept options. Part of the editor-destroy OpenSpec change. Co-Authored-By: Claude Opus 5.5 --- openspec/changes/editor-destroy/tasks.md | 25 +- .../src/api/DocumentAPI/DocumentAPI.spec.ts | 43 +++ .../core/src/api/DocumentAPI/DocumentAPI.ts | 8 +- packages/core/src/api/SelectionAPI.spec.ts | 43 +++ packages/core/src/api/SelectionAPI.ts | 8 +- .../core/src/components/BlockRenderer.spec.ts | 169 ++++++++++++ packages/core/src/components/BlockRenderer.ts | 60 ++++- .../src/components/SelectionManager.spec.ts | 68 +++++ .../core/src/components/SelectionManager.ts | 101 ++++--- .../src/components/UndoRedoManager.spec.ts | 43 +-- .../core/src/components/UndoRedoManager.ts | 64 ++--- packages/core/src/index.spec.ts | 247 +++++++++++++++++- packages/core/src/index.ts | 133 +++++++++- packages/core/src/tools/ToolsManager.spec.ts | 36 +++ packages/core/src/tools/ToolsManager.ts | 8 +- packages/editorjs/src/index.spec.ts | 13 + packages/editorjs/src/index.ts | 8 + packages/model/src/EditorJSModel.ts | 4 +- packages/sdk/src/entities/BlockTool.ts | 8 +- .../sdk/src/entities/BlockToolAdapter.spec.ts | 114 ++++++++ packages/sdk/src/entities/BlockToolAdapter.ts | 21 +- packages/sdk/src/entities/InlineTool.ts | 6 + packages/sdk/src/entities/ToolDestroy.spec.ts | 56 ++++ 23 files changed, 1140 insertions(+), 146 deletions(-) create mode 100644 packages/core/src/tools/ToolsManager.spec.ts create mode 100644 packages/sdk/src/entities/BlockToolAdapter.spec.ts create mode 100644 packages/sdk/src/entities/ToolDestroy.spec.ts diff --git a/openspec/changes/editor-destroy/tasks.md b/openspec/changes/editor-destroy/tasks.md index 1b333d40..7a56bd0b 100644 --- a/openspec/changes/editor-destroy/tasks.md +++ b/openspec/changes/editor-destroy/tasks.md @@ -1,15 +1,16 @@ ## 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 +- [x] 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()` +- [x] 2.1 Add an `options?: AddEventListenerOptions` argument to `EditorJSModel`'s typed `addEventListener` overloads and forward it +- [x] 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()` +- [x] 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 +- [x] 2.5 Move `UndoRedoManager` listener cleanup to one controller, keeping explicit cleanup only for its debounce timer +- [ ] 2.6 Move `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) @@ -27,17 +28,17 @@ ## 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` +- [x] 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` +- [x] 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` +- [x] 5.3 Write a failing `SelectionManager` test, `should stop handling caret updates after destroy`. Implement `SelectionManager.destroy()` by aborting its controller +- [x] 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 +- [x] 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` +- [x] 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()` +- [x] 6.4 Implement the `#destroyed` checks after each `await` in `initialize()`, and the post-destroy guards in `use()` and `initialize()` ## 7. UI teardown diff --git a/packages/core/src/api/DocumentAPI/DocumentAPI.spec.ts b/packages/core/src/api/DocumentAPI/DocumentAPI.spec.ts index c589ae66..d7019984 100644 --- a/packages/core/src/api/DocumentAPI/DocumentAPI.spec.ts +++ b/packages/core/src/api/DocumentAPI/DocumentAPI.spec.ts @@ -84,4 +84,47 @@ describe('DocumentAPI', () => { expect(dispatchEvent).toHaveBeenCalledWith(expect.objectContaining({ name: 'redo' })); }); }); + + describe('.onUpdate()', () => { + /** + * EventTarget stands in for the model, so listeners are really added and removed + * @param target - stand-in for the model + */ + const createAPI = (target: EventTarget): InstanceType => new DocumentAPI( + target as unknown as InstanceType, + {} as unknown as CoreConfigValidated, + { dispatchEvent } as unknown as EventBus + ); + + it('should call the callback on model update', () => { + const target = new EventTarget(); + const callback = jest.fn(); + + createAPI(target).onUpdate(callback); + target.dispatchEvent(new Event('update')); + + expect(callback).toHaveBeenCalledTimes(1); + }); + + it('should stop calling the callback after the returned function is called', () => { + const target = new EventTarget(); + const callback = jest.fn(); + + const unsubscribe = createAPI(target).onUpdate(callback); + + unsubscribe(); + target.dispatchEvent(new Event('update')); + + expect(callback).not.toHaveBeenCalled(); + }); + + it('should subscribe through an abort signal', () => { + const target = new EventTarget(); + const addEventListener = jest.spyOn(target, 'addEventListener'); + + createAPI(target).onUpdate(jest.fn()); + + expect(addEventListener).toHaveBeenCalledWith('update', expect.any(Function), { signal: expect.any(AbortSignal) }); + }); + }); }); diff --git a/packages/core/src/api/DocumentAPI/DocumentAPI.ts b/packages/core/src/api/DocumentAPI/DocumentAPI.ts index e1e26b98..bb39fc1c 100644 --- a/packages/core/src/api/DocumentAPI/DocumentAPI.ts +++ b/packages/core/src/api/DocumentAPI/DocumentAPI.ts @@ -64,11 +64,11 @@ export class DocumentAPI implements DocumentApiInterface { * @param callback - callback called on model update */ public onUpdate(callback: (event: ModelEvents) => void): () => void { - this.#model.addEventListener(EventType.Changed, callback); + const controller = new AbortController(); - return () => { - this.#model.removeEventListener(EventType.Changed, callback); - }; + this.#model.addEventListener(EventType.Changed, callback, { signal: controller.signal }); + + return () => controller.abort(); } /** diff --git a/packages/core/src/api/SelectionAPI.spec.ts b/packages/core/src/api/SelectionAPI.spec.ts index cf8c5d0c..cffc34f4 100644 --- a/packages/core/src/api/SelectionAPI.spec.ts +++ b/packages/core/src/api/SelectionAPI.spec.ts @@ -50,4 +50,47 @@ describe('SelectionAPI', () => { }); }); }); + + describe('.onCaretUpdate()', () => { + /** + * EventTarget stands in for the model, so listeners are really added and removed + * @param target - stand-in for the model + */ + const createAPI = (target: EventTarget): InstanceType => new SelectionAPI( + selectionManager as unknown as InstanceType, + target as unknown as InstanceType, + {} as unknown as CoreConfigValidated + ); + + it('should call the callback on caret update', () => { + const target = new EventTarget(); + const callback = jest.fn(); + + createAPI(target).onCaretUpdate(callback); + target.dispatchEvent(new Event('update')); + + expect(callback).toHaveBeenCalledTimes(1); + }); + + it('should stop calling the callback after the returned function is called', () => { + const target = new EventTarget(); + const callback = jest.fn(); + + const unsubscribe = createAPI(target).onCaretUpdate(callback); + + unsubscribe(); + target.dispatchEvent(new Event('update')); + + expect(callback).not.toHaveBeenCalled(); + }); + + it('should subscribe through an abort signal', () => { + const target = new EventTarget(); + const addEventListener = jest.spyOn(target, 'addEventListener'); + + createAPI(target).onCaretUpdate(jest.fn()); + + expect(addEventListener).toHaveBeenCalledWith('update', expect.any(Function), { signal: expect.any(AbortSignal) }); + }); + }); }); diff --git a/packages/core/src/api/SelectionAPI.ts b/packages/core/src/api/SelectionAPI.ts index 5e2b323a..174a4759 100644 --- a/packages/core/src/api/SelectionAPI.ts +++ b/packages/core/src/api/SelectionAPI.ts @@ -66,11 +66,11 @@ export class SelectionAPI implements SelectionApiInterface { * @param callback - callback for CaretManager updates */ public onCaretUpdate(callback: (event: CaretManagerEvents) => void): () => void { - this.#model.addEventListener(EventType.CaretManagerUpdated, callback); + const controller = new AbortController(); - return () => { - this.#model.removeEventListener(EventType.CaretManagerUpdated, callback); - }; + this.#model.addEventListener(EventType.CaretManagerUpdated, callback, { signal: controller.signal }); + + return () => controller.abort(); } /** diff --git a/packages/core/src/components/BlockRenderer.spec.ts b/packages/core/src/components/BlockRenderer.spec.ts index 4647ec1f..b3e312e7 100644 --- a/packages/core/src/components/BlockRenderer.spec.ts +++ b/packages/core/src/components/BlockRenderer.spec.ts @@ -251,3 +251,172 @@ describe('BlockRenderer (unit, mocked deps)', () => { }); }); }); + +describe('BlockRenderer lifecycle', () => { + interface Setup { + renderer: InstanceType; + emit: (event: unknown) => Promise; + signal: () => AbortSignal | undefined; + dispatchEvent: jest.Mock; + adapter: { createBlockToolAdapter: jest.Mock; + destroyBlockToolAdapter: jest.Mock; }; + } + + /** + * Creates a renderer whose model honours the abort signal, like a real EventTarget + * @param createTool - returns the tool instance for each added block + */ + function setup(createTool: () => { render: () => Promise; + destroy?: () => void; }): Setup { + let listener: ((event: unknown) => void | Promise) | undefined; + let listenerSignal: AbortSignal | undefined; + + const model = { + addEventListener: jest.fn((_type: string, callback: (event: unknown) => void, options?: AddEventListenerOptions) => { + listener = callback; + listenerSignal = options?.signal; + }), + }; + const dispatchEvent = jest.fn(); + const adapter = { + createBlockToolAdapter: jest.fn(() => ({})), + destroyBlockToolAdapter: jest.fn(), + }; + const toolsManager = { + blockTools: { + get: jest.fn(() => ({ + name: 'tool', + create: createTool, + })), + }, + }; + + const renderer = new BlockRenderer( + model as unknown as InstanceType, + { dispatchEvent } as unknown as InstanceType, + toolsManager as unknown as InstanceType, + adapter as unknown as EditorJSAdapterPlugin + ); + + return { + renderer, + emit: async (event: unknown) => { + if (listenerSignal?.aborted === true) { + return; + } + + await listener?.(event); + }, + signal: () => listenerSignal, + dispatchEvent, + adapter, + }; + } + + const added = (id: string, blockIndex = 0): unknown => new BlockAddedEvent( + { blockIndex } as unknown as BlockIndex, + { name: 'tool', + id: id as BlockId, + data: {} }, + USER_ID + ); + + const removed = (id: string, blockIndex = 0): unknown => new BlockRemovedEvent( + { blockIndex } as unknown as BlockIndex, + { name: 'tool', + id: id as BlockId, + data: {} }, + USER_ID + ); + + it('should call tool destroy when its block is removed', async () => { + const destroy = jest.fn(); + const { emit, adapter, dispatchEvent } = setup(() => ({ render: () => Promise.resolve({}), + destroy })); + + await emit(added('a')); + await emit(removed('a')); + + expect(destroy).toHaveBeenCalledTimes(1); + expect(adapter.destroyBlockToolAdapter).toHaveBeenCalledWith('a'); + expect(dispatchEvent).toHaveBeenCalledTimes(2); + }); + + it('should destroy every remaining tool and block adapter on destroy without dispatching BlockRemovedCoreEvent', async () => { + const destroy = jest.fn(); + const { renderer, emit, adapter, dispatchEvent } = setup(() => ({ render: () => Promise.resolve({}), + destroy })); + + await emit(added('a', 0)); + await emit(added('b', 1)); + dispatchEvent.mockClear(); + + renderer.destroy(); + + expect(destroy).toHaveBeenCalledTimes(2); + expect(adapter.destroyBlockToolAdapter).toHaveBeenCalledWith('a'); + expect(adapter.destroyBlockToolAdapter).toHaveBeenCalledWith('b'); + expect(dispatchEvent).not.toHaveBeenCalled(); + }); + + it('should not destroy a removed block again on destroy', async () => { + const destroy = jest.fn(); + const { renderer, emit } = setup(() => ({ render: () => Promise.resolve({}), + destroy })); + + await emit(added('a')); + await emit(removed('a')); + renderer.destroy(); + + expect(destroy).toHaveBeenCalledTimes(1); + }); + + it('should stop handling model events after destroy', async () => { + const { renderer, emit, signal, dispatchEvent } = setup(() => ({ render: () => Promise.resolve({}) })); + + renderer.destroy(); + await emit(added('a')); + + expect(signal()?.aborted).toBe(true); + expect(dispatchEvent).not.toHaveBeenCalled(); + }); + + it('should not dispatch BlockAddedCoreEvent for a render that settles after destroy', async () => { + let resolveRender: (value: unknown) => void = () => undefined; + const destroy = jest.fn(); + const { renderer, emit, adapter, dispatchEvent } = setup(() => ({ + render: () => new Promise((resolve) => { + resolveRender = resolve; + }), + destroy, + })); + + const pending = emit(added('a')); + + renderer.destroy(); + resolveRender({}); + await pending; + + expect(dispatchEvent).not.toHaveBeenCalled(); + expect(destroy).toHaveBeenCalledTimes(1); + expect(adapter.destroyBlockToolAdapter).toHaveBeenCalledWith('a'); + }); + + it('should keep tearing down when a tool destroy throws', async () => { + let calls = 0; + const { renderer, emit, adapter } = setup(() => ({ + render: () => Promise.resolve({}), + destroy: () => { + calls++; + throw new Error('broken tool'); + }, + })); + + await emit(added('a', 0)); + await emit(added('b', 1)); + + expect(() => renderer.destroy()).not.toThrow(); + expect(calls).toBe(2); + expect(adapter.destroyBlockToolAdapter).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/core/src/components/BlockRenderer.ts b/packages/core/src/components/BlockRenderer.ts index 79a93384..9b4d4e71 100644 --- a/packages/core/src/components/BlockRenderer.ts +++ b/packages/core/src/components/BlockRenderer.ts @@ -6,6 +6,7 @@ import { EventType, ModelEvents } from '@editorjs/sdk'; +import type { BlockId, BlockTool } from '@editorjs/sdk'; import 'reflect-metadata'; import { inject, injectable } from 'inversify'; import { TOKENS } from '../tokens.js'; @@ -53,6 +54,16 @@ export class BlockRenderer { */ #adapter: EditorJSAdapterPlugin; + /** + * Tool instance of each rendered block, destroyed when the block is removed or the renderer is destroyed + */ + #blocks = new Map(); + + /** + * Releases the model listener on destroy + */ + readonly #controller = new AbortController(); + /** * BlockRenderer constructor. * All parameters are injected through the IoC container. @@ -73,7 +84,19 @@ export class BlockRenderer { this.#adapter = adapter; // eslint-disable-next-line @typescript-eslint/no-misused-promises -- Need to bubble the promise up in case of errors - this.#model.addEventListener(EventType.Changed, event => this.#handleModelUpdate(event)); + this.#model.addEventListener(EventType.Changed, event => this.#handleModelUpdate(event), { signal: this.#controller.signal }); + } + + /** + * Stops listening to the model and releases every rendered block: its tool instance and its adapter. + * The model is left untouched and no BlockRemovedCoreEvent is dispatched + */ + public destroy(): void { + this.#controller.abort(); + + for (const id of this.#blocks.keys()) { + this.#releaseBlock(id); + } } /** @@ -112,7 +135,8 @@ export class BlockRenderer { throw new Error(`[BlockRenderer] Block Tool ${data.name} not found`); } - const blockToolAdapter = this.#adapter.createBlockToolAdapter(createBlockId(data.id), tool.name); + const blockId = createBlockId(data.id); + const blockToolAdapter = this.#adapter.createBlockToolAdapter(blockId, tool.name); const block = tool.create({ adapter: blockToolAdapter, @@ -121,9 +145,18 @@ export class BlockRenderer { readOnly: false, }); + this.#blocks.set(blockId, block); + try { const blockElement = await block.render(); + /** + * The renderer was destroyed while the block was rendering, so the block is already released + */ + if (this.#controller.signal.aborted) { + return; + } + this.#eventBus.dispatchEvent(new BlockAddedCoreEvent({ tool: tool.name, data: data.data, @@ -149,15 +182,30 @@ export class BlockRenderer { throw new Error('[BlockRenderer] Block index should be defined. Probably something wrong with the Editor Model. Please, report this issue'); } - this.#adapter.destroyBlockToolAdapter(createBlockId(data.id)); + this.#releaseBlock(createBlockId(data.id)); this.#eventBus.dispatchEvent(new BlockRemovedCoreEvent({ tool: data.name, index: blockIndex, })); + } + + /** + * Destroys the block's tool instance and its adapter. + * A throwing tool is logged so the rest of the teardown still runs + * @param id - id of the block to release + */ + #releaseBlock(id: BlockId): void { + const block = this.#blocks.get(id); + + this.#blocks.delete(id); + + try { + block?.destroy?.(); + } catch (error) { + console.error(`[BlockRenderer] Block Tool failed to destroy block ${id}`, error); + } - /** - * @todo clear block tool adapter memory - */ + this.#adapter.destroyBlockToolAdapter(id); } } diff --git a/packages/core/src/components/SelectionManager.spec.ts b/packages/core/src/components/SelectionManager.spec.ts index 8a475340..082c9fa1 100644 --- a/packages/core/src/components/SelectionManager.spec.ts +++ b/packages/core/src/components/SelectionManager.spec.ts @@ -542,4 +542,72 @@ describe('SelectionManager', () => { expect(selectionManager.selectedBlocks()).toEqual([{ id: 'b0' }]); }); }); + + describe('tool and listener teardown', () => { + /** + * Sets up a caret with one valid text segment + * @param segment - segment returned by the caret index + */ + const mockCaret = (segment: Record): void => { + jest.spyOn(model, 'getCaret') + .mockReturnValue({ + index: { getTextSegments: jest.fn(() => [segment]) }, + update: jest.fn(), + } as unknown as ReturnType); + jest.spyOn(model, 'getFragments').mockReturnValue([]); + jest.spyOn(model, 'format').mockImplementation(() => undefined); + }; + + it('should destroy the throwaway inline tool instance after applying it', () => { + const destroy = jest.fn(); + const toolMock = { + getFormattingOptions: jest.fn(() => ({ action: 'format', + range: [0, 3] })), + destroy, + }; + + (toolsManager as unknown as { inlineTools: Map }).inlineTools = new Map([['bold', { create: () => toolMock }]]); + mockCaret({ blockIndex: 0, + dataKey: 'text', + textRange: [0, 3] }); + + selectionManager.applyInlineTool({ toolName: 'bold' as InlineToolName }); + + expect(destroy).toHaveBeenCalledTimes(1); + }); + + it('should destroy the throwaway inline tool instance when applying it throws', () => { + const destroy = jest.fn(); + const toolMock = { getFormattingOptions: jest.fn(), + destroy }; + + (toolsManager as unknown as { inlineTools: Map }).inlineTools = new Map([['bold', { create: () => toolMock }]]); + mockCaret({ blockIndex: 0, + dataKey: 'text' }); + + expect(() => selectionManager.applyInlineTool({ toolName: 'bold' as InlineToolName })).toThrow(); + expect(destroy).toHaveBeenCalledTimes(1); + }); + + it('should stop handling caret updates after destroy', () => { + // @ts-expect-error - Mocked instance + const ownModel = new EditorJSModel(); + let signal: AbortSignal | undefined; + + ownModel.addEventListener = (_type: EventType, _callback: (e: Event) => void, options?: AddEventListenerOptions) => { + signal = options?.signal; + }; + + const manager = new SelectionManager( + { userId: 'user' } as unknown as CoreConfigValidated, + ownModel, + eventBus, + toolsManager + ); + + manager.destroy(); + + expect(signal?.aborted).toBe(true); + }); + }); }); diff --git a/packages/core/src/components/SelectionManager.ts b/packages/core/src/components/SelectionManager.ts index b128ebf2..ea70f17a 100644 --- a/packages/core/src/components/SelectionManager.ts +++ b/packages/core/src/components/SelectionManager.ts @@ -47,6 +47,11 @@ export class SelectionManager { */ #toolsManager: ToolsManager; + /** + * Releases the caret listener on destroy + */ + readonly #controller = new AbortController(); + /** * @param config - Editor's config * @param model - editor model instance @@ -64,7 +69,18 @@ export class SelectionManager { this.#eventBus = eventBus; this.#toolsManager = toolsManager; - this.#model.addEventListener(EventType.CaretManagerUpdated, (event: CaretManagerEvents) => this.#handleCaretManagerUpdate(event)); + this.#model.addEventListener( + EventType.CaretManagerUpdated, + (event: CaretManagerEvents) => this.#handleCaretManagerUpdate(event), + { signal: this.#controller.signal } + ); + } + + /** + * Stops listening to caret updates + */ + public destroy(): void { + this.#controller.abort(); } /** @@ -175,56 +191,63 @@ export class SelectionManager { throw new Error(`SelectionManager[applyInlineTool]: tool ${toolName} is not attached`); } - for (const segment of segments) { - const textRange = segment.textRange; - const blockIndex = segment.blockIndex; - const dataKey = segment.dataKey; - - if (textRange === undefined) { - throw new IndexError('TextRange of the index should be defined. Probably something wrong with the Editor Model. Please, report this issue'); - } + /** + * The instance is only used for this call, so it is destroyed right after + */ + try { + for (const segment of segments) { + const textRange = segment.textRange; + const blockIndex = segment.blockIndex; + const dataKey = segment.dataKey; + + if (textRange === undefined) { + throw new IndexError('TextRange of the index should be defined. Probably something wrong with the Editor Model. Please, report this issue'); + } - if (blockIndex === undefined) { - throw new IndexError('BlockIndex should be defined. Probably something wrong with the Editor Model. Please, report this issue'); - } + if (blockIndex === undefined) { + throw new IndexError('BlockIndex should be defined. Probably something wrong with the Editor Model. Please, report this issue'); + } - if (dataKey === undefined) { - throw new IndexError('DataKey of the index should be defined. Probably something wrong with the Editor Model. Please, report this issue'); - } + if (dataKey === undefined) { + throw new IndexError('DataKey of the index should be defined. Probably something wrong with the Editor Model. Please, report this issue'); + } - const fragments = this.#model.getFragments(blockIndex, dataKey, ...textRange, toolName); + const fragments = this.#model.getFragments(blockIndex, dataKey, ...textRange, toolName); - const { action, range } = tool.getFormattingOptions(textRange, fragments); + const { action, range } = tool.getFormattingOptions(textRange, fragments); - switch (actionOverride ?? action) { - case FormattingAction.Format: - this.#model.format(userId, blockIndex, dataKey, toolName, ...range, createInlineToolData(data)); + switch (actionOverride ?? action) { + case FormattingAction.Format: + this.#model.format(userId, blockIndex, dataKey, toolName, ...range, createInlineToolData(data)); - break; - case FormattingAction.Unformat: - this.#model.unformat(userId, blockIndex, dataKey, toolName, ...range); + break; + case FormattingAction.Unformat: + this.#model.unformat(userId, blockIndex, dataKey, toolName, ...range); - break; - } + break; + } - /** - * Keep selection param is applied only for the current user - */ - if (userId === this.#config.userId) { - if (keepSelection) { - caret?.update(caretIndex); - } else { - // For composite selections, don't try to add textRange since composite indices - // must not have root-level textRange. Only set textRange for single-segment selections. - const selectedSegments = (caretIndex as TextIndex).getTextSegments(); - - if (selectedSegments.length === 1 && selectedSegments[0].textRange !== undefined) { - caret?.update(caretIndex.withTextRange([selectedSegments[0].textRange[1], selectedSegments[0].textRange[1]])); - } else { + /** + * Keep selection param is applied only for the current user + */ + if (userId === this.#config.userId) { + if (keepSelection) { caret?.update(caretIndex); + } else { + // For composite selections, don't try to add textRange since composite indices + // must not have root-level textRange. Only set textRange for single-segment selections. + const selectedSegments = (caretIndex as TextIndex).getTextSegments(); + + if (selectedSegments.length === 1 && selectedSegments[0].textRange !== undefined) { + caret?.update(caretIndex.withTextRange([selectedSegments[0].textRange[1], selectedSegments[0].textRange[1]])); + } else { + caret?.update(caretIndex); + } } } } + } finally { + tool.destroy?.(); } }; diff --git a/packages/core/src/components/UndoRedoManager.spec.ts b/packages/core/src/components/UndoRedoManager.spec.ts index a1893e4a..bfd10a79 100644 --- a/packages/core/src/components/UndoRedoManager.spec.ts +++ b/packages/core/src/components/UndoRedoManager.spec.ts @@ -163,42 +163,44 @@ describe('UndoRedoManager', () => { it('should register a listener for model Changed events', () => { expect(model.addEventListener).toHaveBeenCalledWith( EventType.Changed, - expect.any(Function) + expect.any(Function), + { signal: expect.any(AbortSignal) } ); }); it('should register an undo listener on the eventBus', () => { expect(eventBus.addEventListener).toHaveBeenCalledWith( 'core:undo', - expect.any(Function) + expect.any(Function), + { signal: expect.any(AbortSignal) } ); }); it('should register a redo listener on the eventBus', () => { expect(eventBus.addEventListener).toHaveBeenCalledWith( 'core:redo', - expect.any(Function) + expect.any(Function), + { signal: expect.any(AbortSignal) } ); }); }); describe('.destroy()', () => { - it('should remove the undo listener from eventBus', () => { - manager.destroy(); - - expect(eventBus.removeEventListener).toHaveBeenCalledWith( - 'core:undo', - expect.any(Function) - ); - }); + /** + * Collects the signals passed to every listener registration + * @param mock - mocked addEventListener + */ + const signalsOf = (mock: jest.Mock): (AbortSignal | undefined)[] => mock.mock.calls.map( + call => (call[2] as AddEventListenerOptions | undefined)?.signal + ); - it('should remove the redo listener from eventBus', () => { + it('should abort the signal of the undo and redo listeners', () => { manager.destroy(); - expect(eventBus.removeEventListener).toHaveBeenCalledWith( - 'core:redo', - expect.any(Function) - ); + const signals = signalsOf(eventBus.addEventListener); + + expect(signals).toHaveLength(2); + expect(signals.every(signal => signal?.aborted === true)).toBe(true); }); it('should clear the debounce timer', () => { @@ -211,13 +213,12 @@ describe('UndoRedoManager', () => { expect(clearTimeoutSpy).toHaveBeenCalled(); }); - it('should remove the model updates listener', () => { + it('should abort the signal of the model updates listener', () => { manager.destroy(); - expect(model.removeEventListener).toHaveBeenCalledWith( - 'model:changed', - expect.any(Function) - ); + const [signal] = signalsOf(model.addEventListener); + + expect(signal?.aborted).toBe(true); }); }); diff --git a/packages/core/src/components/UndoRedoManager.ts b/packages/core/src/components/UndoRedoManager.ts index 3bdf018d..8abb1e1d 100644 --- a/packages/core/src/components/UndoRedoManager.ts +++ b/packages/core/src/components/UndoRedoManager.ts @@ -41,11 +41,6 @@ export class UndoRedoManager { */ #model: EditorJSModel; - /** - * Editor's EventBus instance - */ - #eventBus: EventBus; - /** * Editor configuration (provides the current user id). */ @@ -79,36 +74,9 @@ export class UndoRedoManager { #isApplying = false; /** - * Undo Core Event listener. Stored to be removed on destroy - * @param event - undo core event + * Releases the manager's model and EventBus listeners on destroy */ - #undoListener = (event: UndoCoreEvent): void => { - if (event.defaultPrevented) { - return; - } - - this.undo(); - }; - - /** - * Redo Core Event listener. Stored to be removed on destroy - * @param event - redo core event - */ - #redoListener = (event: RedoCoreEvent): void => { - if (event.defaultPrevented) { - return; - } - - this.redo(); - }; - - /** - * Model updates listener. Stored to be removed on destroy - * @param e - model event - */ - #modelUpdatesListener = (e: ModelEvents): void => { - this.#handleEvent(e); - }; + readonly #controller = new AbortController(); /** * UndoRedoManager constructor. @@ -124,12 +92,26 @@ export class UndoRedoManager { ) { this.#config = config; this.#model = model; - this.#eventBus = eventBus; - model.addEventListener(EventType.Changed, this.#modelUpdatesListener); + const { signal } = this.#controller; + + model.addEventListener(EventType.Changed, (e: ModelEvents) => this.#handleEvent(e), { signal }); + + eventBus.addEventListener(`core:${CoreEventType.Undo}`, (event: UndoCoreEvent) => { + if (event.defaultPrevented) { + return; + } + + this.undo(); + }, { signal }); + + eventBus.addEventListener(`core:${CoreEventType.Redo}`, (event: RedoCoreEvent) => { + if (event.defaultPrevented) { + return; + } - eventBus.addEventListener(`core:${CoreEventType.Undo}`, this.#undoListener); - eventBus.addEventListener(`core:${CoreEventType.Redo}`, this.#redoListener); + this.redo(); + }, { signal }); } /** @@ -180,13 +162,11 @@ export class UndoRedoManager { } /** - * Releases resources held by the manager (the pending debounce timer). + * Releases resources held by the manager: its listeners and the pending debounce timer */ public destroy(): void { clearTimeout(this.#debounceTimer); - this.#eventBus.removeEventListener(`core:${CoreEventType.Undo}`, this.#undoListener); - this.#eventBus.removeEventListener(`core:${CoreEventType.Redo}`, this.#redoListener); - this.#model.removeEventListener(EventType.Changed, this.#modelUpdatesListener); + this.#controller.abort(); } /** diff --git a/packages/core/src/index.spec.ts b/packages/core/src/index.spec.ts index 3338540a..a81fe4d8 100644 --- a/packages/core/src/index.spec.ts +++ b/packages/core/src/index.spec.ts @@ -1,7 +1,7 @@ /* eslint-disable jsdoc/require-jsdoc -- inline test stubs */ -import { describe, it, expect } from '@jest/globals'; -import { PluginType, ToolType } from '@editorjs/sdk'; -import type { CoreConfig } from '@editorjs/sdk'; +import { describe, it, expect, jest, beforeEach } from '@jest/globals'; +import { CoreEventType, PluginType, ToolType } from '@editorjs/sdk'; +import type { CoreConfig, EditorjsPluginParams } from '@editorjs/sdk'; import Core from './index.js'; /** @@ -107,4 +107,245 @@ describe('Core', () => { expect(StubPlugin.instances).toBe(1); }); }); + + describe('destroy()', () => { + /** + * Records teardown calls across plugins, tools and the adapter, in order + */ + let log: string[]; + + beforeEach(() => { + log = []; + }); + + /** + * Adapter recording its teardown + */ + const createAdapter = (): unknown => class RecordingAdapter { + public static type = PluginType.Adapter as const; + public static name = 'recording-adapter'; + + public createBlockToolAdapter(): object { + return {}; + } + + /** + * The v2 data import assigns block ids, so the id isn't recorded + */ + public destroyBlockToolAdapter(): void { + log.push('adapter:block'); + } + + public destroy(): void { + log.push('adapter'); + } + }; + + /** + * Block tool recording its construction and teardown + */ + const createBlockTool = (): unknown => class RecordingBlockTool { + public static type = ToolType.Block as const; + public static name = 'paragraph'; + + public render(): Promise { + return Promise.resolve({}); + } + + public destroy(): void { + log.push('tool'); + } + }; + + /** + * Plugin recording its teardown under the given name + * @param name - static name the plugin registers its public API under + * @param onCreate - receives the plugin's constructor params + */ + const createPlugin = (name: string, onCreate?: (params: EditorjsPluginParams) => void): unknown => class RecordingPlugin { + public static type = PluginType.Plugin as const; + public static name = name; + + public publicApi = { name }; + + constructor(params: EditorjsPluginParams) { + onCreate?.(params); + } + + public destroy(): void { + log.push(`plugin:${name}`); + } + }; + + /** + * Creates a core with one block of data and the recording adapter and block tool registered + * @param overrides - config overrides + */ + const createCore = (overrides: Partial = {}): Core => { + const core = new Core(createConfig({ + data: { blocks: [{ id: 'block-1', + type: 'paragraph', + data: {} }] }, + ...overrides, + } as Partial)); + + core.use(createAdapter() as typeof StubAdapter); + core.use(createBlockTool() as typeof StubBlockTool); + + return core; + }; + + /** + * Lets the block render promise settle + */ + const flush = (): Promise => new Promise(resolve => setTimeout(resolve, 0)); + + it('should call plugin destroy in reverse construction order', async () => { + const core = createCore(); + + core.use(createPlugin('a') as typeof StubPlugin); + core.use(createPlugin('b') as typeof StubPlugin); + core.use(createPlugin('c') as typeof StubPlugin); + await core.initialize(); + + core.destroy(); + + expect(log.filter(entry => entry.startsWith('plugin:'))).toEqual(['plugin:c', 'plugin:b', 'plugin:a']); + }); + + it('should unregister plugin public APIs', async () => { + let params: EditorjsPluginParams | undefined; + const core = createCore(); + + core.use(createPlugin('probe', (p) => { + params = p; + }) as typeof StubPlugin); + await core.initialize(); + + expect((params?.api.plugins as Record).probe).toBeDefined(); + + core.destroy(); + + expect((params?.api.plugins as Record).probe).toBeUndefined(); + }); + + it('should continue teardown and log when a plugin destroy throws', async () => { + const error = jest.spyOn(console, 'error').mockImplementation(() => undefined); + const core = createCore(); + + core.use(createPlugin('a') as typeof StubPlugin); + core.use(class BrokenPlugin { + public static type = PluginType.Plugin as const; + public static name = 'broken'; + + public destroy(): void { + throw new Error('broken plugin'); + } + } as unknown as typeof StubPlugin); + await core.initialize(); + await flush(); + + expect(() => core.destroy()).not.toThrow(); + expect(error).toHaveBeenCalled(); + expect(log).toEqual(['plugin:a', 'tool', 'adapter:block', 'adapter']); + + error.mockRestore(); + }); + + it('should destroy plugins, then rendered blocks, then the adapter', async () => { + const core = createCore(); + + core.use(createPlugin('a') as typeof StubPlugin); + await core.initialize(); + await flush(); + + core.destroy(); + + expect(log).toEqual(['plugin:a', 'tool', 'adapter:block', 'adapter']); + }); + + it('should not emit model events during destroy', async () => { + const onModelUpdate = jest.fn(); + const core = createCore({ onModelUpdate }); + + await core.initialize(); + await flush(); + onModelUpdate.mockClear(); + + core.destroy(); + + expect(onModelUpdate).not.toHaveBeenCalled(); + }); + + it('should be a no-op when called twice', async () => { + const core = createCore(); + + core.use(createPlugin('a') as typeof StubPlugin); + await core.initialize(); + await flush(); + + core.destroy(); + core.destroy(); + + expect(log).toEqual(['plugin:a', 'tool', 'adapter:block', 'adapter']); + }); + + it('should not construct anything when destroyed before initialize', () => { + const created = jest.fn(); + const core = createCore(); + + core.use(createPlugin('a', created) as typeof StubPlugin); + + core.destroy(); + + expect(created).not.toHaveBeenCalled(); + expect(log).toEqual([]); + }); + + it('should resolve initialize without dispatching ready when destroyed mid-initialization', async () => { + let resolvePrepare: () => void = () => undefined; + const ready = jest.fn(); + const onModelUpdate = jest.fn(); + const core = createCore({ onModelUpdate }); + + core.use(createPlugin('a', ({ eventBus }) => { + eventBus.addEventListener(`core:${CoreEventType.Ready}`, ready); + }) as typeof StubPlugin); + core.use(class SlowInlineTool { + public static type = ToolType.Inline as const; + public static name = 'slow'; + + public static prepare(): Promise { + return new Promise((resolve) => { + resolvePrepare = resolve; + }); + } + } as unknown as typeof StubBlockTool); + + const initialization = core.initialize(); + + /** + * Let the tools queue reach the pending prepare() + */ + await flush(); + core.destroy(); + resolvePrepare(); + + await expect(initialization).resolves.toBeUndefined(); + await flush(); + + expect(ready).not.toHaveBeenCalled(); + expect(onModelUpdate).not.toHaveBeenCalled(); + expect(log).toEqual(['plugin:a', 'adapter']); + }); + + it('should throw from use and initialize after destroy', async () => { + const core = createCore(); + + core.destroy(); + + expect(() => core.use(createPlugin('a') as typeof StubPlugin)).toThrow(/destroyed/); + await expect(core.initialize()).rejects.toThrow(/destroyed/); + }); + }); }); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 01ab27ae..5a97c5bb 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -12,7 +12,9 @@ import { PluginType, ToolType, type ToolStaticOptions, - type PluginId + type PluginId, + type EditorjsPlugin, + type EditorJSAdapterPlugin } from '@editorjs/sdk'; import { composeDataFromVersion2 } from './utils/composeDataFromVersion2.js'; import ToolsManager from './tools/ToolsManager.js'; @@ -31,6 +33,46 @@ import { PluginRegistry } from './components/PluginRegistry.js'; */ const DEFAULT_HOLDER_ID = 'editorjs'; +/** + * Plugin instance created by Core, with the name its public API is registered under + */ +interface PluginInstance { + /** + * Plugin's static name + */ + name: PluginId; + + /** + * Plugin instance + */ + instance: EditorjsPlugin; +} + +/** + * Core services that hold listeners or resources and need teardown + */ +interface CoreServices { + /** + * Tracks the caret and applies inline tools + */ + selectionManager?: SelectionManager; + + /** + * Renders blocks and owns their tool instances + */ + blockRenderer?: BlockRenderer; + + /** + * Local undo/redo history + */ + undoRedoManager?: UndoRedoManager; + + /** + * Rendering adapter plugin + */ + adapter?: EditorJSAdapterPlugin; +} + /** * Editor entry point * - initializes Model @@ -59,6 +101,22 @@ export default class Core { */ #plugins: Container; + /** + * Plugin instances in construction order, destroyed in reverse order + */ + #pluginInstances: PluginInstance[] = []; + + /** + * Core services resolved during initialization, destroyed by {@link destroy}. + * Kept here so teardown never resolves a service that initialization didn't reach + */ + #services: CoreServices = {}; + + /** + * Set by {@link destroy}. A destroyed core can't be used or initialized again + */ + #destroyed = false; + /** * @param config - Editor configuration */ @@ -129,6 +187,8 @@ export default class Core { pluginOrTool: ToolConstructable | EditorjsPluginConstructor | EditorjsAdapterPluginConstructor, options?: ToolStaticOptions ): Core { + this.#assertNotDestroyed(); + const pluginType = pluginOrTool.type; switch (pluginType) { @@ -155,6 +215,7 @@ export default class Core { * Initializes the core */ public async initialize(): Promise { + this.#assertNotDestroyed(); this.#validatePreconditions(); const { blocks } = composeDataFromVersion2(this.#config.data ?? { blocks: [] }); @@ -167,18 +228,26 @@ export default class Core { * @todo add e2e initialization tests * Currently only BlockRenderer would be enough, but that would be hard to debug. Easier just add every module here */ - this.#iocContainer.get(SelectionManager); + this.#services.selectionManager = this.#iocContainer.get(SelectionManager); this.#iocContainer.get(BlocksManager); - this.#iocContainer.get(BlockRenderer); + this.#services.blockRenderer = this.#iocContainer.get(BlockRenderer); + this.#services.adapter = this.#iocContainer.get(TOKENS.Adapter); this.#initializePlugins(); await this.#initializeTools(); + /** + * destroy() was called while tools were being prepared: it already tore down what exists + */ + if (this.#destroyed) { + return; + } + /** * UndoRedoManager should go after plugins as it uses defaultPrevented on undo/redo events which can be set by plugins * @todo Figure out how to make initialization less complex */ - this.#iocContainer.get(UndoRedoManager); + this.#services.undoRedoManager = this.#iocContainer.get(UndoRedoManager); this.#model.initializeDocument({ blocks }); @@ -187,6 +256,59 @@ export default class Core { eventBus.dispatchEvent(new CoreEventBase(CoreEventType.Ready, undefined)); } + /** + * Tears the editor down: plugins in reverse construction order, core services, rendered blocks, and the adapter. + * The document model is left untouched, so no removal reaches undo history or collaboration. + * Safe to call more than once and while {@link initialize} is pending. After it, `use()` and `initialize()` throw + */ + public destroy(): void { + if (this.#destroyed) { + return; + } + + this.#destroyed = true; + + const registry = this.#iocContainer.get(PluginRegistry); + + for (const { name, instance } of this.#pluginInstances.reverse()) { + this.#safely(`plugin "${name}"`, () => instance.destroy?.()); + registry.unregister(name); + } + + this.#pluginInstances = []; + + const { undoRedoManager, selectionManager, blockRenderer, adapter } = this.#services; + + this.#safely('UndoRedoManager', () => undoRedoManager?.destroy()); + this.#safely('SelectionManager', () => selectionManager?.destroy()); + this.#safely('BlockRenderer', () => blockRenderer?.destroy()); + this.#safely('adapter', () => adapter?.destroy?.()); + + this.#services = {}; + } + + /** + * Runs one teardown step, logging its error so the remaining steps still run + * @param label - what is being destroyed, for the log + * @param step - teardown step + */ + #safely(label: string, step: () => void): void { + try { + step(); + } catch (error) { + console.error(`[Core] Failed to destroy ${label}`, error); + } + } + + /** + * Throws if {@link destroy} has been called + */ + #assertNotDestroyed(): void { + if (this.#destroyed) { + throw new Error('Editor has been destroyed. Create a new instance instead'); + } + } + /** * Validates that the caller has registered everything the headless engine needs before booting. * Throws with a clear message instead of failing deep inside module resolution. @@ -252,6 +374,9 @@ export default class Core { eventBus, }); + this.#pluginInstances.push({ name: plugin.name, + instance }); + if (instance.publicApi !== undefined) { this.#iocContainer.get(PluginRegistry).register(plugin.name, instance.publicApi); } diff --git a/packages/core/src/tools/ToolsManager.spec.ts b/packages/core/src/tools/ToolsManager.spec.ts new file mode 100644 index 00000000..dabb0f3b --- /dev/null +++ b/packages/core/src/tools/ToolsManager.spec.ts @@ -0,0 +1,36 @@ +import { describe, expect, it, jest } from '@jest/globals'; +import type { CoreConfigValidated, InlineToolConstructor } from '@editorjs/sdk'; +import { EventBus, ToolType } from '@editorjs/sdk'; +import ToolsManager from './ToolsManager.js'; +import type { EditorAPI } from '../api/index.js'; + +describe('ToolsManager', () => { + describe('.prepareTools()', () => { + it('should destroy the inline tool instance created for validation', async () => { + const destroy = jest.fn(); + + /** + * Inline tool with a static prepare(), so the validation branch runs + */ + class ProbeInlineTool { + public static type = ToolType.Inline; + + public static name = 'probe'; + + public static prepare = jest.fn(() => Promise.resolve()); + + public destroy = destroy; + } + + const manager = new ToolsManager( + { tools: {} } as unknown as CoreConfigValidated, + () => ({}) as EditorAPI, + new EventBus() + ); + + await manager.prepareTools([[ProbeInlineTool as unknown as InlineToolConstructor, undefined]]); + + expect(destroy).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/packages/core/src/tools/ToolsManager.ts b/packages/core/src/tools/ToolsManager.ts index 297f30fd..42792d38 100644 --- a/packages/core/src/tools/ToolsManager.ts +++ b/packages/core/src/tools/ToolsManager.ts @@ -150,7 +150,13 @@ export default class ToolsManager { * Some Tools validation */ const inlineToolRequiredMethods = ['render']; - const notImplementedMethods = inlineToolRequiredMethods.filter(method => tool.create()[method as keyof InlineTool] !== undefined); + const instance = tool.create(); + const notImplementedMethods = inlineToolRequiredMethods.filter(method => instance[method as keyof InlineTool] !== undefined); + + /** + * The instance is only used for validation + */ + instance.destroy?.(); if (notImplementedMethods.length > 0) { /** diff --git a/packages/editorjs/src/index.spec.ts b/packages/editorjs/src/index.spec.ts index fa64ca57..4cfc0355 100644 --- a/packages/editorjs/src/index.spec.ts +++ b/packages/editorjs/src/index.spec.ts @@ -9,6 +9,7 @@ import { jest, describe, it, expect, beforeEach } from '@jest/globals'; */ const use = jest.fn(); const initialize = jest.fn<() => Promise>(); +const destroy = jest.fn(); /** * Minimal Core stand-in recording `use()` calls and delegating `initialize()`. @@ -16,6 +17,7 @@ const initialize = jest.fn<() => Promise>(); class MockCore { public use = use; public initialize = initialize; + public destroy = destroy; } jest.unstable_mockModule('@editorjs/core', () => ({ default: MockCore })); @@ -40,6 +42,17 @@ describe('EditorJS bundle', () => { beforeEach(() => { use.mockClear(); initialize.mockReset(); + destroy.mockClear(); + }); + + it('should delegate destroy to Core', () => { + initialize.mockResolvedValue(undefined); + + const editor = new EditorJS({} as any); + + editor.destroy(); + + expect(destroy).toHaveBeenCalledTimes(1); }); it('exposes isReady that resolves when Core initialization completes', async () => { diff --git a/packages/editorjs/src/index.ts b/packages/editorjs/src/index.ts index 2a42d0e6..4412ba38 100644 --- a/packages/editorjs/src/index.ts +++ b/packages/editorjs/src/index.ts @@ -99,6 +99,14 @@ export default class EditorJS { public get isReady(): Promise { return this.#readyPromise; } + + /** + * Tears the editor down: removes its DOM, closes the collaboration connection, and releases plugins and tools. + * Safe to call more than once and before {@link isReady} settles + */ + public destroy(): void { + this.#core.destroy(); + } } export { Core }; diff --git a/packages/model/src/EditorJSModel.ts b/packages/model/src/EditorJSModel.ts index 8b9d7521..3682e415 100644 --- a/packages/model/src/EditorJSModel.ts +++ b/packages/model/src/EditorJSModel.ts @@ -36,12 +36,12 @@ export interface EditorJSModel { /** * Overload for EditorDocument events */ - addEventListener(type: EventType.Changed, listener: (event: K) => void): void; + addEventListener(type: EventType.Changed, listener: (event: K) => void, options?: AddEventListenerOptions | boolean): void; /** * Overload for CaretManager events */ - addEventListener(type: EventType.CaretManagerUpdated, listener: (event: K) => void): void; + addEventListener(type: EventType.CaretManagerUpdated, listener: (event: K) => void, options?: AddEventListenerOptions | boolean): void; } /** diff --git a/packages/sdk/src/entities/BlockTool.ts b/packages/sdk/src/entities/BlockTool.ts index 1f77f69c..a696f97c 100644 --- a/packages/sdk/src/entities/BlockTool.ts +++ b/packages/sdk/src/entities/BlockTool.ts @@ -163,7 +163,13 @@ export type BlockTool< */ // eslint-disable-next-line @typescript-eslint/no-unused-vars, @typescript-eslint/no-explicit-any Config extends ToolConfig = any -> = Omit; +> = Omit & { + /** + * Releases resources held by the instance. + * Called once, when the block is removed or the editor is destroyed + */ + destroy?(): void; +}; /** * Block Tool constructor class diff --git a/packages/sdk/src/entities/BlockToolAdapter.spec.ts b/packages/sdk/src/entities/BlockToolAdapter.spec.ts new file mode 100644 index 00000000..78c2465a --- /dev/null +++ b/packages/sdk/src/entities/BlockToolAdapter.spec.ts @@ -0,0 +1,114 @@ +/* eslint-disable jsdoc/require-jsdoc */ + +import { describe, expect, it, jest } from '@jest/globals'; +import type { CoreConfig } from './Config.js'; +import type { EditorAPI } from '../api/index.js'; +import type { BlockId, EventBus, ModelEvents } from '@editorjs/model-types'; +import { BlockToolAdapter } from './BlockToolAdapter.js'; + +const MODEL_EVENT = 'model:update'; +const BUS_EVENT = 'ui:probe'; + +/** + * Adapter subclass that registers its own listener with the inherited signal + */ +class ProbeAdapter extends BlockToolAdapter { + public readonly modelUpdates = jest.fn(); + + public readonly busEvents = jest.fn(); + + constructor(config: Required, api: EditorAPI, eventBus: EventBus) { + super(config, api, eventBus); + + eventBus.addEventListener(BUS_EVENT, () => this.busEvents(), { signal: this.signal }); + } + + protected handleModelUpdate(event: ModelEvents): void { + this.modelUpdates(event); + } +} + +/** + * Adapter under test with the targets standing in for the model and the EventBus + */ +interface Harness { + adapter: ProbeAdapter; + model: EventTarget; + eventBus: EventTarget; +} + +/** + * Creates an adapter wired to real EventTargets standing in for the model and the EventBus + */ +function createAdapter(): Harness { + const model = new EventTarget(); + const eventBus = new EventTarget(); + const blockId = 'block' as BlockId; + + const api = { + document: { + onUpdate: (callback: EventListener) => { + const controller = new AbortController(); + + model.addEventListener(MODEL_EVENT, callback, { signal: controller.signal }); + + return () => controller.abort(); + }, + }, + blocks: { + getIdByIndex: () => blockId, + }, + } as unknown as EditorAPI; + + const adapter = new ProbeAdapter({} as Required, api, eventBus as EventBus); + + adapter.setBlockId(blockId); + + return { adapter, + model, + eventBus }; +} + +/** + * Model event shaped enough for the adapter's index check + */ +function modelEvent(): Event { + return Object.assign(new Event(MODEL_EVENT), { detail: { index: { blockIndex: 0 } } }); +} + +describe('BlockToolAdapter', () => { + it('should handle model updates before destroy', () => { + const { adapter, model } = createAdapter(); + + model.dispatchEvent(modelEvent()); + + expect(adapter.modelUpdates).toHaveBeenCalledTimes(1); + }); + + it('should stop handling model updates after destroy', () => { + const { adapter, model } = createAdapter(); + + adapter.destroy(); + model.dispatchEvent(modelEvent()); + + expect(adapter.modelUpdates).not.toHaveBeenCalled(); + }); + + it('should remove a subclass listener registered with the inherited signal on destroy', () => { + const { adapter, eventBus } = createAdapter(); + + eventBus.dispatchEvent(new Event(BUS_EVENT)); + adapter.destroy(); + eventBus.dispatchEvent(new Event(BUS_EVENT)); + + expect(adapter.busEvents).toHaveBeenCalledTimes(1); + }); + + it('should abort the inherited signal on destroy', () => { + const { adapter } = createAdapter(); + + adapter.destroy(); + + expect((adapter as unknown as { signal: AbortSignal }).signal.aborted).toBe(true); + }); +}); diff --git a/packages/sdk/src/entities/BlockToolAdapter.ts b/packages/sdk/src/entities/BlockToolAdapter.ts index faa6d212..8b828f90 100644 --- a/packages/sdk/src/entities/BlockToolAdapter.ts +++ b/packages/sdk/src/entities/BlockToolAdapter.ts @@ -38,9 +38,15 @@ export abstract class BlockToolAdapter extends EventTarget { protected eventBus: EventBus; /** - * Stored reference to the model change listener so it can be removed on destroy. + * Releases every listener registered by the adapter and its subclasses on destroy */ - #modelChangeListenerCleanup: () => void; + readonly #controller = new AbortController(); + + /** + * Signal subclasses pass to their own listener registrations. + * It is aborted by {@link destroy}, so subclasses don't need to remove listeners themselves + */ + protected readonly signal: AbortSignal = this.#controller.signal; /** * @param config - editor's configuration @@ -54,19 +60,20 @@ export abstract class BlockToolAdapter extends EventTarget { this.config = config; this.eventBus = eventBus; - this.#modelChangeListenerCleanup = this.#api.document.onUpdate( + const unsubscribe = this.#api.document.onUpdate( ((event: ModelEvents) => this.#handleModelUpdate(event)) as EventListener ); + + this.signal.addEventListener('abort', unsubscribe, { once: true }); } /** * Releases all resources held by this adapter. - * Removes the model change listener registered in the constructor. - * Subclasses that register additional listeners should override this method, - * call `super.destroy()`, and then remove their own listeners. + * Aborts {@link signal}, removing the model listener and every listener subclasses registered with it. + * Subclasses override this method only to release resources that aren't listeners, and call `super.destroy()` */ public destroy(): void { - this.#modelChangeListenerCleanup(); + this.#controller.abort(); } /** diff --git a/packages/sdk/src/entities/InlineTool.ts b/packages/sdk/src/entities/InlineTool.ts index 3e167e87..da7be561 100644 --- a/packages/sdk/src/entities/InlineTool.ts +++ b/packages/sdk/src/entities/InlineTool.ts @@ -125,6 +125,12 @@ export interface InlineTool extends Omit; + + /** + * Releases resources held by the instance. + * Called once, when the editor stops using this instance + */ + destroy?(): void; } /** diff --git a/packages/sdk/src/entities/ToolDestroy.spec.ts b/packages/sdk/src/entities/ToolDestroy.spec.ts new file mode 100644 index 00000000..cade0252 --- /dev/null +++ b/packages/sdk/src/entities/ToolDestroy.spec.ts @@ -0,0 +1,56 @@ +import { describe, expect, it, jest } from '@jest/globals'; +import type { BlockTool } from './BlockTool.js'; +import type { InlineTool } from './InlineTool.js'; + +/** + * Builds the minimal inline tool members, so each case only adds what it checks + */ +function inlineToolBase(): Omit { + return { + isActive: () => false, + getFormattingOptions: () => ({ action: 'format', + range: [0, 0] }) as never, + createWrapper: () => ({}) as HTMLElement, + getToolbarConfig: () => ({}) as never, + }; +} + +describe('Tool destroy contract', () => { + it('should accept an inline tool that implements destroy', () => { + const destroy = jest.fn(); + const tool: InlineTool = { + ...inlineToolBase(), + destroy, + }; + + tool.destroy?.(); + + expect(destroy).toHaveBeenCalledTimes(1); + }); + + it('should accept an inline tool without destroy', () => { + const tool: InlineTool = inlineToolBase(); + + expect('destroy' in tool).toBe(false); + }); + + it('should accept a block tool that implements destroy', () => { + const destroy = jest.fn(); + const tool: BlockTool = { + render: () => ({}) as HTMLElement, + destroy, + }; + + tool.destroy?.(); + + expect(destroy).toHaveBeenCalledTimes(1); + }); + + it('should accept a block tool without destroy', () => { + const tool: BlockTool = { + render: () => ({}) as HTMLElement, + }; + + expect('destroy' in tool).toBe(false); + }); +}); From d397f4dd70e8a3662a1619ef4db44c152275313f Mon Sep 17 00:00:00 2001 From: gohabereg Date: Tue, 6 Oct 2026 01:16:38 +0100 Subject: [PATCH 03/10] test(core): kill surviving mutants in the destroy lifecycle - Cover teardown error logging, services that were never resolved, an adapter without destroy(), and blocks removed without a tracked tool. - Cover ToolsManager inline tool validation around the destroyed validation instance. - Drop the redundant destroyed guard in Core.destroy(): clearing plugin instances and services already makes a second call a no-op. - Drop { once: true } on the BlockToolAdapter abort listener: a signal aborts only once. Co-Authored-By: Claude Opus 5.5 --- .../core/src/components/BlockRenderer.spec.ts | 23 +++++ packages/core/src/index.spec.ts | 93 ++++++++++++++++++- packages/core/src/index.ts | 7 +- packages/core/src/tools/ToolsManager.spec.ts | 56 +++++++++++ packages/sdk/src/entities/BlockToolAdapter.ts | 2 +- 5 files changed, 175 insertions(+), 6 deletions(-) diff --git a/packages/core/src/components/BlockRenderer.spec.ts b/packages/core/src/components/BlockRenderer.spec.ts index b3e312e7..62bcedfd 100644 --- a/packages/core/src/components/BlockRenderer.spec.ts +++ b/packages/core/src/components/BlockRenderer.spec.ts @@ -253,6 +253,10 @@ describe('BlockRenderer (unit, mocked deps)', () => { }); describe('BlockRenderer lifecycle', () => { + beforeEach(() => { + (console.error as jest.Mock).mockClear(); + }); + interface Setup { renderer: InstanceType; emit: (event: unknown) => Promise; @@ -418,5 +422,24 @@ describe('BlockRenderer lifecycle', () => { expect(() => renderer.destroy()).not.toThrow(); expect(calls).toBe(2); expect(adapter.destroyBlockToolAdapter).toHaveBeenCalledTimes(2); + expect(console.error).toHaveBeenCalledWith('[BlockRenderer] Block Tool failed to destroy block a', expect.any(Error)); + }); + + it('should not log when a tool has no destroy', async () => { + const { renderer, emit } = setup(() => ({ render: () => Promise.resolve({}) })); + + await emit(added('a')); + renderer.destroy(); + + expect(console.error).not.toHaveBeenCalled(); + }); + + it('should destroy the adapter of a removed block that has no tracked tool instance', async () => { + const { emit, adapter } = setup(() => ({ render: () => Promise.resolve({}) })); + + await emit(removed('unknown')); + + expect(adapter.destroyBlockToolAdapter).toHaveBeenCalledWith('unknown'); + expect(console.error).not.toHaveBeenCalled(); }); }); diff --git a/packages/core/src/index.spec.ts b/packages/core/src/index.spec.ts index a81fe4d8..06b38c0e 100644 --- a/packages/core/src/index.spec.ts +++ b/packages/core/src/index.spec.ts @@ -3,6 +3,9 @@ import { describe, it, expect, jest, beforeEach } from '@jest/globals'; import { CoreEventType, PluginType, ToolType } from '@editorjs/sdk'; import type { CoreConfig, EditorjsPluginParams } from '@editorjs/sdk'; import Core from './index.js'; +import { UndoRedoManager } from './components/UndoRedoManager.js'; +import { SelectionManager } from './components/SelectionManager.js'; +import { BlockRenderer } from './components/BlockRenderer.js'; /** * `Core` is headless: it registers nothing by default, so `initialize()` validates @@ -246,7 +249,7 @@ describe('Core', () => { await flush(); expect(() => core.destroy()).not.toThrow(); - expect(error).toHaveBeenCalled(); + expect(error).toHaveBeenCalledWith('[Core] Failed to destroy plugin "broken"', expect.any(Error)); expect(log).toEqual(['plugin:a', 'tool', 'adapter:block', 'adapter']); error.mockRestore(); @@ -278,6 +281,7 @@ describe('Core', () => { }); it('should be a no-op when called twice', async () => { + const error = jest.spyOn(console, 'error').mockImplementation(() => undefined); const core = createCore(); core.use(createPlugin('a') as typeof StubPlugin); @@ -288,6 +292,93 @@ describe('Core', () => { core.destroy(); expect(log).toEqual(['plugin:a', 'tool', 'adapter:block', 'adapter']); + expect(error).not.toHaveBeenCalled(); + + error.mockRestore(); + }); + + it.each([ + ['UndoRedoManager', UndoRedoManager], + ['SelectionManager', SelectionManager], + ['BlockRenderer', BlockRenderer], + ])('should destroy %s', async (_label, Service) => { + const destroy = jest.spyOn(Service.prototype, 'destroy'); + const core = createCore(); + + await core.initialize(); + core.destroy(); + + expect(destroy).toHaveBeenCalledTimes(1); + + destroy.mockRestore(); + }); + + it.each([ + ['UndoRedoManager', UndoRedoManager], + ['SelectionManager', SelectionManager], + ['BlockRenderer', BlockRenderer], + ])('should log and continue when %s fails to destroy', async (label, Service) => { + const error = jest.spyOn(console, 'error').mockImplementation(() => undefined); + const destroy = jest.spyOn(Service.prototype, 'destroy').mockImplementation(() => { + throw new Error('broken service'); + }); + const core = createCore(); + + await core.initialize(); + core.destroy(); + + expect(error).toHaveBeenCalledWith(`[Core] Failed to destroy ${label}`, expect.any(Error)); + expect(log).toContain('adapter'); + + destroy.mockRestore(); + error.mockRestore(); + }); + + it('should log and continue when the adapter fails to destroy', async () => { + const error = jest.spyOn(console, 'error').mockImplementation(() => undefined); + const core = new Core(createConfig()); + + core.use(class BrokenAdapter { + public static type = PluginType.Adapter as const; + + public destroy(): void { + throw new Error('broken adapter'); + } + } as unknown as typeof StubAdapter); + core.use(StubBlockTool); + await core.initialize(); + + core.destroy(); + + expect(error).toHaveBeenCalledWith('[Core] Failed to destroy adapter', expect.any(Error)); + + error.mockRestore(); + }); + + it('should not log when the adapter has no destroy', async () => { + const error = jest.spyOn(console, 'error').mockImplementation(() => undefined); + const core = new Core(createConfig()); + + core.use(StubAdapter); + core.use(StubBlockTool); + await core.initialize(); + + core.destroy(); + + expect(error).not.toHaveBeenCalled(); + + error.mockRestore(); + }); + + it('should not log when destroyed before initialize', () => { + const error = jest.spyOn(console, 'error').mockImplementation(() => undefined); + const core = createCore(); + + core.destroy(); + + expect(error).not.toHaveBeenCalled(); + + error.mockRestore(); }); it('should not construct anything when destroyed before initialize', () => { diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 5a97c5bb..e29fa08c 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -262,10 +262,9 @@ export default class Core { * Safe to call more than once and while {@link initialize} is pending. After it, `use()` and `initialize()` throw */ public destroy(): void { - if (this.#destroyed) { - return; - } - + /** + * Plugin instances and services are cleared below, so a second call finds nothing left to destroy + */ this.#destroyed = true; const registry = this.#iocContainer.get(PluginRegistry); diff --git a/packages/core/src/tools/ToolsManager.spec.ts b/packages/core/src/tools/ToolsManager.spec.ts index dabb0f3b..c727a48b 100644 --- a/packages/core/src/tools/ToolsManager.spec.ts +++ b/packages/core/src/tools/ToolsManager.spec.ts @@ -32,5 +32,61 @@ describe('ToolsManager', () => { expect(destroy).toHaveBeenCalledTimes(1); }); + + /** + * Prepares a single inline tool whose instance is built by `create` + * @param instance - prototype members of the tool instance + */ + const prepareInlineTool = async (instance: Record): Promise => { + /** + * Inline tool whose instance carries the given members + */ + class ProbeInlineTool { + public static type = ToolType.Inline; + + public static name = 'probe'; + + /** + * Copies the given members onto the instance + */ + constructor() { + Object.assign(this, instance); + } + + /** + * Static prepare, so the validation branch runs + */ + public static prepare(): Promise { + return Promise.resolve(); + } + } + + const manager = new ToolsManager( + { tools: {} } as unknown as CoreConfigValidated, + () => ({}) as EditorAPI, + new EventBus() + ); + + await manager.prepareTools([[ProbeInlineTool as unknown as InlineToolConstructor, undefined]]); + + return manager; + }; + + it('should make an inline tool without render available', async () => { + const manager = await prepareInlineTool({}); + + expect(manager.inlineTools.get('probe')).toBeDefined(); + expect(manager.unavailable.get('probe')).toBeUndefined(); + }); + + it('should mark an inline tool implementing the v2 render method as unavailable', async () => { + const log = jest.spyOn(console, 'log').mockImplementation(() => undefined); + const manager = await prepareInlineTool({ render: () => undefined }); + + expect(manager.inlineTools.get('probe')).toBeUndefined(); + expect(manager.unavailable.get('probe')).toBeDefined(); + + log.mockRestore(); + }); }); }); diff --git a/packages/sdk/src/entities/BlockToolAdapter.ts b/packages/sdk/src/entities/BlockToolAdapter.ts index 8b828f90..0d95045f 100644 --- a/packages/sdk/src/entities/BlockToolAdapter.ts +++ b/packages/sdk/src/entities/BlockToolAdapter.ts @@ -64,7 +64,7 @@ export abstract class BlockToolAdapter extends EventTarget { ((event: ModelEvents) => this.#handleModelUpdate(event)) as EventListener ); - this.signal.addEventListener('abort', unsubscribe, { once: true }); + this.signal.addEventListener('abort', unsubscribe); } /** From e2d0909721c59838499163c004d33ee63b24eb4f Mon Sep 17 00:00:00 2001 From: gohabereg Date: Tue, 6 Oct 2026 18:13:17 +0100 Subject: [PATCH 04/10] feat(core): reject an initialize() interrupted by destroy() with AbortError - Core.initialize() rejects with a DOMException named AbortError when destroy() runs while tools are being prepared, instead of resolving. - The EditorJS bundle handles its own ready promise: an AbortError is ignored, any other initialization error is logged, so neither is reported as an unhandled rejection. Awaiting isReady still rejects. Co-Authored-By: Claude Opus 5.5 --- openspec/changes/editor-destroy/tasks.md | 4 +- packages/core/src/index.spec.ts | 5 +- packages/core/src/index.ts | 2 +- packages/editorjs/src/index.spec.ts | 28 ++++++++++ packages/editorjs/src/index.ts | 9 +++- .../editorjs/src/reportUnlessAborted.spec.ts | 51 +++++++++++++++++++ packages/editorjs/src/reportUnlessAborted.ts | 17 +++++++ 7 files changed, 110 insertions(+), 6 deletions(-) create mode 100644 packages/editorjs/src/reportUnlessAborted.spec.ts create mode 100644 packages/editorjs/src/reportUnlessAborted.ts diff --git a/openspec/changes/editor-destroy/tasks.md b/openspec/changes/editor-destroy/tasks.md index 7a56bd0b..d6c0510c 100644 --- a/openspec/changes/editor-destroy/tasks.md +++ b/openspec/changes/editor-destroy/tasks.md @@ -37,7 +37,7 @@ - [x] 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` - [x] 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` +- [x] 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` - [x] 6.4 Implement the `#destroyed` checks after each `await` in `initialize()`, and the post-destroy guards in `use()` and `initialize()` ## 7. UI teardown @@ -49,7 +49,7 @@ ## 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 +- [x] 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) diff --git a/packages/core/src/index.spec.ts b/packages/core/src/index.spec.ts index 06b38c0e..9016def4 100644 --- a/packages/core/src/index.spec.ts +++ b/packages/core/src/index.spec.ts @@ -393,7 +393,7 @@ describe('Core', () => { expect(log).toEqual([]); }); - it('should resolve initialize without dispatching ready when destroyed mid-initialization', async () => { + it('should reject initialize with an AbortError without dispatching ready when destroyed mid-initialization', async () => { let resolvePrepare: () => void = () => undefined; const ready = jest.fn(); const onModelUpdate = jest.fn(); @@ -422,7 +422,8 @@ describe('Core', () => { core.destroy(); resolvePrepare(); - await expect(initialization).resolves.toBeUndefined(); + await expect(initialization).rejects.toMatchObject({ name: 'AbortError', + message: 'Editor was destroyed during initialization' }); await flush(); expect(ready).not.toHaveBeenCalled(); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index e29fa08c..58031324 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -240,7 +240,7 @@ export default class Core { * destroy() was called while tools were being prepared: it already tore down what exists */ if (this.#destroyed) { - return; + throw new DOMException('Editor was destroyed during initialization', 'AbortError'); } /** diff --git a/packages/editorjs/src/index.spec.ts b/packages/editorjs/src/index.spec.ts index 4cfc0355..0a83ae2a 100644 --- a/packages/editorjs/src/index.spec.ts +++ b/packages/editorjs/src/index.spec.ts @@ -45,6 +45,29 @@ describe('EditorJS bundle', () => { destroy.mockClear(); }); + describe('isReady interrupted by destroy', () => { + /** + * Lets an unhandled rejection surface: Jest fails the test that leaves one behind + */ + const flush = (): Promise => new Promise(resolve => setTimeout(resolve, 0)); + + it('should not leave an AbortError from isReady unhandled', async () => { + initialize.mockRejectedValue(new DOMException('Editor was destroyed during initialization', 'AbortError')); + + void new EditorJS({} as any); + + await expect(flush()).resolves.toBeUndefined(); + }); + + it('should still reject isReady with the AbortError', async () => { + initialize.mockRejectedValue(new DOMException('Editor was destroyed during initialization', 'AbortError')); + + const editor = new EditorJS({} as any); + + await expect(editor.isReady).rejects.toMatchObject({ name: 'AbortError' }); + }); + }); + it('should delegate destroy to Core', () => { initialize.mockResolvedValue(undefined); @@ -65,11 +88,16 @@ describe('EditorJS bundle', () => { }); it('rejects isReady when Core initialization fails', async () => { + const error = jest.spyOn(console, 'error').mockImplementation(() => undefined); + initialize.mockRejectedValue(new Error('init failed')); const editor = new EditorJS({} as any); await expect(editor.isReady).rejects.toThrow('init failed'); + expect(error).toHaveBeenCalledWith('[EditorJS] Initialization failed', expect.any(Error)); + + error.mockRestore(); }); it('registers a rendering adapter on the underlying Core', () => { diff --git a/packages/editorjs/src/index.ts b/packages/editorjs/src/index.ts index 4412ba38..b4910be3 100644 --- a/packages/editorjs/src/index.ts +++ b/packages/editorjs/src/index.ts @@ -10,6 +10,7 @@ import { ClipboardPlugin } from '@editorjs/clipboard-plugin'; import { ShortcutsPlugin } from '@editorjs/shortcuts-plugin'; import { EditorjsUI, BlocksUI, InlineToolbarUI, ToolbarUI, ToolboxUI } from '@editorjs/ui'; import { mergeTools } from './mergeTools.js'; +import { reportUnlessAborted } from './reportUnlessAborted.js'; /** * Default tools registered by the bundle, keyed later by their static `name`. @@ -91,10 +92,16 @@ export default class EditorJS { this.#core.use(ToolboxUI); this.#readyPromise = this.#core.initialize(); + + /** + * `isReady` may be left unawaited: ignore the AbortError a destroy before ready causes, log anything else + */ + reportUnlessAborted(this.#readyPromise); } /** - * Resolves once the editor has finished initializing; rejects if initialization fails. + * Resolves once the editor has finished initializing. Rejects if initialization fails, + * or with an `AbortError` if the editor is destroyed before it is ready */ public get isReady(): Promise { return this.#readyPromise; diff --git a/packages/editorjs/src/reportUnlessAborted.spec.ts b/packages/editorjs/src/reportUnlessAborted.spec.ts new file mode 100644 index 00000000..9f23566e --- /dev/null +++ b/packages/editorjs/src/reportUnlessAborted.spec.ts @@ -0,0 +1,51 @@ +import { afterEach, beforeEach, describe, expect, it, jest } from '@jest/globals'; +import { reportUnlessAborted } from './reportUnlessAborted.js'; + +/** + * Lets the attached handler run + */ +const flush = (): Promise => new Promise(resolve => setTimeout(resolve, 0)); + +describe('reportUnlessAborted', () => { + let error: jest.SpiedFunction; + + beforeEach(() => { + error = jest.spyOn(console, 'error').mockImplementation(() => undefined); + }); + + afterEach(() => { + error.mockRestore(); + }); + + it('should ignore an AbortError rejection', async () => { + reportUnlessAborted(Promise.reject(new DOMException('aborted', 'AbortError'))); + await flush(); + + expect(error).not.toHaveBeenCalled(); + }); + + it('should log any other rejection', async () => { + const failure = new Error('init failed'); + + reportUnlessAborted(Promise.reject(failure)); + await flush(); + + expect(error).toHaveBeenCalledWith('[EditorJS] Initialization failed', failure); + }); + + it('should log a DOMException of another name', async () => { + const failure = new DOMException('bad state', 'InvalidStateError'); + + reportUnlessAborted(Promise.reject(failure)); + await flush(); + + expect(error).toHaveBeenCalledWith('[EditorJS] Initialization failed', failure); + }); + + it('should not log when the promise resolves', async () => { + reportUnlessAborted(Promise.resolve()); + await flush(); + + expect(error).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/editorjs/src/reportUnlessAborted.ts b/packages/editorjs/src/reportUnlessAborted.ts new file mode 100644 index 00000000..b568db7b --- /dev/null +++ b/packages/editorjs/src/reportUnlessAborted.ts @@ -0,0 +1,17 @@ +/** + * Handles the bundle's own ready promise so its rejection is never reported as unhandled. + * + * Destroying the editor before it is ready rejects `isReady` with an `AbortError`: that is expected + * and ignored. Any other initialization error is logged, so it isn't lost when nobody awaits `isReady`. + * Callers awaiting `isReady` still get the rejection, since this only attaches a handler. + * @param promise - the ready promise + */ +export function reportUnlessAborted(promise: Promise): void { + promise.catch((error: unknown) => { + if (error instanceof DOMException && error.name === 'AbortError') { + return; + } + + console.error('[EditorJS] Initialization failed', error); + }); +} From 65f6c062ed10dcbdcc065f0873ffa504b2ec704b Mon Sep 17 00:00:00 2001 From: gohabereg Date: Tue, 6 Oct 2026 18:30:07 +0100 Subject: [PATCH 05/10] 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 2a1ea01cdc6da311b01c34a6814869013544a899 Mon Sep 17 00:00:00 2001 From: gohabereg Date: Wed, 4 Nov 2026 18:38:51 +0000 Subject: [PATCH 06/10] fix(core): destroy each shared inline tool instance once, from its facade InlineToolFacade.create() hands the same cached instance to every consumer, but ToolsManager's validation and SelectionManager destroyed it after each use, so a tool with a destroy() hook was torn down at startup and after every formatting action, and then used again. - InlineToolFacade.destroy() destroys the shared instance once, dropping it before the hook runs. - ToolsManager.destroy() destroys every inline tool facade; a throwing hook is logged and the remaining tools are still destroyed. - Core.destroy() calls it last, after the adapter, since the adapter and the UI use the instances until then. - Validation and SelectionManager.applyInlineTool no longer destroy the instance. Co-Authored-By: Claude Opus 5.5 --- .../src/components/SelectionManager.spec.ts | 8 +- .../core/src/components/SelectionManager.ts | 83 ++++++++-------- packages/core/src/index.spec.ts | 30 ++++++ packages/core/src/index.ts | 13 ++- packages/core/src/tools/ToolsManager.spec.ts | 92 +++++++++++++++++- packages/core/src/tools/ToolsManager.ts | 31 +++++- .../tools/facades/InlineToolFacade.spec.ts | 96 +++++++++++++++++++ .../sdk/src/tools/facades/InlineToolFacade.ts | 18 +++- 8 files changed, 313 insertions(+), 58 deletions(-) create mode 100644 packages/sdk/src/tools/facades/InlineToolFacade.spec.ts diff --git a/packages/core/src/components/SelectionManager.spec.ts b/packages/core/src/components/SelectionManager.spec.ts index 082c9fa1..72e94668 100644 --- a/packages/core/src/components/SelectionManager.spec.ts +++ b/packages/core/src/components/SelectionManager.spec.ts @@ -558,7 +558,7 @@ describe('SelectionManager', () => { jest.spyOn(model, 'format').mockImplementation(() => undefined); }; - it('should destroy the throwaway inline tool instance after applying it', () => { + it('should not destroy the shared inline tool instance after applying it', () => { const destroy = jest.fn(); const toolMock = { getFormattingOptions: jest.fn(() => ({ action: 'format', @@ -573,10 +573,10 @@ describe('SelectionManager', () => { selectionManager.applyInlineTool({ toolName: 'bold' as InlineToolName }); - expect(destroy).toHaveBeenCalledTimes(1); + expect(destroy).not.toHaveBeenCalled(); }); - it('should destroy the throwaway inline tool instance when applying it throws', () => { + it('should not destroy the shared inline tool instance when applying it throws', () => { const destroy = jest.fn(); const toolMock = { getFormattingOptions: jest.fn(), destroy }; @@ -586,7 +586,7 @@ describe('SelectionManager', () => { dataKey: 'text' }); expect(() => selectionManager.applyInlineTool({ toolName: 'bold' as InlineToolName })).toThrow(); - expect(destroy).toHaveBeenCalledTimes(1); + expect(destroy).not.toHaveBeenCalled(); }); it('should stop handling caret updates after destroy', () => { diff --git a/packages/core/src/components/SelectionManager.ts b/packages/core/src/components/SelectionManager.ts index ea70f17a..eb3dda40 100644 --- a/packages/core/src/components/SelectionManager.ts +++ b/packages/core/src/components/SelectionManager.ts @@ -191,63 +191,56 @@ export class SelectionManager { throw new Error(`SelectionManager[applyInlineTool]: tool ${toolName} is not attached`); } - /** - * The instance is only used for this call, so it is destroyed right after - */ - try { - for (const segment of segments) { - const textRange = segment.textRange; - const blockIndex = segment.blockIndex; - const dataKey = segment.dataKey; - - if (textRange === undefined) { - throw new IndexError('TextRange of the index should be defined. Probably something wrong with the Editor Model. Please, report this issue'); - } + for (const segment of segments) { + const textRange = segment.textRange; + const blockIndex = segment.blockIndex; + const dataKey = segment.dataKey; - if (blockIndex === undefined) { - throw new IndexError('BlockIndex should be defined. Probably something wrong with the Editor Model. Please, report this issue'); - } + if (textRange === undefined) { + throw new IndexError('TextRange of the index should be defined. Probably something wrong with the Editor Model. Please, report this issue'); + } - if (dataKey === undefined) { - throw new IndexError('DataKey of the index should be defined. Probably something wrong with the Editor Model. Please, report this issue'); - } + if (blockIndex === undefined) { + throw new IndexError('BlockIndex should be defined. Probably something wrong with the Editor Model. Please, report this issue'); + } - const fragments = this.#model.getFragments(blockIndex, dataKey, ...textRange, toolName); + if (dataKey === undefined) { + throw new IndexError('DataKey of the index should be defined. Probably something wrong with the Editor Model. Please, report this issue'); + } - const { action, range } = tool.getFormattingOptions(textRange, fragments); + const fragments = this.#model.getFragments(blockIndex, dataKey, ...textRange, toolName); - switch (actionOverride ?? action) { - case FormattingAction.Format: - this.#model.format(userId, blockIndex, dataKey, toolName, ...range, createInlineToolData(data)); + const { action, range } = tool.getFormattingOptions(textRange, fragments); - break; - case FormattingAction.Unformat: - this.#model.unformat(userId, blockIndex, dataKey, toolName, ...range); + switch (actionOverride ?? action) { + case FormattingAction.Format: + this.#model.format(userId, blockIndex, dataKey, toolName, ...range, createInlineToolData(data)); - break; - } + break; + case FormattingAction.Unformat: + this.#model.unformat(userId, blockIndex, dataKey, toolName, ...range); - /** - * Keep selection param is applied only for the current user - */ - if (userId === this.#config.userId) { - if (keepSelection) { - caret?.update(caretIndex); + break; + } + + /** + * Keep selection param is applied only for the current user + */ + if (userId === this.#config.userId) { + if (keepSelection) { + caret?.update(caretIndex); + } else { + // For composite selections, don't try to add textRange since composite indices + // must not have root-level textRange. Only set textRange for single-segment selections. + const selectedSegments = (caretIndex as TextIndex).getTextSegments(); + + if (selectedSegments.length === 1 && selectedSegments[0].textRange !== undefined) { + caret?.update(caretIndex.withTextRange([selectedSegments[0].textRange[1], selectedSegments[0].textRange[1]])); } else { - // For composite selections, don't try to add textRange since composite indices - // must not have root-level textRange. Only set textRange for single-segment selections. - const selectedSegments = (caretIndex as TextIndex).getTextSegments(); - - if (selectedSegments.length === 1 && selectedSegments[0].textRange !== undefined) { - caret?.update(caretIndex.withTextRange([selectedSegments[0].textRange[1], selectedSegments[0].textRange[1]])); - } else { - caret?.update(caretIndex); - } + caret?.update(caretIndex); } } } - } finally { - tool.destroy?.(); } }; diff --git a/packages/core/src/index.spec.ts b/packages/core/src/index.spec.ts index 9016def4..0de8ac42 100644 --- a/packages/core/src/index.spec.ts +++ b/packages/core/src/index.spec.ts @@ -6,6 +6,7 @@ import Core from './index.js'; import { UndoRedoManager } from './components/UndoRedoManager.js'; import { SelectionManager } from './components/SelectionManager.js'; import { BlockRenderer } from './components/BlockRenderer.js'; +import ToolsManager from './tools/ToolsManager.js'; /** * `Core` is headless: it registers nothing by default, so `initialize()` validates @@ -267,6 +268,33 @@ describe('Core', () => { expect(log).toEqual(['plugin:a', 'tool', 'adapter:block', 'adapter']); }); + it('should destroy each shared inline tool instance once, after the adapter', async () => { + const core = createCore(); + + core.use(class SharedInlineTool { + public static type = ToolType.Inline as const; + public static name = 'shared'; + + /** + * Gives the tool a validation pass, which creates its shared instance + */ + public static prepare(): Promise { + return Promise.resolve(); + } + + public destroy(): void { + log.push('inline'); + } + } as unknown as typeof StubBlockTool); + await core.initialize(); + await flush(); + + core.destroy(); + core.destroy(); + + expect(log).toEqual(['tool', 'adapter:block', 'adapter', 'inline']); + }); + it('should not emit model events during destroy', async () => { const onModelUpdate = jest.fn(); const core = createCore({ onModelUpdate }); @@ -301,6 +329,7 @@ describe('Core', () => { ['UndoRedoManager', UndoRedoManager], ['SelectionManager', SelectionManager], ['BlockRenderer', BlockRenderer], + ['ToolsManager', ToolsManager], ])('should destroy %s', async (_label, Service) => { const destroy = jest.spyOn(Service.prototype, 'destroy'); const core = createCore(); @@ -317,6 +346,7 @@ describe('Core', () => { ['UndoRedoManager', UndoRedoManager], ['SelectionManager', SelectionManager], ['BlockRenderer', BlockRenderer], + ['ToolsManager', ToolsManager], ])('should log and continue when %s fails to destroy', async (label, Service) => { const error = jest.spyOn(console, 'error').mockImplementation(() => undefined); const destroy = jest.spyOn(Service.prototype, 'destroy').mockImplementation(() => { diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 58031324..fb0a02d2 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -71,6 +71,11 @@ interface CoreServices { * Rendering adapter plugin */ adapter?: EditorJSAdapterPlugin; + + /** + * Owns the tool facades, and through them the shared inline tool instances + */ + toolsManager?: ToolsManager; } /** @@ -232,6 +237,7 @@ export default class Core { this.#iocContainer.get(BlocksManager); this.#services.blockRenderer = this.#iocContainer.get(BlockRenderer); this.#services.adapter = this.#iocContainer.get(TOKENS.Adapter); + this.#services.toolsManager = this.#iocContainer.get(ToolsManager); this.#initializePlugins(); await this.#initializeTools(); @@ -276,13 +282,18 @@ export default class Core { this.#pluginInstances = []; - const { undoRedoManager, selectionManager, blockRenderer, adapter } = this.#services; + const { undoRedoManager, selectionManager, blockRenderer, adapter, toolsManager } = this.#services; this.#safely('UndoRedoManager', () => undoRedoManager?.destroy()); this.#safely('SelectionManager', () => selectionManager?.destroy()); this.#safely('BlockRenderer', () => blockRenderer?.destroy()); this.#safely('adapter', () => adapter?.destroy?.()); + /** + * Last: the shared inline tool instances are used by the adapter and the UI until they're gone + */ + this.#safely('ToolsManager', () => toolsManager?.destroy()); + this.#services = {}; } diff --git a/packages/core/src/tools/ToolsManager.spec.ts b/packages/core/src/tools/ToolsManager.spec.ts index c727a48b..1148da0c 100644 --- a/packages/core/src/tools/ToolsManager.spec.ts +++ b/packages/core/src/tools/ToolsManager.spec.ts @@ -6,7 +6,7 @@ import type { EditorAPI } from '../api/index.js'; describe('ToolsManager', () => { describe('.prepareTools()', () => { - it('should destroy the inline tool instance created for validation', async () => { + it('should not destroy the shared inline tool instance it validates', async () => { const destroy = jest.fn(); /** @@ -30,7 +30,7 @@ describe('ToolsManager', () => { await manager.prepareTools([[ProbeInlineTool as unknown as InlineToolConstructor, undefined]]); - expect(destroy).toHaveBeenCalledTimes(1); + expect(destroy).not.toHaveBeenCalled(); }); /** @@ -89,4 +89,92 @@ describe('ToolsManager', () => { log.mockRestore(); }); }); + + describe('.destroy()', () => { + /** + * Inline tool class whose instances call `onDestroy` from their destroy() hook + * @param name - static name the tool is registered under + * @param onDestroy - called by the instance's destroy() + * @param withPrepare - give the tool a static prepare(), so prepareTools validates it + */ + const inlineTool = (name: string, onDestroy: () => void, withPrepare = false): InlineToolConstructor => { + /** + * Inline tool that reports when its instance is destroyed + */ + class ProbeInlineTool { + public static type = ToolType.Inline; + + public static name = name; + + /** + * Destroy hook under test + */ + public destroy(): void { + onDestroy(); + } + } + + if (withPrepare) { + Object.assign(ProbeInlineTool, { prepare: (): Promise => Promise.resolve() }); + } + + return ProbeInlineTool as unknown as InlineToolConstructor; + }; + + /** + * Creates a manager with the given tools prepared + * @param tools - inline tool classes to prepare + */ + const prepared = async (...tools: InlineToolConstructor[]): Promise => { + const manager = new ToolsManager( + { tools: {} } as unknown as CoreConfigValidated, + () => ({}) as EditorAPI, + new EventBus() + ); + + await manager.prepareTools(tools.map(tool => [tool, undefined])); + + return manager; + }; + + it('should destroy each inline tool instance once', async () => { + const destroyA = jest.fn(); + const destroyB = jest.fn(); + const manager = await prepared(inlineTool('a', destroyA), inlineTool('b', destroyB, true)); + + manager.inlineTools.get('a')?.create(); + + manager.destroy(); + manager.destroy(); + + expect(destroyA).toHaveBeenCalledTimes(1); + expect(destroyB).toHaveBeenCalledTimes(1); + }); + + it('should not create an instance for a tool that was never used', async () => { + const destroy = jest.fn(); + const manager = await prepared(inlineTool('unused', destroy)); + + manager.destroy(); + + expect(destroy).not.toHaveBeenCalled(); + }); + + it('should log a throwing destroy() and still destroy the other tools', async () => { + const error = jest.spyOn(console, 'error').mockImplementation(() => undefined); + const destroyB = jest.fn(); + const manager = await prepared( + inlineTool('a', () => { + throw new Error('broken tool'); + }, true), + inlineTool('b', destroyB, true) + ); + + expect(() => manager.destroy()).not.toThrow(); + expect(destroyB).toHaveBeenCalledTimes(1); + expect(error).toHaveBeenCalledWith('[ToolsManager] Inline tool a failed to destroy', expect.any(Error)); + + error.mockRestore(); + }); + }); }); diff --git a/packages/core/src/tools/ToolsManager.ts b/packages/core/src/tools/ToolsManager.ts index 42792d38..dff6e51f 100644 --- a/packages/core/src/tools/ToolsManager.ts +++ b/packages/core/src/tools/ToolsManager.ts @@ -110,6 +110,29 @@ export default class ToolsManager { this.#validateTools(); } + /** + * Destroys the shared inline tool instances, once each. + * + * Every inline tool facade hands the same instance to all its consumers (formatting, selection, + * the inline toolbar), so none of them destroys it: the facade owns it and is destroyed here, on + * editor teardown. A throwing hook is logged and the remaining tools are still destroyed + */ + public destroy(): void { + const facades = new Set([...this.#availableTools.values(), ...this.#unavailableTools.values()]); + + for (const facade of facades) { + if (!facade.isInline()) { + continue; + } + + try { + facade.destroy(); + } catch (error) { + console.error(`[ToolsManager] Inline tool ${facade.name} failed to destroy`, error); + } + } + } + /** * Calls tools prepare method if it exists and adds tools to relevant collection (available or unavailable tools) * @param tools - tools to prepare and their settings @@ -150,13 +173,11 @@ export default class ToolsManager { * Some Tools validation */ const inlineToolRequiredMethods = ['render']; - const instance = tool.create(); - const notImplementedMethods = inlineToolRequiredMethods.filter(method => instance[method as keyof InlineTool] !== undefined); - /** - * The instance is only used for validation + * The facade's shared instance: it stays alive for the other consumers and is destroyed on teardown */ - instance.destroy?.(); + const instance = tool.create(); + const notImplementedMethods = inlineToolRequiredMethods.filter(method => instance[method as keyof InlineTool] !== undefined); if (notImplementedMethods.length > 0) { /** diff --git a/packages/sdk/src/tools/facades/InlineToolFacade.spec.ts b/packages/sdk/src/tools/facades/InlineToolFacade.spec.ts new file mode 100644 index 00000000..2c07b347 --- /dev/null +++ b/packages/sdk/src/tools/facades/InlineToolFacade.spec.ts @@ -0,0 +1,96 @@ +/* eslint-disable jsdoc/require-jsdoc */ + +import { describe, expect, it, jest } from '@jest/globals'; +import type { InlineToolConstructor } from '../../entities/index.js'; +import { ToolType } from '../../entities/index.js'; +import type { ToolOptions } from './BaseToolFacade.js'; +import { InlineToolFacade } from './InlineToolFacade.js'; +import type { EditorAPI } from '../../api'; + +/** + * Facade under test, with a record of how many instances its tool class constructed + */ +interface Probe { + facade: InlineToolFacade; + constructed: jest.Mock; +} + +/** + * Creates a facade around an inline tool class whose instances record their construction and destruction + * @param destroy - the instances' destroy() hook, or undefined for a tool without one + */ +function createFacade(destroy?: () => void): Probe { + const constructed = jest.fn(); + + class MockInlineTool { + public static type = ToolType.Inline; + + public destroy = destroy; + + constructor() { + constructed(); + } + } + + const facade = new InlineToolFacade({ + api: {} as EditorAPI, + constructable: MockInlineTool as unknown as InlineToolConstructor, + isDefault: false, + name: 'probe', + useToolOptions: {} as ToolOptions, + }); + + return { facade, + constructed }; +} + +describe('InlineToolFacade', () => { + it('should return the same instance from every create()', () => { + const { facade, constructed } = createFacade(); + + expect(facade.create()).toBe(facade.create()); + expect(constructed).toHaveBeenCalledTimes(1); + }); + + it('should call the instance\'s destroy() once on destroy', () => { + const destroy = jest.fn(); + const { facade } = createFacade(destroy); + + facade.create(); + facade.destroy(); + facade.destroy(); + + expect(destroy).toHaveBeenCalledTimes(1); + }); + + it('should not construct an instance on destroy when none was created', () => { + const { facade, constructed } = createFacade(jest.fn()); + + facade.destroy(); + + expect(constructed).not.toHaveBeenCalled(); + }); + + it('should support a tool without destroy()', () => { + const { facade } = createFacade(); + + facade.create(); + + expect(() => facade.destroy()).not.toThrow(); + }); + + it('should drop the instance even when its destroy() throws', () => { + const destroy = jest.fn(() => { + throw new Error('broken tool'); + }); + const { facade } = createFacade(destroy); + + facade.create(); + + expect(() => facade.destroy()).toThrow('broken tool'); + + facade.destroy(); + + expect(destroy).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/sdk/src/tools/facades/InlineToolFacade.ts b/packages/sdk/src/tools/facades/InlineToolFacade.ts index a1faeab4..0d57b4a0 100644 --- a/packages/sdk/src/tools/facades/InlineToolFacade.ts +++ b/packages/sdk/src/tools/facades/InlineToolFacade.ts @@ -38,7 +38,8 @@ export class InlineToolFacade extends BaseToolFacade Date: Wed, 4 Nov 2026 22:32:13 +0000 Subject: [PATCH 07/10] fix(core): release the onModelUpdate listener on destroy Core registered its onModelUpdate model listener without a way to remove it. An integrator that kept the model it was handed could still reach the callback after teardown. The listener now uses Core's own controller, which destroy() aborts. Co-Authored-By: Claude Opus 5.5 --- packages/core/src/index.spec.ts | 19 +++++++++++++++++++ packages/core/src/index.ts | 8 +++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/core/src/index.spec.ts b/packages/core/src/index.spec.ts index 0de8ac42..4fd4c8f3 100644 --- a/packages/core/src/index.spec.ts +++ b/packages/core/src/index.spec.ts @@ -308,6 +308,25 @@ describe('Core', () => { expect(onModelUpdate).not.toHaveBeenCalled(); }); + it('should stop calling onModelUpdate after destroy', async () => { + let model: { removeBlock: (userId: string, index: number) => void } | undefined; + const onModelUpdate = jest.fn((updated: unknown) => { + model = updated as typeof model; + }); + const core = createCore({ onModelUpdate }); + + await core.initialize(); + await flush(); + core.destroy(); + onModelUpdate.mockClear(); + + // An integrator that kept the model it was handed changes it after teardown + model?.removeBlock('user', 0); + + expect(model).toBeDefined(); + expect(onModelUpdate).not.toHaveBeenCalled(); + }); + it('should be a no-op when called twice', async () => { const error = jest.spyOn(console, 'error').mockImplementation(() => undefined); const core = createCore(); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index fb0a02d2..22b344ca 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -117,6 +117,11 @@ export default class Core { */ #services: CoreServices = {}; + /** + * Releases Core's own listeners on destroy + */ + readonly #controller = new AbortController(); + /** * Set by {@link destroy}. A destroyed core can't be used or initialized again */ @@ -164,7 +169,7 @@ export default class Core { if (config.onModelUpdate !== undefined) { this.#model.addEventListener(EventType.Changed, () => { config.onModelUpdate?.(this.#model); - }); + }, { signal: this.#controller.signal }); } } @@ -272,6 +277,7 @@ export default class Core { * Plugin instances and services are cleared below, so a second call finds nothing left to destroy */ this.#destroyed = true; + this.#controller.abort(); const registry = this.#iocContainer.get(PluginRegistry); From 73441291164c3e748d1c64848ae5b7d0d808cd62 Mon Sep 17 00:00:00 2001 From: gohabereg Date: Wed, 4 Nov 2026 23:59:20 +0000 Subject: [PATCH 08/10] 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 09/10] 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 From 3c3ddcefed7d223cba05195a90bd6ef951d31782 Mon Sep 17 00:00:00 2001 From: gohabereg Date: Wed, 4 Nov 2026 23:59:57 +0000 Subject: [PATCH 10/10] fix(core): stop preparing tools once the editor is destroyed The tool preparation queue kept running after destroy(). Tools still queued were prepared, and a tool whose prepare() settled after teardown was created, registered and announced with ToolLoaded. Its inline instance then outlived ToolsManager.destroy(). ToolsManager now owns an AbortController that destroy() aborts. Each queued job checks the signal before calling prepare() and again once it settles. A prepare() already running can't be interrupted, since it takes no signal, so its result is dropped. Co-Authored-By: Claude Opus 5.5 --- packages/core/src/tools/ToolsManager.spec.ts | 108 +++++++++++++++++++ packages/core/src/tools/ToolsManager.ts | 28 ++++- 2 files changed, 135 insertions(+), 1 deletion(-) diff --git a/packages/core/src/tools/ToolsManager.spec.ts b/packages/core/src/tools/ToolsManager.spec.ts index 1148da0c..17ed2116 100644 --- a/packages/core/src/tools/ToolsManager.spec.ts +++ b/packages/core/src/tools/ToolsManager.spec.ts @@ -4,6 +4,26 @@ import { EventBus, ToolType } from '@editorjs/sdk'; import ToolsManager from './ToolsManager.js'; import type { EditorAPI } from '../api/index.js'; +/** + * Inline tool whose static prepare() settles only when the test calls finish() + */ +interface PendingTool { + /** + * The tool class + */ + tool: InlineToolConstructor; + + /** + * Its static prepare() + */ + prepare: jest.Mock; + + /** + * Settles the pending prepare() + */ + finish: () => void; +} + describe('ToolsManager', () => { describe('.prepareTools()', () => { it('should not destroy the shared inline tool instance it validates', async () => { @@ -90,6 +110,94 @@ describe('ToolsManager', () => { }); }); + describe('.prepareTools() interrupted by .destroy()', () => { + /** + * Inline tool class with a static prepare() that settles when the test says so + * @param name - static name the tool is registered under + * @param constructed - called by the tool's constructor + */ + const pendingTool = (name: string, constructed: () => void = jest.fn()): PendingTool => { + let finish: () => void = () => undefined; + const prepare = jest.fn(() => new Promise((resolve) => { + finish = resolve; + })); + + /** + * Inline tool whose preparation the test controls + */ + class PendingInlineTool { + public static type = ToolType.Inline; + + public static name = name; + + public static prepare = prepare; + + /** + * Reports its construction + */ + constructor() { + constructed(); + } + } + + return { + tool: PendingInlineTool as unknown as InlineToolConstructor, + prepare, + finish: () => finish(), + }; + }; + + /** + * Lets queued promise jobs run + */ + const flush = (): Promise => new Promise(resolve => setTimeout(resolve, 0)); + + it('should not prepare the tools still queued', async () => { + const first = pendingTool('first'); + const second = pendingTool('second'); + const manager = new ToolsManager( + { tools: {} } as unknown as CoreConfigValidated, + () => ({}) as EditorAPI, + new EventBus() + ); + + const preparing = manager.prepareTools([[first.tool, undefined], [second.tool, undefined]]); + + await flush(); + manager.destroy(); + first.finish(); + await preparing; + + expect(first.prepare).toHaveBeenCalledTimes(1); + expect(second.prepare).not.toHaveBeenCalled(); + }); + + it('should neither create nor announce a tool whose prepare() settles after destroy', async () => { + const constructed = jest.fn(); + const pending = pendingTool('late', constructed); + const eventBus = new EventBus(); + const dispatch = jest.spyOn(eventBus, 'dispatchEvent'); + const manager = new ToolsManager( + { tools: {} } as unknown as CoreConfigValidated, + () => ({}) as EditorAPI, + eventBus + ); + + const preparing = manager.prepareTools([[pending.tool, undefined]]); + + await flush(); + manager.destroy(); + pending.finish(); + await preparing; + + expect(pending.prepare).toHaveBeenCalledTimes(1); + expect(constructed).not.toHaveBeenCalled(); + expect(dispatch).not.toHaveBeenCalled(); + expect(manager.available.get('late')).toBeUndefined(); + expect(manager.unavailable.get('late')).toBeUndefined(); + }); + }); + describe('.destroy()', () => { /** * Inline tool class whose instances call `onDestroy` from their destroy() hook diff --git a/packages/core/src/tools/ToolsManager.ts b/packages/core/src/tools/ToolsManager.ts index dff6e51f..af2244d7 100644 --- a/packages/core/src/tools/ToolsManager.ts +++ b/packages/core/src/tools/ToolsManager.ts @@ -56,6 +56,12 @@ export default class ToolsManager { */ #unavailableTools = new ToolsCollection(); + /** + * Aborted by {@link destroy}: tools still queued for preparation are skipped, and a tool whose + * prepare() settles afterwards is neither created nor registered + */ + readonly #controller = new AbortController(); + /** * Returns available Tools */ @@ -115,9 +121,13 @@ export default class ToolsManager { * * Every inline tool facade hands the same instance to all its consumers (formatting, selection, * the inline toolbar), so none of them destroys it: the facade owns it and is destroyed here, on - * editor teardown. A throwing hook is logged and the remaining tools are still destroyed + * editor teardown. A throwing hook is logged and the remaining tools are still destroyed. + * + * Also stops a {@link prepareTools} still running: no tool is prepared, created or announced after this */ public destroy(): void { + this.#controller.abort(); + const facades = new Set([...this.#availableTools.values(), ...this.#unavailableTools.values()]); for (const facade of facades) { @@ -139,6 +149,7 @@ export default class ToolsManager { */ public async prepareTools(tools: [ToolConstructable, ToolStaticOptions | undefined][]): Promise { const promiseQueue = new PromiseQueue(); + const { signal } = this.#controller; const setToAvailableToolsCollection = (toolName: string, tool: ToolFacadeClass): void => { this.#availableTools.set(toolName, tool); @@ -157,6 +168,13 @@ export default class ToolsManager { if (isFunction(toolConstructor.prepare)) { void promiseQueue.add(async () => { + /** + * The editor was destroyed while earlier tools were being prepared + */ + if (signal.aborted) { + return; + } + try { const tool = factory.get(toolName); @@ -168,6 +186,14 @@ export default class ToolsManager { config: tool.config, }); + /** + * The editor was destroyed while this tool was preparing. An inline instance created now + * would outlive destroy(), and ToolLoaded would reach a torn-down editor + */ + if (signal.aborted) { + return; + } + if (tool.isInline()) { /** * Some Tools validation