Typed async events (#394)

This switches from nodejs's `EventEmitter` to [emittery](https://npmjs.com/package/emittery) for a few reasons:

- type-safe events. This will prevent people doing things like `.on('migration', ...)` instead of `.on('migrating', ...)` etc. It also types the event payload - especially important now since the payload is changing slightly
- async events. `this.emit(...)` now returns a promise, which we can await. Which means in a follow-up, we can add `beforeAll` and `afterAll` events, which will enable use cases like locking the migration table. That wouldn't be possible with sync events.
-  apparently, less chance of memory leaks 🤷 

The API is smaller than `EventEmitter` which makes this a breaking change by itself. `emittery` only supports single payloads too, meaning we can no longer `emit('migrating', m.name, m)` - but that was pretty redundant anyway, so it makes sense to update to `MigrationParams` as part of the major (v3) release.

README.md

@@ -384,6 +384,20 @@ The `migrations.resolve` parameter replaces `customResolver`. Explicit support f
 
 The constructor option `logging` is replaced by `logger` to allow for `warn` and `error` messages in future. NodeJS's global `console` object can be passed to this. To disable logging, replace `logging: false` with `logger: undefined`.
 
+Events have moved from the default nodejs `EventEmitter` to [emittery](https://www.npmjs.com/package/emittery). It has better design for async code, a less bloated API surface and strong types. But, it doesn't allow passing multiple arguments to callbacks, so listeners have to change slightly, as well as `.addListener(...)` and `.removeListener(...)` no longer being supported (`.on(...)` and `.off(...)` should now be used):
+
+Before:
+
+```js
+umzug.on('migrating', (name, m) => console.log({ name, path: m.path }))
+```
+
+After:
+
+```js
+umzug.on('migrating', ev => console.log({ name: ev.name, path: ev.path }))
+```
+
 The `Umzug#execute` method is removed. Use `Umzug#up` or `Umzug#down`.
 
 The options for `Umguz#up` and `Umzug#down` have changed:
@@ -498,13 +512,15 @@ class CustomStorage implements UmzugStorage {
 
 ### Events
 
-Umzug is an [EventEmitter](https://nodejs.org/docs/latest-v10.x/api/events.html#events_class_eventemitter). Each of the following events will be called with `name` and `migration` as arguments. Events are a convenient place to implement application-specific logic that must run around each migration:
+Umzug is an [emittery event emitter](https://www.npmjs.com/package/emittery). Each of the following events will be called with migration parameters as its payload (with `context`, `name`, and nullable `path` properties). Events are a convenient place to implement application-specific logic that must run around each migration:
 
 * `migrating` - A migration is about to be executed.
 * `migrated` - A migration has successfully been executed.
 * `reverting` - A migration is about to be reverted.
 * `reverted` - A migration has successfully been reverted.
 
+All events are type-safe, so IDEs will prevent typos and supply strong types for the event payloads.
+
 ## License
 
 See the [LICENSE file](./LICENSE)

package.json

@@ -14,9 +14,10 @@
 		"lib"
 	],
 	"dependencies": {
-		"fs-jetpack": "^4.0.0",
-		"glob": "^7.1.6",
-		"type-fest": "^0.19.0"
+		"emittery": "~0.7.2",
+		"fs-jetpack": "~4.0.0",
+		"glob": "~7.1.6",
+		"type-fest": "~0.19.0"
 	},
 	"devDependencies": {
 		"@types/jest": "26.0.15",

src/umzug.ts

@@ -1,9 +1,9 @@
 import * as path from 'path';
-import { EventEmitter } from 'events';
 import { promisify } from 'util';
 import { UmzugStorage, JSONStorage, verifyUmzugStorage } from './storage';
 import * as glob from 'glob';
 import { MergeExclusive } from './type-util';
+import * as emittery from 'emittery';
 
 const globAsync = promisify(glob);
 
@@ -31,14 +31,20 @@ export interface MigrationMeta {
 	path?: string;
 }
 
+export interface MigrationParams<T> {
+	name: string;
+	path?: string;
+	context: T;
+}
+
 /**
  * A runnable migration. Represents a migration object with an `up` function which can be called directly, with no arguments, and an optional `down` function to revert it.
  */
 export interface RunnableMigration<T> extends MigrationMeta {
 	/** The effect of applying the migration */
-	up: (params: { name: string; path?: string; context?: T }) => Promise<unknown>;
+	up: (params: MigrationParams<T>) => Promise<unknown>;
 	/** The effect of reverting the migration */
-	down?: (params: { name: string; path?: string; context?: T }) => Promise<unknown>;
+	down?: (params: MigrationParams<T>) => Promise<unknown>;
 }
 
 /** Glob instructions for migration files */
@@ -63,7 +69,7 @@ export type InputMigrations<T> =
 	| ((context: T) => Promisable<Array<RunnableMigration<T>>>);
 
 /** A function which takes a migration name, path and context, and returns an object with `up` and `down` functions. */
-export type Resolver<T> = (params: { name: string; path?: string; context: T }) => RunnableMigration<T>;
+export type Resolver<T> = (params: MigrationParams<T>) => RunnableMigration<T>;
 
 export const RerunBehavior = {
 	/** Hard error if an up migration that has already been run, or a down migration that hasn't, is encountered */
@@ -115,7 +121,9 @@ export type MigrateDownOptions = MergeExclusive<
 	}
 >;
 
-export class Umzug<Ctx> extends EventEmitter {
+export class Umzug<Ctx> extends emittery.Typed<
+	Record<'migrating' | 'migrated' | 'reverting' | 'reverted', MigrationParams<Ctx>>
+> {
 	private readonly storage: UmzugStorage;
 	private readonly migrations: () => Promise<ReadonlyArray<RunnableMigration<Ctx>>>;
 
@@ -140,7 +148,7 @@ export class Umzug<Ctx> extends EventEmitter {
 	 * export const down: Migration = ({name, context}) => context.query(...)
 	 */
 	declare readonly _types: {
-		migration: (params: { name: string; path?: string; context: Ctx }) => Promise<unknown>;
+		migration: (params: MigrationParams<Ctx>) => Promise<unknown>;
 	};
 
 	/** creates a new Umzug instance */
@@ -151,6 +159,11 @@ export class Umzug<Ctx> extends EventEmitter {
 		this.migrations = this.getMigrationsResolver();
 	}
 
+	private get context(): Ctx {
+		// eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- safe because options.context is only undefined if Ctx's type is undefined
+		return this.options.context!;
+	}
+
 	private logging(message: Record<string, unknown>) {
 		this.options.logger?.info(message);
 	}
@@ -275,16 +288,18 @@ export class Umzug<Ctx> extends EventEmitter {
 
 		for (const m of toBeApplied) {
 			const start = Date.now();
+			const params: MigrationParams<Ctx> = { name: m.name, path: m.path, context: this.context };
+
 			this.logging({ event: 'migrating', name: m.name });
-			this.emit('migrating', m.name, m);
+			await this.emit('migrating', params);
 
-			await m.up({ name: m.name, path: m.path, context: this.options.context });
+			await m.up(params);
 
 			await this.storage.logMigration(m.name);
 
 			const duration = (Date.now() - start) / 1000;
 			this.logging({ event: 'migrated', name: m.name, durationSeconds: duration });
-			this.emit('migrated', m.name, m);
+			await this.emit('migrated', params);
 		}
 
 		return toBeApplied.map(m => ({ name: m.name, path: m.path }));
@@ -327,16 +342,18 @@ export class Umzug<Ctx> extends EventEmitter {
 
 		for (const m of toBeReverted) {
 			const start = Date.now();
+			const params: MigrationParams<Ctx> = { name: m.name, path: m.path, context: this.context };
+
 			this.logging({ event: 'reverting', name: m.name });
-			this.emit('reverting', m.name, m);
+			await this.emit('reverting', params);
 
-			await m.down?.({ name: m.name, path: m.path, context: this.options.context });
+			await m.down?.(params);
 
 			await this.storage.unlogMigration(m.name);
 
 			const duration = Number.parseFloat(((Date.now() - start) / 1000).toFixed(3));
 			this.logging({ event: 'reverted', name: m.name, durationSeconds: duration });
-			this.emit('reverted', m.name, m);
+			await this.emit('reverted', params);
 		}
 
 		return toBeReverted.map(m => ({ name: m.name, path: m.path }));

test/umzug.test.ts

@@ -642,6 +642,38 @@ describe('types', () => {
 			return migrations.slice();
 		});
 	});
+
+	test('event types', () => {
+		const umzug = new Umzug({
+			migrations: [],
+			context: { someCustomSqlClient: {} },
+			logger: undefined,
+		});
+
+		umzug.on('migrating', params => {
+			expectTypeOf(params.name).toBeString();
+			expectTypeOf(params.path).toEqualTypeOf<string | undefined>();
+			expectTypeOf(params.context).toEqualTypeOf({ someCustomSqlClient: {} });
+		});
+
+		umzug.on('migrated', params => {
+			expectTypeOf(params.name).toBeString();
+			expectTypeOf(params.path).toEqualTypeOf<string | undefined>();
+			expectTypeOf(params.context).toEqualTypeOf({ someCustomSqlClient: {} });
+		});
+
+		umzug.on('reverting', params => {
+			expectTypeOf(params.name).toBeString();
+			expectTypeOf(params.path).toEqualTypeOf<string | undefined>();
+			expectTypeOf(params.context).toEqualTypeOf({ someCustomSqlClient: {} });
+		});
+
+		umzug.on('reverted', params => {
+			expectTypeOf(params.name).toBeString();
+			expectTypeOf(params.path).toEqualTypeOf<string | undefined>();
+			expectTypeOf(params.context).toEqualTypeOf({ someCustomSqlClient: {} });
+		});
+	});
 });
 
 describe('error cases', () => {
@@ -703,45 +735,44 @@ describe('events', () => {
 			logger: undefined,
 		});
 
-		// `.addListener` and `.on` are aliases - use both to make sure they're wired up properly
-		umzug.addListener('migrating', spy('migrating'));
+		umzug.on('migrating', spy('migrating'));
 		umzug.on('migrated', spy('migrated'));
 
 		const revertingSpy = spy('reverting');
-		umzug.addListener('reverting', revertingSpy);
+		umzug.on('reverting', revertingSpy);
 		umzug.on('reverted', spy('reverted'));
 
 		await umzug.up();
 
 		expect(mock.mock.calls).toMatchObject([
-			['migrating', 'm1', { name: 'm1' }],
+			['migrating', { name: 'm1' }],
 			['up-m1', { name: 'm1' }],
-			['migrated', 'm1', { name: 'm1' }],
-			['migrating', 'm2', { name: 'm2' }],
+			['migrated', { name: 'm1' }],
+			['migrating', { name: 'm2' }],
 			['up-m2', { name: 'm2' }],
-			['migrated', 'm2', { name: 'm2' }],
+			['migrated', { name: 'm2' }],
 		]);
 
 		mock.mockClear();
 
 		await umzug.down();
 
 		expect(mock.mock.calls).toMatchObject([
-			['reverting', 'm2', { name: 'm2' }],
+			['reverting', { name: 'm2' }],
 			['down-m2', { name: 'm2' }],
-			['reverted', 'm2', { name: 'm2' }],
+			['reverted', { name: 'm2' }],
 		]);
 
 		mock.mockClear();
 
-		umzug.removeListener('reverting', revertingSpy);
+		umzug.off('reverting', revertingSpy);
 
 		await umzug.down();
 
 		expect(mock.mock.calls).toMatchObject([
 			// `reverting` shouldn't be here because the listener was removed
 			['down-m1', { name: 'm1' }],
-			['reverted', 'm1', { name: 'm1' }],
+			['reverted', { name: 'm1' }],
 		]);
 	});
 });