From 6f6af5e622fe99da05acc4158ec65f41348e21d9 Mon Sep 17 00:00:00 2001 From: Christian Fehmer Date: Thu, 31 Jul 2025 22:17:24 +0200 Subject: [PATCH] fix(presets): move migration to Config.apply (@fehmer) (#6814) --- frontend/__tests__/root/config.spec.ts | 40 ++++++++++----- frontend/__tests__/utils/misc.spec.ts | 43 ++++++++++++---- frontend/src/ts/config.ts | 9 +++- .../src/ts/controllers/preset-controller.ts | 3 +- frontend/src/ts/utils/config.ts | 5 +- frontend/src/ts/utils/misc.ts | 51 ++++++++++++------- 6 files changed, 105 insertions(+), 46 deletions(-) diff --git a/frontend/__tests__/root/config.spec.ts b/frontend/__tests__/root/config.spec.ts index 134c6b9ef..a8ff3ed3c 100644 --- a/frontend/__tests__/root/config.spec.ts +++ b/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"); }); }); }); diff --git a/frontend/__tests__/utils/misc.spec.ts b/frontend/__tests__/utils/misc.spec.ts index 5a0fd732d..deac90b51 100644 --- a/frontend/__tests__/utils/misc.spec.ts +++ b/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"); }); }); }); diff --git a/frontend/src/ts/config.ts b/frontend/src/ts/config.ts index 07e61ab30..d2ac943dd 100644 --- a/frontend/src/ts/config.ts +++ b/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 { if (configToApply === undefined || configToApply === null) return; + //remove additional keys, migrate old values if needed + configToApply = sanitizeConfig(replaceLegacyValues(configToApply)); + ConfigEvent.dispatch("fullConfigChange"); const defaultConfig = getDefaultConfig(); diff --git a/frontend/src/ts/controllers/preset-controller.ts b/frontend/src/ts/controllers/preset-controller.ts index b6836ec60..42cd0b212 100644 --- a/frontend/src/ts/controllers/preset-controller.ts +++ b/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 { } await UpdateConfig.apply( - migrateConfig(replaceLegacyValues(presetToApply.config)), + presetToApply.config, !isPartialPreset(presetToApply) ); diff --git a/frontend/src/ts/utils/config.ts b/frontend/src/ts/utils/config.ts index 25a40d330..8ad759647 100644 --- a/frontend/src/ts/utils/config.ts +++ b/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( diff --git a/frontend/src/ts/utils/misc.ts b/frontend/src/ts/utils/misc.ts index 62bdaea32..7cc874a4f 100644 --- a/frontend/src/ts/utils/misc.ts +++ b/frontend/src/ts/utils/misc.ts @@ -723,8 +723,9 @@ export function sanitize( 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 = new Map(); @@ -738,26 +739,40 @@ export function sanitize( } // 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; + + 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 {