Fix bug in commit error handling

.changeset/full-suns-press.md

@@ -0,0 +1,6 @@
+---
+"graphile-migrate": patch
+---
+
+Fixes a bug in error handling for `commit` where `--! include` comments were not
+restored, instead being replaced by the included assets.

__tests__/commit.test.ts

@@ -30,8 +30,10 @@ it("aborts if current.sql is empty", async () => {
 });
 
 it("restores migration headers after a failed commit", async () => {
-  const original = "--! Message: test-migration\n\nselect 1 + ;\n";
+  const original =
+    "--! Message: test-migration\n\nselect 1 + ;\n--! include some-fixture.sql\n";
   mockFs({
+    "migrations/fixtures/some-fixture.sql": "select 1;\n",
     "migrations/current.sql": original,
   });
 

__tests__/readCurrentMigration.test.ts

@@ -28,14 +28,14 @@ it("reads from current.sql", async () => {
   const currentLocation = await getCurrentMigrationLocation(parsedSettings);
 
   const content = await readCurrentMigration(parsedSettings, currentLocation);
-  expect(content).toEqual("-- TEST");
+  expect(content).toEqual({ raw: "-- TEST", compiled: "-- TEST" });
 });
 
 it("returns empty if there's no current.sql", async () => {
   const currentLocation = await getCurrentMigrationLocation(parsedSettings);
 
   const content = await readCurrentMigration(parsedSettings, currentLocation);
-  expect(content).toEqual("");
+  expect(content).toEqual({ raw: "", compiled: "" });
 });
 
 it("returns empty if there's an empty current/", async () => {
@@ -45,7 +45,7 @@ it("returns empty if there's an empty current/", async () => {
   const currentLocation = await getCurrentMigrationLocation(parsedSettings);
 
   const content = await readCurrentMigration(parsedSettings, currentLocation);
-  expect(content).toEqual("");
+  expect(content).toEqual({ raw: "", compiled: "" });
 });
 
 const contentWithSplits = `\
@@ -80,7 +80,7 @@ With multiple lines
 
   const currentLocation = await getCurrentMigrationLocation(parsedSettings);
   const content = await readCurrentMigration(parsedSettings, currentLocation);
-  expect(content).toEqual(contentWithSplits);
+  expect(content.compiled).toEqual(contentWithSplits);
 });
 
 it("ignores extraneous files", async () => {
@@ -100,7 +100,7 @@ With multiple lines
 
   const currentLocation = await getCurrentMigrationLocation(parsedSettings);
   const content = await readCurrentMigration(parsedSettings, currentLocation);
-  expect(content).toEqual(contentWithSplits);
+  expect(content.compiled).toEqual(contentWithSplits);
 });
 
 it("reads from current.sql, and processes included files", async () => {
@@ -111,7 +111,7 @@ it("reads from current.sql, and processes included files", async () => {
 
   const currentLocation = await getCurrentMigrationLocation(parsedSettings);
   const content = await readCurrentMigration(parsedSettings, currentLocation);
-  expect(content).toEqual(`\
+  expect(content.compiled).toEqual(`\
 --! Included foo_current.sql
 -- TEST from foo
 --! EndIncluded foo_current.sql`);

src/commands/commit.ts

@@ -59,7 +59,7 @@ export async function _commit(
 
   const { headers, body } = parseMigrationText(
     currentLocation.path,
-    contents,
+    contents.compiled,
     false,
   );
   const messageFromComment = headers.Message;
@@ -119,7 +119,7 @@ export async function _commit(
     logDbError(parsedSettings, e);
 
     parsedSettings.logger.error("ABORTING...");
-    await writeCurrentMigration(parsedSettings, currentLocation, contents);
+    await writeCurrentMigration(parsedSettings, currentLocation, contents.raw);
     await fsp.unlink(newMigrationFilepath);
     parsedSettings.logger.error("ABORTED AND ROLLED BACK");
     throw e;

src/commands/status.ts

@@ -25,7 +25,7 @@ async function _status(
   // Checks that don't need a database connection
   const currentLocation = await getCurrentMigrationLocation(parsedSettings);
   const body = await readCurrentMigration(parsedSettings, currentLocation);
-  const minifiedBody = pgMinify(body);
+  const minifiedBody = pgMinify(body.compiled);
   const hasCurrentMigration = minifiedBody !== "";
 
   // Checks that need a database connection

src/commands/uncommit.ts

@@ -27,7 +27,7 @@ export async function _uncommit(parsedSettings: ParsedSettings): Promise<void> {
 
   // Check current.sql is blank
   const currentLocation = await getCurrentMigrationLocation(parsedSettings);
-  const currentBody = await readCurrentMigration(
+  const { compiled: currentBody } = await readCurrentMigration(
     parsedSettings,
     currentLocation,
   );

src/current.ts

@@ -55,9 +55,10 @@ export async function getCurrentMigrationLocation(
 export async function readCurrentMigration(
   parsedSettings: ParsedSettings,
   location: CurrentMigrationLocation,
-): Promise<string> {
-  const { body, paths } = await readCurrentMigrationRaw(location);
-  return compileIncludes(parsedSettings, body, new Set(paths));
+): Promise<{ raw: string; compiled: string }> {
+  const { body: raw, paths } = await readCurrentMigrationRaw(location);
+  const compiled = await compileIncludes(parsedSettings, raw, new Set(paths));
+  return { raw, compiled };
 }
 
 async function readCurrentMigrationRaw(

src/currentRunner.ts

@@ -19,7 +19,10 @@ export function makeCurrentMigrationRunner(
   const { shadow = false, forceActions = false } = options;
   async function run(): Promise<void> {
     const currentLocation = await getCurrentMigrationLocation(parsedSettings);
-    const body = await readCurrentMigration(parsedSettings, currentLocation);
+    const { compiled: body } = await readCurrentMigration(
+      parsedSettings,
+      currentLocation,
+    );
     let migrationsAreEquivalent = false;
 
     try {