diff --git a/openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/.openspec.yaml b/openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/.openspec.yaml new file mode 100644 index 00000000..ff5f854a --- /dev/null +++ b/openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-17 diff --git a/openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/design.md b/openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/design.md new file mode 100644 index 00000000..bb6f99c5 --- /dev/null +++ b/openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/design.md @@ -0,0 +1,43 @@ +## Context + +`FormattingInlineNode.format()` (`packages/model/src/entities/inline-fragments/FormattingInlineNode/index.ts:130`) short-circuits with `return []` whenever `tool === this.tool`, so re-applying a data-carrying inline tool never updates the fragment's `data`. `isEqual()` (line 193) and `mergeWith()` (line 213) ignore `data` as well, so `ParentNode.normalize()` can merge adjacent same-tool fragments that differ only in data. Every `format` op reaches the tree as plain `(tool: InlineToolName, start, end, data)` via `EditorJSModel.format → EditorDocument → BlockNode → ParentInlineNode → FormattingInlineNode`, and these ops are replayed for remote users and undo/redo — so the comparison must be deterministic and must not depend on non-serializable tool objects being threaded through the op. + +The `inline-link` tool (`packages/tools/inline-link/src/index.ts:133`) currently works around the model gap by sending `FormattingAction.None` on re-apply, which `SelectionManager` (`packages/core/src/components/SelectionManager.ts:193`) silently ignores (no `None` case) — so the new `href` never reaches the model. + +## Goals / Non-Goals + +**Goals:** +- Re-applying the same tool with **different** data replaces the affected fragment's data; with **equal** data it stays a no-op (preserves the current optimization for dataless tools like bold). +- Correct behavior when the re-applied range only partially covers an existing fragment (split, replace the middle segment). +- Data equality defaults to a deep structural comparison. +- `isEqual()` / `mergeWith()` become data-aware so normalization never merges distinct-data fragments. +- `inline-link` re-apply sends `FormattingAction.Format` and updates the URL. + +**Non-Goals:** +- A full `intersectType` engine (`Extend` / `LeaveBoth`) — only the replace-on-same-tool path is in scope. +- Changing the collaborative op shape or threading functions through it. +- Building the DI registry that lets a tool register a custom comparator — deferred (see Decision 3). + +## Decisions + +### Decision 1: Deep-equality comparison is the model's default + +Add a pure helper (e.g. `isSameInlineData(a?: InlineToolData, b?: InlineToolData): boolean`) performing deep structural equality, treating both-`undefined` as equal. `FormattingInlineNode.format()` uses it: when `tool === this.tool`, return `[]` (no-op) if data is equal, otherwise perform a data replacement. `isEqual()` and `mergeWith()` use the same helper so only truly-equal fragments merge. + +### Decision 2: Replace via split, reusing the existing `unformat` pattern + +For a same-tool re-apply with different data: +- If `[start, end]` fully covers the node (`0..length`): replace the node's `data` in place. +- If partial: split at `start` and `end` (mirroring `unformat`'s `split` logic) so the middle segment is isolated, and give that middle segment the new `data`. + +`data` is currently `readonly`; the implementation will introduce a controlled way to set it (internal setter or reconstruct-and-swap) rather than exposing it publicly. + +### Decision 3: Tool-overridable comparator — seam now, wiring deferred + +Because the op carries only a tool **name**, a custom comparator must be resolved by name from a model-side registry populated at tool-registration time, not passed through the op. That registry spans `sdk → core → model` and is larger than this change warrants. + +**This change:** add an optional `isSameData?(a, b): boolean` to the SDK `InlineTool` interface as a documented extension point, and centralize the model's comparison behind the single `isSameInlineData` helper so the override has an obvious future home. **Deferred:** the DI registry and core wiring that actually resolve a tool's `isSameData` — no built-in tool needs a custom comparator yet (link's `{ href }` is handled correctly by deep equality). + +### Decision 4: Remove the link workaround + +In `inline-link`, drop the `isActive ? FormattingAction.None : FormattingAction.Format` branch and always send `FormattingAction.Format` with `data: { href: linkInput.value }`. The stale `@todo Replace link…` comment is removed since the model now handles it. diff --git a/openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/proposal.md b/openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/proposal.md new file mode 100644 index 00000000..725e2254 --- /dev/null +++ b/openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/proposal.md @@ -0,0 +1,25 @@ +## Why + +When a data-carrying inline tool (like **link**) is re-applied over a range that already has that same tool, the new data is silently dropped — editing a link's URL does nothing and the old `href` sticks. The root cause is in the model: `FormattingInlineNode.format()` returns early whenever the tool matches an already-applied tool, never comparing or replacing `data`. Because the model can't do this, the `inline-link` tool works around it by sending a no-op `FormattingAction.None` on re-apply, so the new value never even reaches the model. + +## What Changes + +- `FormattingInlineNode.format()` stops no-op'ing on same-tool re-apply. When the same tool is re-applied with **different data**, the fragment's data is replaced; when the data is unchanged, it stays a no-op (preserving the current optimization for tools like bold that carry no data). +- Data equality defaults to a deep comparison, but an inline tool MAY provide its own comparator so it can decide what "same data" means for its fragments. +- Node normalization becomes data-aware: `isEqual()` / `mergeWith()` account for `data`, so adjacent same-tool fragments with different data are not incorrectly treated as equal or merged together (which would silently resurrect stale data). +- The `inline-link` tool stops sending the `FormattingAction.None` workaround and sends `FormattingAction.Format` with the new data on re-apply, now that the model handles replacement. + +## Capabilities + +### New Capabilities + + +### Modified Capabilities +- `model`: the "Inline text tree" requirement gains data-aware re-formatting — re-applying the same data-carrying tool over a fragment replaces its data (deep-equality default, tool-overridable comparator) instead of being a no-op, and normalization no longer merges same-tool fragments whose data differs. +- `tools`: the "Inline link tool" requirement changes so that confirming a URL on an already-linked selection re-applies the link with the new `href` via `FormattingAction.Format`, rather than the current `FormattingAction.None` no-op. + +## Impact + +- `packages/model/src/entities/inline-fragments/FormattingInlineNode/index.ts`: make `format()`, `isEqual()`, and `mergeWith()` data-aware. +- `packages/model-types` / `packages/sdk` (`InlineTool`): expose the optional per-tool data comparator so tools can override deep equality. +- `packages/tools/inline-link/src/index.ts`: remove the `FormattingAction.None` re-apply workaround; send `FormattingAction.Format` with the new data. diff --git a/openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/specs/model/spec.md b/openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/specs/model/spec.md new file mode 100644 index 00000000..aaeb5571 --- /dev/null +++ b/openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/specs/model/spec.md @@ -0,0 +1,36 @@ +## MODIFIED Requirements + +### Requirement: Inline text tree +The system SHALL provide `TextNode` (extending `ParentInlineNode`) as the inline-fragment tree root for a text data node, supporting text insert/remove, format/unformat over ranges, fragment listing, and index/range validation with auto-merging/normalization of children. When a data-carrying inline tool is re-applied over a range already formatted with that same tool, the system SHALL replace the affected fragment's data with the newly supplied data instead of ignoring it. Two fragments of the same tool SHALL be considered equal (for normalization/merging) only when their data is also equal; data equality SHALL default to a deep structural comparison, which an inline tool MAY override with its own comparator. + +#### Scenario: Inserting text into an empty tree +- **GIVEN** a `TextNode` with no children +- **WHEN** text is inserted +- **THEN** a child inline text node is auto-created and a `TextAddedEvent` is emitted + +#### Scenario: Removing text with an out-of-range index +- **GIVEN** a `TextNode` with a bounded length +- **WHEN** `removeText` is called with indices outside that range +- **THEN** it throws a range/index validation error + +#### Scenario: Overlapping formatting from the same or different tools +- **GIVEN** a text range that already has formatting applied +- **WHEN** `format` is applied again over an overlapping range (same tool or a different tool) +- **THEN** the resulting fragments nest or merge correctly without duplicating formatting + +#### Scenario: Re-applying the same tool with different data +- **GIVEN** a fragment already formatted with a data-carrying tool (e.g. a `link` with `{ href: "a" }`) +- **WHEN** `format` is applied over that range with the same tool but different data (e.g. `{ href: "b" }`) +- **THEN** the fragment's data is replaced with the new data (`{ href: "b" }`) rather than the call being a no-op + +#### Scenario: Re-applying the same tool with identical data +- **GIVEN** a fragment already formatted with a data-carrying tool +- **WHEN** `format` is applied over that range with the same tool and data that is equal under the effective comparator +- **THEN** the tree is left unchanged (no redundant fragments or mutations) + +#### Scenario: Normalization keeps distinct-data fragments separate +- **GIVEN** two adjacent fragments of the same tool whose data differs +- **WHEN** the tree is normalized +- **THEN** the fragments are NOT merged, so each retains its own data + +Implemented in `src/entities/inline-fragments/ParentInlineNode/index.ts`, `src/entities/inline-fragments/FormattingInlineNode/index.ts`, `src/entities/inline-fragments/TextNode/index.ts`, validated by `src/entities/inline-fragments/TextNode/*.spec.ts`, `src/entities/inline-fragments/FormattingInlineNode/*.spec.ts`, and `src/entities/inline-fragments/specs/InlineTree.integration.spec.ts`. diff --git a/openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/specs/tools/spec.md b/openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/specs/tools/spec.md new file mode 100644 index 00000000..43155468 --- /dev/null +++ b/openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/specs/tools/spec.md @@ -0,0 +1,31 @@ +## MODIFIED Requirements + +### Requirement: Inline link tool +The system SHALL provide `LinkInlineTool`, implementing `InlineTool` with `name: 'link'` and `intersectType: IntersectType.Replace` (a new link replaces any overlapping link rather than merging), rendering an `` wrapper and exposing a popover for entering/editing the URL. Confirming a URL SHALL always re-apply the link via `FormattingAction.Format`, so editing an existing link updates its `href` to the new value. + +#### Scenario: Editing an existing link +- **GIVEN** the current selection is inside an existing link fragment +- **WHEN** the toolbar renders `getToolbarConfig` +- **THEN** the icon is `IconUnlink`, the popover pre-opens with the existing `href` value, and activating the icon calls `api.selection.applyInlineTool` with `FormattingAction.Unformat` to remove the link + +#### Scenario: Creating a new link +- **GIVEN** the current selection is not inside an existing link +- **WHEN** the toolbar renders `getToolbarConfig` +- **THEN** the popover opens with an empty input, autofocused via a microtask + +#### Scenario: Confirming a link URL +- **GIVEN** the popover's URL input has focus +- **WHEN** the user presses Enter +- **THEN** `api.selection.applyInlineTool` is called with `data: { href: }` and `FormattingAction.Format`, whether the tool was already active or not + +#### Scenario: Updating the URL of an existing link +- **GIVEN** the current selection is inside an existing link with `href: "a"` +- **WHEN** the user enters `"b"` in the popover input and presses Enter +- **THEN** the link fragment's `href` is replaced with `"b"` + +#### Scenario: Wrapper href assignment +- **GIVEN** `createWrapper(data)` is called +- **WHEN** `data.href` is a string +- **THEN** the produced `` element's `href` attribute is set to that value; otherwise a bare `` is produced + +Implemented in `packages/tools/inline-link/src/index.ts`. diff --git a/openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/tasks.md b/openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/tasks.md new file mode 100644 index 00000000..7cf468b1 --- /dev/null +++ b/openspec/changes/archive/2026-07-17-replace-inline-fragment-data-on-reformat/tasks.md @@ -0,0 +1,20 @@ +## 1. Model: data-aware comparison + +- [x] 1.1 Add a pure `isSameInlineData(a?, b?)` deep-equality helper in the model's inline-fragments utils +- [x] 1.2 Make `FormattingInlineNode.isEqual()` and `mergeWith()` use `isSameInlineData` so distinct-data fragments don't merge +- [x] 1.3 Rewrite `FormattingInlineNode.format()`: same tool + equal data → no-op; same tool + different data → replace via split (reuse the `unformat` split pattern), full-cover → replace `data` in place +- [x] 1.4 Introduce a controlled way to set the fragment's `data` (private `#data` field + public getter), keeping it non-public +- [x] 1.5 Make `ParentInlineNode.getFragments()` normalization data-aware so the read/serialization path keeps distinct-data fragments separate + +## 2. SDK: comparator seam + +- [x] 2.1 Add optional `isSameData?(a, b): boolean` to the `InlineTool` interface with a doc comment marking it the deferred override point + +## 3. Tool: remove the link workaround + +- [x] 3.1 In `inline-link`, always send `FormattingAction.Format` on confirm; drop the `FormattingAction.None` branch and the stale `@todo` + +## 4. Verify + +- [x] 4.1 Add/extend tests for the spec scenarios: re-apply different data → replaced; re-apply equal data → no-op; partial-range replace splits correctly; normalization keeps distinct-data fragments separate (integration spec) plus a focused `isSameInlineData` unit spec +- [x] 4.2 Run the model + tools test suites and typecheck; confirm green diff --git a/openspec/changes/wire-inline-tool-data-comparator/.openspec.yaml b/openspec/changes/wire-inline-tool-data-comparator/.openspec.yaml new file mode 100644 index 00000000..ff5f854a --- /dev/null +++ b/openspec/changes/wire-inline-tool-data-comparator/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-17 diff --git a/openspec/changes/wire-inline-tool-data-comparator/proposal.md b/openspec/changes/wire-inline-tool-data-comparator/proposal.md new file mode 100644 index 00000000..eade6d2c --- /dev/null +++ b/openspec/changes/wire-inline-tool-data-comparator/proposal.md @@ -0,0 +1,53 @@ +## Why + +The change `replace-inline-fragment-data-on-reformat` (archived `2026-07-17`) made the model +data-aware when re-applying an inline tool, using a **deep-equality** default +(`isSameInlineData`, `packages/model/src/utils/isSameInlineData.ts`). As part of that work it +added an optional `isSameData?(a, b)` to the SDK `InlineTool` interface +(`packages/sdk/src/entities/InlineTool.ts`) as a **documented seam only** — the model does not +yet consult it. See that change's `design.md` Decision 3 for the rationale (the wiring spans +sdk → core → model and no built-in tool needed custom equality yet). + +This is the deferred follow-up: actually let a tool override the model's default data equality. +Do it when a tool genuinely needs equality that differs from deep structural comparison +(e.g. a mention/comment tool that should treat two fragments as equal ignoring a derived or +display-only field, or normalize a URL before comparing). + +## What Changes + +- Resolve a tool's `isSameData` comparator inside the model by **tool name**, defaulting to + `isSameInlineData` when a tool does not provide one. +- The comparator must be registered/looked up at tool-registration time, **not** threaded + through the `format` op — the op is replayed for remote users / undo-redo and carries only a + tool name and plain data, so it must stay serializable and deterministic. +- Consume the resolved comparator wherever the model currently calls `isSameInlineData`: + `FormattingInlineNode.format()` (no-op vs replace), `FormattingInlineNode.isEqual()` / + `mergeWith()`, and `ParentInlineNode.getFragments()` normalization. +- Remove the `@todo`s left at the seam once wired (`InlineTool.isSameData`, + `isSameInlineData`). + +## Capabilities + +### New Capabilities + + +### Modified Capabilities +- `model`: the "Inline text tree" requirement's data-equality default (currently deep + structural comparison) becomes overridable per inline tool via a comparator resolved by tool + name. +- `sdk`: the inline-tool contract's `isSameData` moves from a documented-but-unused seam to a + hook the model actually invokes. + +## Impact + +- `packages/model` — a comparator registry/resolver keyed by `InlineToolName`; the four call + sites listed above consult it instead of calling `isSameInlineData` directly. +- `packages/core` — register each tool's `isSameData` into the model at tool-registration time + (near where tools/IoC are wired). +- `packages/sdk` — `InlineTool.isSameData` doc comment updated (no longer "not yet consulted"). + +## Notes + +This is a **stub** created to track deferred work — proposal only. Specs, design, and tasks +are intentionally not written yet; generate them (e.g. `/opsx:continue +wire-inline-tool-data-comparator` or `/opsx:ff`) when the work is picked up. diff --git a/openspec/specs/model/spec.md b/openspec/specs/model/spec.md index b156dd4e..d9b8bca8 100644 --- a/openspec/specs/model/spec.md +++ b/openspec/specs/model/spec.md @@ -3,9 +3,7 @@ ## Purpose `@editorjs/model` is the in-memory document model engine: a tree of blocks containing text/value data nodes and inline-formatting fragments, plus multi-user caret tracking. It centralizes CRUD on document structure, text content, and inline formatting, emitting typed events (from `@editorjs/model-types`) for every mutation so collaboration and rendering layers can react. It is an internal engine consumed by `core` and `ot-server`; tools and plugins should depend on `@editorjs/sdk` instead. - ## Requirements - ### Requirement: EditorJSModel public facade The system SHALL expose `EditorJSModel` as the single entry point wrapping `EditorDocument` and `CaretManager`, providing block/data/text/caret CRUD, tagging mutations with the acting `_userId`, and re-dispatching document events as `ModelEvents`. @@ -62,7 +60,7 @@ The system SHALL provide `BlockNode` representing a single block's data tree (ne Implemented in `src/entities/BlockNode/index.ts`, validated by its co-located `.spec.ts`. ### Requirement: Inline text tree -The system SHALL provide `TextNode` (extending `ParentInlineNode`) as the inline-fragment tree root for a text data node, supporting text insert/remove, format/unformat over ranges, fragment listing, and index/range validation with auto-merging/normalization of children. +The system SHALL provide `TextNode` (extending `ParentInlineNode`) as the inline-fragment tree root for a text data node, supporting text insert/remove, format/unformat over ranges, fragment listing, and index/range validation with auto-merging/normalization of children. When a data-carrying inline tool is re-applied over a range already formatted with that same tool, the system SHALL replace the affected fragment's data with the newly supplied data instead of ignoring it. Two fragments of the same tool SHALL be considered equal (for normalization/merging) only when their data is also equal; data equality SHALL default to a deep structural comparison, which an inline tool MAY override with its own comparator. #### Scenario: Inserting text into an empty tree - **GIVEN** a `TextNode` with no children @@ -79,7 +77,22 @@ The system SHALL provide `TextNode` (extending `ParentInlineNode`) as the inline - **WHEN** `format` is applied again over an overlapping range (same tool or a different tool) - **THEN** the resulting fragments nest or merge correctly without duplicating formatting -Implemented in `src/entities/inline-fragments/ParentInlineNode/index.ts`, `src/entities/inline-fragments/TextNode/index.ts`, validated by `src/entities/inline-fragments/TextNode/*.spec.ts` and `src/entities/inline-fragments/specs/InlineTree.integration.spec.ts`. +#### Scenario: Re-applying the same tool with different data +- **GIVEN** a fragment already formatted with a data-carrying tool (e.g. a `link` with `{ href: "a" }`) +- **WHEN** `format` is applied over that range with the same tool but different data (e.g. `{ href: "b" }`) +- **THEN** the fragment's data is replaced with the new data (`{ href: "b" }`) rather than the call being a no-op + +#### Scenario: Re-applying the same tool with identical data +- **GIVEN** a fragment already formatted with a data-carrying tool +- **WHEN** `format` is applied over that range with the same tool and data that is equal under the effective comparator +- **THEN** the tree is left unchanged (no redundant fragments or mutations) + +#### Scenario: Normalization keeps distinct-data fragments separate +- **GIVEN** two adjacent fragments of the same tool whose data differs +- **WHEN** the tree is normalized +- **THEN** the fragments are NOT merged, so each retains its own data + +Implemented in `src/entities/inline-fragments/ParentInlineNode/index.ts`, `src/entities/inline-fragments/FormattingInlineNode/index.ts`, `src/entities/inline-fragments/TextNode/index.ts`, validated by `src/entities/inline-fragments/TextNode/*.spec.ts`, `src/entities/inline-fragments/FormattingInlineNode/*.spec.ts`, and `src/entities/inline-fragments/specs/InlineTree.integration.spec.ts`. ### Requirement: Value nodes and block tunes The system SHALL provide `ValueNode` for leaf non-text data storage and `BlockTune` for per-block tune data, both participating in the block's event-emitting data tree. @@ -110,3 +123,4 @@ The system SHALL provide an internal `ToolsRegistry` and IoC container for tool/ - **THEN** the registered instance is returned Implemented in `src/tools/ToolsRegistry.ts`, `src/IoC/`. + diff --git a/openspec/specs/tools/spec.md b/openspec/specs/tools/spec.md index 313cccf7..48dc60d0 100644 --- a/openspec/specs/tools/spec.md +++ b/openspec/specs/tools/spec.md @@ -5,9 +5,7 @@ The built-in tool/plugin packages provide the default block and inline tools shipped with the editor: `@editorjs/paragraph` (block), and `@editorjs/bold`, `@editorjs/italic`, `@editorjs/inline-link` (inline). Each implements the `BlockTool`/`InlineTool` contracts from `@editorjs/sdk`. **Note**: none of these four packages contain test files; the scenarios below are derived directly from source control flow rather than confirmed by dedicated tests. - ## Requirements - ### Requirement: Paragraph block tool The system SHALL provide `Paragraph`, implementing `BlockTool`, as the default block tool rendering plain text, with `conversionConfig: { import: 'text', export: 'text' }`. @@ -59,7 +57,7 @@ The system SHALL provide `ItalicInlineTool`, structurally identical to `BoldInli Implemented in `packages/tools/italic/src/index.ts`. ### Requirement: Inline link tool -The system SHALL provide `LinkInlineTool`, implementing `InlineTool` with `name: 'link'` and `intersectType: IntersectType.Replace` (a new link replaces any overlapping link rather than merging), rendering an `` wrapper and exposing a popover for entering/editing the URL. +The system SHALL provide `LinkInlineTool`, implementing `InlineTool` with `name: 'link'` and `intersectType: IntersectType.Replace` (a new link replaces any overlapping link rather than merging), rendering an `` wrapper and exposing a popover for entering/editing the URL. Confirming a URL SHALL always re-apply the link via `FormattingAction.Format`, so editing an existing link updates its `href` to the new value. #### Scenario: Editing an existing link - **GIVEN** the current selection is inside an existing link fragment @@ -74,7 +72,12 @@ The system SHALL provide `LinkInlineTool`, implementing `InlineTool` with `name: #### Scenario: Confirming a link URL - **GIVEN** the popover's URL input has focus - **WHEN** the user presses Enter -- **THEN** `api.selection.applyInlineTool` is called with `data: { href: }` and `FormattingAction.Format` (new link) or `FormattingAction.None` (editing an existing link's href), depending on whether the tool was active +- **THEN** `api.selection.applyInlineTool` is called with `data: { href: }` and `FormattingAction.Format`, whether the tool was already active or not + +#### Scenario: Updating the URL of an existing link +- **GIVEN** the current selection is inside an existing link with `href: "a"` +- **WHEN** the user enters `"b"` in the popover input and presses Enter +- **THEN** the link fragment's `href` is replaced with `"b"` #### Scenario: Wrapper href assignment - **GIVEN** `createWrapper(data)` is called @@ -92,3 +95,4 @@ The system SHALL have all inline tools (Bold, Italic, Link) implement the same c - **THEN** `isActive` returns `true`, using the same range-containment logic regardless of which tool is evaluating it Cross-referenced across `packages/tools/bold/src/index.ts`, `packages/tools/italic/src/index.ts`, `packages/tools/inline-link/src/index.ts`, and validated indirectly by the generic facade test `packages/sdk/src/tools/facades/BaseToolFacade.spec.ts`. + diff --git a/packages/model/src/entities/inline-fragments/FormattingInlineNode/index.ts b/packages/model/src/entities/inline-fragments/FormattingInlineNode/index.ts index 81e36627..f257f4e1 100644 --- a/packages/model/src/entities/inline-fragments/FormattingInlineNode/index.ts +++ b/packages/model/src/entities/inline-fragments/FormattingInlineNode/index.ts @@ -6,6 +6,7 @@ import type { InlineNode } from '../InlineNode/index.js'; import { ParentNode } from '../mixins/ParentNode/index.js'; import { ChildNode } from '../mixins/ChildNode/index.js'; import { ParentInlineNode } from '../ParentInlineNode/index.js'; +import { isSameInlineData } from '../../../utils/index.js'; export type * from './types/index.js'; @@ -26,10 +27,18 @@ export class FormattingInlineNode extends ParentInlineNode implements InlineNode */ public readonly tool: InlineToolName; + /** + * Any additional data associated with the formatting tool. + * Kept in a private field so it can be replaced on re-formatting while staying read-only to the outside. + */ + #data?: InlineToolData; + /** * Any additional data associated with the formatting tool */ - public readonly data?: InlineToolData; + public get data(): InlineToolData | undefined { + return this.#data; + } /** * Constructor for FormattingInlineNode class. @@ -42,7 +51,7 @@ export class FormattingInlineNode extends ParentInlineNode implements InlineNode super(); this.tool = tool; - this.data = data; + this.#data = data; } /** @@ -128,17 +137,76 @@ export class FormattingInlineNode extends ParentInlineNode implements InlineNode * @param [data] - inline tool data if applicable */ public format(tool: InlineToolName, start: number, end: number, data?: InlineToolData): InlineNode[] { - /** - * In case current tool is the same as new one, do nothing - * @todo Compare data as well - */ if (tool === this.tool) { - return []; + /** + * Re-applying the same tool with the same data is a no-op + */ + if (isSameInlineData(data, this.#data)) { + return []; + } + + /** + * Re-applying the same tool with different data replaces the data over the passed range. + * Split the node so only the affected range gets the new data, then update that segment. + */ + return this.#replaceData(start, end, data); } return super.format(tool, start, end, data); } + /** + * Replaces this node's data over the specified range, splitting the node when the range does + * not cover it fully so the surrounding parts keep their original data. + * @param start - char start index of the range + * @param end - char end index of the range + * @param [data] - new inline tool data to apply + */ + #replaceData(start: number, end: number, data?: InlineToolData): InlineNode[] { + /** + * Range covers the whole node — just swap the data in place + */ + if (start === 0 && end === this.length) { + this.#data = data; + + return [this]; + } + + const middleNode = this.split(start); + const result: FormattingInlineNode[] = []; + + /** + * If start > 0, `split` produced a node covering [start, length). Split it again at `end` + * so `middleNode` is exactly the affected range, and the remainder keeps the old data. + */ + if (middleNode) { + const endNode = middleNode.split(end - this.length); + + middleNode.#data = data; + + result.push(this, middleNode); + + if (endNode) { + result.push(endNode); + } + /** + * If start === 0, split at `end` so `this` becomes the affected range and the tail keeps the old data. + */ + } else { + const endNode = this.split(end); + + this.#data = data; + + result.push(this); + + if (endNode) { + result.push(endNode); + } + } + + return result; + } + /** * Removes formatting from the text for a specified inline tool in the specified range * @param tool - name of inline tool to remove @@ -200,10 +268,9 @@ export class FormattingInlineNode extends ParentInlineNode implements InlineNode } /** - * @todo check data equality + * Fragments with different data must stay separate so normalization does not drop or resurrect stale data */ - - return true; + return isSameInlineData(this.#data, node.#data); } /** @@ -216,7 +283,7 @@ export class FormattingInlineNode extends ParentInlineNode implements InlineNode } /** - * @todo merge data + * `isEqual` guarantees the data is equal, so there is nothing to reconcile — just move the children over */ Array.from(node.children).forEach((child) => { this.append(child); diff --git a/packages/model/src/entities/inline-fragments/ParentInlineNode/index.ts b/packages/model/src/entities/inline-fragments/ParentInlineNode/index.ts index 98bd660e..be19852a 100644 --- a/packages/model/src/entities/inline-fragments/ParentInlineNode/index.ts +++ b/packages/model/src/entities/inline-fragments/ParentInlineNode/index.ts @@ -1,4 +1,5 @@ import { getContext } from '../../../utils/Context.js'; +import { isSameInlineData } from '../../../utils/index.js'; import { IndexBuilder } from '@editorjs/model-types'; import type { InlineNode } from '../InlineNode/index.js'; import type { InlineFragment, InlineTreeNodeSerialized, InlineToolData, InlineToolName } from '@editorjs/model-types'; @@ -155,9 +156,15 @@ export class ParentInlineNode extends EventBus implements InlineNode { const previousFragment = normalized[normalized.length - 1]; /** - * @todo compare data + * Adjacent fragments are only coalesced when they share the same tool AND data, + * so a range whose data was replaced keeps its own fragment in the output. */ - if (previousFragment === undefined || previousFragment.tool !== fragment.tool || previousFragment.range[1] !== fragment.range[0]) { + if ( + previousFragment === undefined + || previousFragment.tool !== fragment.tool + || previousFragment.range[1] !== fragment.range[0] + || !isSameInlineData(previousFragment.data, fragment.data) + ) { normalized.push(fragment); return normalized; diff --git a/packages/model/src/entities/inline-fragments/specs/InlineTree.integration.spec.ts b/packages/model/src/entities/inline-fragments/specs/InlineTree.integration.spec.ts index bb66d1ed..0fe30968 100644 --- a/packages/model/src/entities/inline-fragments/specs/InlineTree.integration.spec.ts +++ b/packages/model/src/entities/inline-fragments/specs/InlineTree.integration.spec.ts @@ -278,6 +278,117 @@ describe('Inline fragments tree integration', () => { ]); }); + describe('re-applying a data-carrying tool', () => { + const linkTool = createInlineToolName('link'); + const initialText = 'Editor.js is a block-styled editor.'; + + it('should replace fragment data when re-applying the same tool with different data', () => { + const tree = new TextNode({ children: [new TextInlineNode({ value: initialText })] }); + + tree.format(linkTool, 0, initialText.length, createInlineToolData({ href: 'https://a.com' })); + tree.format(linkTool, 0, initialText.length, createInlineToolData({ href: 'https://b.com' })); + + expect(tree.getFragments()).toStrictEqual([ + { + tool: linkTool, + range: [0, initialText.length], + data: createInlineToolData({ href: 'https://b.com' }), + }, + ]); + }); + + it('should leave the tree unchanged when re-applying the same tool with identical data', () => { + const tree = new TextNode({ children: [new TextInlineNode({ value: initialText })] }); + const data = createInlineToolData({ href: 'https://a.com' }); + + tree.format(linkTool, 0, initialText.length, data); + tree.format(linkTool, 0, initialText.length, createInlineToolData({ href: 'https://a.com' })); + + expect(tree.getFragments()).toStrictEqual([ + { + tool: linkTool, + range: [0, initialText.length], + data, + }, + ]); + }); + + it('should split the fragment and replace data only within the re-applied range', () => { + const tree = new TextNode({ children: [new TextInlineNode({ value: initialText })] }); + const dataA = createInlineToolData({ href: 'https://a.com' }); + const dataB = createInlineToolData({ href: 'https://b.com' }); + const start = 5; + const end = 10; + + tree.format(linkTool, 0, initialText.length, dataA); + tree.format(linkTool, start, end, dataB); + + expect(tree.getFragments()).toStrictEqual([ + { + tool: linkTool, + range: [0, start], + data: dataA, + }, + { + tool: linkTool, + range: [start, end], + data: dataB, + }, + { + tool: linkTool, + range: [end, initialText.length], + data: dataA, + }, + ]); + }); + + it('should replace data only within a re-applied prefix range', () => { + const tree = new TextNode({ children: [new TextInlineNode({ value: initialText })] }); + const dataA = createInlineToolData({ href: 'https://a.com' }); + const dataB = createInlineToolData({ href: 'https://b.com' }); + const end = 10; + + tree.format(linkTool, 0, initialText.length, dataA); + tree.format(linkTool, 0, end, dataB); + + expect(tree.getFragments()).toStrictEqual([ + { + tool: linkTool, + range: [0, end], + data: dataB, + }, + { + tool: linkTool, + range: [end, initialText.length], + data: dataA, + }, + ]); + }); + + it('should not merge adjacent same-tool fragments whose data differs', () => { + const tree = new TextNode({ children: [new TextInlineNode({ value: initialText })] }); + const dataA = createInlineToolData({ href: 'https://a.com' }); + const dataB = createInlineToolData({ href: 'https://b.com' }); + const boundary = 10; + + tree.format(linkTool, 0, boundary, dataA); + tree.format(linkTool, boundary, initialText.length, dataB); + + expect(tree.getFragments()).toStrictEqual([ + { + tool: linkTool, + range: [0, boundary], + data: dataA, + }, + { + tool: linkTool, + range: [boundary, initialText.length], + data: dataB, + }, + ]); + }); + }); + it('should support overlapping formatting for different tools', () => { const initialText = 'Editor.js is a block-styled editor.'; const child = new TextInlineNode({ value: initialText }); diff --git a/packages/model/src/utils/index.ts b/packages/model/src/utils/index.ts index 23aec98b..a4bc214f 100644 --- a/packages/model/src/utils/index.ts +++ b/packages/model/src/utils/index.ts @@ -1 +1,2 @@ export * from './textUtils.js'; +export * from './isSameInlineData.js'; diff --git a/packages/model/src/utils/isSameInlineData.spec.ts b/packages/model/src/utils/isSameInlineData.spec.ts new file mode 100644 index 00000000..202fd351 --- /dev/null +++ b/packages/model/src/utils/isSameInlineData.spec.ts @@ -0,0 +1,65 @@ +/* eslint-disable @typescript-eslint/no-magic-numbers */ +import { createInlineToolData } from '@editorjs/model-types'; +import { isSameInlineData } from './isSameInlineData.js'; + +describe('isSameInlineData', () => { + it('should treat both-undefined as equal', () => { + expect(isSameInlineData(undefined, undefined)).toBe(true); + }); + + it('should treat undefined and an empty object as equal', () => { + expect(isSameInlineData(undefined, createInlineToolData({}))).toBe(true); + }); + + it('should return true for deeply equal data', () => { + expect(isSameInlineData( + createInlineToolData({ href: 'https://a.com', + meta: { rel: 'nofollow' } }), + createInlineToolData({ href: 'https://a.com', + meta: { rel: 'nofollow' } }) + )).toBe(true); + }); + + it('should return false when a primitive value differs', () => { + expect(isSameInlineData( + createInlineToolData({ href: 'https://a.com' }), + createInlineToolData({ href: 'https://b.com' }) + )).toBe(false); + }); + + it('should return false when a nested value differs', () => { + expect(isSameInlineData( + createInlineToolData({ href: 'https://a.com', + meta: { rel: 'nofollow' } }), + createInlineToolData({ href: 'https://a.com', + meta: { rel: 'noopener' } }) + )).toBe(false); + }); + + it('should return false when the set of keys differs', () => { + expect(isSameInlineData( + createInlineToolData({ href: 'https://a.com' }), + createInlineToolData({ href: 'https://a.com', + title: 'Example' }) + )).toBe(false); + }); + + it('should return false when one value is an array and the other is not', () => { + expect(isSameInlineData( + createInlineToolData({ value: [1, 2, 3] }), + createInlineToolData({ value: { length: 3 } }) + )).toBe(false); + }); + + it('should compare array values by order and content', () => { + expect(isSameInlineData( + createInlineToolData({ items: [1, 2, 3] }), + createInlineToolData({ items: [1, 2, 3] }) + )).toBe(true); + + expect(isSameInlineData( + createInlineToolData({ items: [1, 2, 3] }), + createInlineToolData({ items: [3, 2, 1] }) + )).toBe(false); + }); +}); diff --git a/packages/model/src/utils/isSameInlineData.ts b/packages/model/src/utils/isSameInlineData.ts new file mode 100644 index 00000000..77ae5be0 --- /dev/null +++ b/packages/model/src/utils/isSameInlineData.ts @@ -0,0 +1,53 @@ +import type { InlineToolData } from '@editorjs/model-types'; + +/** + * Deep structural equality check for two plain JSON-like values. + * @param a - first value to compare + * @param b - second value to compare + */ +function deepEqual(a: unknown, b: unknown): boolean { + if (a === b) { + return true; + } + + if (typeof a !== 'object' || a === null || typeof b !== 'object' || b === null) { + return false; + } + + const aIsArray = Array.isArray(a); + + if (aIsArray !== Array.isArray(b)) { + return false; + } + + if (aIsArray) { + const arrA = a as unknown[]; + const arrB = b as unknown[]; + + return arrA.length === arrB.length && arrA.every((item, index) => deepEqual(item, arrB[index])); + } + + const objA = a as Record; + const objB = b as Record; + const keysA = Object.keys(objA); + const keysB = Object.keys(objB); + + return keysA.length === keysB.length + && keysA.every(key => Object.prototype.hasOwnProperty.call(objB, key) && deepEqual(objA[key], objB[key])); +} + +/** + * Determines whether two inline tool data objects should be treated as equal. + * + * This is the model's default comparator, used to decide whether re-applying a tool is a + * no-op and whether two same-tool fragments may be merged. Missing data is treated as empty, + * so `undefined` and `{}` count as equal; otherwise a deep structural comparison is performed. + * @todo Allow an inline tool to override this via the `InlineTool.isSameData` seam + * (declared in `@editorjs/sdk`). Wiring a per-tool comparator through the model is + * deferred until a built-in tool needs custom equality. + * @param a - first inline tool data + * @param b - second inline tool data + */ +export function isSameInlineData(a?: InlineToolData, b?: InlineToolData): boolean { + return deepEqual(a ?? {}, b ?? {}); +} diff --git a/packages/sdk/src/entities/InlineTool.ts b/packages/sdk/src/entities/InlineTool.ts index 6d86de0e..ac12fb96 100644 --- a/packages/sdk/src/entities/InlineTool.ts +++ b/packages/sdk/src/entities/InlineTool.ts @@ -88,6 +88,20 @@ export interface InlineTool extends Omit