fix: set computed value inside store and not the value received

Value received from the set can be a function, but the store
should contain only the computed value. It should be a clone
of the local storage.

Avoid to set initial value inside store on read if a value is already present.

Add test for checking local storage and store updates.

Fix mock of zustand which was not really resetting the store to
initial value between tests.

Add util to export things for tests only.

Update zustand dependency

refs: SHELL-27

__mocks__/zustand/index.ts

@@ -14,11 +14,13 @@ const storeResetFns = new Set<() => void>();
 // when creating a store, we get its initial state, create a reset function and add it in the set
 export const create =
 	<S>() =>
-	(createState: StateCreator<S>): UseBoundStore<StoreApi<S>> => {
-		const store = actualCreate(createState);
-		const initialState = store.getState();
-		storeResetFns.add(() => store.setState(initialState, true));
-		return store;
+	(actualCreateState: StateCreator<S>): UseBoundStore<StoreApi<S>> => {
+		const createState: StateCreator<S> = (setState, getState, store) => {
+			const state = actualCreateState(setState, getState, store);
+			storeResetFns.add(() => store.setState(actualCreateState(setState, getState, store), true));
+			return state;
+		};
+		return actualCreate(createState);
 	};
 
 // Reset all stores after each test run

package.json

@@ -112,7 +112,7 @@
 		"react-widgets-moment": "^4.0.30",
 		"tinymce": "^6.4.0",
 		"ua-parser-js": "^1.0.34",
-		"zustand": "^4.3.6"
+		"zustand": "^4.3.8"
 	},
 	"peerDependencies": {
 		"@reduxjs/toolkit": "^1 >=1.9",

src/shell/hooks/useLocalStorage.test.tsx

@@ -0,0 +1,193 @@
+/*
+ * SPDX-FileCopyrightText: 2023 Zextras <https://www.zextras.com>
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only
+ */
+import React, { useCallback, useEffect, useState } from 'react';
+import { screen, within } from '@testing-library/react';
+import { exportForTest, useLocalStorage } from './useLocalStorage';
+import { setup } from '../../test/utils';
+
+describe('use local storage', () => {
+	const TestComponent = <T,>({
+		initialValue,
+		updatedValue,
+		localStorageKey = 'test-key'
+	}: {
+		initialValue: T;
+		updatedValue: React.SetStateAction<T>;
+		localStorageKey?: string;
+	}): JSX.Element => {
+		const [localStorageValue, setLocalStorageValue] = useLocalStorage(
+			localStorageKey,
+			initialValue
+		);
+		const localStorageStoreValue = exportForTest.useLocalStorageStore?.(
+			(state) => state.storage[localStorageKey]
+		);
+		const [localStorageUpdates, setLocalStorageUpdates] = useState<unknown[]>([]);
+		const [localStorageStoreUpdates, setLocalStorageStoreUpdates] = useState<unknown[]>([]);
+
+		useEffect(() => {
+			setLocalStorageUpdates((prevState) => [...prevState, localStorageValue]);
+		}, [localStorageValue]);
+
+		useEffect(() => {
+			setLocalStorageStoreUpdates((prevState) => [...prevState, localStorageStoreValue]);
+		}, [localStorageStoreValue]);
+
+		const setValueOnClick = useCallback((): void => {
+			setLocalStorageValue(updatedValue);
+		}, [setLocalStorageValue, updatedValue]);
+
+		return (
+			<>
+				<span>LocalStorageValue: {JSON.stringify(localStorageValue)}</span>
+				<span>LocalStorageValueUpdates: {JSON.stringify(localStorageUpdates)}</span>
+				<span>LocalStorageStoreValue: {JSON.stringify(localStorageStoreValue)}</span>
+				<span>LocalStorageStoreValueUpdates: {JSON.stringify(localStorageStoreUpdates)}</span>
+				<button onClick={setValueOnClick}>Update value</button>
+			</>
+		);
+	};
+
+	test('should show localStorage value and store value', () => {
+		const initial = 'initial';
+		const updated = 'updated';
+		const updatesLocalStorage = [initial];
+		const updatesLocalStorageStore = [undefined, initial];
+		setup(<TestComponent initialValue={initial} updatedValue={updated} />);
+		expect(screen.getByText(`LocalStorageValue: ${JSON.stringify(initial)}`)).toBeVisible();
+		expect(
+			screen.getByText(`LocalStorageValueUpdates: ${JSON.stringify(updatesLocalStorage)}`)
+		).toBeVisible();
+		expect(screen.getByText(`LocalStorageStoreValue: ${JSON.stringify(initial)}`)).toBeVisible();
+		expect(
+			screen.getByText(`LocalStorageStoreValueUpdates: ${JSON.stringify(updatesLocalStorageStore)}`)
+		).toBeVisible();
+		expect(screen.queryByText(RegExp(updated, 'i'))).not.toBeInTheDocument();
+	});
+
+	test('should set the new value in the store when set is called with the new value', async () => {
+		const initial = 'initial';
+		const updated = 'updated';
+		const updatesLocalStorage = [initial, updated];
+		const updatesLocalStorageStore = [undefined, initial, updated];
+		const { user } = setup(<TestComponent initialValue={initial} updatedValue={updated} />);
+		await user.click(screen.getByRole('button'));
+		expect(screen.getByText(`LocalStorageValue: ${JSON.stringify(updated)}`)).toBeVisible();
+		expect(
+			screen.getByText(`LocalStorageValueUpdates: ${JSON.stringify(updatesLocalStorage)}`)
+		).toBeVisible();
+		expect(screen.getByText(`LocalStorageStoreValue: ${JSON.stringify(updated)}`)).toBeVisible();
+		expect(
+			screen.getByText(`LocalStorageStoreValueUpdates: ${JSON.stringify(updatesLocalStorageStore)}`)
+		).toBeVisible();
+	});
+
+	test('should set the new value in the store when set is called with the callback', async () => {
+		const initial = 'initial';
+		const updated = 'updated';
+		const updatesLocalStorage = [initial, updated];
+		const updatesLocalStorageStore = [undefined, initial, updated];
+		const updateFn = (): string => updated;
+		const { user } = setup(<TestComponent initialValue={initial} updatedValue={updateFn} />);
+		await user.click(screen.getByRole('button'));
+		expect(screen.getByText(`LocalStorageValue: ${JSON.stringify(updated)}`)).toBeVisible();
+		expect(
+			screen.getByText(`LocalStorageValueUpdates: ${JSON.stringify(updatesLocalStorage)}`)
+		).toBeVisible();
+		expect(screen.getByText(`LocalStorageStoreValue: ${JSON.stringify(updated)}`)).toBeVisible();
+		expect(
+			screen.getByText(`LocalStorageStoreValueUpdates: ${JSON.stringify(updatesLocalStorageStore)}`)
+		).toBeVisible();
+	});
+
+	test('should propagate update of local storage value across different usages of hook', async () => {
+		const initial1 = 'initial1';
+		const initial2 = 'initial2';
+		const updated1 = 'updated1';
+		const updated2 = 'updated2';
+		const updatesLocalStorage1 = [initial1, updated2];
+		const updatesLocalStorageStore = [undefined, initial1, updated2];
+		const updatesLocalStorage2 = [initial2, initial1, updated2];
+		const { user } = setup(
+			<>
+				<div data-testid={'block-1'}>
+					<TestComponent initialValue={initial1} updatedValue={updated1} />
+				</div>
+				<div data-testid={'block-2'}>
+					<TestComponent initialValue={initial2} updatedValue={updated2} />
+				</div>
+			</>
+		);
+		const withinBlock1 = within(screen.getByTestId('block-1'));
+		const withinBlock2 = within(screen.getByTestId('block-2'));
+		expect(withinBlock1.getByText(`LocalStorageValue: ${JSON.stringify(initial1)}`)).toBeVisible();
+		expect(
+			withinBlock1.getByText(`LocalStorageStoreValue: ${JSON.stringify(initial1)}`)
+		).toBeVisible();
+		expect(withinBlock2.getByText(`LocalStorageValue: ${JSON.stringify(initial1)}`)).toBeVisible();
+		expect(
+			withinBlock2.getByText(`LocalStorageStoreValue: ${JSON.stringify(initial1)}`)
+		).toBeVisible();
+
+		await user.click(withinBlock2.getByRole('button'));
+		expect(withinBlock1.getByText(`LocalStorageValue: ${JSON.stringify(updated2)}`)).toBeVisible();
+		expect(
+			withinBlock1.getByText(`LocalStorageValueUpdates: ${JSON.stringify(updatesLocalStorage1)}`)
+		).toBeVisible();
+		expect(
+			withinBlock1.getByText(`LocalStorageStoreValue: ${JSON.stringify(updated2)}`)
+		).toBeVisible();
+		expect(
+			withinBlock1.getByText(
+				`LocalStorageStoreValueUpdates: ${JSON.stringify(updatesLocalStorageStore)}`
+			)
+		).toBeVisible();
+		expect(withinBlock2.getByText(`LocalStorageValue: ${JSON.stringify(updated2)}`)).toBeVisible();
+		expect(
+			withinBlock2.getByText(`LocalStorageValueUpdates: ${JSON.stringify(updatesLocalStorage2)}`)
+		).toBeVisible();
+		expect(
+			withinBlock2.getByText(`LocalStorageStoreValue: ${JSON.stringify(updated2)}`)
+		).toBeVisible();
+		expect(
+			withinBlock2.getByText(
+				`LocalStorageStoreValueUpdates: ${JSON.stringify(updatesLocalStorageStore)}`
+			)
+		).toBeVisible();
+	});
+
+	test('should show the fallback value if the value inside the local storage cannot be parsed', async () => {
+		const lsKey = 'test-key';
+		window.localStorage.setItem(lsKey, 'not a JSON');
+		const initial = 'initial';
+		const updatesLocalStorage = [initial];
+		const updatesLocalStorageStore = [undefined, initial];
+		const actualConsoleError = console.error;
+		console.error = jest.fn<ReturnType<typeof console.error>, Parameters<typeof console.error>>(
+			(error, ...rest) => {
+				if (
+					error instanceof Error &&
+					error.message === 'Unexpected token o in JSON at position 1'
+				) {
+					console.log('Controlled error', error, ...rest);
+				} else {
+					actualConsoleError(error, ...rest);
+				}
+			}
+		);
+		setup(
+			<TestComponent initialValue={initial} updatedValue={'updated'} localStorageKey={lsKey} />
+		);
+		expect(screen.getByText(`LocalStorageValue: ${JSON.stringify(initial)}`)).toBeVisible();
+		expect(
+			screen.getByText(`LocalStorageValueUpdates: ${JSON.stringify(updatesLocalStorage)}`)
+		).toBeVisible();
+		expect(screen.getByText(`LocalStorageStoreValue: ${JSON.stringify(initial)}`)).toBeVisible();
+		expect(
+			screen.getByText(`LocalStorageStoreValueUpdates: ${JSON.stringify(updatesLocalStorageStore)}`)
+		).toBeVisible();
+	});
+});

src/shell/hooks/useLocalStorage.ts

@@ -5,6 +5,7 @@
  */
 import React, { useCallback, useEffect, useMemo, useRef } from 'react';
 import { create } from 'zustand';
+import { createExportForTestOnly } from '../../utils/utils';
 
 function isSameLocalStorageValue(valueA: unknown, valueB: unknown): boolean {
 	return JSON.stringify(valueA) === JSON.stringify(valueB);
@@ -25,7 +26,7 @@ const useLocalStorageStore = create<LocalStorageState>()((setState) => ({
 			const localStorageItem = window.localStorage.getItem(key);
 			const item = localStorageItem !== null ? JSON.parse(localStorageItem) : fallback;
 			setState((state) => {
-				if (!isSameLocalStorageValue(item, state.storage[key])) {
+				if (state.storage[key] === undefined) {
 					const newState = { ...state };
 					newState.storage[key] = item;
 					return newState;
@@ -47,7 +48,7 @@ const useLocalStorageStore = create<LocalStorageState>()((setState) => ({
 			const newState = { ...state };
 			if (!isSameLocalStorageValue(valueToStore, state.storage[key])) {
 				window.localStorage.setItem(key, JSON.stringify(valueToStore));
-				newState.storage[key] = value;
+				newState.storage[key] = valueToStore;
 				return newState;
 			}
 			return state;
@@ -105,3 +106,5 @@ export function useLocalStorage<T>(
 
 	return [storedValue, setValueForKey];
 }
+
+export const exportForTest = createExportForTestOnly({ useLocalStorageStore });

src/shell/hooks/useResize.ts

@@ -5,7 +5,12 @@
  */
 import React, { CSSProperties, useCallback, useEffect, useRef } from 'react';
 import { find, forEach } from 'lodash';
-import { setElementSizeAndPosition, setGlobalCursor, SizeAndPosition } from '../../utils/utils';
+import {
+	createExportForTestOnly,
+	setElementSizeAndPosition,
+	setGlobalCursor,
+	SizeAndPosition
+} from '../../utils/utils';
 import { useLocalStorage } from './useLocalStorage';
 
 /**
@@ -159,4 +164,4 @@ export const useResize = (
 	);
 };
 
-export const exportForTest = process.env.NODE_ENV === 'test' ? { calcNewSizeAndPosition } : {};
+export const exportForTest = createExportForTestOnly({ calcNewSizeAndPosition });

src/utils/utils.ts

@@ -4,6 +4,7 @@
  * SPDX-License-Identifier: AGPL-3.0-only
  */
 import React, { CSSProperties } from 'react';
+import { reduce } from 'lodash';
 
 export type ElementPosition = {
 	top: number;
@@ -46,3 +47,18 @@ export function setElementSizeAndPosition(
 export function stopPropagation(event: Event | React.SyntheticEvent): void {
 	event.stopPropagation();
 }
+
+export function createExportForTestOnly<TObj extends Record<string, unknown>>(
+	objToExport: TObj
+): { [K in keyof TObj]: TObj[K] | undefined } {
+	return process.env.NODE_ENV === 'test'
+		? objToExport
+		: reduce(
+				objToExport,
+				(accumulator, value, key) => {
+					accumulator[key as keyof TObj] = undefined;
+					return accumulator;
+				},
+				{} as Record<keyof TObj, undefined>
+		  );
+}