From 7723a33cc74c72c7662c2fd43a13645de4c9bc25 Mon Sep 17 00:00:00 2001 From: bdbch Date: Sun, 2 Aug 2026 12:34:49 +0200 Subject: [PATCH] wip: refactor extension manager and add tests --- .../src/editor/{__tests__ => }/Editor.spec.ts | 2 +- .../{__tests__ => }/EventEmitter.spec.ts | 2 +- .../src/extensions/ExtensionManager.spec.ts | 72 ++++++++++++ .../core/src/extensions/ExtensionManager.ts | 38 +----- .../helpers/resolveExtensionsOnEditor.spec.ts | 111 ++++++++++++++++++ .../helpers/resolveExtensionsOnEditor.ts | 55 +++++++++ 6 files changed, 245 insertions(+), 35 deletions(-) rename packages/core/src/editor/{__tests__ => }/Editor.spec.ts (96%) rename packages/core/src/editor/{__tests__ => }/EventEmitter.spec.ts (96%) create mode 100644 packages/core/src/extensions/ExtensionManager.spec.ts create mode 100644 packages/core/src/extensions/helpers/resolveExtensionsOnEditor.spec.ts create mode 100644 packages/core/src/extensions/helpers/resolveExtensionsOnEditor.ts diff --git a/packages/core/src/editor/__tests__/Editor.spec.ts b/packages/core/src/editor/Editor.spec.ts similarity index 96% rename from packages/core/src/editor/__tests__/Editor.spec.ts rename to packages/core/src/editor/Editor.spec.ts index 2d653a9..96db589 100644 --- a/packages/core/src/editor/__tests__/Editor.spec.ts +++ b/packages/core/src/editor/Editor.spec.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it } from "vite-plus/test"; -import { Editor } from "../Editor.ts"; +import { Editor } from "./Editor.ts"; import { EditorState } from "@codemirror/state"; import { EditorView } from "@codemirror/view"; diff --git a/packages/core/src/editor/__tests__/EventEmitter.spec.ts b/packages/core/src/editor/EventEmitter.spec.ts similarity index 96% rename from packages/core/src/editor/__tests__/EventEmitter.spec.ts rename to packages/core/src/editor/EventEmitter.spec.ts index 92adfd6..77e5aaf 100644 --- a/packages/core/src/editor/__tests__/EventEmitter.spec.ts +++ b/packages/core/src/editor/EventEmitter.spec.ts @@ -1,5 +1,5 @@ import { describe, it, vi, expect } from "vite-plus/test"; -import { EventEmitter } from "../EventEmitter.ts"; +import { EventEmitter } from "./EventEmitter.ts"; describe("EventEmitter", () => { it("creates listeners and triggers callbacks", () => { diff --git a/packages/core/src/extensions/ExtensionManager.spec.ts b/packages/core/src/extensions/ExtensionManager.spec.ts new file mode 100644 index 0000000..1181e2c --- /dev/null +++ b/packages/core/src/extensions/ExtensionManager.spec.ts @@ -0,0 +1,72 @@ +import { EditorState, StateField } from "@codemirror/state"; +import { keymap } from "@codemirror/view"; +import { describe, expect, it } from "vite-plus/test"; +import type { Editor } from "../editor/Editor.ts"; +import { ExtensionManager } from "./ExtensionManager.ts"; +import type { InkwellExtension } from "./types.ts"; + +describe("ExtensionManager", () => { + const editor = {} as Editor; + + it("exposes resolved extensions", () => { + const child: InkwellExtension = { name: "child" }; + const parent: InkwellExtension = { + name: "parent", + addExtensions: () => [child], + }; + + const manager = new ExtensionManager(editor, [parent]); + + expect(manager.resolvedExtensions).toEqual([parent, child]); + }); + + it("creates working CodeMirror extensions in priority order", () => { + const calls: string[] = []; + const highPriorityField = StateField.define({ + create: () => 10, + update: (value) => value, + }); + const lowPriorityField = StateField.define({ + create: () => 0, + update: (value) => value, + }); + const lowPriorityExtension: InkwellExtension = { + name: "low-priority", + addCodeMirrorExtensions: () => { + calls.push("low-priority"); + return [lowPriorityField]; + }, + }; + const highPriorityExtension: InkwellExtension = { + name: "high-priority", + priority: 10, + addCodeMirrorExtensions: () => { + calls.push("high-priority"); + return [highPriorityField]; + }, + }; + + const manager = new ExtensionManager(editor, [lowPriorityExtension, highPriorityExtension]); + const state = EditorState.create({ extensions: manager.cmExtensions }); + + expect(calls).toEqual(["high-priority", "low-priority"]); + expect(state.field(highPriorityField)).toBe(10); + expect(state.field(lowPriorityField)).toBe(0); + }); + + it("creates a valid CodeMirror keymap from extensions", () => { + const binding = { + key: "Mod-Shift-k", + run: () => true, + }; + const extension: InkwellExtension = { + name: "keybindings", + addKeybinds: () => [binding], + }; + + const manager = new ExtensionManager(editor, [extension]); + const state = EditorState.create({ extensions: manager.cmExtensions }); + + expect(state.facet(keymap)).toContainEqual([binding]); + }); +}); diff --git a/packages/core/src/extensions/ExtensionManager.ts b/packages/core/src/extensions/ExtensionManager.ts index d762795..6632a89 100644 --- a/packages/core/src/extensions/ExtensionManager.ts +++ b/packages/core/src/extensions/ExtensionManager.ts @@ -2,6 +2,7 @@ import type { Extension as CMExtension } from "@codemirror/state"; import { type KeyBinding, keymap } from "@codemirror/view"; import type { Editor } from "../editor/Editor.ts"; import type { InkwellExtension } from "./types.ts"; +import { resolveExtensionsOnEditor } from "./helpers/resolveExtensionsOnEditor.ts"; /** * Manages and resolves extensions so the editor receives one complete CodeMirror setup. @@ -20,7 +21,7 @@ export class ExtensionManager { private _addonCMExtensions: CMExtension[] = []; /** Stores keybindings before they become one CodeMirror keymap. */ - private _keybindings: KeyBinding[] = []; + private _keybindings: CMExtension[] = []; /** * Creates one extension setup lifecycle for the editor and its configured extensions. @@ -50,7 +51,7 @@ export class ExtensionManager { /** Provides CodeMirror extensions ready for the editor state. */ get cmExtensions(): CMExtension[] { - return [keymap.of(this._keybindings), ...this._addonCMExtensions]; + return [...this._keybindings, ...this._addonCMExtensions]; } /** Provides root and child extensions that can contribute to editor setup. */ @@ -58,42 +59,13 @@ export class ExtensionManager { return [...this._resolvedExtensions]; } - /** Provides one CodeMirror keymap containing all configured keybindings. */ - get keybindings(): CMExtension { - return keymap.of(this._keybindings); - } - /** * Expands child extensions so their contributions are included in editor setup. * * @returns The complete extension list, including child extensions. */ resolveExtensions(): InkwellExtension[] { - const resolved: InkwellExtension[] = []; - const visited = new Set(); - const visitedNames = new Set(); - - // TODO: add name-guarding / deduping - const resolve = (ext: InkwellExtension) => { - if (visited.has(ext) || visitedNames.has(ext.name)) return; - - visited.add(ext); - resolved.push(ext); - visitedNames.add(ext.name); - - if (ext.addExtensions) { - const childExtensions = ext.addExtensions({ editor: this.editor }); - for (const childExtension of childExtensions) { - resolve(childExtension); - } - } - }; - - for (const ext of this._extensions) { - resolve(ext); - } - - return resolved; + return resolveExtensionsOnEditor(this.editor, this._extensions); } /** @@ -142,7 +114,7 @@ export class ExtensionManager { const keyBinds = addKeybinds({ editor: this.editor })?.filter((kb): kb is KeyBinding => kb !== undefined) ?? []; - this._keybindings.push(...keyBinds); + this._keybindings.push(keymap.of(keyBinds)); } /** diff --git a/packages/core/src/extensions/helpers/resolveExtensionsOnEditor.spec.ts b/packages/core/src/extensions/helpers/resolveExtensionsOnEditor.spec.ts new file mode 100644 index 0000000..5451808 --- /dev/null +++ b/packages/core/src/extensions/helpers/resolveExtensionsOnEditor.spec.ts @@ -0,0 +1,111 @@ +import { describe, expect, it, vi } from "vite-plus/test"; +import type { Editor } from "../../editor/Editor.ts"; +import type { InkwellExtension } from "../types.ts"; +import { resolveExtensionsOnEditor } from "./resolveExtensionsOnEditor.ts"; + +describe("resolveExtensionsOnEditor", () => { + const editor = {} as Editor; + + it("returns an empty list when no extensions are configured", () => { + expect(resolveExtensionsOnEditor(editor, [])).toEqual([]); + }); + + it("keeps root extensions in their configured order", () => { + const first: InkwellExtension = { name: "first" }; + const second: InkwellExtension = { name: "second" }; + + expect(resolveExtensionsOnEditor(editor, [first, second])).toEqual([first, second]); + }); + + it("resolves child extensions into a flat list", () => { + const grandchild: InkwellExtension = { name: "grandchild" }; + const child: InkwellExtension = { + name: "child", + addExtensions: () => [grandchild], + }; + const parent: InkwellExtension = { + name: "parent", + addExtensions: () => [child], + }; + + expect(resolveExtensionsOnEditor(editor, [parent])).toEqual([parent, child, grandchild]); + }); + + it("passes the editor to child extension hooks", () => { + const addExtensions = vi.fn(() => []); + const extension: InkwellExtension = { name: "test", addExtensions }; + + resolveExtensionsOnEditor(editor, [extension]); + + expect(addExtensions).toHaveBeenCalledOnce(); + expect(addExtensions).toHaveBeenCalledWith({ editor }); + }); + + it("resolves the same extension object only once", () => { + const shared: InkwellExtension = { name: "shared" }; + const first: InkwellExtension = { + name: "first", + addExtensions: () => [shared], + }; + const second: InkwellExtension = { + name: "second", + addExtensions: () => [shared], + }; + + expect(resolveExtensionsOnEditor(editor, [first, second])).toEqual([first, shared, second]); + }); + + it("throws when two different extensions have the same name", () => { + const first: InkwellExtension = { name: "duplicate" }; + const second: InkwellExtension = { name: "duplicate" }; + + expect(() => resolveExtensionsOnEditor(editor, [first, second])).toThrow( + "Duplicate extension name detected: duplicate", + ); + }); + + it("throws when child extensions create a cycle", () => { + const first: InkwellExtension = { + name: "first", + addExtensions: () => [second], + }; + const second: InkwellExtension = { + name: "second", + addExtensions: () => [first], + }; + + expect(() => resolveExtensionsOnEditor(editor, [first])).toThrow( + "Cycle detected in extension graph involving extension: first", + ); + }); + + it("resolves complicated extension graphs correctly", () => { + const grandchild1: InkwellExtension = { name: "grandchild1" }; + const grandchild2: InkwellExtension = { name: "grandchild2" }; + const child1: InkwellExtension = { + name: "child1", + addExtensions: () => [grandchild1], + }; + const child2: InkwellExtension = { + name: "child2", + addExtensions: () => [grandchild2], + }; + const parent1: InkwellExtension = { + name: "parent1", + addExtensions: () => [child1, child2, grandchild2], // grandchild2 is intentionally added here to test deduplication + }; + const parent2: InkwellExtension = { + name: "parent2", + addExtensions: () => [child1], + }; + + expect(resolveExtensionsOnEditor(editor, [parent1, parent2])).toEqual([ + parent1, + child1, + grandchild1, + child2, + grandchild2, + parent2, + ]); + }); +}); diff --git a/packages/core/src/extensions/helpers/resolveExtensionsOnEditor.ts b/packages/core/src/extensions/helpers/resolveExtensionsOnEditor.ts new file mode 100644 index 0000000..6a292c6 --- /dev/null +++ b/packages/core/src/extensions/helpers/resolveExtensionsOnEditor.ts @@ -0,0 +1,55 @@ +import type { Editor } from "../../editor/Editor.ts"; +import type { InkwellExtension } from "../types.ts"; + +/** + * Resolves the extensions by resolving child extensions and returning a flat list of all extensions + deduplicating by name. + * + * @param editor The editor instance. + * @param extensions The extensions to resolve. + * @returns The resolved extensions. + */ +export function resolveExtensionsOnEditor( + editor: Editor, + extensions: InkwellExtension[], +): InkwellExtension[] { + const resolved: InkwellExtension[] = []; + const resolvedChildren = new Set(); + const visitedChildren = new Set(); + const visited = new Set(); + const visitedNames = new Set(); + + // TODO: add name-guarding / deduping + const resolve = (ext: InkwellExtension) => { + if (resolvedChildren.has(ext)) { + return; + } + + if (visitedChildren.has(ext)) { + throw new Error(`Cycle detected in extension graph involving extension: ${ext.name}`); + } + + if (visited.has(ext) || visitedNames.has(ext.name)) { + throw new Error(`Duplicate extension name detected: ${ext.name}`); + } + + visited.add(ext); + resolved.push(ext); + visitedNames.add(ext.name); + + if (ext.addExtensions) { + visitedChildren.add(ext); + const childExtensions = ext.addExtensions({ editor }); + for (const childExtension of childExtensions) { + resolve(childExtension); + resolvedChildren.add(childExtension); + } + visitedChildren.delete(ext); + } + }; + + for (const ext of extensions) { + resolve(ext); + } + + return resolved; +} -- 2.51.2