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
81 changes: 81 additions & 0 deletions openspec/changes/separate-tool-options-and-config/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
## Context

Today a tool's static surface is one bag (`ToolConstructor.options`) that mixes two things with different audiences and different lifetimes:

- **Core/plugin-facing wiring** β€” `toolbox`, `shortcut`, `inlineToolbar`, `tunes`, `conversionConfig`, `canBeSplit`, `isReadOnlySupported`. Read by `BaseToolFacade`/`BlockToolFacade` getters and consumed by plugins (`ToolboxUI`, `ShortcutsPlugin`, inline toolbar, tunes) **before any block instance exists**.
- **Tool-author-facing user data** β€” `ToolConfig`, nested under `options.config`, merged with any `use()`-time `config` override in `BaseToolFacade.config`, and handed to the tool's `constructor`/`prepare()`.

`ToolConfig` itself is not an SDK contract β€” it's re-exported from the legacy `@editorjs/editorjs` package as `type ToolConfig<T extends object = any> = T`, an untyped passthrough. A tool author gets whatever structure they declare in their own `Config` generic parameter, with nothing checking that it's consistent with anything else.

The concrete failure this produces is the Header tool's v3 migration (draft PR [editor-js/header#130](https://github.com/editor-js/header/pull/130)): `HeaderConfig.levels` is declared but never read anywhere β€” the static `options.toolbox` array is a hardcoded 3-entry list (H1–H3) that has no way to depend on `config.levels`, because `toolbox` is evaluated once, statically, at class-definition time, while `config` is only resolved per tool-registration (in `BaseToolFacade.config`) and per block instance (in the constructor).

The lifecycle already has a hook positioned exactly where this could be fixed: `ToolsManager.prepareTools()` calls `toolConstructor.prepare({ toolName, config: tool.config })` β€” with `tool.config` already the fully merged config β€” and only *after* that call resolves does it call `setToAvailableToolsCollection`, which dispatches `ToolLoadedCoreEvent`. `ToolboxUI` only starts reading a tool's `toolbox` getter in response to that event. So `prepare()` already runs at the right time, with the right data; it just isn't a channel that can influence `options` today, and nothing tells a tool author it's meant to be.

Constraints this design has to respect:
- A page can run multiple `Core` instances that both `use()` the same tool class (e.g. two editors sharing one `Header` import). Anything a tool computes from its own resolved config must not be written onto the shared class/static object, or the second instance's config would clobber the first's.
- Per this change's proposal, a breaking change to `ToolConfig`'s type/import is acceptable; a live "change config after mount" API is explicitly out of scope.
- The project's existing TDD convention applies to the facade/manager changes below (see `openspec/config.yaml` rules).

## Goals / Non-Goals

**Goals:**
- Make `ToolConfig` an SDK-owned, real contract (not a re-exported `any`-defaulted passthrough).
- Keep `ToolOptions` (`BlockToolOptions`/`InlineToolOptions`/`BlockTuneOptions`) as the static, core/plugin-facing contract, formally distinct from `ToolConfig`.
- Let a tool compute config-derived static options (starting with `toolbox`) from its fully-resolved `ToolConfig`, resolved once during tool preparation, via the existing `prepare()` hook β€” without mutating the tool's shared static class property.
- Catch the `HeaderConfig.levels`-style failure mode: a config key that's declared but never actually consumed.

**Non-Goals:**
- No live/reactive config updates after the editor has mounted (confirmed out of scope β€” config is resolved once, during preparation).
- No change to how `shortcut`, `inlineToolbar`, or `tunes` are merged β€” this change only adds the config-derived tier to `toolbox`, since that's the concrete, evidenced need; the same mechanism can be extended to other option fields later if a real case shows up.
- Not migrating the `header` tool itself β€” it lives in a separate repo/submodule. This change ships the SDK mechanism; wiring `Header.prepare()` to use it is a follow-up in that repo.
- No new runtime schema-validation dependency (e.g. zod/io-ts).

## Decisions

