diff --git a/packages/core/src/editor/managers/ExtensionManager/ExtensionManager.browser.test.ts b/packages/core/src/editor/managers/ExtensionManager/ExtensionManager.browser.test.ts deleted file mode 100644 index 9d31e69374..0000000000 --- a/packages/core/src/editor/managers/ExtensionManager/ExtensionManager.browser.test.ts +++ /dev/null @@ -1,45 +0,0 @@ -import { afterEach, expect, it } from "vite-plus/test"; - -import { createExtension } from "../../BlockNoteExtension.js"; -import { BlockNoteEditor } from "../../BlockNoteEditor.js"; - -const cleanups: Array<() => void> = []; -afterEach(() => { - for (const cleanup of cleanups.splice(0).reverse()) { - cleanup(); - } -}); - -it("mounts runtime extensions and cleans them up when removed", () => { - let mounts = 0; - let unmounts = 0; - const extension = createExtension(() => ({ - key: "runtime-lifecycle", - mount({ signal }: { signal: AbortSignal }) { - expect(signal.aborted).toBe(false); - mounts++; - return () => { - unmounts++; - }; - }, - })); - const editor = BlockNoteEditor.create(); - const host = document.createElement("div"); - document.body.append(host); - editor.mount(host); - cleanups.push(() => { - editor._tiptapEditor.destroy(); - host.remove(); - }); - editor.registerExtension(extension()); - expect(mounts).toBe(1); - const instance = editor.getExtension(extension)!; - editor.unregisterExtension(extension); - expect(unmounts).toBe(1); - editor.registerExtension(instance); - expect(mounts).toBe(2); - editor.unmount(); - expect(unmounts).toBe(2); - editor.mount(host); - expect(mounts).toBe(3); -}); diff --git a/packages/core/src/editor/managers/ExtensionManager/ExtensionManager.node.test.ts b/packages/core/src/editor/managers/ExtensionManager/ExtensionManager.node.test.ts index a225205177..5fc5ac188a 100644 --- a/packages/core/src/editor/managers/ExtensionManager/ExtensionManager.node.test.ts +++ b/packages/core/src/editor/managers/ExtensionManager/ExtensionManager.node.test.ts @@ -30,6 +30,143 @@ function remountEditor(editor: Pick) { } describe("runtime extension plugin source", () => { + function createEditor( + options?: Parameters[0], + ) { + const editor = BlockNoteEditor.create(options); + editors.push(editor); + rebuildPlugins(editor); + return editor; + } + + it("installs nested plugins in dependency order and retains that order on rebuild", () => { + const syncKey = new PluginKey("sync"); + const cursorKey = new PluginKey("cursor"); + const sync = new Plugin({ + key: syncKey, + state: { init: () => 42, apply: (_tr, value) => value }, + }); + const cursor = new Plugin({ + key: cursorKey, + state: { + init: (_config, state) => syncKey.getState(state), + apply: (_tr, value) => value, + }, + }); + const parent = new Plugin({ key: new PluginKey("parent") }); + const editor = createEditor(); + editor.registerExtension({ + key: "parent", + prosemirrorPlugins: [parent], + blockNoteExtensions: [ + () => ({ key: "cursor", prosemirrorPlugins: [cursor] }), + () => ({ + key: "group", + blockNoteExtensions: [ + () => ({ + key: "sync", + runsBefore: ["cursor"], + prosemirrorPlugins: [sync], + }), + ], + }), + ], + }); + expect(cursorKey.getState(editor.prosemirrorState)).toBe(42); + const expected = [sync, cursor, parent]; + expect( + editor.prosemirrorState.plugins.filter((p) => expected.includes(p)), + ).toEqual(expected); + rebuildPlugins(editor); + expect( + editor.prosemirrorState.plugins.filter((p) => expected.includes(p)), + ).toEqual(expected); + }); + + it("exposes earlier registrations to factories and plugin initializers", () => { + const editor = createEditor(); + const dependency = { key: "dependency", value: 42 }; + const key = new PluginKey("lookup"); + editor.registerExtension([ + dependency, + ({ editor }) => { + expect(editor.getExtension(dependency.key)).toBe(dependency); + return { + key: "consumer", + prosemirrorPlugins: [ + new Plugin({ + key, + state: { + init() { + return editor.getExtension(dependency.key) + ?.value; + }, + apply: (_tr, value) => value, + }, + }), + ], + }; + }, + ]); + expect(key.getState(editor.prosemirrorState)).toBe(42); + }); + + it("does not silently remove an unrelated plugin with the same key", () => { + const key = new PluginKey("collision"); + const original = new Plugin({ key }); + const editor = createEditor({ + extensions: [() => ({ key: "original", prosemirrorPlugins: [original] })], + }); + expect(() => + editor.registerExtension({ + key: "collision", + prosemirrorPlugins: [new Plugin({ key })], + }), + ).toThrow(/keyed plugin/); + expect(key.get(editor.prosemirrorState)).toBe(original); + }); + + it("retains registrations made inside another factory", () => { + const editor = createEditor(); + const plugin = new Plugin({}); + const inner = { key: "inner", prosemirrorPlugins: [plugin] }; + editor.registerExtension(({ editor }) => { + editor.registerExtension(inner); + return { key: "outer" }; + }); + expect(editor.getExtension(inner.key)).toBe(inner); + expect(editor.prosemirrorState.plugins).toContain(plugin); + rebuildPlugins(editor); + expect(editor.prosemirrorState.plugins).toContain(plugin); + }); + + it("removes obsolete parent dependency edges before validating a replacement", () => { + const childPlugin = new Plugin({}); + const parentPlugin = new Plugin({}); + const parent = { + key: "former-parent", + blockNoteExtensions: [ + () => ({ key: "former-child", prosemirrorPlugins: [childPlugin] }), + ], + }; + const editor = createEditor(); + editor.registerExtension(parent); + editor.replaceExtension(parent, { + key: parent.key, + runsBefore: ["former-child"], + prosemirrorPlugins: [parentPlugin], + }); + // The old child-before-parent edge would create a false cycle. + const expected = [parentPlugin, childPlugin]; + expect( + editor.prosemirrorState.plugins.filter((p) => expected.includes(p)), + ).toEqual(expected); + rebuildPlugins(editor); + expect( + editor.prosemirrorState.plugins.filter((p) => expected.includes(p)), + ).toEqual(expected); + }); + it("resets selected plugin state without resetting retained plugins", () => { const key = new PluginKey("reset"); const retainedKey = new PluginKey("retained"); diff --git a/packages/core/src/editor/managers/ExtensionManager/ExtensionManager.test.ts b/packages/core/src/editor/managers/ExtensionManager/ExtensionManager.test.ts index 5e0edadae5..9cc5b40e8c 100644 --- a/packages/core/src/editor/managers/ExtensionManager/ExtensionManager.test.ts +++ b/packages/core/src/editor/managers/ExtensionManager/ExtensionManager.test.ts @@ -2,7 +2,7 @@ * @vitest-environment jsdom */ import { Plugin, PluginKey } from "prosemirror-state"; -import { describe, expect, it, vi } from "vite-plus/test"; +import { afterEach, describe, expect, it, vi } from "vite-plus/test"; import { createExtension } from "../../BlockNoteExtension.js"; import { BlockNoteEditor } from "../../BlockNoteEditor.js"; @@ -15,6 +15,130 @@ function createMountedEditor( return editor; } +describe("runtime extension lifecycle", () => { + const cleanups: Array<() => void> = []; + afterEach(() => { + for (const cleanup of cleanups.splice(0).reverse()) { + cleanup(); + } + }); + + function createEditor() { + const editor = BlockNoteEditor.create(); + const host = document.createElement("div"); + document.body.append(host); + cleanups.push(() => { + editor.unmount(); + editor._tiptapEditor.destroy(); + host.remove(); + }); + editor.mount(host); + return { editor, host }; + } + + it("mounts nested plugin-less extensions once and cleans them up", () => { + const { editor, host } = createEditor(); + const parentCleanup = vi.fn(); + const childCleanup = vi.fn(); + const parentMount = vi.fn(() => parentCleanup); + const childMount = vi.fn(() => childCleanup); + const child = { key: "child", mount: childMount }; + const parent = { + key: "parent", + mount: parentMount, + blockNoteExtensions: [() => child], + }; + editor.registerExtension(parent); + editor.registerExtension(parent); + expect(parentMount).toHaveBeenCalledTimes(1); + expect(childMount).toHaveBeenCalledTimes(1); + editor.unregisterExtension([parent, child]); + expect(parentCleanup).toHaveBeenCalledTimes(1); + expect(childCleanup).toHaveBeenCalledTimes(1); + editor.registerExtension(parent); + editor.unmount(); + editor.mount(host); + expect(parentMount).toHaveBeenCalledTimes(3); + expect(childMount).toHaveBeenCalledTimes(3); + }); + + it("allows cleanup to re-register the same instance after plugin removal", () => { + const { editor, host } = createEditor(); + const key = new PluginKey("cleanup-identical"); + const plugin = new Plugin({ key }); + let resurrect = true; + let registrationError: unknown; + const mount = vi.fn(() => () => { + if (resurrect) { + resurrect = false; + try { + editor.registerExtension(extension); + } catch (error) { + registrationError = error; + } + } + }); + const extension = { + key: "cleanup-identical", + prosemirrorPlugins: [plugin], + mount, + }; + editor.registerExtension(extension); + editor.unregisterExtension(extension); + expect(registrationError).toBeUndefined(); + expect(editor.getExtension(extension.key)).toBe(extension); + expect(mount).toHaveBeenCalledTimes(2); + expect(key.get(editor.prosemirrorState)).toBe(plugin); + editor.unmount(); + editor.mount(host); + expect(key.get(editor.prosemirrorState)).toBe(plugin); + }); + + it("does not mount an extension removed by an earlier mount hook", () => { + const { editor } = createEditor(); + const childMount = vi.fn(); + const child = { key: "removed-child", mount: childMount }; + editor.registerExtension([ + { + key: "removing-parent", + mount() { + editor.unregisterExtension(child); + }, + }, + child, + ]); + expect(editor.getExtension(child.key)).toBeUndefined(); + expect(childMount).not.toHaveBeenCalled(); + }); + + it("mounts runtime extensions and cleans them up when removed", () => { + let mounts = 0; + let unmounts = 0; + const extension = createExtension(() => ({ + key: "runtime-lifecycle", + mount({ signal }: { signal: AbortSignal }) { + expect(signal.aborted).toBe(false); + mounts++; + return () => { + unmounts++; + }; + }, + })); + const { editor, host } = createEditor(); + editor.registerExtension(extension()); + expect(mounts).toBe(1); + const instance = editor.getExtension(extension)!; + editor.unregisterExtension(extension); + expect(unmounts).toBe(1); + editor.registerExtension(instance); + expect(mounts).toBe(2); + editor.unmount(); + expect(unmounts).toBe(2); + editor.mount(host); + expect(mounts).toBe(3); + }); +}); + describe("Extension keyboard shortcuts", () => { it("validates shortcut names at initialization", () => { expect(() => diff --git a/packages/core/src/editor/managers/ExtensionManager/index.ts b/packages/core/src/editor/managers/ExtensionManager/index.ts index 76a8674cc4..cea81bd2c9 100644 --- a/packages/core/src/editor/managers/ExtensionManager/index.ts +++ b/packages/core/src/editor/managers/ExtensionManager/index.ts @@ -37,6 +37,14 @@ export type ExtensionSelector = | undefined; export type ExtensionSelection = ExtensionSelector | ExtensionSelector[]; +function getPluginKey(plugin: Plugin): string | undefined { + // ProseMirror omits this internal property from its public declarations. + // Unlike spec.key, it also covers automatically generated keys. + return "key" in plugin && typeof plugin.key === "string" + ? plugin.key + : undefined; +} + export class ExtensionManager { /** * A set of extension keys which are disabled by the options @@ -120,7 +128,12 @@ export class ExtensionManager { } private mountExtension(extension: Extension): void { - if (!extension.mount || this.abortMap.has(extension)) { + if ( + !extension.mount || + this.editor.headless || + this.abortMap.has(extension) || + !this.extensions.includes(extension) + ) { return; } const controller = new window.AbortController(); @@ -175,7 +188,7 @@ export class ExtensionManager { } if (!instance || this.disabledExtensions.has(instance.key)) { - return undefined as any; + return undefined; } // A sub-extension declared via `blockNoteExtensions` must run before the @@ -198,7 +211,7 @@ export class ExtensionManager { // conflicting when the user (or another extension) registers that same // extension directly. The first registration wins. if (this.extensions.some((e) => e.key === instance.key)) { - return undefined as any; + return undefined; } // Now that we know that the extension is not disabled, we can add it to the extension factories @@ -218,7 +231,7 @@ export class ExtensionManager { } } - return instance as any; + return instance; } /** @@ -244,8 +257,14 @@ export class ExtensionManager { for (const extension of toResolve) { extensions.push(...this.resolveExtensions(extension)); } - } else if (typeof toResolve === "object" && "key" in toResolve) { - extensions.push(toResolve); + } else if ( + toResolve && + typeof toResolve === "object" && + "key" in toResolve + ) { + if (this.extensions.includes(toResolve)) { + extensions.push(toResolve); + } } else if (typeof toResolve === "string") { const instance = this.extensions.find((e) => e.key === toResolve); if (instance) { @@ -276,9 +295,7 @@ export class ExtensionManager { public unregisterExtension( toUnregister: ExtensionSelection, ): Extension | Extension[] | undefined { - const removed = [...new Set(this.resolveExtensions(toUnregister))].filter( - (extension) => this.extensions.includes(extension), - ); + const removed = [...new Set(this.resolveExtensions(toUnregister))]; if (removed.length) { this.replaceExtension(removed, []); } @@ -305,14 +322,22 @@ export class ExtensionManager { | (Extension | ExtensionFactoryInstance)[], options?: { resetPluginStateFor: readonly PluginKey[] }, ): void { - // ---- Remove phase (no updatePlugins call) ---- - const extensionsToRemove = this.resolveExtensions(toUnregister); + const extensionsToRemove = new Set(this.resolveExtensions(toUnregister)); - if (toUnregister && !extensionsToRemove.length) { + if (toUnregister && !extensionsToRemove.size) { // eslint-disable-next-line no-console console.warn(`No extensions found to unregister`, toUnregister); } + this.extensions = this.extensions.filter( + (extension) => !extensionsToRemove.has(extension), + ); + for (const [factory, extension] of this.extensionFactories) { + if (extensionsToRemove.has(extension)) { + this.extensionFactories.delete(factory); + } + } + const controllers: AbortController[] = []; let didWarnUnregister = false; // We collect both plugin references and plugin keys to remove. // Key-based matching is needed because re-entrant dispatches (e.g. from @@ -322,26 +347,32 @@ export class ExtensionManager { const pluginRefsToRemove = new Set(); const pluginKeysToRemove = new Set(); for (const extension of extensionsToRemove) { - this.extensions = this.extensions.filter((e) => e !== extension); - this.extensionFactories.forEach((instance, factory) => { - if (instance === extension) { - this.extensionFactories.delete(factory); - } - }); - this.abortMap.get(extension)?.abort(); - this.abortMap.delete(extension); - - const plugins = this.extensionPlugins.get(extension); - plugins?.forEach((plugin) => { + for (const plugin of this.extensionPlugins.get(extension) ?? []) { pluginRefsToRemove.add(plugin); - const key = (plugin as any).spec?.key; - const keyStr = typeof key === "object" && key ? key.key : key; - if (typeof keyStr === "string") { - pluginKeysToRemove.add(keyStr); + const key = getPluginKey(plugin); + if (key) { + pluginKeysToRemove.add(key); } - }); + } + } + for (const extension of extensionsToRemove) { + // Remove outgoing declarations, but retain incoming declarations from + // parents when a child is replaced with another instance of the same key. + for (const [key, dependents] of this.blockNoteExtensionDependents) { + dependents.delete(extension.key); + if (!dependents.size) { + this.blockNoteExtensionDependents.delete(key); + } + } + this.extensionPlugins.delete(extension); this.runtimeExtensions.delete(extension); + // Detach before callbacks can re-register this same instance. + const controller = this.abortMap.get(extension); + this.abortMap.delete(extension); + if (controller) { + controllers.push(controller); + } if (extension.tiptapExtensions && !didWarnUnregister) { didWarnUnregister = true; @@ -353,17 +384,33 @@ export class ExtensionManager { } } - // ---- Add phase (no updatePlugins call) ---- - const newExtensions = ([] as (Extension | ExtensionFactoryInstance)[]) - .concat(toRegister) - .filter(Boolean) as (Extension | ExtensionFactoryInstance)[]; - - const registeredExtensions = newExtensions - .map((ext) => this.addExtension(ext)) - .filter(Boolean) as Extension[]; - - const pluginsToAdd: Plugin[] = []; + const newExtensions = Array.isArray(toRegister) ? toRegister : [toRegister]; + const extensionsBefore = new Set(this.extensions); + for (const extension of newExtensions) { + this.addExtension(extension); + } + // Recursive registration adds more than the returned parent instance. + const registeredExtensions = this.extensions.filter( + (extension) => !extensionsBefore.has(extension), + ); + // Keep construction-time and unmanaged Tiptap plugins in their existing + // positions. Reinsert existing runtime plugins in dependency order. Include + // registrations made inside factories, but not newly prepared plugins: + // their keys must not silently displace unrelated installed plugins. + for (const extension of this.runtimeExtensions) { + for (const plugin of this.extensionPlugins.get(extension) ?? []) { + pluginRefsToRemove.add(plugin); + const key = getPluginKey(plugin); + if (key) { + pluginKeysToRemove.add(key); + } + } + } for (const extension of registeredExtensions) { + // A factory may already have registered this extension reentrantly. + if (this.runtimeExtensions.has(extension)) { + continue; + } this.runtimeExtensions.add(extension); if (extension?.tiptapExtensions) { // eslint-disable-next-line no-console @@ -381,50 +428,33 @@ export class ExtensionManager { ); } - this.getProsemirrorPluginsFromExtension(extension).plugins.forEach( - (plugin) => { - pluginsToAdd.push(plugin); - }, - ); + this.getProsemirrorPluginsFromExtension(extension); } - // ---- Single atomic plugin update for replacement or an explicit reset ---- - if ( - pluginRefsToRemove.size || - pluginKeysToRemove.size || - pluginsToAdd.length || - options?.resetPluginStateFor.length - ) { - this.updatePlugins( - (plugins) => [ - ...plugins.filter((plugin) => { - // Fast path: exact reference match - if (pluginRefsToRemove.has(plugin)) { - return false; - } - // Fallback: match by key string (handles cases where plugin instances - // in the state differ from the ones we tracked) - if (pluginKeysToRemove.size) { - const key = (plugin as any).spec?.key; - const keyStr = typeof key === "object" && key ? key.key : key; - if ( - typeof keyStr === "string" && - pluginKeysToRemove.has(keyStr) - ) { - return false; - } - } - return true; - }), - ...pluginsToAdd, - ], - options?.resetPluginStateFor, - ); + const currentState = this.editor.prosemirrorState; + const plugins = [ + ...currentState.plugins.filter((plugin) => { + const key = getPluginKey(plugin); + return ( + !pluginRefsToRemove.has(plugin) && + !(key && pluginKeysToRemove.has(key)) + ); + }), + ...this.getRuntimePlugins(), + ]; + const unchanged = + plugins.length === currentState.plugins.length && + plugins.every((plugin, index) => plugin === currentState.plugins[index]); + if (!unchanged || options?.resetPluginStateFor.length) { + this.updatePlugins(() => plugins, options?.resetPluginStateFor); } - if (!this.editor.headless) { - for (const extension of registeredExtensions) { - this.mountExtension(extension); - } + // Cleanup sees the committed registry and complete plugin set. Nothing + // below overwrites changes made by reentrant registrations. + for (const controller of controllers) { + controller.abort(); + } + for (const extension of registeredExtensions) { + this.mountExtension(extension); } } @@ -456,17 +486,8 @@ export class ExtensionManager { this.editor.prosemirrorView.updateState(state); } - /** - * Get all the extensions that are registered to the editor - */ - public getTiptapExtensions(): AnyTiptapExtension[] { - // Start with the default tiptap extensions - const tiptapExtensions = getDefaultTiptapExtensions( - this.editor, - this.options, - ).filter((extension) => !this.disabledExtensions.has(extension.name)); - - const getPriority = sortByDependencies( + private getExtensionPriority() { + return sortByDependencies( this.extensions.map((extension) => { // A sub-extension declared via `blockNoteExtensions` must run before the // extension(s) that declared it, so we merge those parents into its @@ -481,6 +502,30 @@ export class ExtensionManager { }; }), ); + } + + private getRuntimePlugins(): Plugin[] { + const priority = this.getExtensionPriority(); + return ( + this.extensions + .filter((extension) => this.runtimeExtensions.has(extension)) + // Match Tiptap's reverse declaration order for equal priorities. + .reverse() + .sort((a, b) => priority(b.key) - priority(a.key)) + .flatMap((extension) => this.extensionPlugins.get(extension) ?? []) + ); + } + + /** + * Get all the extensions that are registered to the editor + */ + public getTiptapExtensions(): AnyTiptapExtension[] { + // Start with the default tiptap extensions + const tiptapExtensions = getDefaultTiptapExtensions( + this.editor, + this.options, + ).filter((extension) => !this.disabledExtensions.has(extension.name)); + const getPriority = this.getExtensionPriority(); const inputRulesByPriority = new Map(); for (const extension of this.extensions) { @@ -515,16 +560,12 @@ export class ExtensionManager { } } - // Runtime additions have no construction-time Tiptap wrapper. Like - // registerExtension's plugin update, append their already-created plugins. + // Use the same ordered runtime plugin source during updates and remounts. tiptapExtensions.push( TiptapExtension.create({ name: "blocknote-runtime-extensions", priority: Number.NEGATIVE_INFINITY, - addProseMirrorPlugins: () => - this.extensions - .filter((extension) => this.runtimeExtensions.has(extension)) - .flatMap((extension) => this.extensionPlugins.get(extension) ?? []), + addProseMirrorPlugins: () => this.getRuntimePlugins(), }), );