Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(editor): Add typed event bus (no-changelog) #10367

Merged
merged 6 commits into from
Aug 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/design-system/src/components/N8nFormBox/FormBox.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -67,7 +67,7 @@ withDefaults(defineProps<FormBoxProps>(), {
redirectLink: '',
});

const formBus = createEventBus();
const formBus = createFormEventBus();
const emit = defineEmits<{
submit: [value: { [key: string]: Value }];
update: [value: { name: string; value: Value }];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -19,7 +19,7 @@ type Value = string | number | boolean | null | undefined;

const props = withDefaults(defineProps<FormInputsProps>(), {
inputs: () => [],
eventBus: createEventBus,
eventBus: createFormEventBus,
netroy marked this conversation as resolved.
Show resolved Hide resolved
columnView: false,
verticalSpacing: '',
teleported: true,
Expand Down
20 changes: 16 additions & 4 deletions packages/design-system/src/utils/__tests__/event-bus.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,30 @@ describe('createEventBus()', () => {

expect(handler).toHaveBeenCalled();
});
});

it('should return unregister fn', () => {
describe('once()', () => {
it('should register event handler', () => {
const handler = vi.fn();
const eventName = 'test';

const unregister = eventBus.on(eventName, handler);
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';

unregister();
eventBus.once(eventName, handler);

eventBus.emit(eventName, {});
eventBus.emit(eventName, {});

expect(handler).not.toHaveBeenCalled();
expect(handler).toHaveBeenCalledTimes(1);
});
});

Expand Down
105 changes: 69 additions & 36 deletions packages/design-system/src/utils/event-bus.ts
Original file line number Diff line number Diff line change
@@ -1,51 +1,84 @@
// eslint-disable-next-line @typescript-eslint/ban-types
export type CallbackFn = Function;
export type UnregisterFn = () => void;

export interface EventBus {
on: (eventName: string, fn: CallbackFn) => UnregisterFn;
off: (eventName: string, fn: CallbackFn) => void;
emit: <T = Event>(eventName: string, event?: T) => void;
}
type Payloads<ListenerMap> = {
[E in keyof ListenerMap]: unknown;
};

export function createEventBus(): EventBus {
const handlers = new Map<string, CallbackFn[]>();
type Listener<Payload> = (payload: Payload) => void;

function off(eventName: string, fn: CallbackFn) {
const eventFns = handlers.get(eventName);
// TODO: Fix all usages of `createEventBus` and convert `any` to `unknown`
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export interface EventBus<ListenerMap extends Payloads<ListenerMap> = Record<string, any>> {
netroy marked this conversation as resolved.
Show resolved Hide resolved
on<EventName extends keyof ListenerMap & string>(
eventName: EventName,
fn: Listener<ListenerMap[EventName]>,
): void;

if (eventFns) {
eventFns.splice(eventFns.indexOf(fn) >>> 0, 1);
}
}
once<EventName extends keyof ListenerMap & string>(
eventName: EventName,
fn: Listener<ListenerMap[EventName]>,
): void;

function on(eventName: string, fn: CallbackFn): UnregisterFn {
let eventFns = handlers.get(eventName);
off<EventName extends keyof ListenerMap & string>(
eventName: EventName,
fn: Listener<ListenerMap[EventName]>,
): void;

if (!eventFns) {
eventFns = [fn];
} else {
eventFns.push(fn);
}
emit<EventName extends keyof ListenerMap & string>(
eventName: EventName,
event?: ListenerMap[EventName],
): void;
}

handlers.set(eventName, eventFns);
/**
* 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<ListenerMap> = Record<string, any>,
>(): EventBus<ListenerMap> {
const handlers = new Map<string, CallbackFn[]>();

return () => off(eventName, fn);
}
return {
on(eventName, fn) {
let eventFns = handlers.get(eventName);
if (!eventFns) {
eventFns = [fn];
} else {
eventFns.push(fn);
}
handlers.set(eventName, eventFns);
},

function emit<T = Event>(eventName: string, event?: T) {
const eventFns = handlers.get(eventName);
once(eventName, fn) {
const handler: typeof fn = (payload) => {
this.off(eventName, handler);
fn(payload);
};
this.on(eventName, handler);
},

if (eventFns) {
eventFns.slice().forEach(async (handler) => {
await handler(event);
});
}
}
off(eventName, fn) {
const eventFns = handlers.get(eventName);
if (eventFns) {
eventFns.splice(eventFns.indexOf(fn) >>> 0, 1);
}
},

return {
on,
off,
emit,
emit(eventName, event) {
const eventFns = handlers.get(eventName);
if (eventFns) {
eventFns.slice().forEach((handler) => handler(event));
}
},
};
}
12 changes: 12 additions & 0 deletions packages/design-system/src/utils/form-event-bus.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { createEventBus } from './event-bus';

export interface FormEventBusEvents {
submit: never;
}

export type FormEventBus = ReturnType<typeof createFormEventBus>;

/**
* Creates a new event bus to be used with the `FormInputs` component.
*/
export const createFormEventBus = createEventBus<FormEventBusEvents>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

considering that we have so many other instances of createEventBus, do we plan to create a typed alias for each of them? if not, perhaps we should remove this, and use createEventBus<FormEventBusEvents>() instead of createFormEventBus() ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense to have an aliased create function for cases where it's used for a common component. For example for the FormInputs and for modals. I could move this to the FormInputs component file so it's closer to usage.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think that once we migrate all other event buses to be typed, creating aliases for each would seem too verbose. but we can talk about that after we finish porting over the rest of the code.

1 change: 1 addition & 0 deletions packages/design-system/src/utils/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export * from './event-bus';
export * from './form-event-bus';
export * from './markdown';
export * from './typeguards';
export * from './uid';
Expand Down
Loading