From 69b78bff4acbc63a9783d1e5094f157339b74ea2 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 04:02:21 +0000 Subject: [PATCH] refactor(core): retire ActionEngine's zero-caller event-mapping API (#3368) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements the maintainer ruling of 2026-08-11 on objectui#3368: retire `addMapping` under enforce-or-remove — remove the export and its dead registration path. Removed from `@object-ui/core`'s public surface: `ActionEngine.addMapping()`, `ActionEngine.dispatch()`, the private `mappings` registry they shared, and the exported `ActionMapping` interface. Re-measured at this branch point (f762f5bdf): definition plus four call sites, all four in the engine's own test file, zero production callers. Nothing ever registered a mapping, so `dispatch()` had no reachable caller either. The three recorded contract inconsistencies vs `visible` (truthy entry gate, string-only condition, fail-open evaluation) die with the path rather than being fixed on it. A retirement note at the survivor site records what left and why, per the convention measured on d9d346307 (#4328 / PR #4366). Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 Co-Authored-By: Claude Fable 5 --- ...retire-action-engine-event-mapping-3368.md | 25 ++++++ packages/core/src/actions/ActionEngine.ts | 76 ++++++++----------- .../actions/__tests__/ActionEngine.test.ts | 41 ++-------- 3 files changed, 60 insertions(+), 82 deletions(-) create mode 100644 .changeset/retire-action-engine-event-mapping-3368.md diff --git a/.changeset/retire-action-engine-event-mapping-3368.md b/.changeset/retire-action-engine-event-mapping-3368.md new file mode 100644 index 0000000000..5d859ccbbc --- /dev/null +++ b/.changeset/retire-action-engine-event-mapping-3368.md @@ -0,0 +1,25 @@ +--- +'@object-ui/core': minor +--- + +Retire `ActionEngine`'s event-mapping API (objectui#3368). `ActionEngine.addMapping()`, +`ActionEngine.dispatch()`, the private `mappings` registry behind them, and the exported +`ActionMapping` interface are removed under enforce-or-remove: all four were public surface +of `@object-ui/core` with zero production callers. Nothing in the repo ever registered a +mapping, so `dispatch()` had no reachable caller either, and every call site was in the +engine's own test file. + +Breaking for anyone who typed against or called the removed declarations, marked `minor` +per this repository's version-alignment convention (the major tracks `@objectstack`, never +an API-break count). Actions are still entered by name (`executeAction`), by location +(`getActionsForLocation`), by shortcut (`handleShortcut`) and in bulk (`executeBulk`) — +only the event-keyed entry point is gone, and no runtime behaviour changes because no +runtime path reached it. + +The three ways the retired condition gate had drifted from the `visible` contract that +`getActionsForLocation` implements die with the path rather than being fixed on it: it +entered on a raw truthy check (`condition: false` dispatched anyway), typed `condition` as +`string` only (a `{ dialect: 'cel', source }` envelope could not reach the canonical +`@objectstack/formula` engine), and evaluated without `throwOnError` (a throwing predicate +failed OPEN, the opposite of `visible`'s fail-closed posture). Aligning the contract of an +API nobody calls would only have widened behaviour nobody uses. diff --git a/packages/core/src/actions/ActionEngine.ts b/packages/core/src/actions/ActionEngine.ts index 040acb217f..00d8ef9b14 100644 --- a/packages/core/src/actions/ActionEngine.ts +++ b/packages/core/src/actions/ActionEngine.ts @@ -9,9 +9,8 @@ /** * @object-ui/core - Action Engine * - * Declarative action dispatch engine. Manages action registration, - * event-to-action mapping, location-based filtering, keyboard shortcuts, - * and bulk operation support. + * Declarative action engine. Manages action registration, location-based + * filtering, keyboard shortcuts, and bulk operation support. * * Replaces callback-based patterns with a schema-driven pipeline: * ActionSchema[] → ActionEngine → ActionRunner → ActionResult @@ -44,16 +43,6 @@ export interface RegisteredAction { priority?: number; } -/** Event-to-action mapping */ -export interface ActionMapping { - /** Event name (e.g., 'row:click', 'toolbar:save', 'keyboard:ctrl+s') */ - event: string; - /** Action name to execute */ - actionName: string; - /** Optional condition expression */ - condition?: string; -} - /** Keyboard shortcut handler */ export interface ShortcutBinding { /** Key combination (e.g., 'ctrl+s', 'meta+k', 'shift+n') */ @@ -91,9 +80,35 @@ function warnHiddenPredicate(name: unknown, raw: unknown, err: unknown): void { ); } +/** + * Registration + filtering + execution for declared actions. + * + * Entry points are by NAME (`executeAction`), by LOCATION + * (`getActionsForLocation`), by SHORTCUT (`handleShortcut`) and in BULK + * (`executeBulk`). There is deliberately no entry point by arbitrary EVENT + * name: this class used to carry a second, parallel one — `addMapping()` + * registered `{ event, actionName, condition }` triples into a private + * `mappings` array and `dispatch(event)` ran every triple matching an event + * string. It was retired under enforce-or-remove (objectui#3368) because it + * was a public export of `@object-ui/core` with zero production callers: in + * its whole lifetime nothing ever registered a mapping, so `dispatch()` had + * no reachable caller either, and its four call sites were all in this + * class's own test file. + * + * Its condition gate had drifted three ways from the `visible` contract + * `getActionsForLocation` below implements — it entered on a raw truthy check + * (so `condition: false` dispatched anyway), typed `condition` as `string` + * only (so a `{ dialect: 'cel', source }` envelope could not reach the + * canonical engine), and evaluated without `throwOnError` (so a throwing + * predicate failed OPEN, the opposite of `visible`'s fail-closed posture). + * All three were retired WITH the path rather than fixed on it: aligning the + * contract of an API nobody calls only widens behaviour nobody uses. If an + * event-keyed entry point is ever genuinely needed, it must be built on the + * shared predicate definitions (`hasDeclaredPredicate` + `toPredicateInput`) + * that the surviving gate below already uses, not on a fourth spelling. + */ export class ActionEngine { private actions = new Map(); - private mappings: ActionMapping[] = []; private shortcuts: ShortcutBinding[] = []; private normalizedShortcutMap = new Map(); private runner: ActionRunner; @@ -174,12 +189,6 @@ export class ActionEngine { } this.actions.delete(name); this.shortcuts = this.shortcuts.filter(s => s.actionName !== name); - this.mappings = this.mappings.filter(m => m.actionName !== name); - } - - /** Add an event-to-action mapping */ - addMapping(mapping: ActionMapping): void { - this.mappings.push(mapping); } /** @@ -320,30 +329,6 @@ export class ActionEngine { return this.runner.execute(registered.action); } - /** Dispatch an event — finds mapped actions and executes them */ - async dispatch(event: string, contextOverride?: Partial): Promise { - const matchingMappings = this.mappings.filter(m => m.event === event); - - if (matchingMappings.length === 0) { - return []; - } - - const results: ActionResult[] = []; - for (const mapping of matchingMappings) { - // Check condition if present - if (mapping.condition) { - const evaluator = this.runner.getEvaluator(); - const shouldRun = evaluator.evaluateCondition(mapping.condition); - if (!shouldRun) continue; - } - - const result = await this.executeAction(mapping.actionName, contextOverride); - results.push(result); - } - - return results; - } - /** Handle a keyboard shortcut event — returns true if handled */ async handleShortcut(keys: string, contextOverride?: Partial): Promise { const actionName = this.normalizedShortcutMap.get(normalizeShortcut(keys)); @@ -407,10 +392,9 @@ export class ActionEngine { this.runner.updateContext(context); } - /** Clear all registered actions and mappings */ + /** Clear all registered actions and shortcuts */ clear(): void { this.actions.clear(); - this.mappings = []; this.shortcuts = []; this.normalizedShortcutMap.clear(); } diff --git a/packages/core/src/actions/__tests__/ActionEngine.test.ts b/packages/core/src/actions/__tests__/ActionEngine.test.ts index 18bf699519..73e9bd7558 100644 --- a/packages/core/src/actions/__tests__/ActionEngine.test.ts +++ b/packages/core/src/actions/__tests__/ActionEngine.test.ts @@ -72,12 +72,11 @@ describe('ActionEngine', () => { }); describe('unregisterAction', () => { - it('removes action and its shortcuts/mappings', () => { + it('removes action and its shortcuts', () => { engine.registerAction({ name: 'save', type: 'api' }, { shortcut: 'ctrl+s' }); - engine.addMapping({ event: 'toolbar:save', actionName: 'save' }); - + engine.unregisterAction('save'); - + expect(engine.getAction('save')).toBeUndefined(); expect(engine.getShortcuts()).toHaveLength(0); }); @@ -107,35 +106,6 @@ describe('ActionEngine', () => { }); }); - describe('dispatch', () => { - it('executes mapped actions for an event', async () => { - engine.registerAction({ name: 'log', type: 'script', target: '"logged"' }); - engine.addMapping({ event: 'row:click', actionName: 'log' }); - - const results = await engine.dispatch('row:click'); - expect(results).toHaveLength(1); - expect(results[0].success).toBe(true); - }); - - it('returns empty array for unmapped events', async () => { - const results = await engine.dispatch('unknown:event'); - expect(results).toEqual([]); - }); - - it('skips actions when mapping condition is false', async () => { - engine = new ActionEngine({ data: { status: 'locked' } }); - engine.registerAction({ name: 'edit', type: 'script', target: '"edited"' }); - engine.addMapping({ - event: 'row:click', - actionName: 'edit', - condition: '${data.status === "active"}' - }); - - const results = await engine.dispatch('row:click'); - expect(results).toHaveLength(0); - }); - }); - describe('handleShortcut', () => { it('executes action for matching shortcut', async () => { engine.registerAction( @@ -206,10 +176,9 @@ describe('ActionEngine', () => { }); describe('clear', () => { - it('removes all actions, mappings, and shortcuts', () => { + it('removes all actions and shortcuts', () => { engine.registerAction({ name: 'save', type: 'api' }, { shortcut: 'ctrl+s' }); - engine.addMapping({ event: 'test', actionName: 'save' }); - + engine.clear(); expect(engine.getAction('save')).toBeUndefined();