From 2279ac01ec3b69826f7b0c157626ffa1910e4bd5 Mon Sep 17 00:00:00 2001 From: Roman Date: Wed, 3 Mar 2021 12:58:39 +0200 Subject: [PATCH] clean up / responding to comments Signed-off-by: Roman --- integration/__tests__/cluster-pages.tests.ts | 18 ++++--- src/renderer/hooks/useStorage.ts | 2 +- src/renderer/local-storage.ts | 13 +++-- src/renderer/utils/storageHelper.ts | 50 ++++++++------------ 4 files changed, 39 insertions(+), 44 deletions(-) diff --git a/integration/__tests__/cluster-pages.tests.ts b/integration/__tests__/cluster-pages.tests.ts index d145d7f31a..26d519bb3e 100644 --- a/integration/__tests__/cluster-pages.tests.ts +++ b/integration/__tests__/cluster-pages.tests.ts @@ -48,13 +48,13 @@ describe("Lens cluster pages", () => { } }; - function getSidebarSelectors(testId: string) { - const baseSelector = `.Sidebar [data-test-id="${testId}"]`; + function getMainMenuSelectors(itemId: string) { + const baseSelector = `.Sidebar [data-test-id="${itemId}"]`; return { sidebarItemRoot: baseSelector, expandIcon: `${baseSelector} .expand-icon`, - pageLink(href: string){ + pageLink(href: string) { return `${baseSelector} a[href^="/${href}"]`; } }; @@ -74,7 +74,11 @@ describe("Lens cluster pages", () => { testId: string; expectedSelector?: string; expectedText?: string; - subMenu?: Required>[]; + subMenu?: { + href: string; + expectedSelector: string; + expectedText: string; + }[]; }; const sidebarMenu: SidebarItem[] = [ @@ -267,7 +271,7 @@ describe("Lens cluster pages", () => { }]; sidebarMenu.forEach(({ testId, expectedSelector, expectedText, subMenu }) => { - const { sidebarItemRoot, expandIcon, pageLink } = getSidebarSelectors(testId); + const { sidebarItemRoot, expandIcon, pageLink } = getMainMenuSelectors(testId); if (subMenu) { it(`expands submenu for pages in "${testId}"`, async () => { @@ -304,7 +308,7 @@ describe("Lens cluster pages", () => { it(`shows a logs for a pod`, async () => { expect(clusterAdded).toBe(true); // Go to Pods page - await app.client.click(getSidebarSelectors("workloads").expandIcon); + await app.client.click(getMainMenuSelectors("workloads").expandIcon); await app.client.waitUntilTextExists('a[href^="/pods"]', "Pods"); await app.client.click('a[href^="/pods"]'); await app.client.click(".NamespaceSelect"); @@ -371,7 +375,7 @@ describe("Lens cluster pages", () => { it(`creates a pod in ${TEST_NAMESPACE} namespace`, async () => { expect(clusterAdded).toBe(true); - await app.client.click(getSidebarSelectors("workloads").expandIcon); + await app.client.click(getMainMenuSelectors("workloads").expandIcon); await app.client.waitUntilTextExists('a[href^="/pods"]', "Pods"); await app.client.click('a[href^="/pods"]'); diff --git a/src/renderer/hooks/useStorage.ts b/src/renderer/hooks/useStorage.ts index e859cb17fd..f3bf4d5de6 100644 --- a/src/renderer/hooks/useStorage.ts +++ b/src/renderer/hooks/useStorage.ts @@ -1,7 +1,7 @@ import { useState } from "react"; import { createStorage, StorageHelperOptions } from "../local-storage"; -export function useStorage(key: string, initialValue?: T, options?: StorageHelperOptions) { +export function useStorage(key: string, initialValue?: T, options?: StorageHelperOptions) { const storage = createStorage(key, initialValue, options); const [storageValue, setStorageValue] = useState(storage.get()); const setValue = (value: T) => { diff --git a/src/renderer/local-storage.ts b/src/renderer/local-storage.ts index cfc098a199..bff3acf7e0 100644 --- a/src/renderer/local-storage.ts +++ b/src/renderer/local-storage.ts @@ -1,6 +1,6 @@ import path from "path"; import fse from "fs-extra"; -import { isEmpty } from "lodash"; +import { isEmpty, noop } from "lodash"; import { app, remote } from "electron"; import { action, observable, reaction, when, } from "mobx"; import { StorageHelper, StorageHelperOptions } from "./utils/storageHelper"; @@ -37,7 +37,7 @@ export class LensLocalStorage { } @action - async load() { + private async load() { try { await fse.ensureDir(this.folderPath, { mode: 0o755 }); const state = await fse.readJson(this.filePath); @@ -63,14 +63,14 @@ export class LensLocalStorage { @action clear() { this.state.clear(); - fse.unlink(this.filePath).catch(() => null); + fse.unlink(this.filePath).catch(noop); } } export const lensLocalStorage = new LensLocalStorage(); export function createStorage(key: string, defaultValue?: T, options: StorageHelperOptions = {}) { - return new StorageHelper(key, defaultValue, { + return new StorageHelper(key, defaultValue, { ...options, storage: { async getItem(key: string) { @@ -78,9 +78,12 @@ export function createStorage(key: string, defaultValue?: T, options: Storage return lensLocalStorage.state.get(key); }, - async setItem(key: string, value: any) { + setItem(key: string, value: any) { lensLocalStorage.state.set(key, value); }, + removeItem(key: string) { + lensLocalStorage.state.delete(key); + }, }, }); } diff --git a/src/renderer/utils/storageHelper.ts b/src/renderer/utils/storageHelper.ts index f1bfe66727..2502b75403 100755 --- a/src/renderer/utils/storageHelper.ts +++ b/src/renderer/utils/storageHelper.ts @@ -8,36 +8,36 @@ import { isEmpty, isEqual, isFunction } from "lodash"; setAutoFreeze(false); // allow to merge observables -export interface StorageHelperOptions extends StorageConfiguration { +export interface StorageHelperOptions extends StorageConfiguration { autoInit?: boolean; // default: true } -export interface StorageConfiguration { +export interface StorageConfiguration { storage?: StorageAdapter; observable?: CreateObservableOptions; } -export interface StorageAdapter> { - getItem(this: C, key: string): T | Promise; // import - setItem(this: C, key: string, value: T): void; // export - onChange?(this: C, value: T, oldValue?: T): void; +export interface StorageAdapter { + getItem(key: string): T | Promise; // import + setItem(key: string, value: T): void; // export + removeItem?(key: string): void; // if not provided setItem(key,undefined) will be used + onChange?(value: T, oldValue?: T): void; } -export const localStorageAdapter: StorageAdapter = { +export const localStorageAdapter: StorageAdapter> = { getItem(key: string) { return JSON.parse(localStorage.getItem(key)); }, setItem(key: string, value: any) { - if (value != null) { - localStorage.setItem(key, JSON.stringify(value)); - } else { - localStorage.removeItem(key); - } + localStorage.setItem(key, JSON.stringify(value)); + }, + removeItem(key: string) { + localStorage.removeItem(key); } }; export class StorageHelper { - static defaultOptions: StorageHelperOptions = { + static defaultOptions: StorageHelperOptions = { autoInit: true, storage: localStorageAdapter, observable: { @@ -47,11 +47,11 @@ export class StorageHelper { }; private data = observable.box(); - @observable.ref storage: StorageAdapter>; + @observable.ref storage: StorageAdapter; @observable initialized = false; whenReady = when(() => this.initialized); - constructor(readonly key: string, readonly defaultValue?: T, readonly options: StorageHelperOptions = {}) { + constructor(readonly key: string, readonly defaultValue?: T, readonly options: StorageHelperOptions = {}) { this.options = { ...StorageHelper.defaultOptions, ...options }; this.configure(); this.reset(); @@ -88,7 +88,7 @@ export class StorageHelper { } @action - configure({ storage, observable }: StorageConfiguration = this.options): this { + private configure({ storage, observable }: StorageConfiguration = this.options): this { if (storage) this.configureStorage(storage); if (observable) this.configureObservable(observable); @@ -96,8 +96,8 @@ export class StorageHelper { } @action - configureStorage(storage: StorageAdapter) { - this.storage = Object.getOwnPropertyNames(storage).reduce((storage, name: keyof StorageAdapter) => { + protected configureStorage(storage: StorageAdapter) { + this.storage = Object.getOwnPropertyNames(storage).reduce((storage, name: keyof StorageAdapter) => { storage[name] = storage[name]?.bind(this); // bind storage-adapter methods to "this"-context return storage; @@ -105,7 +105,7 @@ export class StorageHelper { } @action - configureObservable(options: CreateObservableOptions = {}) { + protected configureObservable(options: CreateObservableOptions = {}) { this.data = observable.box(this.data.get(), { ...StorageHelper.defaultOptions.observable, // inherit default observability options ...options, @@ -152,18 +152,6 @@ export class StorageHelper { this.set(nextValue); } - // TODO: experiment / proxy to target object - proxyTo(object: object) { - return new Proxy(object, { - get: (target: object, prop: PropertyKey, receiver: any) => { - return Reflect.get(target, prop, receiver); - }, - set: (target: object, prop: PropertyKey, value: any, receiver: any) => { - return Reflect.set(target, prop, value, receiver); - } - }); - } - toJS() { return toJS(this.get(), { recurseEverything: true }); }