#843 - Use JS sort method to sort migrations by name (#844)

Co-authored-by: DavesBorges <[email protected]>
Co-authored-by: Igal Klebanov <[email protected]>

src/migration/migrator.ts

@@ -83,6 +83,7 @@ export class Migrator {
           .withPlugin(this.#schemaPlugin)
           .selectFrom(this.#migrationTable)
           .select(['name', 'timestamp'])
+          .$narrowType<{ name: string; timestamp: string }>()
           .execute()
       : []
 
@@ -577,11 +578,27 @@ export class Migrator {
     const executedMigrations = await db
       .withPlugin(this.#schemaPlugin)
       .selectFrom(this.#migrationTable)
-      .select('name')
-      .orderBy(['timestamp', 'name'])
+      .select(['name', 'timestamp'])
+      .$narrowType<{ name: string; timestamp: string }>()
       .execute()
 
-    return executedMigrations.map((it) => it.name)
+    const nameComparator =
+      this.#props.nameComparator || ((a, b) => a.localeCompare(b))
+
+    return (
+      executedMigrations
+        // https://github.com/kysely-org/kysely/issues/843
+        .sort((a, b) => {
+          if (a.timestamp === b.timestamp) {
+            return nameComparator(a.name, b.name)
+          }
+
+          return (
+            new Date(a.timestamp).getTime() - new Date(b.timestamp).getTime()
+          )
+        })
+        .map((it) => it.name)
+    )
   }
 
   #ensureNoMissingMigrations(
@@ -797,6 +814,14 @@ export interface MigratorProps {
    * order.
    */
   readonly allowUnorderedMigrations?: boolean
+
+  /**
+   * A function that compares migration names, used when sorting migrations in
+   * ascending order.
+   *
+   * Default is `name0.localeCompare(name1)`.
+   */
+  readonly nameComparator?: (name0: string, name1: string) => number
 }
 
 /**

test/node/src/camel-case.test.ts

@@ -313,7 +313,7 @@ for (const dialect of DIALECTS) {
       })
     })
 
-    it.only('should respect `underscoreBeforeDigits` and not add a second underscore in a nested query', async () => {
+    it('should respect `underscoreBeforeDigits` and not add a second underscore in a nested query', async () => {
       const db = camelDb
         .withoutPlugins()
         .withPlugin(new CamelCasePlugin({ underscoreBeforeDigits: true }))

test/node/src/migration.test.ts

@@ -597,6 +597,57 @@ for (const dialect of DIALECTS) {
           expect(executedUpMethods2).to.eql([])
           expect(executedDownMethods2).to.eql(['migration5', 'migration3'])
         })
+
+        it('should migrate down if allowUnorderedMigrations is enabled and migration names are not in order', async () => {
+          const [migrator1, executedUpMethods1] = createMigrations(
+            ['migration1', 'migration3'],
+            { allowUnorderedMigrations: true },
+          )
+
+          const { results: results1 } = await migrator1.migrateToLatest()
+
+          const [migrator2, executedUpMethods2] = createMigrations(
+            ['migration1', 'migration2', 'migration3'],
+            {
+              allowUnorderedMigrations: true,
+            },
+          )
+
+          const { results: results2 } = await migrator2.migrateToLatest()
+
+          expect(results1).to.eql([
+            { migrationName: 'migration1', direction: 'Up', status: 'Success' },
+            { migrationName: 'migration3', direction: 'Up', status: 'Success' },
+          ])
+
+          expect(results2).to.eql([
+            {
+              migrationName: 'migration2',
+              direction: 'Up',
+              status: 'Success',
+            },
+          ])
+
+          expect(executedUpMethods1).to.eql(['migration1', 'migration3'])
+          expect(executedUpMethods2).to.eql(['migration2'])
+
+          const [migrator3, executedUpMethods3, executedDownMethods3] =
+            createMigrations(['migration1', 'migration2', 'migration3'], {
+              allowUnorderedMigrations: true,
+            })
+
+          const { results: results3 } = await migrator3.migrateDown()
+          expect(results3).to.eql([
+            {
+              migrationName: 'migration2',
+              direction: 'Down',
+              status: 'Success',
+            },
+          ])
+
+          expect(executedUpMethods3).to.eql([])
+          expect(executedDownMethods3).to.eql(['migration2'])
+        })
       })
     })
 
@@ -792,6 +843,44 @@ for (const dialect of DIALECTS) {
           'migration1',
         ])
       })
+
+      describe('Migrate up should work when timestamps are equal', () => {
+        let originalToIsoString: typeof Date.prototype.toISOString
+
+        before(() => {
+          originalToIsoString = Date.prototype.toISOString
+          const defaultDateIsoString = new Date(2024, 0, 11).toISOString()
+          Date.prototype.toISOString = () => defaultDateIsoString
+        })
+
+        after(() => {
+          Date.prototype.toISOString = originalToIsoString
+        })
+
+        it('should use the same ordering strategy for migrations for both not executed migrations and executed migrations', async () => {
+          const [migrator1, executedUpMethods1] = createMigrations([
+            '2024-01-01-create-table',
+            '2024-01-01.2-update-table',
+          ])
+
+          await migrator1.migrateToLatest()
+
+          const [migrator2, executedUpMethods2] = createMigrations([
+            '2024-01-01-create-table',
+            '2024-01-01.2-update-table',
+          ])
+
+          const { results: results2, error } = await migrator2.migrateToLatest()
+          expect(error).to.be.undefined
+          expect(results2).to.eql([])
+
+          expect(executedUpMethods1).to.eql([
+            '2024-01-01-create-table',
+            '2024-01-01.2-update-table',
+          ])
+          expect(executedUpMethods2).to.eql([])
+        })
+      })
     })
 
     if (dialect === 'postgres') {