Skip to content

Conversation

@jlucaso1
Copy link

@jlucaso1 jlucaso1 commented Jun 5, 2025

Hey team!

This PR addresses an issue where the Migrator would unexpectedly fail if initialized with a Kysely Transaction instance (trx) while disableTransactions was set to true. The Migrator was attempting to call trx.connection(), which isn't a supported operation on a Transaction object.

The Problem:

When disableTransactions is true, the Migrator falls back to using this.#props.db.connection().execute(...). If this.#props.db is a Transaction instance, this leads to:
Error: calling the connection method for a Transaction is not supported

This prevented a useful pattern: performing a "transactional dry-run" of migrations, where migrations are executed within an outer transaction that is subsequently rolled back.

The Solution:

This change modifies the Migrator to check if the provided db instance isTransaction. If it is, and disableTransactions is also true, the Migrator will now directly use the provided Transaction instance (this.#props.db) to run the migration logic (run(this.#props.db)), bypassing the problematic .connection() call.

Use Case Example (Transactional Migration Validation):

This fix makes it much easier to validate migrations without permanently altering the database, which is super handy for pre-push hooks or CI checks:

import { db } from './your-db-setup'; // Your main Kysely instance
import { Migrator, FileMigrationProvider } from 'kysely';

async function validateMigrationsDryRun() {
  console.log('Attempting transactional dry-run of migrations...');
  try {
    await db.transaction().execute(async (trx) => {
      const migrator = new Migrator({
        db: trx, // Pass the transaction
        provider: new FileMigrationProvider(...),
        disableTransactions: true, // Tell Migrator not to manage its own TX
      });

      const { error, results } = await migrator.migrateToLatest();

      if (error) {
        console.error('Migration validation failed:', error);
        throw error; // This will cause the outer transaction to rollback
      }

      console.log('Migrations appear valid!');
      // IMPORTANT: We must throw an error to force the rollback for a dry run
      throw new Error('DRY_RUN_ROLLBACK');
    });
  } catch (e: any) {
    if (e.message === 'DRY_RUN_ROLLBACK') {
      console.log('Dry run successful, all changes rolled back.');
    } else {
      console.error('Migration validation encountered an error:', e);
      // Potentially exit with error for CI
    }
  }
}

Testing:
I've added a test case to cover this specific scenario, ensuring the Migrator behaves as expected when used with a Transaction and disableTransactions: true.

@vercel
Copy link

vercel bot commented Jun 5, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
kysely 🛑 Canceled (Inspect) Jul 9, 2025 6:10pm

@pkg-pr-new
Copy link

pkg-pr-new bot commented Jun 29, 2025

kysely_koa_example

npm i https://pkg.pr.new/kysely-org/kysely@1480

commit: d98b472

Copy link
Member

@igalklebanov igalklebanov left a comment

Choose a reason for hiding this comment

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

Hey 👋

Thanks! 🙏

Left some comments.

This is not a fix. This is a whole use case that wasn't considered.

@igalklebanov igalklebanov changed the title fix(Migrator): Enable Transactional Dry-Runs by supporting Transaction Instances with disableTransactions: true feat(Migrator): allow passing transactions to Migrator. Jun 29, 2025
@igalklebanov igalklebanov added enhancement New feature or request migrations Related to migrations postgres Related to PostgreSQL mssql Related to MS SQL Server (MSSQL) labels Jun 29, 2025
…tions`

Previously, initializing Migrator with a `Transaction` instance (`trx`)
and `disableTransactions: true` would lead to an error because
Migrator would attempt `trx.connection()`.

This change ensures that if `db` is already a `Transaction` and
`disableTransactions` is true, the Migrator uses the provided `trx`
directly, enabling transactional dry-runs.
jlucaso1 and others added 6 commits July 9, 2025 15:09
Co-authored-by: Igal Klebanov <igalklebanov@gmail.com>
Co-authored-by: Igal Klebanov <igalklebanov@gmail.com>
Co-authored-by: Igal Klebanov <igalklebanov@gmail.com>
Co-authored-by: Igal Klebanov <igalklebanov@gmail.com>
Co-authored-by: Igal Klebanov <igalklebanov@gmail.com>
Co-authored-by: Igal Klebanov <igalklebanov@gmail.com>
@tarqwara
Copy link

tarqwara commented Sep 1, 2025

Any updates on this?

@igalklebanov igalklebanov changed the base branch from master to next October 12, 2025 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request migrations Related to migrations mssql Related to MS SQL Server (MSSQL) postgres Related to PostgreSQL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants