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

Typed async events #394

Merged
merged 5 commits into from
Nov 23, 2020
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
18 changes: 17 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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)
3 changes: 1 addition & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
41 changes: 29 additions & 12 deletions src/umzug.ts
Original file line number Diff line number Diff line change
@@ -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);

Expand Down Expand Up @@ -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 */
Expand All @@ -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 */
Expand Down Expand Up @@ -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>>>;

Expand All @@ -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 */
Expand All @@ -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);
}
Expand Down Expand Up @@ -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 }));
Expand Down Expand Up @@ -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 }));
Expand Down
53 changes: 42 additions & 11 deletions test/umzug.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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' }],
]);
});
});
Expand Down