**1. `ToolConfig` becomes an SDK-owned type, not a re-export of `@editorjs/editorjs`'s passthrough.**
The legacy `ToolConfig<T extends object = any> = T` is how the untyped-escape-hatch problem enters v3 in the first place β€” any tool author who doesn't explicitly parameterize their `Config` generic silently gets `any`. SDK defines its own `ToolConfig` base (still generic per tool, but anchored in `@editorjs/sdk` so it's the type the rest of this design's checks can hook into).
*Alternative considered*: leave the re-export and only tighten the `Config extends ToolConfig` bound on each interface (`BlockToolOptions`, etc.). Rejected β€” the default still resolves to `any` for any tool that skips the generic, which is exactly today's failure mode.

**2. `options` and `config` stay two separate top-level concepts; `options.config` remains the only bridge.**
This matches how they're actually consumed: `options` (via facade getters) is read by core/plugins before any block exists; `config` is read by the tool instance itself. Collapsing them into one `ToolSettings<Config>` bag would erase that timing distinction, which is the actual source of the bug (something that's plugin-timing data, like `toolbox`, has no way to see something that's tool-instance-timing data, like `config.levels`, unless the two are explicitly bridged).
*Alternative considered*: a single unified settings object passed everywhere. Rejected β€” every plugin consumer would need to filter out user-data noise, and it doesn't resolve the "when is this available" question.

**3. Config-derived options are resolved via `prepare()`'s return value, held per-facade-instance β€” not by writing onto `ClassName.options`.**
`prepare()`'s signature extends to:
```ts
prepare?(data: { toolName: string, config: Config }):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think going through prepare is an extra step. We can extends options property to be method which would accept the config

interface ToolConstructor {
  options: ToolOptions | (config: ToolConfig) => ToolOptions
}

Facade would check if options is a method and pass the config if required. So external access via Facade is unaffected

It would simplify the flow and remove new preparedOptions entity

PreparedToolOptions | void | Promise<PreparedToolOptions | void>;
```
where `PreparedToolOptions` is a partial type restricted, for now, to `{ toolbox?: ToolboxConfigEntry | ToolboxConfigEntry[] | false }`. `ToolsManager.prepareTools()` captures this return value and passes it through `ToolsFactory` into the facade constructor as a new `preparedOptions` field, stored on the `BaseToolFacade` instance (not on `constructable`).
*Alternative considered*: have the tool mutate `ClassName.options` directly inside `prepare()` (the shape floated during earlier discussion of this change). Rejected once the multi-`Core`-instance case was worked through: `constructable.options` is one object shared by every facade wrapping that class, so a second `Core` instance's `prepare()` call would overwrite the first instance's computed toolbox for both. Returning a value and letting the facade own it keeps the result properly scoped per editor instance while still using the same trigger point (`prepare()`) already agreed on.

**4. `toolbox` resolution gains a third tier, in this order: static default β†’ prepared (config-derived) β†’ explicit `use()`-time override.**
`BlockToolFacade.toolbox` keeps its existing array/object positional-merge algorithm; it's just seeded from `preparedOptions.toolbox ?? constructable.options.toolbox` instead of `constructable.options.toolbox` alone. All current explicit-override behavior (`toolbox: false` to hide, partial per-entry overrides via `use()`) is preserved unchanged, now layered on top of whatever the tool computed from its own config.
*Alternative considered*: make `preparedOptions.toolbox` and `useToolOptions.toolbox` mutually exclusive. Rejected as an unnecessary behavior cliff β€” keeping one uniform merge means the new tier is additive rather than a special case integrators need to remember.

**5. Verification is a dev-time reachability check on the resolved config object, not a schema-validation library.**
A lightweight check (gated the same way existing `ToolsManager` dev diagnostics are β€” `console.warn`, not thrown, and skippable in production builds) flags a key present in a tool's *resolved* `ToolConfig` object that is never read: neither passed through via `options.config` defaults, nor touched by a `prepare()` that returns `PreparedToolOptions`. This is a reachability check on real objects and function calls at runtime, not a compile-time-only type comparison (TS types don't exist at runtime) β€” so it directly catches the `HeaderConfig.levels`-declared-but-unused case without requiring a new dependency.
*Alternative considered*: full schema validation of `ToolConfig` shapes (zod/io-ts). Rejected as disproportionate to the actual failure mode, which is "declared but never read," not "wrong shape."

