From 2911d20d936cfebe2167e1b501d1d02138ccf50c Mon Sep 17 00:00:00 2001
From: Misha Kaletsky <15040698+mmkal@users.noreply.github.com>
Date: Mon, 13 Dec 2021 00:29:55 +0000
Subject: [PATCH] Remove `.extend(...)` in favour of constructor (#523)

Co-authored-by: Misha Kaletsky <mmkal@users.noreply.github.com>
---
 README.md          | 19 +++++++++++--------
 src/umzug.ts       | 23 +----------------------
 test/umzug.test.ts | 21 +++++++++------------
 3 files changed, 21 insertions(+), 42 deletions(-)

diff --git a/README.md b/README.md
index 5ed473a9..e5d24b77 100644
--- a/README.md
+++ b/README.md
@@ -408,7 +408,7 @@ Note on migration file sorting:
   - so if your migrations are `one/m1.js`, `two/m2.js`, `three/m3.js`, the resultant order will be `one/m1.js`, `three/m3.js`, `two/m2.js`
   - similarly, if your migrations are called `m1.js`, `m2.js`, ... `m10.js`, `m11.js`, the resultant ordering will be `m1.js`, `m10.js`, `m11.js`, ... `m2.js`
 - The easiest way to deal with this is to ensure your migrations appear in a single folder, and their paths match lexicographically with the order they should run in
-- If this isn't possible, the ordering can be customised using `.extend(...)` (see below)
+- If this isn't possible, the ordering can be customised using a new instance (previously, in the beta release for v3, this could be done with `.extend(...)` - see below for example using a new instance)
 
 ### Upgrading from v2.x
 
@@ -472,17 +472,20 @@ const umzug = new Umzug({
 });
 ```
 
-Similarly, you no longer need `migrationSorting`, you can use `Umzug#extend` to manipulate migration lists directly:
+Similarly, you no longer need `migrationSorting`, you can instantiate a new `Umzug` instance to manipulate migration lists directly:
 
 ```js
 const { Umzug } = require('umzug');
 
-const umzug =
-  new Umzug({
-    migrations: { glob: 'migrations/**/*.js' },
-    context: sequelize.getQueryInterface(),
-  })
-  .extend(migrations => migrations.sort((a, b) => b.path.localeCompare(a.path)));
+const parent = new Umzug({
+  migrations: { glob: 'migrations/**/*.js' },
+  context: sequelize.getQueryInterface(),
+})
+
+const umzug = new Umzug({
+  ...parent.options,
+  migrations: ctx => (await parent.migrations()).sort((a, b) => b.path.localeCompare(a.path))
+})
 ```
 
 ### Storages
diff --git a/src/umzug.ts b/src/umzug.ts
index fed362d1..393878da 100644
--- a/src/umzug.ts
+++ b/src/umzug.ts
@@ -14,7 +14,6 @@ import {
 	MigrateUpOptions,
 	MigrationMeta,
 	MigrationParams,
-	Promisable,
 	RerunBehavior,
 	Resolver,
 	RunnableMigration,
@@ -65,7 +64,6 @@ export class MigrationError extends VError {
 
 export class Umzug<Ctx extends object = object> extends emittery<UmzugEvents<Ctx>> {
 	private readonly storage: UmzugStorage<Ctx>;
-	/** @internal */
 	readonly migrations: (ctx: Ctx) => Promise<ReadonlyArray<RunnableMigration<Ctx>>>;
 
 	/**
@@ -98,10 +96,7 @@ export class Umzug<Ctx extends object = object> extends emittery<UmzugEvents<Ctx
 	};
 
 	/** creates a new Umzug instance */
-	constructor(
-		/** @internal */
-		readonly options: UmzugOptions<Ctx>
-	) {
+	constructor(readonly options: UmzugOptions<Ctx>) {
 		super();
 
 		this.storage = verifyUmzugStorage(options.storage ?? new JSONStorage());
@@ -175,22 +170,6 @@ export class Umzug<Ctx extends object = object> extends emittery<UmzugEvents<Ctx
 		return cli.execute(argv);
 	}
 
-	/**
-	 * create a clone of the current Umzug instance, allowing customising the list of migrations.
-	 * This could be used, for example, to sort the list of migrations in a specific order.
-	 */
-	extend(
-		transform: (migrations: ReadonlyArray<RunnableMigration<Ctx>>) => Promisable<Array<RunnableMigration<Ctx>>>
-	): Umzug<Ctx> {
-		return new Umzug({
-			...this.options,
-			migrations: async context => {
-				const migrations = await this.migrations(context);
-				return transform(migrations);
-			},
-		});
-	}
-
 	/** Get the list of migrations which have already been applied */
 	async executed(): Promise<MigrationMeta[]> {
 		return this.runCommand('executed', async ({ context }) => {
diff --git a/test/umzug.test.ts b/test/umzug.test.ts
index acd4a6c0..eb4c5956 100644
--- a/test/umzug.test.ts
+++ b/test/umzug.test.ts
@@ -616,7 +616,7 @@ describe('alternate migration inputs', () => {
 		});
 	});
 
-	test('allows customization via getMigrations', async () => {
+	test('allows customization via parent instance', async () => {
 		const spy = jest.fn();
 
 		const syncer = fsSyncer(path.join(__dirname, 'generated/umzug/customOrdering'), {
@@ -639,7 +639,10 @@ describe('alternate migration inputs', () => {
 			logger: undefined,
 		});
 
-		const umzug = parent.extend(migrations => migrations.slice().reverse());
+		const umzug = new Umzug({
+			...parent.options,
+			migrations: async context => (await parent.migrations(context)).slice().reverse(),
+		});
 
 		await umzug.up();
 
@@ -683,8 +686,10 @@ describe('alternate migration inputs', () => {
 			logger: undefined,
 		});
 
-		const umzug = withDefaultOrdering.extend(standard => {
-			return standard.slice().sort((a, b) => a.name.localeCompare(b.name));
+		const umzug = new Umzug({
+			...withDefaultOrdering.options,
+			migrations: async ctx =>
+				(await withDefaultOrdering.migrations(ctx)).slice().sort((a, b) => a.name.localeCompare(b.name)),
 		});
 
 		await umzug.up();
@@ -863,14 +868,6 @@ describe('types', () => {
 		});
 	});
 
-	test(`extend function doesn't allow modifying migrations array`, () => {
-		const parent = new Umzug({ migrations: [], logger: undefined });
-		parent.extend(migrations => {
-			expectTypeOf(migrations).not.toHaveProperty('reverse');
-			return migrations.slice();
-		});
-	});
-
 	test('event types', () => {
 		const umzug = new Umzug({
 			migrations: [],