@samitouri / QOS-React / commits / e33acfd67f

refactor[react-devtools]: propagate settings from global hook object to frontend (#30610)

Stacked on https://github.com/facebook/react/pull/30597 and whats under it. See [this commit](https://github.com/facebook/react/pull/30610/commits/59b4efa72377bf62f5ec8c0e32e56902cf73fbd7). With this change, the initial values for console patching settings are propagated from hook (which is the source of truth now, because of https://github.com/facebook/react/pull/30596) to the UI. Instead of reading from `localStorage` the frontend is now requesting it from the hook. This happens when settings modal is rendered, and wrapped in a transition. Also, this is happening even if settings modal is not opened yet, so we have enough time to fetch this data without displaying loader or similar UI.

Ruslan Lesiutin committed Sep 18, 2024 at 18:19 UTC e33acfd67f0003272a9aec7a0725d19a429f2460

11 files changed +145 -99

packages/react-devtools-core/src/backend.js

+2 -2

@@ -162,7 +162,7 @@ export function connectToDevTools(options: ?ConnectOptions) {

162 );

163

164 if (devToolsSettingsManager != null && bridge != null) {

165 - bridge.addListener('updateConsolePatchSettings', consolePatchSettings =>

165 + bridge.addListener('updateHookSettings', consolePatchSettings =>

166 cacheConsolePatchSettings(

167 devToolsSettingsManager,

168 consolePatchSettings,

@@ -368,7 +368,7 @@ export function connectWithCustomMessagingProtocol({

368 );

369

370 if (settingsManager != null) {

371 - bridge.addListener('updateConsolePatchSettings', consolePatchSettings =>

371 + bridge.addListener('updateHookSettings', consolePatchSettings =>

372 cacheConsolePatchSettings(settingsManager, consolePatchSettings),

373 );

374 }

packages/react-devtools-core/src/cachedSettings.js

+1 -1

@@ -66,7 +66,7 @@ function parseConsolePatchSettings(

67 export function cacheConsolePatchSettings(

68 devToolsSettingsManager: DevToolsSettingsManager,

69 - value: ConsolePatchSettings,

69 + value: $ReadOnly<ConsolePatchSettings>,

70 ): void {

71 if (devToolsSettingsManager.setConsolePatchSettings == null) {

72 return;

packages/react-devtools-shared/src/backend/agent.js

+23 -14

@@ -150,6 +150,7 @@ export default class Agent extends EventEmitter<{

150 disableTraceUpdates: [],

151 getIfHasUnsupportedRendererVersion: [],

152 updateHookSettings: [DevToolsHookSettings],

153 + getHookSettings: [],

154 }> {

155 _bridge: BackendBridge;

156 _isProfiling: boolean = false;

@@ -213,10 +214,10 @@ export default class Agent extends EventEmitter<{

214 this.syncSelectionFromBuiltinElementsPanel,

215 );

216 bridge.addListener('shutdown', this.shutdown);

216 - bridge.addListener(

217 - 'updateConsolePatchSettings',

218 - this.updateConsolePatchSettings,

219 - );

217 +

218 + bridge.addListener('updateHookSettings', this.updateHookSettings);

219 + bridge.addListener('getHookSettings', this.getHookSettings);

220 +

221 bridge.addListener('updateComponentFilters', this.updateComponentFilters);

222 bridge.addListener('getEnvironmentNames', this.getEnvironmentNames);

223 bridge.addListener(

@@ -802,18 +803,26 @@ export default class Agent extends EventEmitter<{

803 }

804 };

805

805 - updateConsolePatchSettings: (

806 - settings: $ReadOnly<DevToolsHookSettings>,

807 - ) => void = settings => {

808 - // Propagate the settings, so Backend can subscribe to it and modify hook

809 - this.emit('updateHookSettings', {

810 - appendComponentStack: settings.appendComponentStack,

811 - breakOnConsoleErrors: settings.breakOnConsoleErrors,

812 - showInlineWarningsAndErrors: settings.showInlineWarningsAndErrors,

813 - hideConsoleLogsInStrictMode: settings.hideConsoleLogsInStrictMode,

814 - });

806 + updateHookSettings: (settings: $ReadOnly<DevToolsHookSettings>) => void =

807 + settings => {

808 + // Propagate the settings, so Backend can subscribe to it and modify hook

809 + this.emit('updateHookSettings', {

810 + appendComponentStack: settings.appendComponentStack,

811 + breakOnConsoleErrors: settings.breakOnConsoleErrors,

812 + showInlineWarningsAndErrors: settings.showInlineWarningsAndErrors,

813 + hideConsoleLogsInStrictMode: settings.hideConsoleLogsInStrictMode,

814 + });

815 + };

816 +

817 + getHookSettings: () => void = () => {

818 + this.emit('getHookSettings');

819 };

820

821 + onHookSettings: (settings: $ReadOnly<DevToolsHookSettings>) => void =

822 + settings => {

823 + this._bridge.send('hookSettings', settings);

824 + };

825 +

826 updateComponentFilters: (componentFilters: Array<ComponentFilter>) => void =

827 componentFilters => {

828 for (const rendererIDString in this._rendererInterfaces) {

packages/react-devtools-shared/src/backend/index.js

@@ -54,6 +54,7 @@ export function initBackend(

54 hook.sub('fastRefreshScheduled', agent.onFastRefreshScheduled),

55 hook.sub('operations', agent.onHookOperations),

56 hook.sub('traceUpdates', agent.onTraceUpdates),

57 + hook.sub('settingsInitialized', agent.onHookSettings),

59 // TODO Add additional subscriptions required for profiling mode

60 ];

@@ -87,6 +88,12 @@ export function initBackend(

88 hook.settings = settings;

89 });

91 + agent.addListener('getHookSettings', () => {

92 + if (hook.settings != null) {

93 + agent.onHookSettings(hook.settings);

94 + }

95 + });

96 +

97 return () => {

98 subs.forEach(fn => fn());

99 };

packages/react-devtools-shared/src/bridge.js

+5 -1

@@ -207,6 +207,8 @@ export type BackendEvents = {

207 {isSupported: boolean, validAttributes: ?$ReadOnlyArray<string>},

208 ],

209 NativeStyleEditor_styleAndLayout: [StyleAndLayoutPayload],

210 +

211 + hookSettings: [$ReadOnly<DevToolsHookSettings>],

212 };

213

214 type FrontendEvents = {

@@ -241,7 +243,7 @@ type FrontendEvents = {

243 storeAsGlobal: [StoreAsGlobalParams],

244 updateComponentFilters: [Array<ComponentFilter>],

245 getEnvironmentNames: [],

244 - updateConsolePatchSettings: [DevToolsHookSettings],

246 + updateHookSettings: [$ReadOnly<DevToolsHookSettings>],

247 viewAttributeSource: [ViewAttributeSourceParams],

248 viewElementSource: [ElementAndRendererID],

249

@@ -267,6 +269,8 @@ type FrontendEvents = {

269

270 resumeElementPolling: [],

271 pauseElementPolling: [],

272 +

273 + getHookSettings: [],

274 };

275

276 class Bridge<

packages/react-devtools-shared/src/devtools/store.js

+25

@@ -49,6 +49,7 @@ import type {

49 BridgeProtocol,

50 } from 'react-devtools-shared/src/bridge';

51 import UnsupportedBridgeOperationError from 'react-devtools-shared/src/UnsupportedBridgeOperationError';

52 +import type {DevToolsHookSettings} from '../backend/types';

54 const debug = (methodName: string, ...args: Array<string>) => {

55 if (__DEBUG__) {

@@ -94,6 +95,7 @@ export default class Store extends EventEmitter<{

95 collapseNodesByDefault: [],

96 componentFilters: [],

97 error: [Error],

98 + hookSettings: [$ReadOnly<DevToolsHookSettings>],

99 mutated: [[Array<number>, Map<number, number>]],

100 recordChangeDescriptions: [],

101 roots: [],

@@ -192,6 +194,7 @@ export default class Store extends EventEmitter<{

194 _weightAcrossRoots: number = 0;

195

196 _shouldCheckBridgeProtocolCompatibility: boolean = false;

197 + _hookSettings: $ReadOnly<DevToolsHookSettings> | null = null;

198

199 constructor(bridge: FrontendBridge, config?: Config) {

200 super();

@@ -270,6 +273,7 @@ export default class Store extends EventEmitter<{

273

274 bridge.addListener('backendVersion', this.onBridgeBackendVersion);

275 bridge.addListener('saveToClipboard', this.onSaveToClipboard);

276 + bridge.addListener('hookSettings', this.onHookSettings);

277 bridge.addListener('backendInitialized', this.onBackendInitialized);

278 }

279

@@ -1501,8 +1505,29 @@ export default class Store extends EventEmitter<{

1505

1506 this._bridge.send('getBackendVersion');

1507 this._bridge.send('getIfHasUnsupportedRendererVersion');

1508 + this._bridge.send('getHookSettings'); // Warm up cached hook settings

1509 };

1510

1511 + getHookSettings: () => void = () => {

1512 + if (this._hookSettings != null) {

1513 + this.emit('hookSettings', this._hookSettings);

1514 + } else {

1515 + this._bridge.send('getHookSettings');

1516 + }

1517 + };

1518 +

1519 + updateHookSettings: (settings: $ReadOnly<DevToolsHookSettings>) => void =

1520 + settings => {

1521 + this._hookSettings = settings;

1522 + this._bridge.send('updateHookSettings', settings);

1523 + };

1524 +

1525 + onHookSettings: (settings: $ReadOnly<DevToolsHookSettings>) => void =

1526 + settings => {

1527 + this._hookSettings = settings;

1528 + this.emit('hookSettings', settings);

1529 + };

1530 +

1531 // The Store should never throw an Error without also emitting an event.

1532 // Otherwise Store errors will be invisible to users,

1533 // but the downstream errors they cause will be reported as bugs.

packages/react-devtools-shared/src/devtools/views/Settings/DebuggingSettings.js

+37 -10

@@ -8,22 +8,49 @@

8 */

10 import * as React from 'react';

11 -import {useContext} from 'react';

12 -import {SettingsContext} from './SettingsContext';

11 +import {use, useState, useEffect} from 'react';

12 +

13 +import type {DevToolsHookSettings} from 'react-devtools-shared/src/backend/types';

14 +import type Store from 'react-devtools-shared/src/devtools/store';

16 import styles from './SettingsShared.css';

16 -export default function DebuggingSettings(_: {}): React.Node {

17 - const {

18 +type Props = {

19 + hookSettings: Promise<$ReadOnly<DevToolsHookSettings>>,

20 + store: Store,

21 +};

22 +

23 +export default function DebuggingSettings({

24 + hookSettings,

25 + store,

26 +}: Props): React.Node {

27 + const usedHookSettings = use(hookSettings);

28 +

29 + const [appendComponentStack, setAppendComponentStack] = useState(

30 + usedHookSettings.appendComponentStack,

31 + );

32 + const [breakOnConsoleErrors, setBreakOnConsoleErrors] = useState(

33 + usedHookSettings.breakOnConsoleErrors,

34 + );

35 + const [hideConsoleLogsInStrictMode, setHideConsoleLogsInStrictMode] =

36 + useState(usedHookSettings.hideConsoleLogsInStrictMode);

37 + const [showInlineWarningsAndErrors, setShowInlineWarningsAndErrors] =

38 + useState(usedHookSettings.showInlineWarningsAndErrors);

39 +

40 + useEffect(() => {

41 + store.updateHookSettings({

42 + appendComponentStack,

43 + breakOnConsoleErrors,

44 + showInlineWarningsAndErrors,

45 + hideConsoleLogsInStrictMode,

46 + });

47 + }, [

48 + store,

49 appendComponentStack,

50 breakOnConsoleErrors,

20 - hideConsoleLogsInStrictMode,

21 - setAppendComponentStack,

22 - setBreakOnConsoleErrors,

23 - setShowInlineWarningsAndErrors,

51 showInlineWarningsAndErrors,

25 - setHideConsoleLogsInStrictMode,

26 - } = useContext(SettingsContext);

52 + hideConsoleLogsInStrictMode,

53 + ]);

55 return (

56 <div className={styles.Settings}>

packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js

-55

@@ -20,11 +20,7 @@ import {

20 import {

21 LOCAL_STORAGE_BROWSER_THEME,

22 LOCAL_STORAGE_PARSE_HOOK_NAMES_KEY,

23 - LOCAL_STORAGE_SHOULD_BREAK_ON_CONSOLE_ERRORS,

24 - LOCAL_STORAGE_SHOULD_APPEND_COMPONENT_STACK_KEY,

23 LOCAL_STORAGE_TRACE_UPDATES_ENABLED_KEY,

26 - LOCAL_STORAGE_SHOW_INLINE_WARNINGS_AND_ERRORS_KEY,

27 - LOCAL_STORAGE_HIDE_CONSOLE_LOGS_IN_STRICT_MODE,

24 } from 'react-devtools-shared/src/constants';

25 import {

26 COMFORTABLE_LINE_HEIGHT,

@@ -118,30 +114,10 @@ function SettingsContextController({

114 LOCAL_STORAGE_BROWSER_THEME,

115 'auto',

116 );

121 - const [appendComponentStack, setAppendComponentStack] =

122 - useLocalStorageWithLog<boolean>(

123 - LOCAL_STORAGE_SHOULD_APPEND_COMPONENT_STACK_KEY,

124 - true,

125 - );

126 - const [breakOnConsoleErrors, setBreakOnConsoleErrors] =

127 - useLocalStorageWithLog<boolean>(

128 - LOCAL_STORAGE_SHOULD_BREAK_ON_CONSOLE_ERRORS,

129 - false,

130 - );

117 const [parseHookNames, setParseHookNames] = useLocalStorageWithLog<boolean>(

118 LOCAL_STORAGE_PARSE_HOOK_NAMES_KEY,

119 false,

120 );

135 - const [hideConsoleLogsInStrictMode, setHideConsoleLogsInStrictMode] =

136 - useLocalStorageWithLog<boolean>(

137 - LOCAL_STORAGE_HIDE_CONSOLE_LOGS_IN_STRICT_MODE,

138 - false,

139 - );

140 - const [showInlineWarningsAndErrors, setShowInlineWarningsAndErrors] =

141 - useLocalStorageWithLog<boolean>(

142 - LOCAL_STORAGE_SHOW_INLINE_WARNINGS_AND_ERRORS_KEY,

143 - true,

144 - );

121 const [traceUpdatesEnabled, setTraceUpdatesEnabled] =

122 useLocalStorageWithLog<boolean>(

123 LOCAL_STORAGE_TRACE_UPDATES_ENABLED_KEY,

@@ -196,64 +172,33 @@ function SettingsContextController({

172 }

173 }, [browserTheme, theme, documentElements]);

174

199 - useEffect(() => {

200 - bridge.send('updateConsolePatchSettings', {

201 - appendComponentStack,

202 - breakOnConsoleErrors,

203 - showInlineWarningsAndErrors,

204 - hideConsoleLogsInStrictMode,

205 - });

206 - }, [

207 - bridge,

208 - appendComponentStack,

209 - breakOnConsoleErrors,

210 - showInlineWarningsAndErrors,

211 - hideConsoleLogsInStrictMode,

212 - ]);

213 -

175 useEffect(() => {

176 bridge.send('setTraceUpdatesEnabled', traceUpdatesEnabled);

177 }, [bridge, traceUpdatesEnabled]);

178

179 const value = useMemo(

180 () => ({

220 - appendComponentStack,

221 - breakOnConsoleErrors,

181 displayDensity,

182 lineHeight:

183 displayDensity === 'compact'

184 ? COMPACT_LINE_HEIGHT

185 : COMFORTABLE_LINE_HEIGHT,

186 parseHookNames,

228 - setAppendComponentStack,

229 - setBreakOnConsoleErrors,

187 setDisplayDensity,

188 setParseHookNames,

189 setTheme,

190 setTraceUpdatesEnabled,

234 - setShowInlineWarningsAndErrors,

235 - showInlineWarningsAndErrors,

236 - setHideConsoleLogsInStrictMode,

237 - hideConsoleLogsInStrictMode,

191 theme,

192 browserTheme,

193 traceUpdatesEnabled,

194 }),

195 [

243 - appendComponentStack,

244 - breakOnConsoleErrors,

196 displayDensity,

197 parseHookNames,

247 - setAppendComponentStack,

248 - setBreakOnConsoleErrors,

198 setDisplayDensity,

199 setParseHookNames,

200 setTheme,

201 setTraceUpdatesEnabled,

253 - setShowInlineWarningsAndErrors,

254 - showInlineWarningsAndErrors,

255 - setHideConsoleLogsInStrictMode,

256 - hideConsoleLogsInStrictMode,

202 theme,

203 browserTheme,

204 traceUpdatesEnabled,

packages/react-devtools-shared/src/devtools/views/Settings/SettingsModal.js

+10 -7

@@ -26,9 +26,11 @@ import ProfilerSettings from './ProfilerSettings';

27 import styles from './SettingsModal.css';

29 -type TabID = 'general' | 'components' | 'profiler';

29 +import type Store from 'react-devtools-shared/src/devtools/store';

31 -export default function SettingsModal(_: {}): React.Node {

31 +type TabID = 'general' | 'debugging' | 'components' | 'profiler';

32 +

33 +export default function SettingsModal(): React.Node {

34 const {isModalShowing, setIsModalShowing} = useContext(SettingsModalContext);

35 const store = useContext(StoreContext);

36 const {profilerStore} = store;

@@ -54,11 +56,13 @@ export default function SettingsModal(_: {}): React.Node {

56 return null;

57 }

57 - return <SettingsModalImpl />;

59 + return <SettingsModalImpl store={store} />;

60 }

60 -function SettingsModalImpl(_: {}) {

61 - const {setIsModalShowing, environmentNames} =

62 +type ImplProps = {store: Store};

63 +

64 +function SettingsModalImpl({store}: ImplProps) {

65 + const {setIsModalShowing, environmentNames, hookSettings} =

66 useContext(SettingsModalContext);

67 const dismissModal = useCallback(

68 () => setIsModalShowing(false),

@@ -84,9 +88,8 @@ function SettingsModalImpl(_: {}) {

88 case 'components':

89 view = <ComponentsSettings environmentNames={environmentNames} />;

90 break;

87 - // $FlowFixMe[incompatible-type] is this missing in TabID?

91 case 'debugging':

89 - view = <DebuggingSettings />;

92 + view = <DebuggingSettings hookSettings={hookSettings} store={store} />;

93 break;

94 case 'general':

95 view = <GeneralSettings />;

packages/react-devtools-shared/src/devtools/views/Settings/SettingsModalContext.js

+33 -9

@@ -18,8 +18,11 @@ import {

18 startTransition,

19 } from 'react';

21 -import {BridgeContext} from '../context';

21 +import {BridgeContext, StoreContext} from '../context';

22 +

23 import type {FrontendBridge} from '../../../bridge';

24 +import type {DevToolsHookSettings} from '../../../backend/types';

25 +import type Store from '../../store';

27 export type DisplayDensity = 'comfortable' | 'compact';

28 export type Theme = 'auto' | 'light' | 'dark';

@@ -28,6 +31,7 @@ type Context = {

31 isModalShowing: boolean,

32 setIsModalShowing: (value: boolean) => void,

33 environmentNames: null | Promise<Array<string>>,

34 + hookSettings: null | Promise<$ReadOnly<DevToolsHookSettings>>,

35 };

37 const SettingsModalContext: ReactContext<Context> = createContext<Context>(

@@ -46,27 +50,47 @@ function fetchEnvironmentNames(bridge: FrontendBridge): Promise<Array<string>> {

50 });

51 }

53 +function fetchHookSettings(

54 + store: Store,

55 +): Promise<$ReadOnly<DevToolsHookSettings>> {

56 + return new Promise(resolve => {

57 + function onHookSettings(settings: $ReadOnly<DevToolsHookSettings>) {

58 + store.removeListener('hookSettings', onHookSettings);

59 + resolve(settings);

60 + }

61 +

62 + store.addListener('hookSettings', onHookSettings);

63 + store.getHookSettings();

64 + });

65 +}

66 +

67 function SettingsModalContextController({

68 children,

69 }: {

70 children: React$Node,

71 }): React.Node {

72 const bridge = useContext(BridgeContext);

73 + const store = useContext(StoreContext);

56 - const setIsModalShowing: boolean => void = useCallback((value: boolean) => {

57 - startTransition(() => {

58 - setContext({

59 - isModalShowing: value,

60 - setIsModalShowing,

61 - environmentNames: value ? fetchEnvironmentNames(bridge) : null,

75 + const setIsModalShowing: boolean => void = useCallback(

76 + (value: boolean) => {

77 + startTransition(() => {

78 + setContext({

79 + isModalShowing: value,

80 + setIsModalShowing,

81 + environmentNames: value ? fetchEnvironmentNames(bridge) : null,

82 + hookSettings: value ? fetchHookSettings(store) : null,

83 + });

84 });

63 - });

64 - });

85 + },

86 + [bridge, store],

87 + );

89 const [currentContext, setContext] = useState<Context>({

90 isModalShowing: false,

91 setIsModalShowing,

92 environmentNames: null,

93 + hookSettings: null,

94 });

96 return (

packages/react-devtools-shared/src/hook.js

@@ -655,6 +655,8 @@ export function installHook(

655 Promise.resolve(maybeSettingsOrSettingsPromise)

656 .then(settings => {

657 hook.settings = settings;

658 + hook.emit('settingsInitialized', settings);

659 +

660 patchConsoleForErrorsAndWarnings();

661 })

662 .catch(() => {