From 6221dce8f6caf6325f5e7b0a0f03d3b34f071d85 Mon Sep 17 00:00:00 2001 From: Christian Fehmer Date: Sun, 25 May 2025 11:40:28 +0200 Subject: [PATCH 1/3] perf: use cache in local-storage-with-schema (@fehmer) --- .../utils/local-storage-with-schema.spec.ts | 138 +++++++++++++----- .../src/ts/utils/local-storage-with-schema.ts | 22 ++- 2 files changed, 115 insertions(+), 45 deletions(-) diff --git a/frontend/__tests__/utils/local-storage-with-schema.spec.ts b/frontend/__tests__/utils/local-storage-with-schema.spec.ts index fa74cf0407d9..ad86fb7dbfb0 100644 --- a/frontend/__tests__/utils/local-storage-with-schema.spec.ts +++ b/frontend/__tests__/utils/local-storage-with-schema.spec.ts @@ -2,41 +2,49 @@ import { z } from "zod"; import { LocalStorageWithSchema } from "../../src/ts/utils/local-storage-with-schema"; describe("local-storage-with-schema.ts", () => { - describe("LocalStorageWithSchema", () => { - const objectSchema = z.object({ - punctuation: z.boolean(), - mode: z.enum(["words", "time"]), - fontSize: z.number(), - }); + const objectSchema = z.object({ + punctuation: z.boolean(), + mode: z.enum(["words", "time"]), + fontSize: z.number(), + }); - const defaultObject: z.infer = { - punctuation: true, - mode: "words", - fontSize: 16, - }; + const defaultObject: z.infer = { + punctuation: true, + mode: "words", + fontSize: 16, + }; - const ls = new LocalStorageWithSchema({ - key: "config", - schema: objectSchema, - fallback: defaultObject, - }); + let ls = new LocalStorageWithSchema({ + key: "config", + schema: objectSchema, + fallback: defaultObject, + }); - const getItemMock = vi.fn(); - const setItemMock = vi.fn(); - const removeItemMock = vi.fn(); + const getItemMock = vi.fn(); + const setItemMock = vi.fn(); + const removeItemMock = vi.fn(); - vi.stubGlobal("localStorage", { - getItem: getItemMock, - setItem: setItemMock, - removeItem: removeItemMock, - }); + vi.stubGlobal("localStorage", { + getItem: getItemMock, + setItem: setItemMock, + removeItem: removeItemMock, + }); + + afterEach(() => { + getItemMock.mockReset(); + setItemMock.mockReset(); + removeItemMock.mockReset(); + }); - afterEach(() => { - getItemMock.mockReset(); - setItemMock.mockReset(); - removeItemMock.mockReset(); + beforeEach(() => { + ls = new LocalStorageWithSchema({ + key: "config", + schema: objectSchema, + fallback: defaultObject, }); + }); + describe("set", () => { it("should save to localStorage if schema is correct and return true", () => { const res = ls.set(defaultObject); @@ -58,14 +66,44 @@ describe("local-storage-with-schema.ts", () => { expect(res).toBe(false); }); + it("should update cache on set", () => { + ls.set(defaultObject); + + expect(ls.get()).toStrictEqual(defaultObject); + + const update = { ...defaultObject, fontSize: 5 }; + ls.set(update); + + expect(ls.get()).toStrictEqual(update); + + expect(getItemMock).not.toHaveBeenCalled(); + }); + + it("should get last valid value if schema is incorrect", () => { + ls.set(defaultObject); + + ls.set({ hi: "hello" } as any); + + expect(ls.get()).toEqual(defaultObject); + + expect(setItemMock).toHaveBeenCalledOnce(); + expect(getItemMock).not.toHaveBeenCalled(); + }); + }); + + describe("get", () => { it("should revert to the fallback value if localstorage is null", () => { getItemMock.mockReturnValue(null); const res = ls.get(); - expect(localStorage.getItem).toHaveBeenCalledWith("config"); - expect(localStorage.setItem).not.toHaveBeenCalled(); + expect(getItemMock).toHaveBeenCalledWith("config"); + expect(setItemMock).not.toHaveBeenCalled(); expect(res).toEqual(defaultObject); + + //cache used + expect(ls.get()).toEqual(res); + expect(getItemMock).toHaveBeenCalledOnce(); }); it("should revert to the fallback value if localstorage json is malformed", () => { @@ -73,12 +111,16 @@ describe("local-storage-with-schema.ts", () => { const res = ls.get(); - expect(localStorage.getItem).toHaveBeenCalledWith("config"); - expect(localStorage.setItem).toHaveBeenCalledWith( + expect(getItemMock).toHaveBeenCalledWith("config"); + expect(setItemMock).toHaveBeenCalledWith( "config", JSON.stringify(defaultObject) ); expect(res).toEqual(defaultObject); + + //cache used + expect(ls.get()).toEqual(defaultObject); + expect(getItemMock).toHaveBeenCalledOnce(); }); it("should get from localStorage", () => { @@ -86,9 +128,13 @@ describe("local-storage-with-schema.ts", () => { const res = ls.get(); - expect(localStorage.getItem).toHaveBeenCalledWith("config"); - expect(localStorage.setItem).not.toHaveBeenCalled(); + expect(getItemMock).toHaveBeenCalledWith("config"); + expect(setItemMock).not.toHaveBeenCalled(); expect(res).toEqual(defaultObject); + + //cache used + expect(ls.get()).toEqual(res); + expect(getItemMock).toHaveBeenCalledOnce(); }); it("should revert to fallback value if no migrate function and schema failed", () => { @@ -101,12 +147,16 @@ describe("local-storage-with-schema.ts", () => { const res = ls.get(); - expect(localStorage.getItem).toHaveBeenCalledWith("config"); - expect(localStorage.setItem).toHaveBeenCalledWith( + expect(getItemMock).toHaveBeenCalledWith("config"); + expect(setItemMock).toHaveBeenCalledWith( "config", JSON.stringify(defaultObject) ); expect(res).toEqual(defaultObject); + + //cache used + expect(ls.get()).toEqual(defaultObject); + expect(getItemMock).toHaveBeenCalledOnce(); }); it("should migrate (when function is provided) if schema failed", () => { @@ -131,16 +181,20 @@ describe("local-storage-with-schema.ts", () => { const res = ls.get(); - expect(localStorage.getItem).toHaveBeenCalledWith("config"); + expect(getItemMock).toHaveBeenCalledWith("config"); expect(migrateFnMock).toHaveBeenCalledWith( existingValue, expect.any(Array) ); - expect(localStorage.setItem).toHaveBeenCalledWith( + expect(setItemMock).toHaveBeenCalledWith( "config", JSON.stringify(migrated) ); expect(res).toEqual(migrated); + + //cache used + expect(ls.get()).toEqual(migrated); + expect(getItemMock).toHaveBeenCalledOnce(); }); it("should revert to fallback if migration ran but schema still failed", () => { @@ -165,16 +219,20 @@ describe("local-storage-with-schema.ts", () => { const res = ls.get(); - expect(localStorage.getItem).toHaveBeenCalledWith("config"); + expect(getItemMock).toHaveBeenCalledWith("config"); expect(migrateFnMock).toHaveBeenCalledWith( existingValue, expect.any(Array) ); - expect(localStorage.setItem).toHaveBeenCalledWith( + expect(setItemMock).toHaveBeenCalledWith( "config", JSON.stringify(defaultObject) ); expect(res).toEqual(defaultObject); + + //cache used + expect(ls.get()).toEqual(defaultObject); + expect(getItemMock).toHaveBeenCalledOnce(); }); }); }); diff --git a/frontend/src/ts/utils/local-storage-with-schema.ts b/frontend/src/ts/utils/local-storage-with-schema.ts index 7d2477334cb0..9f941155c11f 100644 --- a/frontend/src/ts/utils/local-storage-with-schema.ts +++ b/frontend/src/ts/utils/local-storage-with-schema.ts @@ -12,6 +12,7 @@ export class LocalStorageWithSchema { value: Record | unknown[], zodIssues?: ZodIssue[] ) => T; + private cache?: T; constructor(options: { key: string; @@ -31,11 +32,17 @@ export class LocalStorageWithSchema { public get(): T { console.debug(`LS ${this.key} Getting value from localStorage`); + if (this.cache !== undefined) { + console.debug(`LS ${this.key} Got cached value:`, this.cache); + return this.cache; + } + const value = window.localStorage.getItem(this.key); if (value === null) { console.debug(`LS ${this.key} No value found, returning fallback`); - return this.fallback; + this.cache = this.fallback; + return this.cache; } let migrated = false; @@ -49,12 +56,14 @@ export class LocalStorageWithSchema { console.debug( `LS ${this.key} Migrating from old format to new format` ); - return this.migrate(oldData, zodIssues); + this.cache = this.migrate(oldData, zodIssues); + return this.cache; } else { console.debug( `LS ${this.key} No migration function provided, returning fallback` ); - return this.fallback; + this.cache = this.fallback; + return this.cache; } }, }) @@ -65,7 +74,8 @@ export class LocalStorageWithSchema { `LS ${this.key} Failed to parse from localStorage: ${error.message}` ); window.localStorage.setItem(this.key, JSON.stringify(this.fallback)); - return this.fallback; + this.cache = this.fallback; + return this.cache; } if (migrated || parsed === this.fallback) { @@ -74,7 +84,8 @@ export class LocalStorageWithSchema { } console.debug(`LS ${this.key} Got value:`, parsed); - return parsed; + this.cache = parsed; + return this.cache; } public set(data: T): boolean { @@ -83,6 +94,7 @@ export class LocalStorageWithSchema { const parsed = this.schema.parse(data); console.debug(`LS ${this.key} Setting in localStorage`); window.localStorage.setItem(this.key, JSON.stringify(parsed)); + this.cache = parsed; return true; } catch (e) { let message = "Unknown error occurred"; From f75c2b52f357376fe5617e8f4ca522a3251db547 Mon Sep 17 00:00:00 2001 From: Christian Fehmer Date: Tue, 27 May 2025 09:32:26 +0200 Subject: [PATCH 2/3] cleanup --- .../utils/local-storage-with-schema.spec.ts | 374 +++++++++--------- .../src/ts/utils/local-storage-with-schema.ts | 3 +- 2 files changed, 189 insertions(+), 188 deletions(-) diff --git a/frontend/__tests__/utils/local-storage-with-schema.spec.ts b/frontend/__tests__/utils/local-storage-with-schema.spec.ts index ad86fb7dbfb0..d722e579e1c5 100644 --- a/frontend/__tests__/utils/local-storage-with-schema.spec.ts +++ b/frontend/__tests__/utils/local-storage-with-schema.spec.ts @@ -2,237 +2,239 @@ import { z } from "zod"; import { LocalStorageWithSchema } from "../../src/ts/utils/local-storage-with-schema"; describe("local-storage-with-schema.ts", () => { - const objectSchema = z.object({ - punctuation: z.boolean(), - mode: z.enum(["words", "time"]), - fontSize: z.number(), - }); - - const defaultObject: z.infer = { - punctuation: true, - mode: "words", - fontSize: 16, - }; - - let ls = new LocalStorageWithSchema({ - key: "config", - schema: objectSchema, - fallback: defaultObject, - }); - - const getItemMock = vi.fn(); - const setItemMock = vi.fn(); - const removeItemMock = vi.fn(); - - vi.stubGlobal("localStorage", { - getItem: getItemMock, - setItem: setItemMock, - removeItem: removeItemMock, - }); + describe("LocalStorageWithSchema", () => { + const objectSchema = z.object({ + punctuation: z.boolean(), + mode: z.enum(["words", "time"]), + fontSize: z.number(), + }); - afterEach(() => { - getItemMock.mockReset(); - setItemMock.mockReset(); - removeItemMock.mockReset(); - }); + const defaultObject: z.infer = { + punctuation: true, + mode: "words", + fontSize: 16, + }; - beforeEach(() => { - ls = new LocalStorageWithSchema({ + let ls = new LocalStorageWithSchema({ key: "config", schema: objectSchema, fallback: defaultObject, }); - }); - describe("set", () => { - it("should save to localStorage if schema is correct and return true", () => { - const res = ls.set(defaultObject); + const getItemMock = vi.fn(); + const setItemMock = vi.fn(); + const removeItemMock = vi.fn(); - expect(localStorage.setItem).toHaveBeenCalledWith( - "config", - JSON.stringify(defaultObject) - ); - expect(res).toBe(true); + vi.stubGlobal("localStorage", { + getItem: getItemMock, + setItem: setItemMock, + removeItem: removeItemMock, }); - it("should fail to save to localStorage if schema is incorrect and return false", () => { - const obj = { - hi: "hello", - }; - - const res = ls.set(obj as any); - - expect(localStorage.setItem).not.toHaveBeenCalled(); - expect(res).toBe(false); + afterEach(() => { + getItemMock.mockReset(); + setItemMock.mockReset(); + removeItemMock.mockReset(); }); - it("should update cache on set", () => { - ls.set(defaultObject); - - expect(ls.get()).toStrictEqual(defaultObject); - - const update = { ...defaultObject, fontSize: 5 }; - ls.set(update); - - expect(ls.get()).toStrictEqual(update); - - expect(getItemMock).not.toHaveBeenCalled(); + beforeEach(() => { + ls = new LocalStorageWithSchema({ + key: "config", + schema: objectSchema, + fallback: defaultObject, + }); }); - it("should get last valid value if schema is incorrect", () => { - ls.set(defaultObject); - - ls.set({ hi: "hello" } as any); + describe("set", () => { + it("should save to localStorage if schema is correct and return true", () => { + const res = ls.set(defaultObject); - expect(ls.get()).toEqual(defaultObject); - - expect(setItemMock).toHaveBeenCalledOnce(); - expect(getItemMock).not.toHaveBeenCalled(); - }); - }); - - describe("get", () => { - it("should revert to the fallback value if localstorage is null", () => { - getItemMock.mockReturnValue(null); + expect(localStorage.setItem).toHaveBeenCalledWith( + "config", + JSON.stringify(defaultObject) + ); + expect(res).toBe(true); + }); - const res = ls.get(); + it("should fail to save to localStorage if schema is incorrect and return false", () => { + const obj = { + hi: "hello", + }; - expect(getItemMock).toHaveBeenCalledWith("config"); - expect(setItemMock).not.toHaveBeenCalled(); - expect(res).toEqual(defaultObject); + const res = ls.set(obj as any); - //cache used - expect(ls.get()).toEqual(res); - expect(getItemMock).toHaveBeenCalledOnce(); - }); + expect(localStorage.setItem).not.toHaveBeenCalled(); + expect(res).toBe(false); + }); - it("should revert to the fallback value if localstorage json is malformed", () => { - getItemMock.mockReturnValue("badjson"); + it("should update cache on set", () => { + ls.set(defaultObject); - const res = ls.get(); + expect(ls.get()).toStrictEqual(defaultObject); - expect(getItemMock).toHaveBeenCalledWith("config"); - expect(setItemMock).toHaveBeenCalledWith( - "config", - JSON.stringify(defaultObject) - ); - expect(res).toEqual(defaultObject); + const update = { ...defaultObject, fontSize: 5 }; + ls.set(update); - //cache used - expect(ls.get()).toEqual(defaultObject); - expect(getItemMock).toHaveBeenCalledOnce(); - }); + expect(ls.get()).toStrictEqual(update); - it("should get from localStorage", () => { - getItemMock.mockReturnValue(JSON.stringify(defaultObject)); + expect(getItemMock).not.toHaveBeenCalled(); + }); - const res = ls.get(); + it("should get last valid value if schema is incorrect", () => { + ls.set(defaultObject); - expect(getItemMock).toHaveBeenCalledWith("config"); - expect(setItemMock).not.toHaveBeenCalled(); - expect(res).toEqual(defaultObject); + ls.set({ hi: "hello" } as any); - //cache used - expect(ls.get()).toEqual(res); - expect(getItemMock).toHaveBeenCalledOnce(); - }); + expect(ls.get()).toEqual(defaultObject); - it("should revert to fallback value if no migrate function and schema failed", () => { - getItemMock.mockReturnValue(JSON.stringify({ hi: "hello" })); - const ls = new LocalStorageWithSchema({ - key: "config", - schema: objectSchema, - fallback: defaultObject, + expect(setItemMock).toHaveBeenCalledOnce(); + expect(getItemMock).not.toHaveBeenCalled(); }); + }); - const res = ls.get(); + describe("get", () => { + it("should revert to the fallback value if localstorage is null", () => { + getItemMock.mockReturnValue(null); - expect(getItemMock).toHaveBeenCalledWith("config"); - expect(setItemMock).toHaveBeenCalledWith( - "config", - JSON.stringify(defaultObject) - ); - expect(res).toEqual(defaultObject); + const res = ls.get(); - //cache used - expect(ls.get()).toEqual(defaultObject); - expect(getItemMock).toHaveBeenCalledOnce(); - }); + expect(getItemMock).toHaveBeenCalledWith("config"); + expect(setItemMock).not.toHaveBeenCalled(); + expect(res).toEqual(defaultObject); - it("should migrate (when function is provided) if schema failed", () => { - const existingValue = { hi: "hello" }; + //cache used + expect(ls.get()).toEqual(res); + expect(getItemMock).toHaveBeenCalledOnce(); + }); - getItemMock.mockReturnValue(JSON.stringify(existingValue)); + it("should revert to the fallback value if localstorage json is malformed", () => { + getItemMock.mockReturnValue("badjson"); - const migrated = { - punctuation: false, - mode: "time", - fontSize: 1, - }; + const res = ls.get(); - const migrateFnMock = vi.fn(() => migrated as any); + expect(getItemMock).toHaveBeenCalledWith("config"); + expect(setItemMock).toHaveBeenCalledWith( + "config", + JSON.stringify(defaultObject) + ); + expect(res).toEqual(defaultObject); - const ls = new LocalStorageWithSchema({ - key: "config", - schema: objectSchema, - fallback: defaultObject, - migrate: migrateFnMock, + //cache used + expect(ls.get()).toEqual(defaultObject); + expect(getItemMock).toHaveBeenCalledOnce(); }); - const res = ls.get(); - - expect(getItemMock).toHaveBeenCalledWith("config"); - expect(migrateFnMock).toHaveBeenCalledWith( - existingValue, - expect.any(Array) - ); - expect(setItemMock).toHaveBeenCalledWith( - "config", - JSON.stringify(migrated) - ); - expect(res).toEqual(migrated); - - //cache used - expect(ls.get()).toEqual(migrated); - expect(getItemMock).toHaveBeenCalledOnce(); - }); + it("should get from localStorage", () => { + getItemMock.mockReturnValue(JSON.stringify(defaultObject)); - it("should revert to fallback if migration ran but schema still failed", () => { - const existingValue = { hi: "hello" }; + const res = ls.get(); - getItemMock.mockReturnValue(JSON.stringify(existingValue)); + expect(getItemMock).toHaveBeenCalledWith("config"); + expect(setItemMock).not.toHaveBeenCalled(); + expect(res).toEqual(defaultObject); - const invalidMigrated = { - punctuation: 1, - mode: "time", - fontSize: 1, - }; + //cache used + expect(ls.get()).toEqual(res); + expect(getItemMock).toHaveBeenCalledOnce(); + }); - const migrateFnMock = vi.fn(() => invalidMigrated as any); + it("should revert to fallback value if no migrate function and schema failed", () => { + getItemMock.mockReturnValue(JSON.stringify({ hi: "hello" })); + const ls = new LocalStorageWithSchema({ + key: "config", + schema: objectSchema, + fallback: defaultObject, + }); + + const res = ls.get(); + + expect(getItemMock).toHaveBeenCalledWith("config"); + expect(setItemMock).toHaveBeenCalledWith( + "config", + JSON.stringify(defaultObject) + ); + expect(res).toEqual(defaultObject); + + //cache used + expect(ls.get()).toEqual(defaultObject); + expect(getItemMock).toHaveBeenCalledOnce(); + }); - const ls = new LocalStorageWithSchema({ - key: "config", - schema: objectSchema, - fallback: defaultObject, - migrate: migrateFnMock, + it("should migrate (when function is provided) if schema failed", () => { + 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: migrateFnMock, + }); + + const res = ls.get(); + + expect(getItemMock).toHaveBeenCalledWith("config"); + expect(migrateFnMock).toHaveBeenCalledWith( + existingValue, + expect.any(Array) + ); + expect(setItemMock).toHaveBeenCalledWith( + "config", + JSON.stringify(migrated) + ); + expect(res).toEqual(migrated); + + //cache used + expect(ls.get()).toEqual(migrated); + expect(getItemMock).toHaveBeenCalledOnce(); }); - const res = ls.get(); - - expect(getItemMock).toHaveBeenCalledWith("config"); - expect(migrateFnMock).toHaveBeenCalledWith( - existingValue, - expect.any(Array) - ); - expect(setItemMock).toHaveBeenCalledWith( - "config", - JSON.stringify(defaultObject) - ); - expect(res).toEqual(defaultObject); - - //cache used - expect(ls.get()).toEqual(defaultObject); - expect(getItemMock).toHaveBeenCalledOnce(); + 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(getItemMock).toHaveBeenCalledWith("config"); + expect(migrateFnMock).toHaveBeenCalledWith( + existingValue, + expect.any(Array) + ); + expect(setItemMock).toHaveBeenCalledWith( + "config", + JSON.stringify(defaultObject) + ); + expect(res).toEqual(defaultObject); + + //cache used + expect(ls.get()).toEqual(defaultObject); + expect(getItemMock).toHaveBeenCalledOnce(); + }); }); }); }); diff --git a/frontend/src/ts/utils/local-storage-with-schema.ts b/frontend/src/ts/utils/local-storage-with-schema.ts index 9f941155c11f..64637f182973 100644 --- a/frontend/src/ts/utils/local-storage-with-schema.ts +++ b/frontend/src/ts/utils/local-storage-with-schema.ts @@ -30,13 +30,12 @@ export class LocalStorageWithSchema { } public get(): T { - console.debug(`LS ${this.key} Getting value from localStorage`); - if (this.cache !== undefined) { console.debug(`LS ${this.key} Got cached value:`, this.cache); return this.cache; } + console.debug(`LS ${this.key} Getting value from localStorage`); const value = window.localStorage.getItem(this.key); if (value === null) { From eda700156db6916b7290632ea5c06b0a4f17bd70 Mon Sep 17 00:00:00 2001 From: Christian Fehmer Date: Tue, 27 May 2025 09:39:27 +0200 Subject: [PATCH 3/3] optimize getData --- frontend/src/ts/test/custom-text.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/frontend/src/ts/test/custom-text.ts b/frontend/src/ts/test/custom-text.ts index 844e4ec62058..00567fe9036d 100644 --- a/frontend/src/ts/test/custom-text.ts +++ b/frontend/src/ts/test/custom-text.ts @@ -144,12 +144,7 @@ export type CustomTextData = Omit & { }; export function getData(): CustomTextData { - return { - text: getText(), - mode: getMode(), - limit: getLimit(), - pipeDelimiter: getPipeDelimiter(), - }; + return customTextSettings.get(); } export function getCustomText(name: string, long = false): string[] {