From 121492b30309d041fa867b7e825a5826269ee093 Mon Sep 17 00:00:00 2001
From: Joakim Carlstein <joakim@aboviq.com>
Date: Mon, 22 Jan 2024 13:39:38 +0100
Subject: [PATCH] fix(cli): sort migrations lexicographically for real

---
 .changeset/funny-pants-heal.md          |   5 +
 packages/cli/src/get-migrations.test.ts | 134 ++++++++++++++++++++++++
 packages/cli/src/get-migrations.ts      |  16 +--
 3 files changed, 144 insertions(+), 11 deletions(-)
 create mode 100644 .changeset/funny-pants-heal.md
 create mode 100644 packages/cli/src/get-migrations.test.ts

diff --git a/.changeset/funny-pants-heal.md b/.changeset/funny-pants-heal.md
new file mode 100644
index 0000000..2e4e2e7
--- /dev/null
+++ b/.changeset/funny-pants-heal.md
@@ -0,0 +1,5 @@
+---
+'@emigrate/cli': patch
+---
+
+Sort migration files lexicographically correctly by using the default Array.sort implementation
diff --git a/packages/cli/src/get-migrations.test.ts b/packages/cli/src/get-migrations.test.ts
new file mode 100644
index 0000000..3391b04
--- /dev/null
+++ b/packages/cli/src/get-migrations.test.ts
@@ -0,0 +1,134 @@
+import fs from 'node:fs/promises';
+import { afterEach, beforeEach, describe, it, mock } from 'node:test';
+import assert from 'node:assert';
+import { getMigrations } from './get-migrations.js';
+
+const originalReaddir = fs.readdir;
+const readdirMock = mock.fn(originalReaddir);
+
+describe('get-migrations', () => {
+  beforeEach(() => {
+    fs.readdir = readdirMock;
+  });
+
+  afterEach(() => {
+    readdirMock.mock.restore();
+    fs.readdir = originalReaddir;
+  });
+
+  it('should skip files with leading periods', async () => {
+    readdirMock.mock.mockImplementation(async () => ['.foo.js', 'bar.js', 'baz.js']);
+
+    const migrations = await getMigrations('/cwd/', 'directory');
+
+    assert.deepStrictEqual(migrations, [
+      {
+        name: 'bar.js',
+        filePath: '/cwd/directory/bar.js',
+        relativeFilePath: 'directory/bar.js',
+        extension: '.js',
+        directory: 'directory',
+        cwd: '/cwd/',
+      },
+      {
+        name: 'baz.js',
+        filePath: '/cwd/directory/baz.js',
+        relativeFilePath: 'directory/baz.js',
+        extension: '.js',
+        directory: 'directory',
+        cwd: '/cwd/',
+      },
+    ]);
+  });
+
+  it('should skip files with leading underscores', async () => {
+    readdirMock.mock.mockImplementation(async () => ['_foo.js', 'bar.js', 'baz.js']);
+
+    const migrations = await getMigrations('/cwd/', 'directory');
+
+    assert.deepStrictEqual(migrations, [
+      {
+        name: 'bar.js',
+        filePath: '/cwd/directory/bar.js',
+        relativeFilePath: 'directory/bar.js',
+        extension: '.js',
+        directory: 'directory',
+        cwd: '/cwd/',
+      },
+      {
+        name: 'baz.js',
+        filePath: '/cwd/directory/baz.js',
+        relativeFilePath: 'directory/baz.js',
+        extension: '.js',
+        directory: 'directory',
+        cwd: '/cwd/',
+      },
+    ]);
+  });
+
+  it('should skip files without file extensions', async () => {
+    readdirMock.mock.mockImplementation(async () => ['foo', 'bar.js', 'baz.js']);
+
+    const migrations = await getMigrations('/cwd/', 'directory');
+
+    assert.deepStrictEqual(migrations, [
+      {
+        name: 'bar.js',
+        filePath: '/cwd/directory/bar.js',
+        relativeFilePath: 'directory/bar.js',
+        extension: '.js',
+        directory: 'directory',
+        cwd: '/cwd/',
+      },
+      {
+        name: 'baz.js',
+        filePath: '/cwd/directory/baz.js',
+        relativeFilePath: 'directory/baz.js',
+        extension: '.js',
+        directory: 'directory',
+        cwd: '/cwd/',
+      },
+    ]);
+  });
+
+  it('should sort them in lexicographical order', async () => {
+    readdirMock.mock.mockImplementation(async () => ['foo.js', 'bar_data.js', 'bar.js', 'baz.js']);
+
+    const migrations = await getMigrations('/cwd/', 'directory');
+
+    assert.deepStrictEqual(migrations, [
+      {
+        name: 'bar.js',
+        filePath: '/cwd/directory/bar.js',
+        relativeFilePath: 'directory/bar.js',
+        extension: '.js',
+        directory: 'directory',
+        cwd: '/cwd/',
+      },
+      {
+        name: 'bar_data.js',
+        filePath: '/cwd/directory/bar_data.js',
+        relativeFilePath: 'directory/bar_data.js',
+        extension: '.js',
+        directory: 'directory',
+        cwd: '/cwd/',
+      },
+      {
+        name: 'baz.js',
+        filePath: '/cwd/directory/baz.js',
+        relativeFilePath: 'directory/baz.js',
+        extension: '.js',
+        directory: 'directory',
+        cwd: '/cwd/',
+      },
+      {
+        name: 'foo.js',
+        filePath: '/cwd/directory/foo.js',
+        relativeFilePath: 'directory/foo.js',
+        extension: '.js',
+        directory: 'directory',
+        cwd: '/cwd/',
+      },
+    ]);
+  });
+});
diff --git a/packages/cli/src/get-migrations.ts b/packages/cli/src/get-migrations.ts
index e208ac7..228b132 100644
--- a/packages/cli/src/get-migrations.ts
+++ b/packages/cli/src/get-migrations.ts
@@ -1,17 +1,14 @@
 import path from 'node:path';
 import fs from 'node:fs/promises';
-import { type Dirent } from 'node:fs';
 import { type MigrationMetadata } from '@emigrate/types';
 import { withLeadingPeriod } from './with-leading-period.js';
 import { BadOptionError } from './errors.js';
 
 export type GetMigrationsFunction = typeof getMigrations;
 
-const tryReadDirectory = async (directoryPath: string): Promise<Dirent[]> => {
+const tryReadDirectory = async (directoryPath: string): Promise<string[]> => {
   try {
-    return await fs.readdir(directoryPath, {
-      withFileTypes: true,
-    });
+    return await fs.readdir(directoryPath);
   } catch {
     throw BadOptionError.fromOption('directory', `Couldn't read directory: ${directoryPath}`);
   }
@@ -23,12 +20,9 @@ export const getMigrations = async (cwd: string, directory: string): Promise<Mig
   const allFilesInMigrationDirectory = await tryReadDirectory(directoryPath);
 
   const migrationFiles: MigrationMetadata[] = allFilesInMigrationDirectory
-    .filter(
-      (file) =>
-        file.isFile() && !file.name.startsWith('.') && !file.name.startsWith('_') && path.extname(file.name) !== '',
-    )
-    .sort((a, b) => a.name.localeCompare(b.name))
-    .map(({ name }) => {
+    .filter((name) => !name.startsWith('.') && !name.startsWith('_') && path.extname(name) !== '')
+    .sort()
+    .map((name) => {
       const filePath = path.join(directoryPath, name);
 
       return {