Browse Source

fix(common): `PersistenceService` call site compat (#4799)

Shreyas 1 week ago
parent
commit
02bf634f25

+ 56 - 0
packages/hoppscotch-common/src/services/persistence/__tests__/index.spec.ts

@@ -1,5 +1,7 @@
 /* eslint-disable no-restricted-globals, no-restricted-syntax */
 /* eslint-disable no-restricted-globals, no-restricted-syntax */
 
 
+import * as E from "fp-ts/Either"
+
 import {
 import {
   translateToNewGQLCollection,
   translateToNewGQLCollection,
   translateToNewRESTCollection,
   translateToNewRESTCollection,
@@ -1875,4 +1877,58 @@ describe("PersistenceService", () => {
     )
     )
     expect(removeItemSpy).toHaveBeenCalledWith(`${STORE_NAMESPACE}:${testKey}`)
     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}`)
+  })
 })
 })

+ 71 - 5
packages/hoppscotch-common/src/services/persistence/index.ts

@@ -4,7 +4,7 @@ import * as E from "fp-ts/Either"
 import { z } from "zod"
 import { z } from "zod"
 
 
 import { Service } from "dioc"
 import { Service } from "dioc"
-import { watchDebounced } from "@vueuse/core"
+import { StorageLike, watchDebounced } from "@vueuse/core"
 import { assign, clone, isEmpty } from "lodash-es"
 import { assign, clone, isEmpty } from "lodash-es"
 
 
 import {
 import {
@@ -167,6 +167,9 @@ const migrations: Migration[] = [
 export class PersistenceService extends Service {
 export class PersistenceService extends Service {
   public static readonly ID = "PERSISTENCE_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 restTabService = this.bind(RESTTabService)
   private readonly gqlTabService = this.bind(GQLTabService)
   private readonly gqlTabService = this.bind(GQLTabService)
   private readonly secretEnvironmentService = this.bind(
   private readonly secretEnvironmentService = this.bind(
@@ -197,9 +200,8 @@ export class PersistenceService extends Service {
       STORE_NAMESPACE,
       STORE_NAMESPACE,
       STORE_KEYS.SCHEMA_VERSION
       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"
     const targetVersion = "1"
 
 
     if (currentVersion !== targetVersion) {
     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<T>(
+    key: (typeof STORE_KEYS)[keyof typeof STORE_KEYS]
+  ): Promise<E.Either<StoreError, T | undefined>> {
+    return await Store.get<T>(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<T>(
+    key: (typeof STORE_KEYS)[keyof typeof STORE_KEYS]
+  ): Promise<T | null> {
+    const r = await Store.get<T>(STORE_NAMESPACE, key)
+
+    if (E.isLeft(r)) return null
+
+    return r.right ?? null
+  }
+
+  /**
+   * Gets a value from local config
+   * @deprecated Use get<T>() instead which provides automatic deserialization and type safety.
+   * With get<T>(), 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(
   public async getLocalConfig(
     name: string
     name: string
@@ -928,8 +963,26 @@ export class PersistenceService extends Service {
     return null
     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<T>(
+    key: (typeof STORE_KEYS)[keyof typeof STORE_KEYS],
+    value: T
+  ): Promise<E.Either<StoreError, void>> {
+    return await Store.set(STORE_NAMESPACE, key, value)
+  }
+
   /**
   /**
    * Sets a value in persistence
    * Sets a value in persistence
+   * @deprecated Use set<T>() instead which provides automatic serialization and type safety.
+   * With set<T>(), 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<void> {
   public async setLocalConfig(key: string, value: string): Promise<void> {
     await Store.set(STORE_NAMESPACE, key, value)
     await Store.set(STORE_NAMESPACE, key, value)
@@ -937,6 +990,19 @@ export class PersistenceService extends Service {
 
 
   /**
   /**
    * Clear config value from persistence
    * 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<E.Either<StoreError, boolean>> {
+    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<void> {
   public async removeLocalConfig(key: string): Promise<void> {
     await Store.remove(STORE_NAMESPACE, key)
     await Store.remove(STORE_NAMESPACE, key)

+ 5 - 11
packages/hoppscotch-common/src/services/tab/graphql.ts

@@ -5,7 +5,7 @@ import { TabService } from "./tab"
 import { computed } from "vue"
 import { computed } from "vue"
 import { Container } from "dioc"
 import { Container } from "dioc"
 import { getService } from "~/modules/dioc"
 import { getService } from "~/modules/dioc"
-import { PersistenceService } from "../persistence"
+import { PersistenceService, STORE_KEYS } from "../persistence"
 import { PersistableTabState } from "."
 import { PersistableTabState } from "."
 
 
 export class GQLTabService extends TabService<HoppGQLDocument> {
 export class GQLTabService extends TabService<HoppGQLDocument> {
@@ -46,16 +46,10 @@ export class GQLTabService extends TabService<HoppGQLDocument> {
 
 
   protected async loadPersistedState(): Promise<PersistableTabState<HoppGQLDocument> | null> {
   protected async loadPersistedState(): Promise<PersistableTabState<HoppGQLDocument> | null> {
     const persistenceService = getService(PersistenceService)
     const persistenceService = getService(PersistenceService)
-    const savedState = await persistenceService.getLocalConfig("gqlTabs")
-
-    if (savedState) {
-      try {
-        return JSON.parse(savedState) as PersistableTabState<HoppGQLDocument>
-      } catch {
-        return null
-      }
-    }
-    return null
+    const savedState = await persistenceService.getNullable<
+      PersistableTabState<HoppGQLDocument>
+    >(STORE_KEYS.GQL_TABS)
+    return savedState
   }
   }
 
 
   public getTabRefWithSaveContext(ctx: HoppGQLSaveContext) {
   public getTabRefWithSaveContext(ctx: HoppGQLSaveContext) {

+ 5 - 11
packages/hoppscotch-common/src/services/tab/rest.ts

@@ -4,7 +4,7 @@ import { computed } from "vue"
 import { getDefaultRESTRequest } from "~/helpers/rest/default"
 import { getDefaultRESTRequest } from "~/helpers/rest/default"
 import { HoppRESTSaveContext, HoppTabDocument } from "~/helpers/rest/document"
 import { HoppRESTSaveContext, HoppTabDocument } from "~/helpers/rest/document"
 import { getService } from "~/modules/dioc"
 import { getService } from "~/modules/dioc"
-import { PersistenceService } from "../persistence"
+import { PersistenceService, STORE_KEYS } from "../persistence"
 import { TabService } from "./tab"
 import { TabService } from "./tab"
 import { PersistableTabState } from "."
 import { PersistableTabState } from "."
 
 
@@ -65,16 +65,10 @@ export class RESTTabService extends TabService<HoppTabDocument> {
 
 
   protected async loadPersistedState(): Promise<PersistableTabState<HoppTabDocument> | null> {
   protected async loadPersistedState(): Promise<PersistableTabState<HoppTabDocument> | null> {
     const persistenceService = getService(PersistenceService)
     const persistenceService = getService(PersistenceService)
-    const savedState = await persistenceService.getLocalConfig("restTabs")
-
-    if (savedState) {
-      try {
-        return JSON.parse(savedState) as PersistableTabState<HoppTabDocument>
-      } catch {
-        return null
-      }
-    }
-    return null
+    const savedState = await persistenceService.getNullable<
+      PersistableTabState<HoppTabDocument>
+    >(STORE_KEYS.REST_TABS)
+    return savedState
   }
   }
 
 
   public getTabRefWithSaveContext(ctx: HoppRESTSaveContext) {
   public getTabRefWithSaveContext(ctx: HoppRESTSaveContext) {