fix(presets): move migration to Config.apply (@fehmer) (#6814)

frontend/__tests__/root/config.spec.ts

@@ -685,7 +685,6 @@ describe("Config", () => {
   it("setAds", () => {
     expect(Config.setAds("on")).toBe(true);
     expect(Config.setAds("sellout")).toBe(true);
-    expect(Config.setAds("invalid" as any)).toBe(false);
   });
   it("setRepeatQuotes", () => {
     expect(Config.setRepeatQuotes("off")).toBe(true);
@@ -1098,9 +1097,9 @@ describe("Config", () => {
   });
 
   describe("apply", () => {
-    it("should fill missing values with defaults", () => {
+    it("should fill missing values with defaults", async () => {
       //GIVEN
-      Config.apply({
+      await Config.apply({
         numbers: true,
         punctuation: true,
       });
@@ -1124,18 +1123,29 @@ describe("Config", () => {
         },
         {
           display: "mode incompatible with funbox",
-          value: { mode: "quote", funbox: ["58008"] as any },
+          value: { mode: "quote", funbox: ["58008"] },
           expected: { funbox: [] },
         },
         {
-          display: "invalid customLayoutfluid",
-          value: { funbox: ["58008", "gibberish"] as any },
+          display: "invalid combination of funboxes",
+          value: { funbox: ["58008", "gibberish"] },
           expected: { funbox: [] },
         },
+        {
+          display: "sanitizes config, remove extra keys",
+          value: { mode: "zen", unknownKey: true, unknownArray: [1, 2] } as any,
+          expected: { mode: "zen" },
+        },
+        {
+          display: "applies config migration",
+          value: { mode: "zen", swapEscAndTab: true } as any,
+          expected: { mode: "zen", quickRestart: "esc" },
+        },
       ];
 
-      it.each(testCases)("$display", ({ value, expected }) => {
-        Config.apply(value);
+      it.each(testCases)("$display", async ({ value, expected }) => {
+        await Config.apply(value);
+
         const config = getConfig();
         const applied = Object.fromEntries(
           Object.entries(config).filter(([key]) =>
@@ -1159,8 +1169,8 @@ describe("Config", () => {
         },
       ];
 
-      it.each(testCases)("$display", ({ value, expected }) => {
-        Config.apply(value);
+      it.each(testCases)("$display", async ({ value, expected }) => {
+        await Config.apply(value);
         const config = getConfig();
         const applied = Object.fromEntries(
           Object.entries(config).filter(([key]) =>
@@ -1171,22 +1181,23 @@ describe("Config", () => {
       });
     });
 
-    it("should apply a partial config but keep the rest unchanged", () => {
+    it("should apply a partial config but keep the rest unchanged", async () => {
       replaceConfig({
         numbers: true,
       });
-      Config.apply({
+      await Config.apply({
         punctuation: true,
       });
       const config = getConfig();
       expect(config.numbers).toBe(true);
     });
 
-    it("should reset all values to default if fullReset is true", () => {
+    it("should reset all values to default if fullReset is true", async () => {
       replaceConfig({
         numbers: true,
+        theme: "serika",
       });
-      Config.apply(
+      await Config.apply(
         {
           punctuation: true,
         },
@@ -1194,6 +1205,7 @@ describe("Config", () => {
       );
       const config = getConfig();
       expect(config.numbers).toBe(false);
+      expect(config.theme).toEqual("serika_dark");
     });
   });
 });

frontend/__tests__/utils/misc.spec.ts

@@ -231,11 +231,14 @@ describe("misc.ts", () => {
     });
   });
   describe("sanitize function", () => {
-    const schema = z.object({
-      name: z.string(),
-      age: z.number().positive(),
-      tags: z.array(z.string()),
-    });
+    const schema = z
+      .object({
+        name: z.string(),
+        age: z.number().positive(),
+        tags: z.array(z.string()),
+      })
+      .partial()
+      .strip();
 
     it("should return the same object if it is valid", () => {
       const obj = { name: "Alice", age: 30, tags: ["developer", "coder"] };
@@ -266,20 +269,42 @@ describe("misc.ts", () => {
 
     it("should remove entire property if all array elements are invalid", () => {
       const obj = { name: "Alice", age: 30, tags: [123, 456] as any };
-      expect(sanitize(schema, obj)).toEqual({
+      const sanitized = sanitize(schema, obj);
+      expect(sanitized).toEqual({
         name: "Alice",
         age: 30,
-        tags: undefined,
       });
+      expect(sanitized).not.toHaveProperty("tags");
     });
 
     it("should remove object properties if they are invalid", () => {
       const obj = { name: 123 as any, age: 30, tags: ["developer", "coder"] };
-      expect(sanitize(schema, obj)).toEqual({
+      const sanitized = sanitize(schema, obj);
+      expect(sanitized).toEqual({
         age: 30,
         tags: ["developer", "coder"],
-        name: undefined,
       });
+      expect(sanitized).not.toHaveProperty("name");
+    });
+
+    it("should strip extra keys", () => {
+      const obj = {
+        name: "bob",
+        age: 30,
+        tags: ["developer", "coder"],
+        powerLevel: 9001,
+      } as any;
+      const stripped = sanitize(schema.strip(), obj);
+      expect(stripped).not.toHaveProperty("powerLevel");
+    });
+    it("should strip extra keys on error", () => {
+      const obj = {
+        name: "bob",
+        age: 30,
+        powerLevel: 9001,
+      } as any;
+      const stripped = sanitize(schema.strip(), obj);
+      expect(stripped).not.toHaveProperty("powerLevel");
     });
   });
 });

frontend/src/ts/config.ts

@@ -20,7 +20,11 @@ import { Config, FunboxName } from "@monkeytype/schemas/configs";
 import { Mode } from "@monkeytype/schemas/shared";
 import { Language } from "@monkeytype/schemas/languages";
 import { LocalStorageWithSchema } from "./utils/local-storage-with-schema";
-import { migrateConfig } from "./utils/config";
+import {
+  migrateConfig,
+  replaceLegacyValues,
+  sanitizeConfig,
+} from "./utils/config";
 import { getDefaultConfig } from "./constants/default-config";
 import { parseWithSchema as parseJsonWithSchema } from "@monkeytype/util/json";
 import { ZodSchema } from "zod";
@@ -821,6 +825,9 @@ export async function apply(
 ): Promise<void> {
   if (configToApply === undefined || configToApply === null) return;
 
+  //remove additional keys, migrate old values if needed
+  configToApply = sanitizeConfig(replaceLegacyValues(configToApply));
+
   ConfigEvent.dispatch("fullConfigChange");
 
   const defaultConfig = getDefaultConfig();

frontend/src/ts/controllers/preset-controller.ts

@@ -3,7 +3,6 @@ import * as UpdateConfig from "../config";
 import * as DB from "../db";
 import * as Notifications from "../elements/notifications";
 import * as TestLogic from "../test/test-logic";
-import { migrateConfig, replaceLegacyValues } from "../utils/config";
 import * as TagController from "./tag-controller";
 import { SnapshotPreset } from "../constants/default-snapshot";
 
@@ -17,7 +16,7 @@ export async function apply(_id: string): Promise<void> {
   }
 
   await UpdateConfig.apply(
-    migrateConfig(replaceLegacyValues(presetToApply.config)),
+    presetToApply.config,
     !isPartialPreset(presetToApply)
   );
 

frontend/src/ts/utils/config.ts

@@ -30,10 +30,11 @@ function mergeWithDefaultConfig(config: PartialConfig): Config {
 /**
  * remove all values from the config which are not valid
  */
-function sanitizeConfig(
+export function sanitizeConfig(
   config: ConfigSchemas.PartialConfig
 ): ConfigSchemas.PartialConfig {
-  return sanitize(ConfigSchemas.PartialConfigSchema, config);
+  //make sure to use strip()
+  return sanitize(ConfigSchemas.PartialConfigSchema.strip(), config);
 }
 
 export function replaceLegacyValues(

frontend/src/ts/utils/misc.ts

@@ -723,8 +723,9 @@ export function sanitize<T extends z.ZodTypeAny>(
   const validate = schema.safeParse(obj);
 
   if (validate.success) {
+    //use the parsed data, not the obj. keys might been removed
     // eslint-disable-next-line @typescript-eslint/no-unsafe-return
-    return obj;
+    return validate.data;
   }
 
   const errors: Map<string, number[] | undefined> = new Map();
@@ -738,26 +739,40 @@ export function sanitize<T extends z.ZodTypeAny>(
   }
 
   // eslint-disable-next-line @typescript-eslint/no-unsafe-return
-  return Object.fromEntries(
-    Object.entries(obj).map(([key, value]) => {
-      if (!errors.has(key)) {
-        return [key, value];
-      }
+  const cleanedObject = Object.fromEntries(
+    Object.entries(obj)
+      .map(([key, value]) => {
+        if (!errors.has(key)) {
+          return [key, value];
+        }
 
-      const error = errors.get(key);
+        const error = errors.get(key);
 
-      if (
-        Array.isArray(value) &&
-        error !== undefined && //error is not on the array itself
-        error.length < value.length //not all items in the array are invalid
-      ) {
-        //some items of the array are invalid
-        return [key, value.filter((_element, index) => !error.includes(index))];
-      } else {
-        return [key, undefined];
-      }
-    })
+        if (
+          Array.isArray(value) &&
+          error !== undefined && //error is not on the array itself
+          error.length < value.length //not all items in the array are invalid
+        ) {
+          //some items of the array are invalid
+          return [
+            key,
+            value.filter((_element, index) => !error.includes(index)),
+          ];
+        } else {
+          return [key, undefined];
+        }
+      })
+      .filter((it) => it[1] !== undefined)
   ) as z.infer<T>;
+
+  const cleanValidate = schema.safeParse(cleanedObject);
+  if (cleanValidate.success) {
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-return
+    return cleanValidate.data;
+  }
+  throw new Error(
+    "unable to sanitize:  " + cleanValidate.error.errors.join(",")
+  );
 }
 
 export function triggerResize(): void {