From 59b4efa72377bf62f5ec8c0e32e56902cf73fbd7 Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Tue, 6 Aug 2024 15:36:59 +0100 Subject: [PATCH] refactor[react-devtools]: propagate settings from global hook object to frontend --- packages/react-devtools-core/src/backend.js | 4 +- .../react-devtools-core/src/cachedSettings.js | 2 +- .../src/backend/agent.js | 37 ++++++++----- .../src/backend/index.js | 7 +++ packages/react-devtools-shared/src/bridge.js | 6 +- .../src/devtools/store.js | 25 +++++++++ .../views/Settings/DebuggingSettings.js | 47 ++++++++++++---- .../views/Settings/SettingsContext.js | 55 ------------------- .../devtools/views/Settings/SettingsModal.js | 17 +++--- .../views/Settings/SettingsModalContext.js | 42 +++++++++++--- packages/react-devtools-shared/src/hook.js | 2 + 11 files changed, 145 insertions(+), 99 deletions(-) diff --git a/packages/react-devtools-core/src/backend.js b/packages/react-devtools-core/src/backend.js index d588d4f91e9f0..6949d6019145a 100644 --- a/packages/react-devtools-core/src/backend.js +++ b/packages/react-devtools-core/src/backend.js @@ -162,7 +162,7 @@ export function connectToDevTools(options: ?ConnectOptions) { ); if (devToolsSettingsManager != null && bridge != null) { - bridge.addListener('updateConsolePatchSettings', consolePatchSettings => + bridge.addListener('updateHookSettings', consolePatchSettings => cacheConsolePatchSettings( devToolsSettingsManager, consolePatchSettings, @@ -368,7 +368,7 @@ export function connectWithCustomMessagingProtocol({ ); if (settingsManager != null) { - bridge.addListener('updateConsolePatchSettings', consolePatchSettings => + bridge.addListener('updateHookSettings', consolePatchSettings => cacheConsolePatchSettings(settingsManager, consolePatchSettings), ); } diff --git a/packages/react-devtools-core/src/cachedSettings.js b/packages/react-devtools-core/src/cachedSettings.js index afe12bfdbc5ad..5c603447e8d88 100644 --- a/packages/react-devtools-core/src/cachedSettings.js +++ b/packages/react-devtools-core/src/cachedSettings.js @@ -66,7 +66,7 @@ function parseConsolePatchSettings( export function cacheConsolePatchSettings( devToolsSettingsManager: DevToolsSettingsManager, - value: ConsolePatchSettings, + value: $ReadOnly, ): void { if (devToolsSettingsManager.setConsolePatchSettings == null) { return; diff --git a/packages/react-devtools-shared/src/backend/agent.js b/packages/react-devtools-shared/src/backend/agent.js index 814c6c1c40e67..277b743f3f6a4 100644 --- a/packages/react-devtools-shared/src/backend/agent.js +++ b/packages/react-devtools-shared/src/backend/agent.js @@ -150,6 +150,7 @@ export default class Agent extends EventEmitter<{ disableTraceUpdates: [], getIfHasUnsupportedRendererVersion: [], updateHookSettings: [DevToolsHookSettings], + getHookSettings: [], }> { _bridge: BackendBridge; _isProfiling: boolean = false; @@ -213,10 +214,10 @@ export default class Agent extends EventEmitter<{ this.syncSelectionFromBuiltinElementsPanel, ); bridge.addListener('shutdown', this.shutdown); - bridge.addListener( - 'updateConsolePatchSettings', - this.updateConsolePatchSettings, - ); + + bridge.addListener('updateHookSettings', this.updateHookSettings); + bridge.addListener('getHookSettings', this.getHookSettings); + bridge.addListener('updateComponentFilters', this.updateComponentFilters); bridge.addListener('getEnvironmentNames', this.getEnvironmentNames); bridge.addListener( @@ -802,18 +803,26 @@ export default class Agent extends EventEmitter<{ } }; - updateConsolePatchSettings: ( - settings: $ReadOnly, - ) => void = settings => { - // Propagate the settings, so Backend can subscribe to it and modify hook - this.emit('updateHookSettings', { - appendComponentStack: settings.appendComponentStack, - breakOnConsoleErrors: settings.breakOnConsoleErrors, - showInlineWarningsAndErrors: settings.showInlineWarningsAndErrors, - hideConsoleLogsInStrictMode: settings.hideConsoleLogsInStrictMode, - }); + updateHookSettings: (settings: $ReadOnly) => void = + settings => { + // Propagate the settings, so Backend can subscribe to it and modify hook + this.emit('updateHookSettings', { + appendComponentStack: settings.appendComponentStack, + breakOnConsoleErrors: settings.breakOnConsoleErrors, + showInlineWarningsAndErrors: settings.showInlineWarningsAndErrors, + hideConsoleLogsInStrictMode: settings.hideConsoleLogsInStrictMode, + }); + }; + + getHookSettings: () => void = () => { + this.emit('getHookSettings'); }; + onHookSettings: (settings: $ReadOnly) => void = + settings => { + this._bridge.send('hookSettings', settings); + }; + updateComponentFilters: (componentFilters: Array) => void = componentFilters => { for (const rendererIDString in this._rendererInterfaces) { diff --git a/packages/react-devtools-shared/src/backend/index.js b/packages/react-devtools-shared/src/backend/index.js index 5d84bf6bb70f0..5893424b394c8 100644 --- a/packages/react-devtools-shared/src/backend/index.js +++ b/packages/react-devtools-shared/src/backend/index.js @@ -54,6 +54,7 @@ export function initBackend( hook.sub('fastRefreshScheduled', agent.onFastRefreshScheduled), hook.sub('operations', agent.onHookOperations), hook.sub('traceUpdates', agent.onTraceUpdates), + hook.sub('settingsInitialized', agent.onHookSettings), // TODO Add additional subscriptions required for profiling mode ]; @@ -87,6 +88,12 @@ export function initBackend( hook.settings = settings; }); + agent.addListener('getHookSettings', () => { + if (hook.settings != null) { + agent.onHookSettings(hook.settings); + } + }); + return () => { subs.forEach(fn => fn()); }; diff --git a/packages/react-devtools-shared/src/bridge.js b/packages/react-devtools-shared/src/bridge.js index 156319b366d71..4751f70cfaefb 100644 --- a/packages/react-devtools-shared/src/bridge.js +++ b/packages/react-devtools-shared/src/bridge.js @@ -207,6 +207,8 @@ export type BackendEvents = { {isSupported: boolean, validAttributes: ?$ReadOnlyArray}, ], NativeStyleEditor_styleAndLayout: [StyleAndLayoutPayload], + + hookSettings: [$ReadOnly], }; type FrontendEvents = { @@ -241,7 +243,7 @@ type FrontendEvents = { storeAsGlobal: [StoreAsGlobalParams], updateComponentFilters: [Array], getEnvironmentNames: [], - updateConsolePatchSettings: [DevToolsHookSettings], + updateHookSettings: [$ReadOnly], viewAttributeSource: [ViewAttributeSourceParams], viewElementSource: [ElementAndRendererID], @@ -267,6 +269,8 @@ type FrontendEvents = { resumeElementPolling: [], pauseElementPolling: [], + + getHookSettings: [], }; class Bridge< diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js index d351306a44546..1798e0952b8c2 100644 --- a/packages/react-devtools-shared/src/devtools/store.js +++ b/packages/react-devtools-shared/src/devtools/store.js @@ -49,6 +49,7 @@ import type { BridgeProtocol, } from 'react-devtools-shared/src/bridge'; import UnsupportedBridgeOperationError from 'react-devtools-shared/src/UnsupportedBridgeOperationError'; +import type {DevToolsHookSettings} from '../backend/types'; const debug = (methodName: string, ...args: Array) => { if (__DEBUG__) { @@ -94,6 +95,7 @@ export default class Store extends EventEmitter<{ collapseNodesByDefault: [], componentFilters: [], error: [Error], + hookSettings: [$ReadOnly], mutated: [[Array, Map]], recordChangeDescriptions: [], roots: [], @@ -192,6 +194,7 @@ export default class Store extends EventEmitter<{ _weightAcrossRoots: number = 0; _shouldCheckBridgeProtocolCompatibility: boolean = false; + _hookSettings: $ReadOnly | null = null; constructor(bridge: FrontendBridge, config?: Config) { super(); @@ -270,6 +273,7 @@ export default class Store extends EventEmitter<{ bridge.addListener('backendVersion', this.onBridgeBackendVersion); bridge.addListener('saveToClipboard', this.onSaveToClipboard); + bridge.addListener('hookSettings', this.onHookSettings); bridge.addListener('backendInitialized', this.onBackendInitialized); } @@ -1501,8 +1505,29 @@ export default class Store extends EventEmitter<{ this._bridge.send('getBackendVersion'); this._bridge.send('getIfHasUnsupportedRendererVersion'); + this._bridge.send('getHookSettings'); // Warm up cached hook settings }; + getHookSettings: () => void = () => { + if (this._hookSettings != null) { + this.emit('hookSettings', this._hookSettings); + } else { + this._bridge.send('getHookSettings'); + } + }; + + updateHookSettings: (settings: $ReadOnly) => void = + settings => { + this._hookSettings = settings; + this._bridge.send('updateHookSettings', settings); + }; + + onHookSettings: (settings: $ReadOnly) => void = + settings => { + this._hookSettings = settings; + this.emit('hookSettings', settings); + }; + // The Store should never throw an Error without also emitting an event. // Otherwise Store errors will be invisible to users, // but the downstream errors they cause will be reported as bugs. diff --git a/packages/react-devtools-shared/src/devtools/views/Settings/DebuggingSettings.js b/packages/react-devtools-shared/src/devtools/views/Settings/DebuggingSettings.js index b28ca67c0bcb4..8bde14e62e606 100644 --- a/packages/react-devtools-shared/src/devtools/views/Settings/DebuggingSettings.js +++ b/packages/react-devtools-shared/src/devtools/views/Settings/DebuggingSettings.js @@ -8,22 +8,49 @@ */ import * as React from 'react'; -import {useContext} from 'react'; -import {SettingsContext} from './SettingsContext'; +import {use, useState, useEffect} from 'react'; + +import type {DevToolsHookSettings} from 'react-devtools-shared/src/backend/types'; +import type Store from 'react-devtools-shared/src/devtools/store'; import styles from './SettingsShared.css'; -export default function DebuggingSettings(_: {}): React.Node { - const { +type Props = { + hookSettings: Promise<$ReadOnly>, + store: Store, +}; + +export default function DebuggingSettings({ + hookSettings, + store, +}: Props): React.Node { + const usedHookSettings = use(hookSettings); + + const [appendComponentStack, setAppendComponentStack] = useState( + usedHookSettings.appendComponentStack, + ); + const [breakOnConsoleErrors, setBreakOnConsoleErrors] = useState( + usedHookSettings.breakOnConsoleErrors, + ); + const [hideConsoleLogsInStrictMode, setHideConsoleLogsInStrictMode] = + useState(usedHookSettings.hideConsoleLogsInStrictMode); + const [showInlineWarningsAndErrors, setShowInlineWarningsAndErrors] = + useState(usedHookSettings.showInlineWarningsAndErrors); + + useEffect(() => { + store.updateHookSettings({ + appendComponentStack, + breakOnConsoleErrors, + showInlineWarningsAndErrors, + hideConsoleLogsInStrictMode, + }); + }, [ + store, appendComponentStack, breakOnConsoleErrors, - hideConsoleLogsInStrictMode, - setAppendComponentStack, - setBreakOnConsoleErrors, - setShowInlineWarningsAndErrors, showInlineWarningsAndErrors, - setHideConsoleLogsInStrictMode, - } = useContext(SettingsContext); + hideConsoleLogsInStrictMode, + ]); return (
diff --git a/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js b/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js index 33487c2f553d6..17514d94648ac 100644 --- a/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js @@ -20,11 +20,7 @@ import { import { LOCAL_STORAGE_BROWSER_THEME, LOCAL_STORAGE_PARSE_HOOK_NAMES_KEY, - LOCAL_STORAGE_SHOULD_BREAK_ON_CONSOLE_ERRORS, - LOCAL_STORAGE_SHOULD_APPEND_COMPONENT_STACK_KEY, LOCAL_STORAGE_TRACE_UPDATES_ENABLED_KEY, - LOCAL_STORAGE_SHOW_INLINE_WARNINGS_AND_ERRORS_KEY, - LOCAL_STORAGE_HIDE_CONSOLE_LOGS_IN_STRICT_MODE, } from 'react-devtools-shared/src/constants'; import { COMFORTABLE_LINE_HEIGHT, @@ -118,30 +114,10 @@ function SettingsContextController({ LOCAL_STORAGE_BROWSER_THEME, 'auto', ); - const [appendComponentStack, setAppendComponentStack] = - useLocalStorageWithLog( - LOCAL_STORAGE_SHOULD_APPEND_COMPONENT_STACK_KEY, - true, - ); - const [breakOnConsoleErrors, setBreakOnConsoleErrors] = - useLocalStorageWithLog( - LOCAL_STORAGE_SHOULD_BREAK_ON_CONSOLE_ERRORS, - false, - ); const [parseHookNames, setParseHookNames] = useLocalStorageWithLog( LOCAL_STORAGE_PARSE_HOOK_NAMES_KEY, false, ); - const [hideConsoleLogsInStrictMode, setHideConsoleLogsInStrictMode] = - useLocalStorageWithLog( - LOCAL_STORAGE_HIDE_CONSOLE_LOGS_IN_STRICT_MODE, - false, - ); - const [showInlineWarningsAndErrors, setShowInlineWarningsAndErrors] = - useLocalStorageWithLog( - LOCAL_STORAGE_SHOW_INLINE_WARNINGS_AND_ERRORS_KEY, - true, - ); const [traceUpdatesEnabled, setTraceUpdatesEnabled] = useLocalStorageWithLog( LOCAL_STORAGE_TRACE_UPDATES_ENABLED_KEY, @@ -196,64 +172,33 @@ function SettingsContextController({ } }, [browserTheme, theme, documentElements]); - useEffect(() => { - bridge.send('updateConsolePatchSettings', { - appendComponentStack, - breakOnConsoleErrors, - showInlineWarningsAndErrors, - hideConsoleLogsInStrictMode, - }); - }, [ - bridge, - appendComponentStack, - breakOnConsoleErrors, - showInlineWarningsAndErrors, - hideConsoleLogsInStrictMode, - ]); - useEffect(() => { bridge.send('setTraceUpdatesEnabled', traceUpdatesEnabled); }, [bridge, traceUpdatesEnabled]); const value = useMemo( () => ({ - appendComponentStack, - breakOnConsoleErrors, displayDensity, lineHeight: displayDensity === 'compact' ? COMPACT_LINE_HEIGHT : COMFORTABLE_LINE_HEIGHT, parseHookNames, - setAppendComponentStack, - setBreakOnConsoleErrors, setDisplayDensity, setParseHookNames, setTheme, setTraceUpdatesEnabled, - setShowInlineWarningsAndErrors, - showInlineWarningsAndErrors, - setHideConsoleLogsInStrictMode, - hideConsoleLogsInStrictMode, theme, browserTheme, traceUpdatesEnabled, }), [ - appendComponentStack, - breakOnConsoleErrors, displayDensity, parseHookNames, - setAppendComponentStack, - setBreakOnConsoleErrors, setDisplayDensity, setParseHookNames, setTheme, setTraceUpdatesEnabled, - setShowInlineWarningsAndErrors, - showInlineWarningsAndErrors, - setHideConsoleLogsInStrictMode, - hideConsoleLogsInStrictMode, theme, browserTheme, traceUpdatesEnabled, diff --git a/packages/react-devtools-shared/src/devtools/views/Settings/SettingsModal.js b/packages/react-devtools-shared/src/devtools/views/Settings/SettingsModal.js index 24884098a0bfc..f6652ada3a860 100644 --- a/packages/react-devtools-shared/src/devtools/views/Settings/SettingsModal.js +++ b/packages/react-devtools-shared/src/devtools/views/Settings/SettingsModal.js @@ -26,9 +26,11 @@ import ProfilerSettings from './ProfilerSettings'; import styles from './SettingsModal.css'; -type TabID = 'general' | 'components' | 'profiler'; +import type Store from 'react-devtools-shared/src/devtools/store'; -export default function SettingsModal(_: {}): React.Node { +type TabID = 'general' | 'debugging' | 'components' | 'profiler'; + +export default function SettingsModal(): React.Node { const {isModalShowing, setIsModalShowing} = useContext(SettingsModalContext); const store = useContext(StoreContext); const {profilerStore} = store; @@ -54,11 +56,13 @@ export default function SettingsModal(_: {}): React.Node { return null; } - return ; + return ; } -function SettingsModalImpl(_: {}) { - const {setIsModalShowing, environmentNames} = +type ImplProps = {store: Store}; + +function SettingsModalImpl({store}: ImplProps) { + const {setIsModalShowing, environmentNames, hookSettings} = useContext(SettingsModalContext); const dismissModal = useCallback( () => setIsModalShowing(false), @@ -84,9 +88,8 @@ function SettingsModalImpl(_: {}) { case 'components': view = ; break; - // $FlowFixMe[incompatible-type] is this missing in TabID? case 'debugging': - view = ; + view = ; break; case 'general': view = ; diff --git a/packages/react-devtools-shared/src/devtools/views/Settings/SettingsModalContext.js b/packages/react-devtools-shared/src/devtools/views/Settings/SettingsModalContext.js index 55336a9d650b0..98e491f44b6eb 100644 --- a/packages/react-devtools-shared/src/devtools/views/Settings/SettingsModalContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Settings/SettingsModalContext.js @@ -18,8 +18,11 @@ import { startTransition, } from 'react'; -import {BridgeContext} from '../context'; +import {BridgeContext, StoreContext} from '../context'; + import type {FrontendBridge} from '../../../bridge'; +import type {DevToolsHookSettings} from '../../../backend/types'; +import type Store from '../../store'; export type DisplayDensity = 'comfortable' | 'compact'; export type Theme = 'auto' | 'light' | 'dark'; @@ -28,6 +31,7 @@ type Context = { isModalShowing: boolean, setIsModalShowing: (value: boolean) => void, environmentNames: null | Promise>, + hookSettings: null | Promise<$ReadOnly>, }; const SettingsModalContext: ReactContext = createContext( @@ -46,27 +50,47 @@ function fetchEnvironmentNames(bridge: FrontendBridge): Promise> { }); } +function fetchHookSettings( + store: Store, +): Promise<$ReadOnly> { + return new Promise(resolve => { + function onHookSettings(settings: $ReadOnly) { + store.removeListener('hookSettings', onHookSettings); + resolve(settings); + } + + store.addListener('hookSettings', onHookSettings); + store.getHookSettings(); + }); +} + function SettingsModalContextController({ children, }: { children: React$Node, }): React.Node { const bridge = useContext(BridgeContext); + const store = useContext(StoreContext); - const setIsModalShowing: boolean => void = useCallback((value: boolean) => { - startTransition(() => { - setContext({ - isModalShowing: value, - setIsModalShowing, - environmentNames: value ? fetchEnvironmentNames(bridge) : null, + const setIsModalShowing: boolean => void = useCallback( + (value: boolean) => { + startTransition(() => { + setContext({ + isModalShowing: value, + setIsModalShowing, + environmentNames: value ? fetchEnvironmentNames(bridge) : null, + hookSettings: value ? fetchHookSettings(store) : null, + }); }); - }); - }); + }, + [bridge, store], + ); const [currentContext, setContext] = useState({ isModalShowing: false, setIsModalShowing, environmentNames: null, + hookSettings: null, }); return ( diff --git a/packages/react-devtools-shared/src/hook.js b/packages/react-devtools-shared/src/hook.js index 21cf438373c1c..f47b2d859ed8f 100644 --- a/packages/react-devtools-shared/src/hook.js +++ b/packages/react-devtools-shared/src/hook.js @@ -645,6 +645,8 @@ export function installHook( Promise.resolve(maybeSettingsOrSettingsPromise) .then(settings => { hook.settings = settings; + hook.emit('settingsInitialized', settings); + patchConsoleForErrorsAndWarnings(); }) .catch(() => {