**6. Tools without a `prepare()` method are unaffected at runtime.**
`bold`, `italic`, `inline-link`, and `paragraph` don't currently need config-derived options; their migration is limited to importing the new SDK-owned `ToolConfig` type. Even though a breaking change is acceptable per this change's scope, there's no reason to force a runtime-behavior migration where only the type source moved.

## Risks / Trade-offs

- **[Risk]** A third merge tier (static β†’ prepared β†’ `use()`-override) adds a step to an already non-trivial merge chain in `BlockToolFacade.toolbox`, raising the bar for contributors reading it. β†’ **Mitigation**: keep all three tiers resolved in one getter (as today), document them with a spec scenario (see delta spec), and add a facade unit test per tier combination (static-only, prepared-only, both, both-plus-explicit-override).
- **[Risk]** Widening `prepare()`'s return type is itself a breaking change for any hypothetical tool already using `prepare()` for pure side effects that happens to return a truthy non-`undefined` value. β†’ **Mitigation**: only recognized keys (`toolbox`, for now) are read off the returned object; anything else is ignored with a dev-time warning rather than silently applied or throwing.
- **[Risk]** The "unused config key" check can only see what's reachable at runtime, not a tool's full declared type (erased at compile time), so it can miss a key that exists in the `Config` type but was never included in a given call's resolved config object. β†’ **Mitigation**: scope the check's guarantee accordingly β€” it catches "this resolved config has a key nothing reads," which is exactly the Header failure mode, not "this type has a key that's structurally unreachable."
- **[Risk]** The validating real-world case (`editor-js/header#130`) lives outside this repo and could merge with the bug still present before this change ships its mechanism. β†’ **Mitigation**: tasks.md treats that PR as an acceptance reference, not a task owned by this change; wiring `Header.prepare()` to the new mechanism is explicitly a follow-up in the `header` repo.

## Migration Plan

- Ship the SDK contract changes (`ToolConfig`, `ToolOptions`, `prepare()` signature), facade changes, and `ToolsManager`/`ToolsFactory` plumbing together β€” they're tightly coupled; a partial rollout would leave the new `prepare()` return type with no consumer.
- Update the four in-repo tools' `Config` type imports (mechanical, type-only, no runtime behavior change).
- No feature flag or staged rollout: this is a pre-1.0 internal SDK surface, and per this change's confirmed scope a breaking change is acceptable with no external in-repo consumers beyond the tools already covered.
- Rollback: revert the SDK/facade/core commits together; the tools' type-only import changes revert cleanly since they carry no runtime behavior.

## Open Questions

