diff --git a/lib/papi-dts/papi.d.ts b/lib/papi-dts/papi.d.ts index cad6d43a21..62addea595 100644 --- a/lib/papi-dts/papi.d.ts +++ b/lib/papi-dts/papi.d.ts @@ -180,16 +180,26 @@ declare module 'shared/models/web-view.model' { * _＠param_ `stateKey` Key of the state value to use. The webview state holds a unique value per * key. * - * NOTE: `stateKey` needs to be a constant string, not something that could change during execution. + * WARNING: MUST BE STABLE - const or wrapped in useState, useMemo, etc. The reference must not be + * updated every render * * _＠param_ `defaultStateValue` Value to use if the web view state didn't contain a value for the * given 'stateKey' * - * _＠returns_ `[stateValue, setStateValue]` + * Note: this parameter is internally assigned to a `ref`, so changing it will not cause any hooks + * to re-run with its new value. Running `resetWebViewState()` will always update the state value + * returned to the latest `defaultStateValue`, and changing the `stateKey` will use the latest + * `defaultStateValue`. However, if `defaultStateValue` is changed while a state is + * `defaultStateValue` (meaning it is reset and has no value), the returned state value will not be + * updated to the new `defaultStateValue`. + * + * _＠returns_ `[stateValue, setStateValue, resetWebViewState]` * - * - `stateValue`: the current value for the web view state at the key specified or + * - `webViewStateValue`: The current value for the web view state at the key specified or * `defaultStateValue` if a state was not found - * - `setStateValue`: function to use to update the web view state value at the key specified + * - `setWebViewState`: Function to use to update the web view state value at the key specified + * - `resetWebViewState`: Function that removes the web view state and resets the value to + * `defaultStateValue` * * _＠example_ * @@ -200,7 +210,11 @@ declare module 'shared/models/web-view.model' { export type UseWebViewStateHook = ( stateKey: string, defaultStateValue: T, - ) => [webViewState: T, setWebViewState: (stateValue: T) => void, resetWebViewState: () => void]; + ) => [ + webViewStateValue: T, + setWebViewState: (stateValue: T) => void, + resetWebViewState: () => void, + ]; /** * * Gets the updatable properties on this WebView's WebView definition @@ -239,16 +253,26 @@ declare module 'shared/models/web-view.model' { * _＠param_ `stateKey` Key of the state value to use. The webview state holds a unique value per * key. * - * NOTE: `stateKey` needs to be a constant string, not something that could change during execution. + * WARNING: MUST BE STABLE - const or wrapped in useState, useMemo, etc. The reference must not be + * updated every render * * _＠param_ `defaultStateValue` Value to use if the web view state didn't contain a value for the * given 'stateKey' * - * _＠returns_ `[stateValue, setStateValue]` + * Note: this parameter is internally assigned to a `ref`, so changing it will not cause any hooks + * to re-run with its new value. Running `resetWebViewState()` will always update the state value + * returned to the latest `defaultStateValue`, and changing the `stateKey` will use the latest + * `defaultStateValue`. However, if `defaultStateValue` is changed while a state is + * `defaultStateValue` (meaning it is reset and has no value), the returned state value will not be + * updated to the new `defaultStateValue`. + * + * _＠returns_ `[stateValue, setStateValue, resetWebViewState]` * - * - `stateValue`: the current value for the web view state at the key specified or + * - `webViewStateValue`: The current value for the web view state at the key specified or * `defaultStateValue` if a state was not found - * - `setStateValue`: function to use to update the web view state value at the key specified + * - `setWebViewState`: Function to use to update the web view state value at the key specified + * - `resetWebViewState`: Function that removes the web view state and resets the value to + * `defaultStateValue` * * _＠example_ * @@ -349,16 +373,26 @@ declare module 'shared/global-this.model' { * _＠param_ `stateKey` Key of the state value to use. The webview state holds a unique value per * key. * - * NOTE: `stateKey` needs to be a constant string, not something that could change during execution. + * WARNING: MUST BE STABLE - const or wrapped in useState, useMemo, etc. The reference must not be + * updated every render * * _＠param_ `defaultStateValue` Value to use if the web view state didn't contain a value for the * given 'stateKey' * - * _＠returns_ `[stateValue, setStateValue]` + * Note: this parameter is internally assigned to a `ref`, so changing it will not cause any hooks + * to re-run with its new value. Running `resetWebViewState()` will always update the state value + * returned to the latest `defaultStateValue`, and changing the `stateKey` will use the latest + * `defaultStateValue`. However, if `defaultStateValue` is changed while a state is + * `defaultStateValue` (meaning it is reset and has no value), the returned state value will not be + * updated to the new `defaultStateValue`. * - * - `stateValue`: the current value for the web view state at the key specified or + * _＠returns_ `[stateValue, setStateValue, resetWebViewState]` + * + * - `webViewStateValue`: The current value for the web view state at the key specified or * `defaultStateValue` if a state was not found - * - `setStateValue`: function to use to update the web view state value at the key specified + * - `setWebViewState`: Function to use to update the web view state value at the key specified + * - `resetWebViewState`: Function that removes the web view state and resets the value to + * `defaultStateValue` * * _＠example_ * @@ -4048,14 +4082,18 @@ declare module 'renderer/hooks/papi-hooks/use-setting.hook' { * @param defaultState The default state of the setting. If the setting already has a value set to * it in local storage, this parameter will be ignored. * - * WARNING: MUST BE STABLE - const or wrapped in useState, useMemo, etc. The reference must not be - * updated every render + * Note: this parameter is internally assigned to a `ref`, so changing it will not cause any hooks + * to re-run with its new value. Running `resetSetting()` will always update the setting value + * returned to the latest `defaultState`, and changing the `key` will use the latest + * `defaultState`. However, if `defaultState` is changed while a setting is `defaultState` + * (meaning it is reset and has no value), the returned setting value will not be updated to the + * new `defaultState`. * @returns `[setting, setSetting, resetSetting]` * - * - `setting`: The current state of the setting, either the defaultState or the stored state on the + * - `setting`: The current state of the setting, either `defaultState` or the stored state on the * papi, if any * - `setSetting`: Function that updates the setting to a new value - * - `resetSetting`: Function that removes the setting + * - `resetSetting`: Function that removes the setting and resets the value to `defaultState` * * @throws When subscription callback function is called with an update that has an unexpected * message type @@ -4063,7 +4101,11 @@ declare module 'renderer/hooks/papi-hooks/use-setting.hook' { const useSetting: ( key: SettingName, defaultState: SettingTypes[SettingName], - ) => [SettingTypes[SettingName], (newSetting: SettingTypes[SettingName]) => void, () => void]; + ) => [ + setting: SettingTypes[SettingName], + setSetting: (newSetting: SettingTypes[SettingName]) => void, + resetSetting: () => void, + ]; export default useSetting; } declare module 'renderer/hooks/papi-hooks/use-project-data-provider.hook' { diff --git a/src/renderer/hooks/papi-hooks/use-setting.hook.ts b/src/renderer/hooks/papi-hooks/use-setting.hook.ts index 3d85c47be8..99401839ac 100644 --- a/src/renderer/hooks/papi-hooks/use-setting.hook.ts +++ b/src/renderer/hooks/papi-hooks/use-setting.hook.ts @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useState } from 'react'; +import { useCallback, useEffect, useRef, useState } from 'react'; import settingsService from '@shared/services/settings.service'; import { SettingNames, SettingTypes } from 'papi-shared-types'; @@ -13,14 +13,18 @@ import { SettingNames, SettingTypes } from 'papi-shared-types'; * @param defaultState The default state of the setting. If the setting already has a value set to * it in local storage, this parameter will be ignored. * - * WARNING: MUST BE STABLE - const or wrapped in useState, useMemo, etc. The reference must not be - * updated every render + * Note: this parameter is internally assigned to a `ref`, so changing it will not cause any hooks + * to re-run with its new value. Running `resetSetting()` will always update the setting value + * returned to the latest `defaultState`, and changing the `key` will use the latest + * `defaultState`. However, if `defaultState` is changed while a setting is `defaultState` + * (meaning it is reset and has no value), the returned setting value will not be updated to the + * new `defaultState`. * @returns `[setting, setSetting, resetSetting]` * - * - `setting`: The current state of the setting, either the defaultState or the stored state on the + * - `setting`: The current state of the setting, either `defaultState` or the stored state on the * papi, if any * - `setSetting`: Function that updates the setting to a new value - * - `resetSetting`: Function that removes the setting + * - `resetSetting`: Function that removes the setting and resets the value to `defaultState` * * @throws When subscription callback function is called with an update that has an unexpected * message type @@ -28,22 +32,27 @@ import { SettingNames, SettingTypes } from 'papi-shared-types'; const useSetting = ( key: SettingName, defaultState: SettingTypes[SettingName], -): [SettingTypes[SettingName], (newSetting: SettingTypes[SettingName]) => void, () => void] => { - const [setting, setSettingInternal] = useState(settingsService.get(key, defaultState)); +): [ + setting: SettingTypes[SettingName], + setSetting: (newSetting: SettingTypes[SettingName]) => void, + resetSetting: () => void, +] => { + // Use defaultState as a ref so it doesn't update dependency arrays + const defaultStateRef = useRef(defaultState); + defaultStateRef.current = defaultState; - useEffect(() => { - const updateSettingFromService = (newSettingState: SettingTypes[SettingName]) => { - setSettingInternal(newSettingState); - }; + const [setting, setSettingInternal] = useState(settingsService.get(key, defaultStateRef.current)); - const initialSetting = settingsService.get(key, defaultState); - updateSettingFromService(initialSetting); + useEffect(() => { + // Get the setting for the new key when the key changes + setSettingInternal(settingsService.get(key, defaultStateRef.current)); + // Subscribe to changes to the setting const unsubscriber = settingsService.subscribe(key, (newSetting) => { if (newSetting.type === 'update-setting') { - updateSettingFromService(newSetting.setting); + setSettingInternal(newSetting.setting); } else if (newSetting.type === 'reset-setting') { - setSettingInternal(defaultState); + setSettingInternal(defaultStateRef.current); } else { throw new Error('Unexpected message type used for updating setting'); } @@ -52,7 +61,7 @@ const useSetting = ( return () => { unsubscriber(); }; - }, [defaultState, key]); + }, [key]); const setSetting = useCallback( (newSetting: SettingTypes[SettingName]) => { @@ -64,8 +73,8 @@ const useSetting = ( const resetSetting = useCallback(() => { settingsService.reset(key); - setSettingInternal(defaultState); - }, [defaultState, key]); + setSettingInternal(defaultStateRef.current); + }, [key]); return [setting, setSetting, resetSetting]; }; diff --git a/src/renderer/hooks/use-web-view-state.hook.ts b/src/renderer/hooks/use-web-view-state.hook.ts index 1088c2c8c0..b35319df78 100644 --- a/src/renderer/hooks/use-web-view-state.hook.ts +++ b/src/renderer/hooks/use-web-view-state.hook.ts @@ -1,4 +1,4 @@ -import { useState, useCallback, useEffect } from 'react'; +import { useState, useCallback, useEffect, useRef } from 'react'; // We don't add this to PAPI directly like other hooks because `this` has to be bound to a web view's iframe context /** See `web-view.model.ts` for normal hook documentation */ @@ -10,31 +10,36 @@ export default function useWebViewState( }, stateKey: string, defaultStateValue: T, -): [webViewState: T, setWebViewState: (newStateValue: T) => void, resetWebViewState: () => void] { +): [ + webViewStateValue: T, + setWebViewState: (newStateValue: T) => void, + resetWebViewState: () => void, +] { + // Use defaultStateValue as a ref so it doesn't update dependency arrays + const defaultStateValueRef = useRef(defaultStateValue); + defaultStateValueRef.current = defaultStateValue; + const [state, setStateInternal] = useState(() => - this.getWebViewState(stateKey, defaultStateValue), + this.getWebViewState(stateKey, defaultStateValueRef.current), ); useEffect(() => { - if ( - this.getWebViewState(stateKey, defaultStateValue) === defaultStateValue && - state !== defaultStateValue - ) - setStateInternal(defaultStateValue); - }, [defaultStateValue, state, stateKey]); + // Get the setting for the new key when the key changes + setStateInternal(this.getWebViewState(stateKey, defaultStateValueRef.current)); + }, [stateKey]); const setState = useCallback( (newStateValue: T) => { - setStateInternal(newStateValue); this.setWebViewState(stateKey, newStateValue); + setStateInternal(newStateValue); }, [stateKey], ); const resetState = useCallback(() => { - setStateInternal(defaultStateValue); this.resetWebViewState(stateKey); - }, [defaultStateValue, stateKey]); + setStateInternal(defaultStateValueRef.current); + }, [stateKey]); return [state, setState, resetState]; } diff --git a/src/shared/models/web-view.model.ts b/src/shared/models/web-view.model.ts index d1bac84769..89119d4f40 100644 --- a/src/shared/models/web-view.model.ts +++ b/src/shared/models/web-view.model.ts @@ -199,16 +199,26 @@ export type WebViewDefinitionUpdateInfo = Partial( stateKey: string, defaultStateValue: T, -) => [webViewState: T, setWebViewState: (stateValue: T) => void, resetWebViewState: () => void]; +) => [ + webViewStateValue: T, + setWebViewState: (stateValue: T) => void, + resetWebViewState: () => void, +]; // Note: the following comment uses ＠, not the actual @ character, to hackily provide @param and // such on this type. It seem that, for some reason, JSDoc does not carry these annotations on