From 1d00301b19ebf302bbd9ad8bedcc0e6b92563f89 Mon Sep 17 00:00:00 2001 From: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> Date: Tue, 13 Aug 2024 10:11:09 +0300 Subject: [PATCH 1/5] refactor(editor): Add typed event bus --- .../src/components/N8nFormBox/FormBox.vue | 4 +- .../components/N8nFormInputs/FormInputs.vue | 8 +-- .../src/utils/__tests__/event-bus.spec.ts | 25 ++++++++ packages/design-system/src/utils/event-bus.ts | 58 +++++++++++++++++-- .../design-system/src/utils/form-event-bus.ts | 12 ++++ packages/design-system/src/utils/index.ts | 1 + 6 files changed, 97 insertions(+), 11 deletions(-) create mode 100644 packages/design-system/src/utils/form-event-bus.ts diff --git a/packages/design-system/src/components/N8nFormBox/FormBox.vue b/packages/design-system/src/components/N8nFormBox/FormBox.vue index ed01219eb914e..7881c4b5c9ba4 100644 --- a/packages/design-system/src/components/N8nFormBox/FormBox.vue +++ b/packages/design-system/src/components/N8nFormBox/FormBox.vue @@ -44,7 +44,7 @@ import N8nHeading from '../N8nHeading'; import N8nLink from '../N8nLink'; import N8nButton from '../N8nButton'; import type { IFormInput } from 'n8n-design-system/types'; -import { createEventBus } from '../../utils'; +import { createFormEventBus } from '../../utils'; interface FormBoxProps { title?: string; @@ -67,7 +67,7 @@ withDefaults(defineProps(), { redirectLink: '', }); -const formBus = createEventBus(); +const formBus = createFormEventBus(); const emit = defineEmits<{ submit: [value: { [key: string]: Value }]; update: [value: { name: string; value: Value }]; diff --git a/packages/design-system/src/components/N8nFormInputs/FormInputs.vue b/packages/design-system/src/components/N8nFormInputs/FormInputs.vue index 620778cd2cb9d..977a10a88fe16 100644 --- a/packages/design-system/src/components/N8nFormInputs/FormInputs.vue +++ b/packages/design-system/src/components/N8nFormInputs/FormInputs.vue @@ -3,12 +3,12 @@ import { computed, onMounted, reactive, ref, watch } from 'vue'; import N8nFormInput from '../N8nFormInput'; import type { IFormInput } from '../../types'; import ResizeObserver from '../ResizeObserver'; -import type { EventBus } from '../../utils'; -import { createEventBus } from '../../utils'; +import type { FormEventBus } from '../../utils'; +import { createFormEventBus } from '../../utils'; export type FormInputsProps = { inputs?: IFormInput[]; - eventBus?: EventBus; + eventBus?: FormEventBus; columnView?: boolean; verticalSpacing?: '' | 'xs' | 's' | 'm' | 'l' | 'xl'; teleported?: boolean; @@ -19,7 +19,7 @@ type Value = string | number | boolean | null | undefined; const props = withDefaults(defineProps(), { inputs: () => [], - eventBus: createEventBus, + eventBus: createFormEventBus, columnView: false, verticalSpacing: '', teleported: true, diff --git a/packages/design-system/src/utils/__tests__/event-bus.spec.ts b/packages/design-system/src/utils/__tests__/event-bus.spec.ts index e403b61008f4e..016209a316767 100644 --- a/packages/design-system/src/utils/__tests__/event-bus.spec.ts +++ b/packages/design-system/src/utils/__tests__/event-bus.spec.ts @@ -29,6 +29,31 @@ describe('createEventBus()', () => { }); }); + describe('once()', () => { + it('should register event handler', () => { + const handler = vi.fn(); + const eventName = 'test'; + + eventBus.once(eventName, handler); + + eventBus.emit(eventName, {}); + + expect(handler).toHaveBeenCalled(); + }); + + it('should unregister event handler after first call', () => { + const handler = vi.fn(); + const eventName = 'test'; + + eventBus.once(eventName, handler); + + eventBus.emit(eventName, {}); + eventBus.emit(eventName, {}); + + expect(handler).toHaveBeenCalledTimes(1); + }); + }); + describe('off()', () => { it('should register event handler', () => { const handler = vi.fn(); diff --git a/packages/design-system/src/utils/event-bus.ts b/packages/design-system/src/utils/event-bus.ts index f6ffc597f5fd6..1b7cb10182a48 100644 --- a/packages/design-system/src/utils/event-bus.ts +++ b/packages/design-system/src/utils/event-bus.ts @@ -2,13 +2,51 @@ export type CallbackFn = Function; export type UnregisterFn = () => void; -export interface EventBus { - on: (eventName: string, fn: CallbackFn) => UnregisterFn; - off: (eventName: string, fn: CallbackFn) => void; - emit: (eventName: string, event?: T) => void; +export type Listener = (payload: Payload) => void; + +export type Payloads = { + [E in keyof ListenerMap]: unknown; +}; + +// TODO: Fix all usages of `createEventBus` and convert `any` to `unknown` +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export interface EventBus = Record> { + on: ( + eventName: EventName, + fn: Listener, + ) => UnregisterFn; + + once: ( + eventName: EventName, + fn: Listener, + ) => UnregisterFn; + + off: ( + eventName: EventName, + fn: Listener, + ) => void; + + emit: ( + eventName: EventName, + event?: ListenerMap[EventName], + ) => void; } -export function createEventBus(): EventBus { +/** + * Creates an event bus with the given listener map. + * + * @example + * ```ts + * const eventBus = createEventBus<{ + * 'user-logged-in': { username: string }; + * 'user-logged-out': never; + * }>(); + */ +export function createEventBus< + // TODO: Fix all usages of `createEventBus` and convert `any` to `unknown` + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ListenerMap extends Payloads = Record, +>(): EventBus { const handlers = new Map(); function off(eventName: string, fn: CallbackFn) { @@ -33,6 +71,15 @@ export function createEventBus(): EventBus { return () => off(eventName, fn); } + function once(eventName: string, fn: CallbackFn): UnregisterFn { + const unregister = on(eventName, (...args: unknown[]) => { + unregister(); + fn(...args); + }); + + return unregister; + } + function emit(eventName: string, event?: T) { const eventFns = handlers.get(eventName); @@ -45,6 +92,7 @@ export function createEventBus(): EventBus { return { on, + once, off, emit, }; diff --git a/packages/design-system/src/utils/form-event-bus.ts b/packages/design-system/src/utils/form-event-bus.ts new file mode 100644 index 0000000000000..5b518c8a4233d --- /dev/null +++ b/packages/design-system/src/utils/form-event-bus.ts @@ -0,0 +1,12 @@ +import { createEventBus } from './event-bus'; + +export interface FormEventBusEvents { + submit: never; +} + +export type FormEventBus = ReturnType; + +/** + * Creates a new event bus to be used with the `FormInputs` component. + */ +export const createFormEventBus = createEventBus; diff --git a/packages/design-system/src/utils/index.ts b/packages/design-system/src/utils/index.ts index cdc1b91597fa1..3f4ed339f0f73 100644 --- a/packages/design-system/src/utils/index.ts +++ b/packages/design-system/src/utils/index.ts @@ -1,4 +1,5 @@ export * from './event-bus'; +export * from './form-event-bus'; export * from './markdown'; export * from './typeguards'; export * from './uid'; From bb4ce348b597f1695cdb06ffaf08ab748cd85f82 Mon Sep 17 00:00:00 2001 From: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> Date: Tue, 13 Aug 2024 13:24:10 +0300 Subject: [PATCH 2/5] refactor(editor): Convert event bus to class, remove unregister fn --- .../src/utils/__tests__/event-bus.spec.ts | 13 --- packages/design-system/src/utils/event-bus.ts | 92 +++++++------------ 2 files changed, 34 insertions(+), 71 deletions(-) diff --git a/packages/design-system/src/utils/__tests__/event-bus.spec.ts b/packages/design-system/src/utils/__tests__/event-bus.spec.ts index 016209a316767..2fb0735404ee0 100644 --- a/packages/design-system/src/utils/__tests__/event-bus.spec.ts +++ b/packages/design-system/src/utils/__tests__/event-bus.spec.ts @@ -14,19 +14,6 @@ describe('createEventBus()', () => { expect(handler).toHaveBeenCalled(); }); - - it('should return unregister fn', () => { - const handler = vi.fn(); - const eventName = 'test'; - - const unregister = eventBus.on(eventName, handler); - - unregister(); - - eventBus.emit(eventName, {}); - - expect(handler).not.toHaveBeenCalled(); - }); }); describe('once()', () => { diff --git a/packages/design-system/src/utils/event-bus.ts b/packages/design-system/src/utils/event-bus.ts index 1b7cb10182a48..a2bf3885afcf5 100644 --- a/packages/design-system/src/utils/event-bus.ts +++ b/packages/design-system/src/utils/event-bus.ts @@ -10,55 +10,19 @@ export type Payloads = { // TODO: Fix all usages of `createEventBus` and convert `any` to `unknown` // eslint-disable-next-line @typescript-eslint/no-explicit-any -export interface EventBus = Record> { - on: ( - eventName: EventName, - fn: Listener, - ) => UnregisterFn; +class EventBusImpl = Record> { + private readonly handlers = new Map(); - once: ( - eventName: EventName, - fn: Listener, - ) => UnregisterFn; - - off: ( - eventName: EventName, - fn: Listener, - ) => void; - - emit: ( - eventName: EventName, - event?: ListenerMap[EventName], - ) => void; -} - -/** - * Creates an event bus with the given listener map. - * - * @example - * ```ts - * const eventBus = createEventBus<{ - * 'user-logged-in': { username: string }; - * 'user-logged-out': never; - * }>(); - */ -export function createEventBus< - // TODO: Fix all usages of `createEventBus` and convert `any` to `unknown` - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ListenerMap extends Payloads = Record, ->(): EventBus { - const handlers = new Map(); - - function off(eventName: string, fn: CallbackFn) { - const eventFns = handlers.get(eventName); + off(eventName: string, fn: CallbackFn) { + const eventFns = this.handlers.get(eventName); if (eventFns) { eventFns.splice(eventFns.indexOf(fn) >>> 0, 1); } } - function on(eventName: string, fn: CallbackFn): UnregisterFn { - let eventFns = handlers.get(eventName); + on(eventName: string, fn: CallbackFn) { + let eventFns = this.handlers.get(eventName); if (!eventFns) { eventFns = [fn]; @@ -66,22 +30,18 @@ export function createEventBus< eventFns.push(fn); } - handlers.set(eventName, eventFns); - - return () => off(eventName, fn); + this.handlers.set(eventName, eventFns); } - function once(eventName: string, fn: CallbackFn): UnregisterFn { - const unregister = on(eventName, (...args: unknown[]) => { - unregister(); + once(eventName: string, fn: CallbackFn) { + this.on(eventName, (...args: unknown[]) => { + this.off(eventName, fn); fn(...args); }); - - return unregister; } - function emit(eventName: string, event?: T) { - const eventFns = handlers.get(eventName); + emit(eventName: string, event?: T) { + const eventFns = this.handlers.get(eventName); if (eventFns) { eventFns.slice().forEach(async (handler) => { @@ -89,11 +49,27 @@ export function createEventBus< }); } } +} - return { - on, - once, - off, - emit, - }; +/** + * Creates an event bus with the given listener map. + * + * @example + * ```ts + * const eventBus = createEventBus<{ + * 'user-logged-in': { username: string }; + * 'user-logged-out': never; + * }>(); + */ +export function createEventBus< + // TODO: Fix all usages of `createEventBus` and convert `any` to `unknown` + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ListenerMap extends Payloads = Record, +>() { + return new EventBusImpl(); } + +// TODO: Fix all usages of `createEventBus` and convert `any` to `unknown` +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export type EventBus = Record> = + InstanceType>; From 928ef4ccffcb92f9f7848711d7225d6c0b724682 Mon Sep 17 00:00:00 2001 From: Tomi Turtiainen <10324676+tomi@users.noreply.github.com> Date: Tue, 13 Aug 2024 13:57:39 +0300 Subject: [PATCH 3/5] refactor: Fix EventBus type errors --- packages/design-system/src/utils/event-bus.ts | 35 ++++++++++++------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/packages/design-system/src/utils/event-bus.ts b/packages/design-system/src/utils/event-bus.ts index a2bf3885afcf5..411e538644e2b 100644 --- a/packages/design-system/src/utils/event-bus.ts +++ b/packages/design-system/src/utils/event-bus.ts @@ -5,15 +5,26 @@ export type UnregisterFn = () => void; export type Listener = (payload: Payload) => void; export type Payloads = { - [E in keyof ListenerMap]: unknown; + // TODO: Fix all usages of `createEventBus` and convert `any` to `unknown` + // eslint-disable-next-line @typescript-eslint/no-explicit-any + [E in keyof ListenerMap]: any; }; // TODO: Fix all usages of `createEventBus` and convert `any` to `unknown` // eslint-disable-next-line @typescript-eslint/no-explicit-any -class EventBusImpl = Record> { +export interface EventBus = Record> { + off(eventName: K, fn: Listener): void; + on(eventName: K, fn: Listener): void; + once(eventName: K, fn: Listener): void; + emit(eventName: K, event?: ListenerMap[K]): void; +} + +// TODO: Fix all usages of `createEventBus` and convert `any` to `unknown` +// eslint-disable-next-line @typescript-eslint/no-explicit-any +class EventBusImpl> implements EventBus { private readonly handlers = new Map(); - off(eventName: string, fn: CallbackFn) { + off(eventName: EventName, fn: CallbackFn) { const eventFns = this.handlers.get(eventName); if (eventFns) { @@ -21,7 +32,7 @@ class EventBusImpl = Record(eventName: EventName, fn: CallbackFn) { let eventFns = this.handlers.get(eventName); if (!eventFns) { @@ -33,14 +44,17 @@ class EventBusImpl = Record(eventName: EventName, fn: CallbackFn) { this.on(eventName, (...args: unknown[]) => { this.off(eventName, fn); fn(...args); }); } - emit(eventName: string, event?: T) { + emit( + eventName: EventName, + event?: ListenerMap[EventName], + ) { const eventFns = this.handlers.get(eventName); if (eventFns) { @@ -65,11 +79,6 @@ export function createEventBus< // TODO: Fix all usages of `createEventBus` and convert `any` to `unknown` // eslint-disable-next-line @typescript-eslint/no-explicit-any ListenerMap extends Payloads = Record, ->() { - return new EventBusImpl(); +>(): EventBus { + return new EventBusImpl(); } - -// TODO: Fix all usages of `createEventBus` and convert `any` to `unknown` -// eslint-disable-next-line @typescript-eslint/no-explicit-any -export type EventBus = Record> = - InstanceType>; From 112fe15d21c38d48be7ab474d00d09a2938e500d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Tue, 13 Aug 2024 13:04:37 +0200 Subject: [PATCH 4/5] revert back to using an interface --- packages/design-system/src/utils/event-bus.ts | 103 +++++++++--------- 1 file changed, 51 insertions(+), 52 deletions(-) diff --git a/packages/design-system/src/utils/event-bus.ts b/packages/design-system/src/utils/event-bus.ts index 411e538644e2b..e1e9d8b3e70fe 100644 --- a/packages/design-system/src/utils/event-bus.ts +++ b/packages/design-system/src/utils/event-bus.ts @@ -1,68 +1,34 @@ // eslint-disable-next-line @typescript-eslint/ban-types export type CallbackFn = Function; -export type UnregisterFn = () => void; -export type Listener = (payload: Payload) => void; - -export type Payloads = { - // TODO: Fix all usages of `createEventBus` and convert `any` to `unknown` - // eslint-disable-next-line @typescript-eslint/no-explicit-any - [E in keyof ListenerMap]: any; +type Payloads = { + [E in keyof ListenerMap]: unknown; }; -// TODO: Fix all usages of `createEventBus` and convert `any` to `unknown` -// eslint-disable-next-line @typescript-eslint/no-explicit-any -export interface EventBus = Record> { - off(eventName: K, fn: Listener): void; - on(eventName: K, fn: Listener): void; - once(eventName: K, fn: Listener): void; - emit(eventName: K, event?: ListenerMap[K]): void; -} +type Listener = (payload: Payload) => void; // TODO: Fix all usages of `createEventBus` and convert `any` to `unknown` // eslint-disable-next-line @typescript-eslint/no-explicit-any -class EventBusImpl> implements EventBus { - private readonly handlers = new Map(); - - off(eventName: EventName, fn: CallbackFn) { - const eventFns = this.handlers.get(eventName); - - if (eventFns) { - eventFns.splice(eventFns.indexOf(fn) >>> 0, 1); - } - } - - on(eventName: EventName, fn: CallbackFn) { - let eventFns = this.handlers.get(eventName); - - if (!eventFns) { - eventFns = [fn]; - } else { - eventFns.push(fn); - } +export interface EventBus = Record> { + on( + eventName: EventName, + fn: Listener, + ): void; - this.handlers.set(eventName, eventFns); - } + once( + eventName: EventName, + fn: Listener, + ): void; - once(eventName: EventName, fn: CallbackFn) { - this.on(eventName, (...args: unknown[]) => { - this.off(eventName, fn); - fn(...args); - }); - } + off( + eventName: EventName, + fn: Listener, + ): void; emit( eventName: EventName, event?: ListenerMap[EventName], - ) { - const eventFns = this.handlers.get(eventName); - - if (eventFns) { - eventFns.slice().forEach(async (handler) => { - await handler(event); - }); - } - } + ): void; } /** @@ -80,5 +46,38 @@ export function createEventBus< // eslint-disable-next-line @typescript-eslint/no-explicit-any ListenerMap extends Payloads = Record, >(): EventBus { - return new EventBusImpl(); + const handlers = new Map(); + + return { + on(eventName, fn) { + let eventFns = handlers.get(eventName); + if (!eventFns) { + eventFns = [fn]; + } else { + eventFns.push(fn); + } + handlers.set(eventName, eventFns); + }, + + once(eventName, fn) { + this.on(eventName, (payload) => { + this.off(eventName, fn); + fn(payload); + }); + }, + + off(eventName, fn) { + const eventFns = handlers.get(eventName); + if (eventFns) { + eventFns.splice(eventFns.indexOf(fn) >>> 0, 1); + } + }, + + emit(eventName, event) { + const eventFns = handlers.get(eventName); + if (eventFns) { + eventFns.slice().forEach((handler) => handler(event)); + } + }, + }; } From 27bc0eee10b4e686bde8e19caf17871692f95dda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Tue, 13 Aug 2024 13:12:29 +0200 Subject: [PATCH 5/5] fix `once` implementation --- packages/design-system/src/utils/event-bus.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/design-system/src/utils/event-bus.ts b/packages/design-system/src/utils/event-bus.ts index e1e9d8b3e70fe..eb08228b47afe 100644 --- a/packages/design-system/src/utils/event-bus.ts +++ b/packages/design-system/src/utils/event-bus.ts @@ -60,10 +60,11 @@ export function createEventBus< }, once(eventName, fn) { - this.on(eventName, (payload) => { - this.off(eventName, fn); + const handler: typeof fn = (payload) => { + this.off(eventName, handler); fn(payload); - }); + }; + this.on(eventName, handler); }, off(eventName, fn) {