refactor(editor): Refactor workflow document composables from handleAction to set / apply (n8n-io#26178)

Author: r00gm

packages/frontend/editor-ui/src/app/stores/workflowDocument.store.ts

@@ -2,21 +2,9 @@ import { defineStore, getActivePinia, type StoreGeneric } from 'pinia';
 import { STORES } from '@n8n/stores';
 import { inject } from 'vue';
 import { WorkflowDocumentStoreKey } from '@/app/constants/injectionKeys';
-import {
-	useWorkflowDocumentActive,
-	isActiveAction,
-	type ActiveAction,
-} from './workflowDocument/useWorkflowDocumentActive';
-import {
-	useWorkflowDocumentPinData,
-	isPinDataAction,
-	type PinDataAction,
-} from './workflowDocument/useWorkflowDocumentPinData';
-import {
-	useWorkflowDocumentTags,
-	isTagAction,
-	type TagAction,
-} from './workflowDocument/useWorkflowDocumentTags';
+import { useWorkflowDocumentActive } from './workflowDocument/useWorkflowDocumentActive';
+import { useWorkflowDocumentPinData } from './workflowDocument/useWorkflowDocumentPinData';
+import { useWorkflowDocumentTags } from './workflowDocument/useWorkflowDocumentTags';
 
 export {
 	getPinDataSize,
@@ -37,8 +25,6 @@ export function createWorkflowDocumentId(
 	return `${workflowId}@${version}`;
 }
 
-type WorkflowDocumentAction = ActiveAction | TagAction | PinDataAction;
-
 /**
  * Gets the store ID for a workflow document store.
  */
@@ -60,65 +46,16 @@ export function useWorkflowDocumentStore(id: WorkflowDocumentId) {
 	return defineStore(getWorkflowDocumentStoreId(id), () => {
 		const [workflowId, workflowVersion] = id.split('@');
 
-		/**
-		 * Handle all document actions in a CRDT-like manner.
-		 * Single entry point for all mutations, enabling future CRDT sync integration.
-		 */
-		function onChange(action: WorkflowDocumentAction) {
-			if (isActiveAction(action)) {
-				handleActiveAction(action);
-			} else if (isTagAction(action)) {
-				handleTagAction(action);
-			} else if (isPinDataAction(action)) {
-				handlePinDataAction(action);
-			}
-		}
-
-		const {
-			active,
-			activeVersionId,
-			activeVersion,
-			setActiveState,
-			handleAction: handleActiveAction,
-		} = useWorkflowDocumentActive(onChange);
-
-		const {
-			tags,
-			setTags,
-			addTags,
-			removeTag,
-			handleAction: handleTagAction,
-		} = useWorkflowDocumentTags(onChange);
-
-		const {
-			pinData,
-			setPinData,
-			pinNodeData,
-			unpinNodeData,
-			renamePinDataNode,
-			getPinDataSnapshot,
-			getNodePinData,
-			handleAction: handlePinDataAction,
-		} = useWorkflowDocumentPinData(onChange);
+		const workflowDocumentActive = useWorkflowDocumentActive();
+		const workflowDocumentTags = useWorkflowDocumentTags();
+		const workflowDocumentPinData = useWorkflowDocumentPinData();
 
 		return {
 			workflowId,
 			workflowVersion,
-			active,
-			activeVersionId,
-			activeVersion,
-			setActiveState,
-			tags,
-			setTags,
-			addTags,
-			removeTag,
-			pinData,
-			setPinData,
-			pinNodeData,
-			unpinNodeData,
-			renamePinDataNode,
-			getPinDataSnapshot,
-			getNodePinData,
+			...workflowDocumentActive,
+			...workflowDocumentTags,
+			...workflowDocumentPinData,
 		};
 	})();
 }

packages/frontend/editor-ui/src/app/stores/workflowDocument/CLAUDE.md

@@ -0,0 +1,71 @@
+# workflowDocument store — Agent Guidelines
+
+## Core pattern: apply/public method split
+
+Every composable in this folder follows a two-layer pattern:
+
+**Public methods** (exposed) — represent user intent. Handle normalization,
+deduplication, and preparation. Call apply methods internally.
+
+**Apply methods** (private) — the **only** functions that mutate refs. Each
+apply method writes to the ref and fires an event hook. Never expose apply
+methods from the composable.
+
+```
+Component → publicMethod() → normalize → applyXxx() → ref + event hook
+```
+
+This split exists to support CRDT in the future: local user actions,
+remote CRDT sync, and undo/redo all converge on the same private apply
+methods inside the composable.
+
+## Event hooks
+
+Every composable exposes change notifications via `createEventHook` from
+`@vueuse/core`. Event payloads must extend `ChangeEvent` from `./types.ts`:
+
+```typescript
+import { createEventHook } from '@vueuse/core';
+import type { ChangeEvent, ChangeAction } from './types';
+
+type MyChangeEvent = ChangeEvent<{ /* domain-specific fields */ }>;
+const onMyChange = createEventHook<MyChangeEvent>();
+```
+
+- Fire `void onMyChange.trigger(...)` inside every apply method
+- Expose only the `.on` subscriber: `onMyChange: onMyChange.on`
+- Use `CHANGE_ACTION.ADD | UPDATE | DELETE` for the `action` field
+
+## Adding a new composable — checklist
+
+1. Create event hook with typed payload extending `ChangeEvent`
+2. Write private `apply*()` methods — each mutates the ref and fires the hook
+3. Write public methods — normalize input, then call apply
+4. Return: readonly refs, public methods, `onXxxChange: hook.on`
+5. Never return apply methods
+
+## Anti-patterns
+
+| Don't | Do instead |
+|-------|------------|
+| Mutate refs outside apply methods | All ref writes go through apply |
+| Expose apply methods from composable | Keep them private (not in return) |
+| Use action objects / `onChange` router | Public methods call apply directly |
+| Use `dataPinningEventBus` or global event bus | Use scoped `createEventHook` |
+| Import global stores inside composable | Inject dependencies via function params |
+
+## Dependency injection
+
+Composables receive external dependencies as constructor params, not via
+global store imports. This keeps them testable and makes coupling explicit:
+
+```typescript
+export function useWorkflowDocumentFoo(deps: {
+  getNodeByName: (name: string) => INodeUi | undefined;
+}) { ... }
+```
+
+## Reference implementation
+
+See `useWorkflowDocumentActive.ts` — the simplest complete example of the
+apply/public pattern with event hooks.

packages/frontend/editor-ui/src/app/stores/workflowDocument/types.ts

@@ -0,0 +1,13 @@
+export const CHANGE_ACTION = {
+	ADD: 'add',
+	UPDATE: 'update',
+	DELETE: 'delete',
+} as const;
+
+export type ChangeAction = (typeof CHANGE_ACTION)[keyof typeof CHANGE_ACTION];
+
+/** Base type all composable change events must extend */
+export interface ChangeEvent<T = unknown> {
+	action: ChangeAction;
+	payload: T;
+}

packages/frontend/editor-ui/src/app/stores/workflowDocument/useWorkflowDocumentActive.test.ts

@@ -0,0 +1,66 @@
+import { describe, it, expect, vi } from 'vitest';
+import { useWorkflowDocumentActive } from './useWorkflowDocumentActive';
+
+function createActive() {
+	return useWorkflowDocumentActive();
+}
+
+describe('useWorkflowDocumentActive', () => {
+	describe('initial state', () => {
+		it('should start inactive with null values', () => {
+			const { active, activeVersionId, activeVersion } = createActive();
+			expect(active.value).toBe(false);
+			expect(activeVersionId.value).toBeNull();
+			expect(activeVersion.value).toBeNull();
+		});
+	});
+
+	describe('setActiveState', () => {
+		it('should set active state and fire event hook', () => {
+			const { active, activeVersionId, activeVersion, setActiveState, onActiveChange } =
+				createActive();
+			const hookSpy = vi.fn();
+			onActiveChange(hookSpy);
+
+			setActiveState({ activeVersionId: 'v1', activeVersion: null });
+
+			expect(active.value).toBe(true);
+			expect(activeVersionId.value).toBe('v1');
+			expect(activeVersion.value).toBeNull();
+			expect(hookSpy).toHaveBeenCalledWith({
+				action: 'update',
+				payload: { activeVersionId: 'v1', activeVersion: null },
+			});
+		});
+
+		it('should clear active state', () => {
+			const { active, activeVersionId, setActiveState } = createActive();
+			setActiveState({ activeVersionId: 'v1', activeVersion: null });
+
+			setActiveState({ activeVersionId: null, activeVersion: null });
+
+			expect(active.value).toBe(false);
+			expect(activeVersionId.value).toBeNull();
+		});
+
+		it('should replace existing active state', () => {
+			const { activeVersionId, setActiveState } = createActive();
+			setActiveState({ activeVersionId: 'v1', activeVersion: null });
+
+			setActiveState({ activeVersionId: 'v2', activeVersion: null });
+
+			expect(activeVersionId.value).toBe('v2');
+		});
+
+		it('should fire event hook on every call', () => {
+			const { setActiveState, onActiveChange } = createActive();
+			const hookSpy = vi.fn();
+			onActiveChange(hookSpy);
+
+			setActiveState({ activeVersionId: 'v1', activeVersion: null });
+			setActiveState({ activeVersionId: null, activeVersion: null });
+
+			expect(hookSpy).toHaveBeenCalledTimes(2);
+		});
+	});
+});

packages/frontend/editor-ui/src/app/stores/workflowDocument/useWorkflowDocumentActive.ts

@@ -1,56 +1,41 @@
 import { ref, readonly, computed } from 'vue';
+import { createEventHook } from '@vueuse/core';
 import type { WorkflowHistory } from '@n8n/rest-api-client';
+import { CHANGE_ACTION } from './types';
+import type { ChangeAction, ChangeEvent } from './types';
 
-type Action<N, P> = { name: N; payload: P };
+export type ActiveStatePayload = {
+	activeVersionId: string | null;
+	activeVersion: WorkflowHistory | null;
+};
 
-type SetActiveStateAction = Action<
-	'setActiveState',
-	{ activeVersionId: string | null; activeVersion: WorkflowHistory | null }
->;
+export type ActiveChangeEvent = ChangeEvent<ActiveStatePayload>;
 
-export type ActiveAction = SetActiveStateAction;
-
-const ACTIVE_ACTION_NAMES = new Set<string>(['setActiveState']);
-
-export function isActiveAction(action: { name: string }): action is ActiveAction {
-	return ACTIVE_ACTION_NAMES.has(action.name);
-}
-
-/**
- * Composable that encapsulates active state, public API, and mutation logic
- * for a workflow document store.
- *
- * Accepts an `onChange` callback that routes actions through the store's
- * unified dispatcher for CRDT migration readiness.
- */
-export function useWorkflowDocumentActive(onChange: (action: ActiveAction) => void) {
+export function useWorkflowDocumentActive() {
 	const activeVersionId = ref<string | null>(null);
 	const activeVersion = ref<WorkflowHistory | null>(null);
 	const active = computed(() => activeVersionId.value !== null);
 
-	function setActiveState(state: {
-		activeVersionId: string | null;
-		activeVersion: WorkflowHistory | null;
-	}) {
-		onChange({ name: 'setActiveState', payload: state });
+	const onActiveChange = createEventHook<ActiveChangeEvent>();
+
+	function applyActiveState(
+		state: ActiveStatePayload,
+		action: ChangeAction = CHANGE_ACTION.UPDATE,
+	) {
+		activeVersionId.value = state.activeVersionId;
+		activeVersion.value = state.activeVersion;
+		void onActiveChange.trigger({ action, payload: state });
 	}
 
-	/**
-	 * Apply an active state action to the state.
-	 * Called by the store's unified onChange dispatcher.
-	 */
-	function handleAction(action: ActiveAction) {
-		if (action.name === 'setActiveState') {
-			activeVersionId.value = action.payload.activeVersionId;
-			activeVersion.value = action.payload.activeVersion;
-		}
+	function setActiveState(state: ActiveStatePayload) {
+		applyActiveState(state);
 	}
 
 	return {
 		active,
 		activeVersionId: readonly(activeVersionId),
 		activeVersion: readonly(activeVersion),
 		setActiveState,
-		handleAction,
+		onActiveChange: onActiveChange.on,
 	};
 }