Check if NaN before incrementing highestMigrationNumber

.changeset/afraid-singers-pretend.md

@@ -0,0 +1,7 @@
+---
+"wrangler": patch
+---
+
+fix: number d1 migrations properly even if previous migrations aren't prefixed by a number
+
+We expected d1 migration names to be "0001_migration-name.sql", but a user could manually create a migration that in this format like "init.sql". Subsequent migrations would be named "0NaN_migration-name.sql" - this fixes that bug.

packages/wrangler/src/__tests__/d1/migrate.test.ts

@@ -1,3 +1,4 @@
+import * as fs from "fs";
 import { cwd } from "process";
 import { http, HttpResponse } from "msw";
 import { reinitialiseAuthTokens } from "../../user";
@@ -14,7 +15,7 @@ import { writeWranglerConfig } from "../helpers/write-wrangler-config";
 
 describe("migrate", () => {
 	runInTempDir();
-	mockConsoleMethods();
+	const std = mockConsoleMethods();
 	mockSetTimeout();
 
 	const { setIsTTY } = useMockIsTTY();
@@ -32,6 +33,28 @@ describe("migrate", () => {
 				runWrangler("d1 migrations create test some-message --local DATABASE")
 			).rejects.toThrowError(`Unknown argument: local`);
 		});
+
+		it("numbers a migration even if prior migrations do not have a number prefix", async () => {
+			setIsTTY(false);
+			writeWranglerConfig({
+				d1_databases: [
+					{ binding: "DATABASE", database_name: "db", database_id: "xxxx" },
+				],
+			});
+			fs.mkdirSync("./migrations");
+			fs.writeFileSync("./migrations/init.sql", "");
+			await runWrangler("d1 migrations create db some-message");
+
+			mockConfirm({
+				text: `About to apply 1 migration(s)
+Your database may not be available to serve requests during the migration, continue?`,
+				result: true,
+			});
+			// regression test for 0NaN_some-message.sql
+			expect(std.out).toContain(
+				`✅ Successfully created Migration '0001_some-message.sql'!`
+			);
+		});
 	});
 
 	describe("apply", () => {

packages/wrangler/src/d1/migrations/helpers.ts

@@ -150,8 +150,11 @@ function getMigrationNames(migrationsPath: string): Array<string> {
  * Returns the highest current migration number plus one, ignoring any missing numbers.
  */
 export function getNextMigrationNumber(migrationsPath: string): number {
-	const migrationNumbers = getMigrationNames(migrationsPath).map((migration) =>
-		parseInt(migration.split("_")[0])
+	const migrationNumbers = getMigrationNames(migrationsPath).map(
+		(migration) => {
+			const num = parseInt(migration.split("_")[0]);
+			return isNaN(num) ? 0 : num;
+		}
 	);
 	const highestMigrationNumber = Math.max(...migrationNumbers, 0);