From 02bf634f25075a35c8bff9f52b25a17fd0739aae Mon Sep 17 00:00:00 2001 From: Shreyas Date: Mon, 3 Mar 2025 12:39:48 +0530 Subject: [PATCH] fix(common): `PersistenceService` call site compat (#4799) --- .../persistence/__tests__/index.spec.ts | 56 ++++++++++++++ .../src/services/persistence/index.ts | 76 +++++++++++++++++-- .../src/services/tab/graphql.ts | 16 ++-- .../src/services/tab/rest.ts | 16 ++-- 4 files changed, 137 insertions(+), 27 deletions(-) diff --git a/packages/hoppscotch-common/src/services/persistence/__tests__/index.spec.ts b/packages/hoppscotch-common/src/services/persistence/__tests__/index.spec.ts index 4a992716..fb3d2d31 100644 --- a/packages/hoppscotch-common/src/services/persistence/__tests__/index.spec.ts +++ b/packages/hoppscotch-common/src/services/persistence/__tests__/index.spec.ts @@ -1,5 +1,7 @@ /* eslint-disable no-restricted-globals, no-restricted-syntax */ +import * as E from "fp-ts/Either" + import { translateToNewGQLCollection, translateToNewRESTCollection, @@ -1875,4 +1877,58 @@ describe("PersistenceService", () => { ) expect(removeItemSpy).toHaveBeenCalledWith(`${STORE_NAMESPACE}:${testKey}`) }) + + it("`set` method sets a value in localStorage", async () => { + const testKey = "temp" + const testValue = "test-value" + + const setItemSpy = spyOnSetItem() + + const service = bindPersistenceService() + + await service.set(testKey, testValue) + expect(setItemSpy).toHaveBeenCalledWith( + `${STORE_NAMESPACE}:${testKey}`, + expect.stringContaining(testValue) + ) + }) + + it("`get` method gets a value from localStorage", async () => { + const testKey = "temp" + const testValue = "test-value" + + const setItemSpy = spyOnSetItem() + const getItemSpy = spyOnGetItem() + + const service = bindPersistenceService() + + await service.set(testKey, testValue) + const retrievedValue = await service.get(testKey) + + expect(setItemSpy).toHaveBeenCalledWith( + `${STORE_NAMESPACE}:${testKey}`, + expect.stringContaining(testValue) + ) + expect(getItemSpy).toHaveBeenCalledWith(`${STORE_NAMESPACE}:${testKey}`) + expect(retrievedValue).toStrictEqual(E.right(testValue)) + }) + + it("`remove` method clears a value in localStorage", async () => { + const testKey = "temp" + const testValue = "test-value" + + const setItemSpy = spyOnSetItem() + const removeItemSpy = spyOnRemoveItem() + + const service = bindPersistenceService() + + await service.set(testKey, testValue) + await service.remove(testKey) + + expect(setItemSpy).toHaveBeenCalledWith( + `${STORE_NAMESPACE}:${testKey}`, + expect.stringContaining(testValue) + ) + expect(removeItemSpy).toHaveBeenCalledWith(`${STORE_NAMESPACE}:${testKey}`) + }) }) diff --git a/packages/hoppscotch-common/src/services/persistence/index.ts b/packages/hoppscotch-common/src/services/persistence/index.ts index 5c8f8313..79b8e6d5 100644 --- a/packages/hoppscotch-common/src/services/persistence/index.ts +++ b/packages/hoppscotch-common/src/services/persistence/index.ts @@ -4,7 +4,7 @@ import * as E from "fp-ts/Either" import { z } from "zod" import { Service } from "dioc" -import { watchDebounced } from "@vueuse/core" +import { StorageLike, watchDebounced } from "@vueuse/core" import { assign, clone, isEmpty } from "lodash-es" import { @@ -167,6 +167,9 @@ const migrations: Migration[] = [ export class PersistenceService extends Service { public static readonly ID = "PERSISTENCE_SERVICE" + // TODO: Consider swapping this with platform dependent `StoreLike` impl + public hoppLocalConfigStorage: StorageLike = localStorage + private readonly restTabService = this.bind(RESTTabService) private readonly gqlTabService = this.bind(GQLTabService) private readonly secretEnvironmentService = this.bind( @@ -197,9 +200,8 @@ export class PersistenceService extends Service { STORE_NAMESPACE, STORE_KEYS.SCHEMA_VERSION ) - const currentVersion = E.isRight(versionResult) - ? versionResult.right || "0" - : "0" + const perhapsVersion = E.isRight(versionResult) ? versionResult.right : "0" + const currentVersion = perhapsVersion ?? "0" const targetVersion = "1" if (currentVersion !== targetVersion) { @@ -916,7 +918,40 @@ export class PersistenceService extends Service { } /** - * Gets a value from persistence + * Gets a typed value from persistence, deserialization is automatic. + * No need to use JSON.parse on the result, it's handled internally by the store. + * @param key The key to retrieve + * @returns Either containing the typed value or an error + */ + public async get( + key: (typeof STORE_KEYS)[keyof typeof STORE_KEYS] + ): Promise> { + return await Store.get(STORE_NAMESPACE, key) + } + + /** + * Gets a value from persistence, discards error and returns null on failure. + * No need to use JSON.parse on the result, it's handled internally by the store. + * NOTE: Use this cautiously, try to always use `get`, handling error at call site is better + * @param key The key to retrieve + * @returns The typed value or null if not found or on error + */ + public async getNullable( + key: (typeof STORE_KEYS)[keyof typeof STORE_KEYS] + ): Promise { + const r = await Store.get(STORE_NAMESPACE, key) + + if (E.isLeft(r)) return null + + return r.right ?? null + } + + /** + * Gets a value from local config + * @deprecated Use get() instead which provides automatic deserialization and type safety. + * With get(), there's no need to use JSON.parse on the result. + * @param name The name of the config to retrieve + * @returns The config value as string, null or undefined */ public async getLocalConfig( name: string @@ -928,8 +963,26 @@ export class PersistenceService extends Service { return null } + /** + * Sets a value in persistence with proper type safety and automatic serialization. + * No need to use JSON.stringify on the value, it's handled internally by the store. + * @param key The key to set + * @param value The value to set (passed directly without manual serialization) + * @returns Either containing void or an error + */ + public async set( + key: (typeof STORE_KEYS)[keyof typeof STORE_KEYS], + value: T + ): Promise> { + return await Store.set(STORE_NAMESPACE, key, value) + } + /** * Sets a value in persistence + * @deprecated Use set() instead which provides automatic serialization and type safety. + * With set(), there's no need to use JSON.stringify on the value before passing it. + * @param key The key to set + * @param value The value to set as string */ public async setLocalConfig(key: string, value: string): Promise { await Store.set(STORE_NAMESPACE, key, value) @@ -937,6 +990,19 @@ export class PersistenceService extends Service { /** * Clear config value from persistence + * @param key The key to remove + * @returns Either containing boolean or an error + */ + public async remove( + key: (typeof STORE_KEYS)[keyof typeof STORE_KEYS] + ): Promise> { + return await Store.remove(STORE_NAMESPACE, key) + } + + /** + * Clear config value from persistence + * @deprecated Use remove() instead which provides proper error handling and type safety. + * @param key The key to remove */ public async removeLocalConfig(key: string): Promise { await Store.remove(STORE_NAMESPACE, key) diff --git a/packages/hoppscotch-common/src/services/tab/graphql.ts b/packages/hoppscotch-common/src/services/tab/graphql.ts index 5741ed09..decdc4bc 100644 --- a/packages/hoppscotch-common/src/services/tab/graphql.ts +++ b/packages/hoppscotch-common/src/services/tab/graphql.ts @@ -5,7 +5,7 @@ import { TabService } from "./tab" import { computed } from "vue" import { Container } from "dioc" import { getService } from "~/modules/dioc" -import { PersistenceService } from "../persistence" +import { PersistenceService, STORE_KEYS } from "../persistence" import { PersistableTabState } from "." export class GQLTabService extends TabService { @@ -46,16 +46,10 @@ export class GQLTabService extends TabService { protected async loadPersistedState(): Promise | null> { const persistenceService = getService(PersistenceService) - const savedState = await persistenceService.getLocalConfig("gqlTabs") - - if (savedState) { - try { - return JSON.parse(savedState) as PersistableTabState - } catch { - return null - } - } - return null + const savedState = await persistenceService.getNullable< + PersistableTabState + >(STORE_KEYS.GQL_TABS) + return savedState } public getTabRefWithSaveContext(ctx: HoppGQLSaveContext) { diff --git a/packages/hoppscotch-common/src/services/tab/rest.ts b/packages/hoppscotch-common/src/services/tab/rest.ts index 1a4e6265..b19e16f1 100644 --- a/packages/hoppscotch-common/src/services/tab/rest.ts +++ b/packages/hoppscotch-common/src/services/tab/rest.ts @@ -4,7 +4,7 @@ import { computed } from "vue" import { getDefaultRESTRequest } from "~/helpers/rest/default" import { HoppRESTSaveContext, HoppTabDocument } from "~/helpers/rest/document" import { getService } from "~/modules/dioc" -import { PersistenceService } from "../persistence" +import { PersistenceService, STORE_KEYS } from "../persistence" import { TabService } from "./tab" import { PersistableTabState } from "." @@ -65,16 +65,10 @@ export class RESTTabService extends TabService { protected async loadPersistedState(): Promise | null> { const persistenceService = getService(PersistenceService) - const savedState = await persistenceService.getLocalConfig("restTabs") - - if (savedState) { - try { - return JSON.parse(savedState) as PersistableTabState - } catch { - return null - } - } - return null + const savedState = await persistenceService.getNullable< + PersistableTabState + >(STORE_KEYS.REST_TABS) + return savedState } public getTabRefWithSaveContext(ctx: HoppRESTSaveContext) {