fix: warn on unexpected fields on migrations (#1166)

This adds a warning for unexpected fields on `[migrations]` config, reported in #1165. It also adds a test for incorrect `renamed_classes` in a migration.

Author: threepointone

.changeset/heavy-monkeys-relax.md

@@ -0,0 +1,7 @@
+---
+"wrangler": patch
+---
+
+fix: warn on unexpected fields on migrations
+
+This adds a warning for unexpected fields on `[migrations]` config, reported in https://github.com/cloudflare/wrangler2/issues/1165. It also adds a test for incorrect `renamed_classes` in a migration.

packages/wrangler/src/__tests__/configuration.test.ts

@@ -165,68 +165,110 @@ describe("normalizeAndValidateConfig()", () => {
       `);
     });
 
-    it("should override `migrations` config defaults with provided values", () => {
-      const expectedConfig: RawConfig = {
-        migrations: [
-          {
-            tag: "TAG",
-            new_classes: ["CLASS_1", "CLASS_2"],
-            renamed_classes: [
-              {
-                from: "FROM_CLASS",
-                to: "TO_CLASS",
-              },
-            ],
-            deleted_classes: ["CLASS_3", "CLASS_4"],
-          },
-        ],
-      };
+    describe("migrations", () => {
+      it("should override `migrations` config defaults with provided values", () => {
+        const expectedConfig: RawConfig = {
+          migrations: [
+            {
+              tag: "TAG",
+              new_classes: ["CLASS_1", "CLASS_2"],
+              renamed_classes: [
+                {
+                  from: "FROM_CLASS",
+                  to: "TO_CLASS",
+                },
+              ],
+              deleted_classes: ["CLASS_3", "CLASS_4"],
+            },
+          ],
+        };
 
-      const { config, diagnostics } = normalizeAndValidateConfig(
-        expectedConfig,
-        undefined,
-        { env: undefined }
-      );
+        const { config, diagnostics } = normalizeAndValidateConfig(
+          expectedConfig,
+          undefined,
+          { env: undefined }
+        );
 
-      expect(config).toEqual(expect.objectContaining(expectedConfig));
-      expect(diagnostics.hasErrors()).toBe(false);
-      expect(diagnostics.hasWarnings()).toBe(false);
-    });
+        expect(config).toEqual(expect.objectContaining(expectedConfig));
+        expect(diagnostics.hasErrors()).toBe(false);
+        expect(diagnostics.hasWarnings()).toBe(false);
+      });
 
-    it("should error on invalid `migrations` values", () => {
-      const expectedConfig = {
-        migrations: [
-          {
-            tag: 111,
-            new_classes: [222, 333],
-            renamed_classes: [
-              {
-                from: 444,
-                to: 555,
-              },
-            ],
-            deleted_classes: [666, 777],
-          },
-        ],
-      };
+      it("should error on invalid `migrations` values", () => {
+        const expectedConfig = {
+          migrations: [
+            {
+              tag: 111,
+              new_classes: [222, 333],
+              renamed_classes: [
+                {
+                  from: 444,
+                  to: 555,
+                },
+              ],
+              deleted_classes: [666, 777],
+            },
+          ],
+        };
 
-      const { config, diagnostics } = normalizeAndValidateConfig(
-        expectedConfig as unknown as RawConfig,
-        undefined,
-        { env: undefined }
-      );
+        const { config, diagnostics } = normalizeAndValidateConfig(
+          expectedConfig as unknown as RawConfig,
+          undefined,
+          { env: undefined }
+        );
 
-      expect(config).toEqual(expect.objectContaining(expectedConfig));
-      expect(diagnostics.hasWarnings()).toBe(false);
-      expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
-        "Processing wrangler configuration:
-          - Expected \\"migrations[0].tag\\" to be of type string but got 111.
-          - Expected \\"migrations[0].new_classes.[0]\\" to be of type string but got 222.
-          - Expected \\"migrations[0].new_classes.[1]\\" to be of type string but got 333.
-          - Expected \\"migrations[0].renamed_classes\\" to be an array of \\"{from: string, to: string}\\" objects but got [{\\"from\\":444,\\"to\\":555}].
-          - Expected \\"migrations[0].deleted_classes.[0]\\" to be of type string but got 666.
-          - Expected \\"migrations[0].deleted_classes.[1]\\" to be of type string but got 777."
-      `);
+        expect(config).toEqual(expect.objectContaining(expectedConfig));
+        expect(diagnostics.hasWarnings()).toBe(false);
+        expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
+          "Processing wrangler configuration:
+            - Expected \\"migrations[0].tag\\" to be of type string but got 111.
+            - Expected \\"migrations[0].new_classes.[0]\\" to be of type string but got 222.
+            - Expected \\"migrations[0].new_classes.[1]\\" to be of type string but got 333.
+            - Expected \\"migrations[0].renamed_classes\\" to be an array of \\"{from: string, to: string}\\" objects but got [{\\"from\\":444,\\"to\\":555}].
+            - Expected \\"migrations[0].deleted_classes.[0]\\" to be of type string but got 666.
+            - Expected \\"migrations[0].deleted_classes.[1]\\" to be of type string but got 777."
+        `);
+      });
+
+      it("should warn/error on unexpected fields on `migrations`", async () => {
+        const expectedConfig = {
+          migrations: [
+            {
+              tag: "TAG",
+              new_classes: ["CLASS_1", "CLASS_2"],
+              renamed_classes: [
+                {
+                  from: "FROM_CLASS",
+                  to: "TO_CLASS",
+                },
+                {
+                  a: "something",
+                  b: "someone",
+                },
+              ],
+              deleted_classes: ["CLASS_3", "CLASS_4"],
+              unrecognized_field: "FOO",
+            },
+          ],
+        };
+
+        const { config, diagnostics } = normalizeAndValidateConfig(
+          expectedConfig as unknown as RawConfig,
+          undefined,
+          { env: undefined }
+        );
+
+        expect(config).toEqual(expect.objectContaining(expectedConfig));
+        expect(diagnostics.hasErrors()).toBe(true);
+        expect(diagnostics.renderWarnings()).toMatchInlineSnapshot(`
+          "Processing wrangler configuration:
+            - Unexpected fields found in migrations field: \\"unrecognized_field\\""
+        `);
+        expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
+          "Processing wrangler configuration:
+            - Expected \\"migrations[0].renamed_classes\\" to be an array of \\"{from: string, to: string}\\" objects but got [{\\"from\\":\\"FROM_CLASS\\",\\"to\\":\\"TO_CLASS\\"},{\\"a\\":\\"something\\",\\"b\\":\\"someone\\"}]."
+        `);
+      });
     });
 
     describe("site", () => {

packages/wrangler/src/config/validation.ts

@@ -384,29 +384,38 @@ function normalizeAndValidateMigrations(
     return [];
   } else {
     for (let i = 0; i < rawMigrations.length; i++) {
-      const migration = rawMigrations[i];
+      const { tag, new_classes, renamed_classes, deleted_classes, ...rest } =
+        rawMigrations[i];
+
+      validateAdditionalProperties(
+        diagnostics,
+        "migrations",
+        Object.keys(rest),
+        []
+      );
+
       validateRequiredProperty(
         diagnostics,
         `migrations[${i}]`,
         `tag`,
-        migration.tag,
+        tag,
         "string"
       );
       validateOptionalTypedArray(
         diagnostics,
         `migrations[${i}].new_classes`,
-        migration.new_classes,
+        new_classes,
         "string"
       );
-      if (migration.renamed_classes !== undefined) {
-        if (!Array.isArray(migration.renamed_classes)) {
+      if (renamed_classes !== undefined) {
+        if (!Array.isArray(renamed_classes)) {
           diagnostics.errors.push(
             `Expected "migrations[${i}].renamed_classes" to be an array of "{from: string, to: string}" objects but got ${JSON.stringify(
-              migration.renamed_classes
+              renamed_classes
             )}.`
           );
         } else if (
-          migration.renamed_classes.some(
+          renamed_classes.some(
             (c) =>
               typeof c !== "object" ||
               !isRequiredProperty(c, "from", "string") ||
@@ -415,15 +424,15 @@ function normalizeAndValidateMigrations(
         ) {
           diagnostics.errors.push(
             `Expected "migrations[${i}].renamed_classes" to be an array of "{from: string, to: string}" objects but got ${JSON.stringify(
-              migration.renamed_classes
+              renamed_classes
             )}.`
           );
         }
       }
       validateOptionalTypedArray(
         diagnostics,
         `migrations[${i}].deleted_classes`,
-        migration.deleted_classes,
+        deleted_classes,
         "string"
       );
     }