feat(migrator): allow disabling transactions in migrate methods. (#1517)

Author: igalklebanov

src/migration/migrator.ts

@@ -146,8 +146,8 @@ export class Migrator {
    * }
    * ```
    */
-  async migrateToLatest(): Promise<MigrationResultSet> {
-    return this.#migrate(() => ({ direction: 'Up', step: Infinity }))
+  async migrateToLatest(options?: MigrateOptions): Promise<MigrationResultSet> {
+    return this.#migrate(() => ({ direction: 'Up', step: Infinity }), options)
   }
 
   /**
@@ -203,6 +203,7 @@ export class Migrator {
    */
   async migrateTo(
     targetMigrationName: string | NoMigrations,
+    options?: MigrateOptions,
   ): Promise<MigrationResultSet> {
     return this.#migrate(
       ({
@@ -226,6 +227,7 @@ export class Migrator {
         const executedIndex = executedMigrations.indexOf(
           targetMigrationName as string,
         )
+
         const pendingIndex = pendingMigrations.findIndex(
           (m) => m.name === (targetMigrationName as string),
         )
@@ -235,14 +237,17 @@ export class Migrator {
             direction: 'Down',
             step: executedMigrations.length - executedIndex - 1,
           }
-        } else if (pendingIndex !== -1) {
+        }
+
+        if (pendingIndex !== -1) {
           return { direction: 'Up', step: pendingIndex + 1 }
-        } else {
-          throw new Error(
-            `migration "${targetMigrationName}" isn't executed or pending`,
-          )
         }
+
+        throw new Error(
+          `migration "${targetMigrationName}" isn't executed or pending`,
+        )
       },
+      options,
     )
   }
 
@@ -274,8 +279,8 @@ export class Migrator {
    * await migrator.migrateUp()
    * ```
    */
-  async migrateUp(): Promise<MigrationResultSet> {
-    return this.#migrate(() => ({ direction: 'Up', step: 1 }))
+  async migrateUp(options?: MigrateOptions): Promise<MigrationResultSet> {
+    return this.#migrate(() => ({ direction: 'Up', step: 1 }), options)
   }
 
   /**
@@ -306,19 +311,20 @@ export class Migrator {
    * await migrator.migrateDown()
    * ```
    */
-  async migrateDown(): Promise<MigrationResultSet> {
-    return this.#migrate(() => ({ direction: 'Down', step: 1 }))
+  async migrateDown(options?: MigrateOptions): Promise<MigrationResultSet> {
+    return this.#migrate(() => ({ direction: 'Down', step: 1 }), options)
   }
 
   async #migrate(
     getMigrationDirectionAndStep: (state: MigrationState) => {
       direction: MigrationDirection
       step: number
     },
+    options: MigrateOptions | undefined,
   ): Promise<MigrationResultSet> {
     try {
       await this.#ensureMigrationTablesExists()
-      return await this.#runMigrations(getMigrationDirectionAndStep)
+      return await this.#runMigrations(getMigrationDirectionAndStep, options)
     } catch (error) {
       if (error instanceof MigrationResultSetError) {
         return error.resultSet
@@ -489,6 +495,7 @@ export class Migrator {
       direction: MigrationDirection
       step: number
     },
+    options: MigrateOptions | undefined,
   ): Promise<MigrationResultSet> {
     const adapter = this.#props.db.getExecutor().adapter
 
@@ -526,11 +533,14 @@ export class Migrator {
       }
     }
 
-    if (adapter.supportsTransactionalDdl && !this.#props.disableTransactions) {
-      return this.#props.db.transaction().execute(run)
-    } else {
+    const disableTransaction =
+      options?.disableTransactions ?? this.#props.disableTransactions
+
+    if (!adapter.supportsTransactionalDdl || disableTransaction) {
       return this.#props.db.connection().execute(run)
     }
+
+    return this.#props.db.transaction().execute(run)
   }
 
   async #getState(db: Kysely<any>): Promise<MigrationState> {
@@ -752,7 +762,18 @@ export class Migrator {
   }
 }
 
-export interface MigratorProps {
+export interface MigrateOptions {
+  /**
+   * When `true`, don't run migrations in transactions even if the dialect supports transactional DDL.
+   *
+   * Default is `false`.
+   *
+   * This is useful when some migrations include queries that would fail otherwise.
+   */
+  readonly disableTransactions?: boolean
+}
+
+export interface MigratorProps extends MigrateOptions {
   readonly db: Kysely<any>
   readonly provider: MigrationProvider
 
@@ -825,15 +846,6 @@ export interface MigratorProps {
    * Default is `name0.localeCompare(name1)`.
    */
   readonly nameComparator?: (name0: string, name1: string) => number
-
-  /**
-   * When `true`, don't run migrations in transactions even if the dialect supports transactional DDL.
-   *
-   * Default is `false`.
-   *
-   * This is useful when some migrations include queries that would fail otherwise.
-   */
-  readonly disableTransactions?: boolean
 }
 
 /**

test/node/src/migration.test.ts

@@ -791,7 +791,7 @@ for (const dialect of DIALECTS) {
         expect(executedUpMethods2).to.eql(['migration2', 'migration4'])
       })
 
-      it('should not execute in transaction if disableTransactions is true', async () => {
+      it('should not execute in transaction if disableTransactions is true on the `Migrator` instance', async () => {
         const [migrator, executedUpMethods] = createMigrations(['migration1'], {
           disableTransactions: true,
         })
@@ -807,7 +807,25 @@ for (const dialect of DIALECTS) {
         expect(transactionSpy.called).to.be.false
       })
 
-      it('should execute in transaction if disableTransactions is false and transactionDdl supported', async () => {
+      it('should not execute in transaction if disableTransactions is true when calling `migrateUp`', async () => {
+        const [migrator, executedUpMethods] = createMigrations(['migration1'], {
+          disableTransactions: false,
+        })
+
+        const { results } = await migrator.migrateUp({
+          disableTransactions: true,
+        })
+
+        expect(results).to.eql([
+          { migrationName: 'migration1', direction: 'Up', status: 'Success' },
+        ])
+
+        expect(executedUpMethods).to.eql(['migration1'])
+
+        expect(transactionSpy.called).to.be.false
+      })
+
+      it('should execute in transaction if disableTransactions is false on the `Migrator` instance and transactionDdl supported', async () => {
         const [migrator, executedUpMethods] = createMigrations(['migration1'], {
           disableTransactions: false,
         })
@@ -826,6 +844,28 @@ for (const dialect of DIALECTS) {
           expect(transactionSpy.called).to.be.false
         }
       })
+
+      it('should execute in transaction if disableTransactions is false when calling `migrateUp` and transactionDdl supported', async () => {
+        const [migrator, executedUpMethods] = createMigrations(['migration1'], {
+          disableTransactions: true,
+        })
+
+        const { results } = await migrator.migrateUp({
+          disableTransactions: false,
+        })
+
+        expect(results).to.eql([
+          { migrationName: 'migration1', direction: 'Up', status: 'Success' },
+        ])
+
+        expect(executedUpMethods).to.eql(['migration1'])
+
+        if (ctx.db.getExecutor().adapter.supportsTransactionalDdl) {
+          expect(transactionSpy.called).to.be.true
+        } else {
+          expect(transactionSpy.called).to.be.false
+        }
+      })
     })
 
     describe('migrateDown', () => {