diff --git a/frontend/__tests__/utils/local-storage-with-schema.spec.ts b/frontend/__tests__/utils/local-storage-with-schema.spec.ts index 75db29071..7a79229d3 100644 --- a/frontend/__tests__/utils/local-storage-with-schema.spec.ts +++ b/frontend/__tests__/utils/local-storage-with-schema.spec.ts @@ -64,6 +64,7 @@ describe("local-storage-with-schema.ts", () => { const res = ls.get(); expect(localStorage.getItem).toHaveBeenCalledWith("config"); + expect(localStorage.setItem).not.toHaveBeenCalled(); expect(res).toEqual(defaultObject); }); @@ -74,6 +75,7 @@ describe("local-storage-with-schema.ts", () => { expect(localStorage.getItem).toHaveBeenCalledWith("config"); expect(localStorage.removeItem).toHaveBeenCalledWith("config"); + expect(localStorage.setItem).not.toHaveBeenCalled(); expect(res).toEqual(defaultObject); }); @@ -83,6 +85,7 @@ describe("local-storage-with-schema.ts", () => { const res = ls.get(); expect(localStorage.getItem).toHaveBeenCalledWith("config"); + expect(localStorage.setItem).not.toHaveBeenCalled(); expect(res).toEqual(defaultObject); }); @@ -97,30 +100,79 @@ describe("local-storage-with-schema.ts", () => { const res = ls.get(); expect(localStorage.getItem).toHaveBeenCalledWith("config"); + expect(localStorage.setItem).toHaveBeenCalledWith( + "config", + JSON.stringify(defaultObject) + ); expect(res).toEqual(defaultObject); }); it("should migrate (when function is provided) if schema failed", () => { - getItemMock.mockReturnValue(JSON.stringify({ hi: "hello" })); + const existingValue = { hi: "hello" }; + + getItemMock.mockReturnValue(JSON.stringify(existingValue)); const migrated = { punctuation: false, mode: "time", fontSize: 1, }; + + const migrateFnMock = vi.fn(() => migrated as any); + const ls = new LocalStorageWithSchema({ key: "config", schema: objectSchema, fallback: defaultObject, - migrate: () => { - return migrated; - }, + migrate: migrateFnMock, }); const res = ls.get(); expect(localStorage.getItem).toHaveBeenCalledWith("config"); + expect(migrateFnMock).toHaveBeenCalledWith( + existingValue, + expect.any(Array) + ); + expect(localStorage.setItem).toHaveBeenCalledWith( + "config", + JSON.stringify(migrated) + ); expect(res).toEqual(migrated); }); + + it("should revert to fallback if migration ran but schema still failed", () => { + const existingValue = { hi: "hello" }; + + getItemMock.mockReturnValue(JSON.stringify(existingValue)); + + const invalidMigrated = { + punctuation: 1, + mode: "time", + fontSize: 1, + }; + + const migrateFnMock = vi.fn(() => invalidMigrated as any); + + const ls = new LocalStorageWithSchema({ + key: "config", + schema: objectSchema, + fallback: defaultObject, + migrate: migrateFnMock, + }); + + const res = ls.get(); + + expect(localStorage.getItem).toHaveBeenCalledWith("config"); + expect(migrateFnMock).toHaveBeenCalledWith( + existingValue, + expect.any(Array) + ); + expect(localStorage.setItem).toHaveBeenCalledWith( + "config", + JSON.stringify(defaultObject) + ); + expect(res).toEqual(defaultObject); + }); }); }); diff --git a/frontend/src/ts/utils/local-storage-with-schema.ts b/frontend/src/ts/utils/local-storage-with-schema.ts index f196a2030..f88434382 100644 --- a/frontend/src/ts/utils/local-storage-with-schema.ts +++ b/frontend/src/ts/utils/local-storage-with-schema.ts @@ -1,4 +1,4 @@ -import { ZodIssue } from "zod"; +import { ZodError, ZodIssue } from "zod"; export class LocalStorageWithSchema { private key: string; @@ -29,7 +29,7 @@ export class LocalStorageWithSchema { try { jsonParsed = JSON.parse(value); } catch (e) { - console.error( + console.log( `Value from localStorage ${this.key} was not a valid JSON, using fallback`, e ); @@ -43,12 +43,25 @@ export class LocalStorageWithSchema { return schemaParsed.data; } - console.error( + console.log( `Value from localStorage ${this.key} failed schema validation, migrating`, schemaParsed.error ); - const newValue = - this.migrate?.(jsonParsed, schemaParsed.error.issues) ?? this.fallback; + + let newValue = this.fallback; + if (this.migrate) { + const migrated = this.migrate(jsonParsed, schemaParsed.error.issues); + const parse = this.schema.safeParse(migrated); + if (parse.success) { + newValue = migrated; + } else { + console.error( + `Value from localStorage ${this.key} failed schema validation after migration! This is very bad!`, + parse.error.issues + ); + } + } + window.localStorage.setItem(this.key, JSON.stringify(newValue)); return newValue; } @@ -59,7 +72,11 @@ export class LocalStorageWithSchema { window.localStorage.setItem(this.key, JSON.stringify(parsed)); return true; } catch (e) { - console.error(`Failed to set ${this.key} in localStorage`, e); + console.error( + `Failed to set ${this.key} in localStorage`, + data, + (e as ZodError).issues + ); return false; } }