diff --git a/package.json b/package.json index ab927fd245..5a452a19d3 100644 --- a/package.json +++ b/package.json @@ -205,7 +205,7 @@ "grapheme-splitter": "^1.0.4", "handlebars": "^4.7.7", "http-proxy": "^1.18.1", - "immer": "^8.0.4", + "immer": "^9.0.6", "joi": "^17.4.2", "js-yaml": "^3.14.0", "jsdom": "^16.7.0", diff --git a/src/renderer/utils/__tests__/storageHelper.test.ts b/src/renderer/utils/__tests__/storageHelper.test.ts index 11127ee3c4..57141450c1 100644 --- a/src/renderer/utils/__tests__/storageHelper.test.ts +++ b/src/renderer/utils/__tests__/storageHelper.test.ts @@ -19,149 +19,109 @@ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -import { reaction } from "mobx"; -import { StorageAdapter, StorageHelper } from "../storageHelper"; +import { observable, reaction } from "mobx"; +import { StorageHelper } from "../storageHelper"; import { delay } from "../../../common/utils/delay"; -describe("renderer/utils/StorageHelper", () => { - describe("window.localStorage might be used as StorageAdapter", () => { - type StorageModel = string; +type StorageModel = { + [prop: string]: any /*json-serializable*/; + message?: string; + description?: any; +}; +describe("renderer/utils/StorageHelper", () => { + describe("Using custom StorageAdapter", () => { const storageKey = "ui-settings"; + const remoteStorageMock = observable.map(); let storageHelper: StorageHelper; + let storageHelperAsync: StorageHelper; beforeEach(() => { - localStorage.clear(); + remoteStorageMock.set(storageKey, { + message: "saved-before", // pretending as previously saved data + }); storageHelper = new StorageHelper(storageKey, { autoInit: false, - storage: localStorage, - defaultValue: "test", - }); - }); - - it("initialized with default value", async () => { - localStorage.setItem(storageKey, "saved"); // pretending it was saved previously - - expect(storageHelper.key).toBe(storageKey); - expect(storageHelper.defaultValue).toBe("test"); - expect(storageHelper.get()).toBe("test"); - - storageHelper.init(); - - expect(storageHelper.key).toBe(storageKey); - expect(storageHelper.defaultValue).toBe("test"); - expect(storageHelper.get()).toBe("saved"); - }); - - it("updates storage", async () => { - storageHelper.init(); - - storageHelper.set("test2"); - expect(localStorage.getItem(storageKey)).toBe("test2"); - - localStorage.setItem(storageKey, "test3"); - storageHelper.init({ force: true }); // reload from underlying storage and merge - expect(storageHelper.get()).toBe("test3"); - }); - }); - - describe("Using custom StorageAdapter", () => { - type SettingsStorageModel = { - [key: string]: any; - message: string; - }; - - const storageKey = "mySettings"; - const storageMock: Record = {}; - let storageHelper: StorageHelper; - let storageHelperAsync: StorageHelper; - let storageAdapter: StorageAdapter; - - const storageHelperDefaultValue: SettingsStorageModel = { - message: "hello-world", - anyOtherStorableData: 123, - }; - - beforeEach(() => { - storageMock[storageKey] = { - message: "saved-before", - } as SettingsStorageModel; - - storageAdapter = { - onChange: jest.fn(), - getItem: jest.fn((key: string) => { - return storageMock[key]; - }), - setItem: jest.fn((key: string, value: any) => { - storageMock[key] = value; - }), - removeItem: jest.fn((key: string) => { - delete storageMock[key]; - }), - }; - - storageHelper = new StorageHelper(storageKey, { - autoInit: false, - defaultValue: storageHelperDefaultValue, - storage: storageAdapter, + defaultValue: { + message: "blabla", + description: "default" + }, + storage: { + getItem(key: string): StorageModel { + return Object.assign( + storageHelper.defaultValue, + remoteStorageMock.get(key), + ); + }, + setItem(key: string, value: StorageModel) { + remoteStorageMock.set(key, value); + }, + removeItem(key: string) { + remoteStorageMock.delete(key); + } + }, }); storageHelperAsync = new StorageHelper(storageKey, { autoInit: false, - defaultValue: storageHelperDefaultValue, + defaultValue: storageHelper.defaultValue, storage: { - ...storageAdapter, - async getItem(key: string): Promise { + ...storageHelper.storage, + async getItem(key: string): Promise { await delay(500); // fake loading timeout - return storageAdapter.getItem(key); + return storageHelper.storage.getItem(key); } }, }); }); - it("loads data from storage with fallback to default-value", () => { - expect(storageHelper.get()).toEqual(storageHelperDefaultValue); + it("initialized with default value", async () => { storageHelper.init(); - - expect(storageHelper.get().message).toBe("saved-before"); - expect(storageAdapter.getItem).toHaveBeenCalledWith(storageHelper.key); + expect(storageHelper.key).toBe(storageKey); + expect(storageHelper.get()).toEqual(storageHelper.defaultValue); }); it("async loading from storage supported too", async () => { expect(storageHelperAsync.initialized).toBeFalsy(); storageHelperAsync.init(); await delay(300); - expect(storageHelperAsync.get()).toEqual(storageHelperDefaultValue); + expect(storageHelperAsync.get()).toEqual(storageHelper.defaultValue); await delay(200); expect(storageHelperAsync.get().message).toBe("saved-before"); }); it("set() fully replaces data in storage", () => { storageHelper.init(); - storageHelper.set({ message: "test2" }); - expect(storageHelper.get().message).toBe("test2"); - expect(storageMock[storageKey]).toEqual({ message: "test2" }); - expect(storageAdapter.setItem).toHaveBeenCalledWith(storageHelper.key, { message: "test2" }); + storageHelper.set({ message: "msg" }); + storageHelper.get().description = "desc"; + expect(storageHelper.get().message).toBe("msg"); + expect(storageHelper.get().description).toBe("desc"); + expect(remoteStorageMock.get(storageKey)).toEqual({ + message: "msg", + description: "desc", + } as StorageModel); }); it("merge() does partial data tree updates", () => { storageHelper.init(); storageHelper.merge({ message: "updated" }); - expect(storageHelper.get()).toEqual({ ...storageHelperDefaultValue, message: "updated" }); - expect(storageAdapter.setItem).toHaveBeenCalledWith(storageHelper.key, { ...storageHelperDefaultValue, message: "updated" }); + expect(storageHelper.get()).toEqual({ ...storageHelper.defaultValue, message: "updated" }); + expect(remoteStorageMock.get(storageKey)).toEqual({ ...storageHelper.defaultValue, message: "updated" }); + // `draft` modified inside, returning `void` is expected storageHelper.merge(draft => { draft.message = "updated2"; }); - expect(storageHelper.get()).toEqual({ ...storageHelperDefaultValue, message: "updated2" }); + expect(storageHelper.get()).toEqual({ ...storageHelper.defaultValue, message: "updated2" }); + // returning object modifies `draft` as well storageHelper.merge(draft => ({ message: draft.message.replace("2", "3") })); - expect(storageHelper.get()).toEqual({ ...storageHelperDefaultValue, message: "updated3" }); + expect(storageHelper.get()).toEqual({ ...storageHelper.defaultValue, message: "updated3" }); }); }); diff --git a/src/renderer/utils/storageHelper.ts b/src/renderer/utils/storageHelper.ts index a7931ae41e..80c5df8fba 100755 --- a/src/renderer/utils/storageHelper.ts +++ b/src/renderer/utils/storageHelper.ts @@ -20,10 +20,9 @@ */ // Helper for working with storages (e.g. window.localStorage, NodeJS/file-system, etc.) - import { action, comparer, makeObservable, observable, toJS, when, } from "mobx"; -import produce, { Draft } from "immer"; -import { isEqual, isFunction, isPlainObject } from "lodash"; +import produce, { Draft, isDraft } from "immer"; +import { isEqual } from "lodash"; import logger from "../../main/logger"; export interface StorageAdapter { @@ -150,15 +149,22 @@ export class StorageHelper { @action merge(value: Partial | ((draft: Draft) => Partial | void)) { - const nextValue = produce(this.toJSON(), (state: Draft) => { - const newValue = isFunction(value) ? value(state) : value; + const nextValue = produce(this.toJSON(), (draft: Draft) => { - return isPlainObject(newValue) - ? Object.assign(state, newValue) // partial updates for returned plain objects - : newValue; + if (typeof value == "function") { + const newValue = value(draft); + + // merge returned plain objects from `value-as-callback` usage + // otherwise `draft` can be just modified inside a callback without returning any value (void) + if (newValue && !isDraft(newValue)) { + Object.assign(draft, newValue); + } + } else { + Object.assign(draft, value); + } }); - this.set(nextValue as T); + this.set(nextValue); } toJSON(): T { diff --git a/yarn.lock b/yarn.lock index 32ae056719..b6a5f1bbde 100644 --- a/yarn.lock +++ b/yarn.lock @@ -7278,10 +7278,10 @@ immediate@~3.0.5: resolved "https://registry.yarnpkg.com/immediate/-/immediate-3.0.6.tgz#9db1dbd0faf8de6fbe0f5dd5e56bb606280de69b" integrity sha1-nbHb0Pr43m++D13V5Wu2BigN5ps= -immer@^8.0.4: - version "8.0.4" - resolved "https://registry.yarnpkg.com/immer/-/immer-8.0.4.tgz#3a21605a4e2dded852fb2afd208ad50969737b7a" - integrity sha512-jMfL18P+/6P6epANRvRk6q8t+3gGhqsJ9EuJ25AXE+9bNTYtssvzeYbEd0mXRYWCmmXSIbnlpz6vd6iJlmGGGQ== +immer@^9.0.6: + version "9.0.6" + resolved "https://registry.yarnpkg.com/immer/-/immer-9.0.6.tgz#7a96bf2674d06c8143e327cbf73539388ddf1a73" + integrity sha512-G95ivKpy+EvVAnAab4fVa4YGYn24J1SpEktnJX7JJ45Bd7xqME/SCplFzYFmTbrkwZbQ4xJK1xMTUYBkN6pWsQ== import-cwd@^3.0.0: version "3.0.0"