Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-07-28
109 changes: 109 additions & 0 deletions openspec/changes/separate-tool-options-and-config/design.md

Large diffs are not rendered by default.

32 changes: 32 additions & 0 deletions openspec/changes/separate-tool-options-and-config/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
## Why

Tools expose two static surfaces with no formal separation: `static options` (core/plugin-facing wiring — `toolbox`, `shortcut`, `inlineToolbar`, `tunes`, `conversionConfig`, `canBeSplit`) and `ToolConfig` (plugin-specific user data, typed only via a generic `Config extends ToolConfig = any` re-exported from the legacy `@editorjs/editorjs` package, where `ToolConfig<T extends object = any> = T` is an untyped passthrough). Because `options` is a plain static value evaluated once at class-definition time, a tool has no way to express "these option values are derived from my resolved config" — there is no contract connecting a `ToolConfig` field to the `options` it is meant to drive.

The gap is structural rather than stylistic. `options` is read off the class before any registration exists, while `config` is resolved per tool registration (in `BaseToolFacade.config`) and per block instance (in the tool's constructor). A config field meant to drive an option value — toolbox entries following a set of variants, a shortcut depending on a mode, conversion behavior gated by a flag — has no path to it. The field stays declared in the tool's `Config` type and documented for integrators, while the option it was meant to control keeps whatever the class hardcoded, and nothing reports the mismatch.

## What Changes

- Define `ToolConfig` as an SDK-owned, per-tool-type-parameterized contract, replacing the untyped passthrough currently imported from `@editorjs/editorjs`. **BREAKING**: a tool's `Config` generic must conform to the new contract's shape and import path.
- Allow a tool's static `options` to be, in addition to a plain object, a **synchronous factory** `(config: ToolConfig) => ToolOptions`. The factory receives the `config` supplied at `use(Tool, { config })` time and returns the tool's complete static option set — including any config defaults it applies, under `options.config`, exactly as the object form declares them. **BREAKING** for the `static options` type of every tool.
- Resolve that factory exactly once, when the tool's facade is constructed, into a private per-facade field. Every option-reading getter (`options`, `config`, `toolbox`, `isReadOnlySupported`) reads the resolved value instead of `constructable.options`, so nothing is ever written back onto the shared tool class.
- Explicitly **do not** change `prepare()`. It stays the `void`-returning, optionally-async, side-effectful initialization hook it is today; deriving static options from config is a separate, pure, synchronous concern and is not routed through it.
- Migrate the four in-repo tools (`paragraph`, `bold`, `italic`, `inline-link`) to the new `ToolConfig`/`ToolOptions` contracts. All four keep the plain-object form of `options`; none needs config-derived options.
- Out of scope: changing a tool's config after the editor has already mounted (no live/reactive "hot-swap" API). Config is resolved once, during tool registration, before the editor renders its UI.
- Out of scope: dev-time detection of a declared-but-unused `ToolConfig` key. It is a distinct diagnostics concern with its own open question (what counts as "used" — a key read in a factory, in `prepare()`, or in the tool's constructor?) and does not belong in the contract change.

## Capabilities

### New Capabilities
(none — this reshapes the existing tool-contract behavior rather than introducing a new capability area)

### Modified Capabilities
- `sdk`: the "Tool and tune contracts" requirement changes — `ToolConfig` becomes a dedicated SDK contract (no longer a passthrough re-export), and `BaseToolConstructor.options` widens from a plain object to "a plain object **or** a synchronous factory of the resolved config", with the facade owning a single resolution of that factory per registration.

## Impact

- `packages/sdk/src/entities/{BaseTool.ts, BlockTool.ts, InlineTool.ts, BlockTune.ts}`: SDK-owned `ToolConfig`; `options` widened to accept a config factory. `prepare()`'s signature is unchanged.
- `packages/sdk/src/tools/facades/BaseToolFacade.ts`: a private per-instance field holding the resolved static options, populated in the constructor; the `options` and `config` getters read it instead of `constructable.options`.
- `packages/sdk/src/tools/facades/BlockToolFacade.ts`: the `toolbox` and `isReadOnlySupported` getters read the resolved static options. The `toolbox` merge algorithm and its two tiers (tool-side value, then `use()`-time override) are unchanged.
- `packages/core/src/tools/ToolsManager.ts`: unchanged. Resolution happens entirely inside the SDK facade, so the tool-registration lifecycle gains no new step and no new ordering guarantee.
- `packages/tools/{paragraph,bold,italic,inline-link}`: migrate to the new `ToolConfig` import/contract (type-only change; all keep object-form `options`).
- `openspec/specs/sdk/spec.md`: delta spec updates to the "Tool and tune contracts" requirement.
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
## MODIFIED Requirements

### Requirement: Tool and tune contracts
The system SHALL define the static/instance contracts that block tools, inline tools, and block tunes must satisfy: `BaseTool`/`BaseToolConstructor` (common `name`, `options`, `prepare()`, `reset()`), `BlockTool`/`BlockToolConstructor` (adds `toolbox`, `shortcut`, `inlineToolbar`, `tunes`, `conversionConfig`, `canBeSplit`), `InlineTool`/`InlineToolConstructor` (adds `isActive`, `getFormattingOptions`, `createWrapper`, `getToolbarConfig`), and `BlockTune`/`BlockTuneConstructor`. `ToolConfig` SHALL be an SDK-owned contract (not a re-exported passthrough from `@editorjs/editorjs`), kept formally distinct from `ToolOptions`: `options` describes core/plugin-facing wiring available before any block instance exists, while `ToolConfig` describes tool-author-facing user data resolved per tool registration. A tool's static `options` SHALL be either a plain options object or a synchronous factory taking the tool's `ToolConfig` and returning that options object; the facade SHALL resolve the factory exactly once, at construction, and serve every option-reading getter from that resolved value without writing it back onto the tool class.

#### Scenario: Options and config merging in a tool facade
- **GIVEN** a tool class has static `options` (and optionally `options.config` typed as `ToolConfig`)
- **WHEN** the tool is registered via `use(Tool, options)` with overriding options
- **THEN** the facade's `options` getter merges the tool's resolved static options with `use()`-time options, with `use()`-time keys taking precedence, and the `config` getter merges similarly, injecting `defaultPlaceholder` only when `isDefault` is true and no `placeholder` key is already present

#### Scenario: Resolving static options from a config factory
- **GIVEN** a tool class declares `static options` as a function of its `ToolConfig` rather than as a plain object
- **WHEN** its facade is constructed for a registration made via `use(Tool, { config })`
- **THEN** the function is invoked exactly once with the `config` supplied at `use()` time, its returned options object becomes the tool's resolved static options for that facade, and the tool class's own `options` property is left untouched

#### Scenario: Deriving a toolbox entry from a config value
- **GIVEN** a block tool's `options` factory computes `toolbox` entries from a field of the config it receives
- **WHEN** the facade's `toolbox` getter is read
- **THEN** the entries reflect the config supplied at `use()` time, and any explicit `use()`-time `toolbox` override is merged on top using the existing array/object positional-merge algorithm, with a `use()`-time `toolbox: false` still hiding the tool from the toolbox

#### Scenario: Isolating factory-derived options per facade instance
- **GIVEN** two `Core` instances each register the same tool class through `use()` with a different `config`
- **WHEN** each instance's facade resolves the tool's `options` factory
- **THEN** each facade holds its own resolved options and neither instance's derived values are observable from the other

#### Scenario: Text content conversion without config
- **GIVEN** a block tool has no `conversionConfig`
- **WHEN** `exportTextContent`/`importTextContent` is called on its facade
- **THEN** it throws a descriptive error stating the tool does not have export/import configuration

#### Scenario: Text content conversion with a keypath
- **GIVEN** a block tool's `conversionConfig` specifies a dot-notation string key (including nested array paths, e.g. `items.0.text`)
- **WHEN** `exportTextContent`/`importTextContent` is called
- **THEN** the value at that keypath is read/written, producing or consuming a `TextNodeSerialized` value tagged with the hidden `Text` block-child-type marker

Implemented in `src/entities/BaseTool.ts`, `BlockTool.ts`, `InlineTool.ts`, `BlockTune.ts`, `src/tools/facades/{BaseToolFacade,BlockToolFacade,InlineToolFacade,BlockTuneFacade}.ts`, validated by `src/tools/facades/BaseToolFacade.spec.ts`.
41 changes: 41 additions & 0 deletions openspec/changes/separate-tool-options-and-config/tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
## 1. SDK-owned `ToolConfig`

- [x] 1.1 Add a failing test in `packages/sdk/src/entities/BaseTool.spec.ts` — `it('should expose ToolConfig from the SDK itself rather than re-export it from @editorjs/editorjs')` — asserting `BaseToolOptions` references the SDK-owned type
- [x] 1.2 Define `ToolConfig` in `packages/sdk/src/entities/BaseTool.ts` as an SDK-owned generic type, replacing the `@editorjs/editorjs` re-export, and update `BaseToolOptions<Config extends ToolConfig>`
- [x] 1.3 Update `BlockTool.ts`, `InlineTool.ts`, and `BlockTune.ts` to import `ToolConfig` from the new location — each re-declares its own `Config extends ToolConfig = ToolConfig` bound rather than inheriting `BaseToolOptions`' one, so this is covered by its own test (`should keep the config option checked on every tool subtype`)

## 2. `options` as a config factory

- [x] 2.1 Add a failing test in `packages/sdk/src/entities/BaseTool.spec.ts` — `it('should accept a synchronous factory of the tool config as static options')` — asserting a `BaseToolConstructor` type-checks with `options` declared as `(config: Config) => Options`
- [x] 2.2 Add a failing test asserting the object form still type-checks unchanged — `it('should accept a plain options object as static options')`
- [x] 2.3 Define `ToolOptionsFactory<Config, Options>` in `packages/sdk/src/entities/BaseTool.ts` and widen `BaseToolConstructor.options` to `Options | ToolOptionsFactory<Config, Options>`
- [x] 2.4 Propagate the widened `options` type through `BlockToolConstructor`, `InlineToolConstructor`, and `BlockTuneConstructor` — no production change was needed: all three extend `BaseToolConstructor<Config, XOptions>` without re-declaring `options`, so they inherit the union. Pinned by `it('should carry the factory form through to every tool subtype constructor')`
- [x] 2.5 Confirm `BaseToolConstructor.prepare()` keeps its current `void | Promise<void>` return type — design.md Decision 3 deliberately leaves this hook untouched

## 3. Resolving the factory in `BaseToolFacade`

- [x] 3.1 Add a failing test in `packages/sdk/src/tools/facades/BaseToolFacade.spec.ts` — `it('should call the options factory once with the config passed at use() time')`
- [x] 3.2 Add a failing test — `it('should not call the options factory again when option getters are read repeatedly')`
- [x] 3.3 Add a failing test — `it('should leave the tool class options untouched after resolving the factory')`
- [x] 3.4 Add a failing test — `it('should resolve options independently for two facades wrapping the same tool class with different configs')` (the multi-`Core` case, design.md Decision 6)
- [x] 3.5 Add a failing test — `it('should merge factory-returned config defaults with the use()-time config')` — covering that a tool instance receives the defaults the factory applied
- [x] 3.6 Implement the private resolved-options field in the `BaseToolFacade` constructor: invoke the factory with `useToolOptions.config ?? {}` when `options` is a function, otherwise use the object as-is. Declared `protected readonly` rather than `#private` because `BlockToolFacade` reads it (group 5); per-instance semantics are unchanged
- [x] 3.7 Point the `options` and `config` getters at the resolved field instead of `constructable.options`

## 4. Reading resolved options in `BlockToolFacade`

- [x] 4.1 Add failing tests for `BlockToolFacade.toolbox` covering both tiers against a factory-derived value — `it('should derive toolbox entries from the config passed at use() time')`, `it('should merge a use()-time toolbox override onto a factory-derived value')`, and `it('should hide the tool when a use()-time toolbox is false despite a factory-derived value')`
- [x] 4.2 Add a failing test — `it('should read isReadOnlySupported from the resolved static options')`
- [x] 4.3 Point `BlockToolFacade.toolbox` and `isReadOnlySupported` at the resolved static options, leaving the array/object positional-merge algorithm and its two tiers unchanged
- [x] 4.4 Confirm the existing object-form toolbox tests still pass unmodified — **none existed**: the toolbox merge algorithm had no test coverage at all. Added five characterization tests in the new `BlockToolFacade.spec.ts` (empty, single-entry wrapping, `false` hiding, array-onto-array positional merge, object-onto-object merge) and confirmed they pass before and after 4.3

## 5. Migrating in-repo tools

- [x] 5.1 Update `packages/tools/{paragraph,bold,italic,inline-link}` to import `ToolConfig`/their `Config` type from the new SDK location, keeping the plain-object form of `static options` — only `paragraph` referenced `ToolConfig`; `bold`, `italic`, and `inline-link` declare no config type
- [x] 5.2 Confirm `yarn workspace <pkg> typecheck` and `yarn lint` pass for each of the four tools with no runtime behavior change

## 6. Verification and documentation alignment

- [x] 6.1 Confirm `yarn workspace @editorjs/sdk test`, `yarn workspace @editorjs/core test`, and `yarn lint` pass — 34 SDK tests, 161 core tests, lint clean
- [x] 6.2 Run `openspec validate --changes separate-tool-options-and-config --strict` and fix any delta-spec formatting issues
- [x] 6.3 Re-check `docs/plugins.md` and `docs/architecture.md` for statements about `static options` always being a plain object, and update any that no longer hold — neither file makes such a statement, no change needed
87 changes: 87 additions & 0 deletions packages/sdk/src/entities/BaseTool.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/* eslint-disable jsdoc/require-jsdoc */

import { describe, expect, it } from '@jest/globals';
import type {
BaseToolConstructor,
BaseToolOptions,
BlockToolOptions,
BlockTuneOptions,
InlineToolOptions,
ToolConfig
} from './BaseTool.js';
import type {
BlockToolConstructor,
BlockToolData,
BlockTuneConstructor,
InlineToolConstructor
} from './index.js';

interface TestToolConfig {
level?: number;
}

/**
* Resolves to `true` only when `T` is exactly `any`.
*
* `1 & any` collapses back to `any`, and `0 extends any` holds — a relation no
* other type satisfies. This is what distinguishes an SDK-owned `ToolConfig`
* from a re-export of the legacy `ToolConfig<T extends object = any> = T`.
*/
type IsAny<T> = 0 extends (1 & T) ? true : false;

describe('ToolConfig', () => {
it('should expose ToolConfig from the SDK itself rather than re-export it from @editorjs/editorjs', () => {
const toolConfigIsAny: IsAny<ToolConfig> = false;
const optionsConfigIsAny: IsAny<NonNullable<BaseToolOptions['config']>> = false;

expect(toolConfigIsAny).toBe(false);
expect(optionsConfigIsAny).toBe(false);
});

it('should keep the config option checked on every tool subtype', () => {
const blockConfigIsAny: IsAny<NonNullable<BlockToolOptions['config']>> = false;
const inlineConfigIsAny: IsAny<NonNullable<InlineToolOptions['config']>> = false;
const tuneConfigIsAny: IsAny<NonNullable<BlockTuneOptions['config']>> = false;

expect(blockConfigIsAny).toBe(false);
expect(inlineConfigIsAny).toBe(false);
expect(tuneConfigIsAny).toBe(false);
});
});

describe('BaseToolConstructor.options', () => {
it('should accept a synchronous factory of the tool config as static options', () => {
const tool: BaseToolConstructor<TestToolConfig> = {
name: 'factory-tool',
options: config => ({
config: { level: config.level ?? 1 },
}),
};

expect(typeof tool.options).toBe('function');
});

it('should accept a plain options object as static options', () => {
const tool: BaseToolConstructor<TestToolConfig> = {
name: 'object-tool',
options: {
config: { level: 1 },
},
};

expect(typeof tool.options).toBe('object');
});

it('should carry the factory form through to every tool subtype constructor', () => {
const blockOptions: BlockToolConstructor<BlockToolData, TestToolConfig>['options'] = config => ({
toolbox: [{ title: `Level ${config.level ?? 1}`,
icon: '' }],
});
const inlineOptions: InlineToolConstructor['options'] = () => ({ config: {} });
const tuneOptions: BlockTuneConstructor['options'] = () => ({ config: {} });

expect(typeof blockOptions).toBe('function');
expect(typeof inlineOptions).toBe('function');
expect(typeof tuneOptions).toBe('function');
});
});
Loading
Loading