- Should `PreparedToolOptions` generalize beyond `toolbox` (e.g. `shortcut`, `inlineToolbar`) once a second concrete need appears, or stay toolbox-only indefinitely? Leaning toolbox-only until evidenced otherwise (YAGNI).
- Should the "unused config key" dev warning live in SDK core (always on outside production, matching existing `ToolsManager` console diagnostics) or as an opt-in lint/test helper tool authors run in CI? Leaning SDK-core for now; worth revisiting if it proves noisy.
- How would a third-party plugin (i.e. not one of the framework's own Toolbox/Shortcuts/InlineToolbar/Tunes consumers) read its own config-derived data off a tool? Today `options`/`PreparedToolOptions` are a closed, core-defined schema, not an extensible registry a third party can add fields to. `BlockToolOptions`/`InlineToolOptions`/`BlockTuneOptions` already carry a `[key: string]: unknown` escape hatch for custom static fields a tool author reads back themselves; `PreparedToolOptions` currently doesn't. Not adding that now (no evidenced consumer) β€” worth revisiting together with the bullet above if a real third-party case shows up.
31 changes: 31 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,31 @@
## 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.toolbox` is a plain static value evaluated once at class-definition time, a tool has no way to express "toolbox entries depend on my resolved config" β€” there is no formal contract, no type checking, and no runtime verification connecting a `ToolConfig` field to the `options` it's meant to drive.

This is not hypothetical: it is live today in the Header tool's v3 SDK migration (draft PR [editor-js/header#130](https://github.com/editor-js/header/pull/130)). `HeaderConfig` declares `levels` and `defaultLevel`, and `defaultLevel` does flow through (it's read inside the constructor from the resolved `config` object). But `levels` is dead β€” the static `options.toolbox` array is hardcoded to exactly three entries (H1–H3) and never consults `config.levels`, so a user who configures `levels: [1]` sees no change in the toolbox. The mechanism to fix this already exists in the core tool lifecycle (`prepare({ toolName, config })` runs once per registered tool, before the tool is announced to the Toolbox/Shortcuts/InlineToolbar/Tunes plugins), but nothing in the SDK's contracts guides a tool author to use it for config-derived options, and nothing catches the drift when, as in Header, a declared config field ends up unused.

## 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.
- Keep `ToolOptions` (`BlockToolOptions` / `InlineToolOptions` / `BlockTuneOptions`) as the static, declarative, core/plugin-facing contract, formally separated from `ToolConfig` β€” `options.config` remains the channel for config *defaults*, but is no longer the only thing standing in for "everything a tool needs at runtime".
- Extend the `prepare()` contract so a tool can return config-derived option values (starting with `toolbox`) after receiving its fully-resolved `ToolConfig`, and have the result flow into the tool's effective options before it is advertised to plugins β€” without mutating the tool's shared static class property (the current pattern of writing directly to `ClassName.options` would leak across multiple `Core` instances sharing the same tool class on one page).
- Add compile-time typing and a dev-time check that flags a `ToolConfig` field which no computed option / consumer reads (directly addressing the `HeaderConfig.levels` drift).
- Migrate the four in-repo tools (`paragraph`, `bold`, `italic`, `inline-link`) to the new `ToolConfig`/`ToolOptions` contracts. **BREAKING** for any tool relying on the current plain-object-only `static options` typing.
- 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 preparation, before the editor renders its UI.

## 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), the static `options`/`config` merge behavior gains a "config-derived options resolved via `prepare()`" step that runs before static options are read by consumers (Toolbox, Shortcuts, InlineToolbar, Tunes), and `BaseToolConstructor.prepare()`'s signature/return type changes accordingly.

## Impact

- `packages/sdk/src/entities/{BaseTool.ts, BlockTool.ts, InlineTool.ts, BlockTune.ts}`: new `ToolConfig`/`ToolOptions` contracts; extended `prepare()` signature and return type.
- `packages/sdk/src/tools/facades/{BaseToolFacade.ts, BlockToolFacade.ts, InlineToolFacade.ts, BlockTuneFacade.ts}`: a per-facade-instance slot for `prepare()`-computed options, inserted into the existing static/`use()`-time merge chain (the `toolbox` getter's merge algorithm gains a tier between static defaults and explicit `use()`-time overrides).
- `packages/core/src/tools/{ToolsManager.ts, ToolsFactory.ts}`: capture `prepare()`'s return value and thread computed options into facade construction, ahead of the `ToolLoadedCoreEvent` dispatch that Toolbox/etc. listen for.
- `packages/tools/{paragraph,bold,italic,inline-link}`: migrate to the new `ToolConfig` import/contract (type-only change; no tool here currently needs config-derived options).
- `editor-js/header` (external submodule repo, PR #130): not modified by this change directly, but is the motivating and validating case β€” its `HeaderConfig.levels` drift is the concrete bug this change makes fixable.
- `openspec/specs/sdk/spec.md`: delta spec updates to the "Tool and tune contracts" requirement.
Loading
Loading