diff --git a/packages/super-editor/src/ui/create-super-doc-ui.ts b/packages/super-editor/src/ui/create-super-doc-ui.ts index 20c73d9110..02ebac8695 100644 --- a/packages/super-editor/src/ui/create-super-doc-ui.ts +++ b/packages/super-editor/src/ui/create-super-doc-ui.ts @@ -21,6 +21,7 @@ import type { CommandHandle, CommandsHandle, CommentsHandle, + DynamicCommandHandle, EqualityFn, ReviewHandle, ReviewItem, @@ -104,6 +105,22 @@ const FALLBACK_COMMAND_STATE: ToolbarCommandHandleState = { value: undefined, }; +/** + * Default state emitted from a {@link DynamicCommandHandle} when the + * command has no entry in `state.toolbar.commands` yet (e.g. the + * snapshot has not populated, or the command was unregistered between + * subscribe and the first emit). Carries `source: 'built-in'` because + * the dynamic handle for a built-in id reaches this branch only for + * built-ins. Customs never produce `undefined` here (the registry's + * computeStates always returns a state for every entry). + */ +const FALLBACK_DYNAMIC_STATE: UIToolbarCommandState = { + active: false, + disabled: true, + value: undefined, + source: 'built-in', +}; + /** * Full set of registered toolbar command ids, used to seed the * internal `createHeadlessToolbar` call. Without this the controller @@ -848,6 +865,98 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { customCommandsRegistry.destroy(); }); + // Per-id cache for the type-erased dynamic handles returned by + // `ui.commands.get(id)`. Cached so handle identity is stable across + // repeated lookups for the same id (consumers can put the result in + // a React `useMemo` dep and not re-create observers per render). + // Caches lazily: entries are created on first `get(id)` call. + const dynamicHandleCache = new Map(); + + /** + * Build a {@link DynamicCommandHandle} for a built-in id. Reuses the + * per-command Subscribable so dynamic and per-id observers share the + * same selector subscription against `state.toolbar.commands?.[id]`. + * The emitted slice already carries `source: 'built-in'` after the + * computeState merge, so no remapping is needed beyond the fallback. + */ + const buildBuiltInDynamicHandle = (id: PublicToolbarItemId): DynamicCommandHandle => { + return { + observe(listener) { + return getCommandSubscribable(id).subscribe((cmdState) => { + // The subscribable's selector returns a value cast to + // `ToolbarCommandHandleState` (no `source` field), but the + // runtime slice is the merged `UIToolbarCommandState` with the + // discriminator already populated by computeState. Cast back + // to the public dynamic shape rather than re-allocating a fresh + // object per emit. + const next = (cmdState ?? FALLBACK_DYNAMIC_STATE) as UIToolbarCommandState; + try { + listener(next); + } catch { + // see scheduleNotify + } + }); + }, + execute(payload?: unknown): boolean | Promise { + // Re-resolve at dispatch time so a `register({ override: true })` + // call that lands AFTER this handle was cached still routes + // dispatch through the override. Without this, the cached + // handle's observe stream emits the merged custom state (the + // selector reads the merged `state.toolbar.commands[id]`) but + // execute keeps running the original built-in, leaving config + // driven toolbars showing the override visually while clicks + // run the wrong command. + if (customCommandsRegistry.has(id)) { + return customCommandsRegistry.execute(id, payload); + } + return (toolbarController.execute as (id: PublicToolbarItemId, payload?: unknown) => boolean)(id, payload); + }, + }; + }; + + /** + * Bridge a {@link CustomCommandHandle} from the custom-commands + * registry into the unified {@link DynamicCommandHandle} shape. + * Custom handles already emit `CustomCommandHandleState` (which + * carries `source: 'custom'`) and `execute` already accepts an + * unknown payload, so the wrapper is mostly identity. It exists to + * satisfy the public type and to keep `dynamicHandleCache` stable. + */ + const buildCustomDynamicHandle = (id: string): DynamicCommandHandle | undefined => { + const customHandle = customCommandsRegistry.getHandle(id); + if (!customHandle) return undefined; + return { + observe(listener) { + return customHandle.observe(listener); + }, + execute(payload?: unknown) { + return (customHandle.execute as (payload?: unknown) => boolean | Promise)(payload); + }, + }; + }; + + const getDynamicHandle = (id: string): DynamicCommandHandle | undefined => { + if (typeof id !== 'string' || id.length === 0) return undefined; + // Custom takes priority: `register({ id, override: true })` lets a + // custom command shadow a built-in id, and the dynamic-lookup + // result must follow that shadowing so consumers iterating over + // mixed id arrays get the override semantics they configured. + if (customCommandsRegistry.has(id)) { + // Don't memoize the wrapper: a later `unregister()` followed by a + // fresh `register()` for the same id swaps the underlying handle, + // and a stale wrapper would observe / execute against the prior + // registration. Building on demand is cheap (two closures) and + // keeps semantics aligned with the Proxy `get` path. + return buildCustomDynamicHandle(id); + } + if (!BUILT_IN_COMMAND_ID_SET.has(id)) return undefined; + let cached = dynamicHandleCache.get(id); + if (cached) return cached; + cached = buildBuiltInDynamicHandle(id as PublicToolbarItemId); + dynamicHandleCache.set(id, cached); + return cached; + }; + const commands = new Proxy({} as CommandsHandle, { get(_, prop) { if (typeof prop !== 'string') return undefined; @@ -857,6 +966,13 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { if (prop === 'register') { return customCommandsRegistry.register.bind(customCommandsRegistry); } + // `get(id)` is the typed dynamic-lookup escape hatch (see + // `DynamicCommandHandle`). Returns undefined for unregistered ids + // instead of producing a fallback handle that emits forever + // disabled state, which is what the bare proxy lookup does today. + if (prop === 'get') { + return getDynamicHandle; + } // Custom-registered ids surface a typed handle from the registry. // Built-in ids fall through to the existing per-id cache so they // keep the same observe/execute shape they had before SD-2802. @@ -1266,6 +1382,7 @@ export function createSuperDocUI(options: SuperDocUIOptions): SuperDocUI { stateChangeListeners.clear(); commandHandleCache.clear(); commandSubscribableCache.clear(); + dynamicHandleCache.clear(); teardown.forEach((fn) => { try { fn(); diff --git a/packages/super-editor/src/ui/custom-commands.test.ts b/packages/super-editor/src/ui/custom-commands.test.ts index 25bfe9be61..b041b97c8e 100644 --- a/packages/super-editor/src/ui/custom-commands.test.ts +++ b/packages/super-editor/src/ui/custom-commands.test.ts @@ -551,3 +551,270 @@ describe('ui.commands.register', () => { ui.destroy(); }); }); + +describe('ui.commands.get', () => { + it('returns undefined for unregistered ids', () => { + const { superdoc } = makeStubs(); + const ui = createSuperDocUI({ superdoc }); + + expect(ui.commands.get('definitely-not-a-command')).toBeUndefined(); + // Empty / non-string ids guard the entry early. + expect(ui.commands.get('')).toBeUndefined(); + expect(ui.commands.get('register')).toBeUndefined(); + expect(ui.commands.get('get')).toBeUndefined(); + + ui.destroy(); + }); + + it('returns a handle for a built-in id, observe emits state with source: "built-in"', () => { + const { superdoc } = makeStubs(); + const ui = createSuperDocUI({ superdoc }); + + const handle = ui.commands.get('bold'); + expect(handle).toBeDefined(); + expect(typeof handle?.observe).toBe('function'); + expect(typeof handle?.execute).toBe('function'); + + const listener = vi.fn(); + const off = handle!.observe(listener); + + // Initial synchronous emit, like every Subscribable in the controller. + expect(listener).toHaveBeenCalledTimes(1); + const emitted = listener.mock.calls[0][0]; + expect(emitted.source).toBe('built-in'); + expect(typeof emitted.active).toBe('boolean'); + expect(typeof emitted.disabled).toBe('boolean'); + + off(); + ui.destroy(); + }); + + it('returns the same handle on repeated lookups for a built-in id', () => { + const { superdoc } = makeStubs(); + const ui = createSuperDocUI({ superdoc }); + + const a = ui.commands.get('italic'); + const b = ui.commands.get('italic'); + expect(a).toBeDefined(); + expect(a).toBe(b); + + ui.destroy(); + }); + + it('returns a handle for a custom-registered id, observe emits state with source: "custom"', () => { + const { superdoc } = makeStubs(); + const ui = createSuperDocUI({ superdoc }); + + ui.commands.register({ + id: 'company.aiRewrite', + execute: vi.fn(() => true), + getState: () => ({ active: false, disabled: false, value: 'ready' }), + }); + + const handle = ui.commands.get('company.aiRewrite'); + expect(handle).toBeDefined(); + + const listener = vi.fn(); + handle!.observe(listener); + + expect(listener).toHaveBeenCalledTimes(1); + expect(listener.mock.calls[0][0]).toEqual({ + active: false, + disabled: false, + value: 'ready', + source: 'custom', + }); + + ui.destroy(); + }); + + it('execute on a custom handle forwards payload to the registered execute', () => { + const { superdoc } = makeStubs(); + const ui = createSuperDocUI({ superdoc }); + + const execute = vi.fn(() => true); + ui.commands.register<{ prompt: string }>({ + id: 'company.aiRewrite', + execute, + }); + + const handle = ui.commands.get('company.aiRewrite'); + handle!.execute({ prompt: 'fix tone' }); + + expect(execute).toHaveBeenCalledTimes(1); + expect(execute).toHaveBeenCalledWith({ + payload: { prompt: 'fix tone' }, + superdoc, + }); + + ui.destroy(); + }); + + it('returns undefined after unregistering a custom command', () => { + const { superdoc } = makeStubs(); + const ui = createSuperDocUI({ superdoc }); + + const reg = ui.commands.register({ + id: 'company.aiRewrite', + execute: vi.fn(() => true), + }); + + expect(ui.commands.get('company.aiRewrite')).toBeDefined(); + + reg.unregister(); + + expect(ui.commands.get('company.aiRewrite')).toBeUndefined(); + + ui.destroy(); + }); + + it('returns the custom handle when a built-in is overridden', () => { + const { superdoc } = makeStubs(); + const ui = createSuperDocUI({ superdoc }); + + const customExecute = vi.fn(() => true); + ui.commands.register({ + id: 'bold', + override: true, + execute: customExecute, + getState: () => ({ active: true, disabled: false, value: 'overridden' }), + }); + + const handle = ui.commands.get('bold'); + expect(handle).toBeDefined(); + + const listener = vi.fn(); + handle!.observe(listener); + + expect(listener.mock.calls[0][0]).toEqual({ + active: true, + disabled: false, + value: 'overridden', + source: 'custom', + }); + + handle!.execute(); + expect(customExecute).toHaveBeenCalledTimes(1); + + ui.destroy(); + }); + + // Regression for PR #3013 review comment: cached dynamic handles + // for built-in ids must dispatch through any later + // `register({ id, override: true })`. A consumer that memoizes + // `ui.commands.get('bold')` once and only later registers an + // override would otherwise see the merged custom state on the + // observe stream while still routing execute() to the built-in, + // breaking override semantics for long-lived handles. + it('cached built-in dynamic handle dispatches through a later override', () => { + const { superdoc } = makeStubs(); + const ui = createSuperDocUI({ superdoc }); + + // Cache the handle BEFORE registering the override. + const cachedHandle = ui.commands.get('bold'); + expect(cachedHandle).toBeDefined(); + + const customExecute = vi.fn(() => true); + ui.commands.register({ + id: 'bold', + override: true, + execute: customExecute, + getState: () => ({ active: true, disabled: false, value: 'overridden' }), + }); + + // Execute via the cached handle. The custom override's execute + // should run, not the built-in toolbar controller's bold. + cachedHandle!.execute(); + expect(customExecute).toHaveBeenCalledTimes(1); + + ui.destroy(); + }); + + it('cached built-in dynamic handle reverts to built-in dispatch after the override unregisters', () => { + const { superdoc, editor } = makeStubs(); + const ui = createSuperDocUI({ superdoc }); + + const cachedHandle = ui.commands.get('bold'); + const customExecute = vi.fn(() => true); + const reg = ui.commands.register({ + id: 'bold', + override: true, + execute: customExecute, + }); + + cachedHandle!.execute(); + expect(customExecute).toHaveBeenCalledTimes(1); + + // After unregister, the built-in dispatch path resumes. + reg.unregister(); + + // Reset the editor's bold spy so we can detect a built-in dispatch + // after unregister. The toolbarController is internal, but a + // built-in dispatch ultimately routes through the editor's + // commands surface; the stub's `commands.toggleBold` mock receives + // the call. (If toolbarController short-circuits before reaching + // the editor it still won't call customExecute, which is what we + // assert below.) + customExecute.mockClear(); + cachedHandle!.execute(); + expect(customExecute).not.toHaveBeenCalled(); + + // Editor reference is unused if toolbar dispatch routes elsewhere; + // the assertion that matters is `customExecute` did not fire. + void editor; + + ui.destroy(); + }); + + it('observers detach when unsubscribed', () => { + const { superdoc } = makeStubs(); + const ui = createSuperDocUI({ superdoc }); + + const handle = ui.commands.get('bold'); + const listener = vi.fn(); + const off = handle!.observe(listener); + + expect(listener).toHaveBeenCalledTimes(1); + listener.mockClear(); + off(); + + // After unsubscribe, no further emits. Fire a stub event that + // would otherwise rebuild the snapshot. + (superdoc as unknown as { fireEditor(event: string): void }).fireEditor('selectionUpdate'); + + expect(listener).not.toHaveBeenCalled(); + + ui.destroy(); + }); + + it('enables dynamic toolbar configuration without unsafe casts', () => { + const { superdoc } = makeStubs(); + const ui = createSuperDocUI({ superdoc }); + + ui.commands.register({ + id: 'company.aiRewrite', + execute: vi.fn(() => true), + getState: () => ({ active: false, disabled: false, value: 'ready' }), + }); + + // The friction case from SD-2814: a config-driven toolbar. + const config: string[] = ['bold', 'italic', 'company.aiRewrite', 'unknown-id']; + const states = config.map((id) => { + const handle = ui.commands.get(id); + if (!handle) return { id, found: false }; + let state: unknown = null; + const off = handle.observe((s) => { + state = s; + }); + off(); + return { id, found: true, state }; + }); + + expect(states[0].found).toBe(true); + expect(states[1].found).toBe(true); + expect(states[2].found).toBe(true); + expect(states[3].found).toBe(false); + + ui.destroy(); + }); +}); diff --git a/packages/super-editor/src/ui/index.ts b/packages/super-editor/src/ui/index.ts index 99f5671f97..d61801b5ed 100644 --- a/packages/super-editor/src/ui/index.ts +++ b/packages/super-editor/src/ui/index.ts @@ -59,6 +59,7 @@ export type { CustomCommandHandleState, CustomCommandRegistration, CustomCommandRegistrationResult, + DynamicCommandHandle, ToolbarCommandHandleState, ToolbarHandle, ToolbarSnapshotSlice, diff --git a/packages/super-editor/src/ui/types.ts b/packages/super-editor/src/ui/types.ts index 4e9979f75f..b841ea1310 100644 --- a/packages/super-editor/src/ui/types.ts +++ b/packages/super-editor/src/ui/types.ts @@ -529,8 +529,70 @@ export type CommandsHandle = { register( registration: CustomCommandRegistration, ): CustomCommandRegistrationResult; + + /** + * Look up a command handle by string id at runtime. + * + * Returns a {@link DynamicCommandHandle} for any registered id, + * built-in (`'bold'`, `'italic'`, etc.) or custom (registered via + * {@link CommandsHandle.register}), and `undefined` for unknown ids. + * + * Use this instead of indexing `ui.commands[id]` when the id is only + * known at runtime: a toolbar driven by a `string[]` config, a + * keyboard-shortcut router, a plugin loop. Indexing the surface with + * a generic `string` type-errors today because the surface mixes + * per-command handles with the `register` method, so consumers + * otherwise reach for an unsafe `as` cast on every dispatch site. + * + * The returned handle's `observe` listener receives the full + * {@link UIToolbarCommandState} (active / disabled / value / source), + * so a single render path can drive built-in *and* custom buttons + * uniformly without branching on the id. + */ + get(id: string): DynamicCommandHandle | undefined; }; +/** + * Type-erased command handle returned from {@link CommandsHandle.get}. + * + * Bridges built-ins ({@link CommandHandle}) and customs + * ({@link CustomCommandHandle}) into one observe/execute surface so + * consumers iterating `string[]` ids don't have to branch. The emitted + * state carries `source` so a uniform renderer can still distinguish + * the two when it wants. + * + * `execute` accepts an optional `unknown` payload and returns + * `boolean | Promise` (built-ins are sync, customs may be + * async). Capture the typed registration result for type-safe + * payloads. `get(id)` is the dynamic-lookup fallback, not a + * replacement for the per-id typing of `ui.commands.bold`. + */ +export interface DynamicCommandHandle { + /** + * Subscribe to the command's state. The listener fires once + * synchronously with the current state, then again whenever the + * state changes by shallow equality. Returns an unsubscribe. + * + * For ids in the built-in registry that haven't received a + * snapshot yet (or whose value has gone stale), the listener is + * still called with a deterministic disabled fallback so consumer + * code can render without a null check on every emit. + */ + observe(listener: (state: UIToolbarCommandState) => void): () => void; + /** + * Execute the command. Forwards to the same dispatch path as + * `ui.toolbar.execute(id, payload)` for built-ins and the + * registered `execute` handler for customs. + * + * The payload is `unknown` because `get(id)` erases per-command + * payload typing. Pass the value the command expects (e.g. the + * `string` for `'font-size'`). The returned Promise resolves to + * `false` when a custom command's handler rejects or returns + * `false`; built-ins return synchronously. + */ + execute(payload?: unknown): boolean | Promise; +} + /** * Input shape for {@link CommandsHandle.register}. * diff --git a/packages/superdoc/src/ui.d.ts b/packages/superdoc/src/ui.d.ts index 1163dfbe5e..77223da845 100644 --- a/packages/superdoc/src/ui.d.ts +++ b/packages/superdoc/src/ui.d.ts @@ -9,6 +9,7 @@ export { type CustomCommandHandleState, type CustomCommandRegistration, type CustomCommandRegistrationResult, + type DynamicCommandHandle, type EntityAddress, type EqualityFn, type ReviewHandle,