@samitouri / QOS-React-1 / commits / bfe91fbecf

refactor[react-devtools]: flatten reload and profile config (#31132)

Stacked on https://github.com/facebook/react/pull/31131. See last commit. This is a clean-up and a pre-requisite for next changes: 1. `ReloadAndProfileConfig` is now split into boolean value and settings object. This is mainly because I will add one more setting soon, and also because settings might be persisted for a longer time than the flag which signals if the Backend was reloaded for profiling. Ideally, this settings should probably be moved to the global Hook object, same as we did for console patching. 2. Host is now responsible for reseting the cached values, Backend will execute provided `onReloadAndProfileFlagsReset` callback.

Ruslan Lesiutin committed Oct 9, 2024 at 13:57 UTC bfe91fbecf183f85fc1c4f909e12a6833a247319
10 files changed +131 -99
packages/react-devtools-core/src/backend.js
+31 -10
@@ -26,8 +26,7 @@ import type {
26 import type {
27 DevToolsHook,
28 DevToolsHookSettings,
29 - ReloadAndProfileConfig,
30 - ReloadAndProfileConfigPersistence,
29 + ProfilingSettings,
30 } from 'react-devtools-shared/src/backend/types';
31 import type {ResolveNativeStyle} from 'react-devtools-shared/src/backend/NativeStyleEditor/setupNativeStyleEditor';
32
@@ -42,7 +41,9 @@ type ConnectOptions = {
41 websocket?: ?WebSocket,
42 onSettingsUpdated?: (settings: $ReadOnly<DevToolsHookSettings>) => void,
43 isReloadAndProfileSupported?: boolean,
45 - reloadAndProfileConfigPersistence?: ReloadAndProfileConfigPersistence,
44 + isProfiling?: boolean,
45 + onReloadAndProfile?: (recordChangeDescriptions: boolean) => void,
46 + onReloadAndProfileFlagsReset?: () => void,
47 };
48
49 let savedComponentFilters: Array<ComponentFilter> =
@@ -63,9 +64,15 @@ export function initialize(
64 maybeSettingsOrSettingsPromise?:
65 | DevToolsHookSettings
66 | Promise<DevToolsHookSettings>,
66 - reloadAndProfileConfig?: ReloadAndProfileConfig,
67 + shouldStartProfilingNow: boolean = false,
68 + profilingSettings?: ProfilingSettings,
69 ) {
68 - installHook(window, maybeSettingsOrSettingsPromise, reloadAndProfileConfig);
70 + installHook(
71 + window,
72 + maybeSettingsOrSettingsPromise,
73 + shouldStartProfilingNow,
74 + profilingSettings,
75 + );
76 }
77
78 export function connectToDevTools(options: ?ConnectOptions) {
@@ -86,7 +93,9 @@ export function connectToDevTools(options: ?ConnectOptions) {
93 isAppActive = () => true,
94 onSettingsUpdated,
95 isReloadAndProfileSupported = getIsReloadAndProfileSupported(),
89 - reloadAndProfileConfigPersistence,
96 + isProfiling,
97 + onReloadAndProfile,
98 + onReloadAndProfileFlagsReset,
99 } = options || {};
100
101 const protocol = useHttps ? 'wss' : 'ws';
@@ -180,7 +189,11 @@ export function connectToDevTools(options: ?ConnectOptions) {
189
190 // TODO (npm-packages) Warn if "isBackendStorageAPISupported"
191 // $FlowFixMe[incompatible-call] found when upgrading Flow
183 - const agent = new Agent(bridge, reloadAndProfileConfigPersistence);
192 + const agent = new Agent(bridge, isProfiling, onReloadAndProfile);
193 + if (typeof onReloadAndProfileFlagsReset === 'function') {
194 + onReloadAndProfileFlagsReset();
195 + }
196 +
197 if (onSettingsUpdated != null) {
198 agent.addListener('updateHookSettings', onSettingsUpdated);
199 }
@@ -320,7 +333,9 @@ type ConnectWithCustomMessagingOptions = {
333 resolveRNStyle?: ResolveNativeStyle,
334 onSettingsUpdated?: (settings: $ReadOnly<DevToolsHookSettings>) => void,
335 isReloadAndProfileSupported?: boolean,
323 - reloadAndProfileConfigPersistence?: ReloadAndProfileConfigPersistence,
336 + isProfiling?: boolean,
337 + onReloadAndProfile?: (recordChangeDescriptions: boolean) => void,
338 + onReloadAndProfileFlagsReset?: () => void,
339 };
340
341 export function connectWithCustomMessagingProtocol({
@@ -331,7 +346,9 @@ export function connectWithCustomMessagingProtocol({
346 resolveRNStyle,
347 onSettingsUpdated,
348 isReloadAndProfileSupported = getIsReloadAndProfileSupported(),
334 - reloadAndProfileConfigPersistence,
349 + isProfiling,
350 + onReloadAndProfile,
351 + onReloadAndProfileFlagsReset,
352 }: ConnectWithCustomMessagingOptions): Function {
353 const hook: ?DevToolsHook = window.__REACT_DEVTOOLS_GLOBAL_HOOK__;
354 if (hook == null) {
@@ -368,7 +385,11 @@ export function connectWithCustomMessagingProtocol({
385 bridge.send('overrideComponentFilters', savedComponentFilters);
386 }
387
371 - const agent = new Agent(bridge, reloadAndProfileConfigPersistence);
388 + const agent = new Agent(bridge, isProfiling, onReloadAndProfile);
389 + if (typeof onReloadAndProfileFlagsReset === 'function') {
390 + onReloadAndProfileFlagsReset();
391 + }
392 +
393 if (onSettingsUpdated != null) {
394 agent.addListener('updateHookSettings', onSettingsUpdated);
395 }
packages/react-devtools-extensions/src/contentScripts/backendManager.js
+14 -1
@@ -14,6 +14,11 @@ import type {
14 import {hasAssignedBackend} from 'react-devtools-shared/src/backend/utils';
15 import {COMPACT_VERSION_NAME} from 'react-devtools-extensions/src/utils';
16 import {getIsReloadAndProfileSupported} from 'react-devtools-shared/src/utils';
17 +import {
18 + getIfReloadedAndProfiling,
19 + onReloadAndProfile,
20 + onReloadAndProfileFlagsReset,
21 +} from 'react-devtools-shared/src/utils';
22
23 let welcomeHasInitialized = false;
24 const requiredBackends = new Set<string>();
@@ -140,7 +145,15 @@ function activateBackend(version: string, hook: DevToolsHook) {
145 },
146 });
147
143 - const agent = new Agent(bridge);
148 + const agent = new Agent(
149 + bridge,
150 + getIfReloadedAndProfiling(),
151 + onReloadAndProfile,
152 + );
153 + // Agent read flags successfully, we can count it as successful launch
154 + // Clean up flags, so that next reload won't start profiling
155 + onReloadAndProfileFlagsReset();
156 +
157 agent.addListener('shutdown', () => {
158 // If we received 'shutdown' from `agent`, we assume the `bridge` is already shutting down,
159 // and that caused the 'shutdown' event on the `agent`, so we don't need to call `bridge.shutdown()` here.
packages/react-devtools-extensions/src/contentScripts/installHook.js
+12 -1
@@ -1,4 +1,8 @@
1 import {installHook} from 'react-devtools-shared/src/hook';
2 +import {
3 + getIfReloadedAndProfiling,
4 + getProfilingSettings,
5 +} from 'react-devtools-shared/src/utils';
6
7 let resolveHookSettingsInjection;
8
@@ -34,8 +38,15 @@ if (!window.hasOwnProperty('__REACT_DEVTOOLS_GLOBAL_HOOK__')) {
38 payload: {handshake: true},
39 });
40
41 + const shouldStartProfiling = getIfReloadedAndProfiling();
42 + const profilingSettings = getProfilingSettings();
43 // Can't delay hook installation, inject settings lazily
38 - installHook(window, hookSettingsPromise);
44 + installHook(
45 + window,
46 + hookSettingsPromise,
47 + shouldStartProfiling,
48 + profilingSettings,
49 + );
50
51 // Detect React
52 window.__REACT_DEVTOOLS_GLOBAL_HOOK__.on(
packages/react-devtools-inline/src/backend.js
+12 -2
@@ -8,7 +8,12 @@ import setupNativeStyleEditor from 'react-devtools-shared/src/backend/NativeStyl
8
9 import type {BackendBridge} from 'react-devtools-shared/src/bridge';
10 import type {Wall} from 'react-devtools-shared/src/frontend/types';
11 -import {getIsReloadAndProfileSupported} from 'react-devtools-shared/src/utils';
11 +import {
12 + getIfReloadedAndProfiling,
13 + getIsReloadAndProfileSupported,
14 + onReloadAndProfile,
15 + onReloadAndProfileFlagsReset,
16 +} from 'react-devtools-shared/src/utils';
17
18 function startActivation(contentWindow: any, bridge: BackendBridge) {
19 const onSavedPreferences = (data: $FlowFixMe) => {
@@ -63,7 +68,12 @@ function startActivation(contentWindow: any, bridge: BackendBridge) {
68 }
69
70 function finishActivation(contentWindow: any, bridge: BackendBridge) {
66 - const agent = new Agent(bridge);
71 + const agent = new Agent(
72 + bridge,
73 + getIfReloadedAndProfiling(),
74 + onReloadAndProfile,
75 + );
76 + onReloadAndProfileFlagsReset();
77
78 const hook = contentWindow.__REACT_DEVTOOLS_GLOBAL_HOOK__;
79 if (hook) {
packages/react-devtools-shared/src/attachRenderer.js
+5 -3
@@ -12,8 +12,8 @@ import type {
12 RendererInterface,
13 DevToolsHook,
14 RendererID,
15 + ProfilingSettings,
16 } from 'react-devtools-shared/src/backend/types';
16 -import type {ReloadAndProfileConfig} from './backend/types';
17
18 import {attach as attachFlight} from 'react-devtools-shared/src/backend/flight/renderer';
19 import {attach as attachFiber} from 'react-devtools-shared/src/backend/fiber/renderer';
@@ -30,7 +30,8 @@ export default function attachRenderer(
30 id: RendererID,
31 renderer: ReactRenderer,
32 global: Object,
33 - reloadAndProfileConfig: ReloadAndProfileConfig,
33 + shouldStartProfilingNow: boolean,
34 + profilingSettings: ProfilingSettings,
35 ): RendererInterface | void {
36 // only attach if the renderer is compatible with the current version of the backend
37 if (!isMatchingRender(renderer.reconcilerVersion || renderer.version)) {
@@ -55,7 +56,8 @@ export default function attachRenderer(
56 id,
57 renderer,
58 global,
58 - reloadAndProfileConfig,
59 + shouldStartProfilingNow,
60 + profilingSettings,
61 );
62 } else if (renderer.ComponentTree) {
63 // react-dom v15
packages/react-devtools-shared/src/backend/agent.js
+8 -25
@@ -26,11 +26,9 @@ import type {
26 RendererID,
27 RendererInterface,
28 DevToolsHookSettings,
29 - ReloadAndProfileConfigPersistence,
29 } from './types';
30 import type {ComponentFilter} from 'react-devtools-shared/src/frontend/types';
31 import {isReactNativeEnvironment} from './utils';
33 -import {defaultReloadAndProfileConfigPersistence} from '../utils';
32 import {
33 sessionStorageGetItem,
34 sessionStorageRemoveItem,
@@ -151,33 +149,21 @@ export default class Agent extends EventEmitter<{
149 }> {
150 _bridge: BackendBridge;
151 _isProfiling: boolean = false;
154 - _recordChangeDescriptions: boolean = false;
152 _rendererInterfaces: {[key: RendererID]: RendererInterface, ...} = {};
153 _persistedSelection: PersistedSelection | null = null;
154 _persistedSelectionMatch: PathMatch | null = null;
155 _traceUpdatesEnabled: boolean = false;
159 - _reloadAndProfileConfigPersistence: ReloadAndProfileConfigPersistence;
156 + _onReloadAndProfile: ((recordChangeDescriptions: boolean) => void) | void;
157
158 constructor(
159 bridge: BackendBridge,
163 - reloadAndProfileConfigPersistence?: ReloadAndProfileConfigPersistence = defaultReloadAndProfileConfigPersistence,
160 + isProfiling: boolean = false,
161 + onReloadAndProfile?: (recordChangeDescriptions: boolean) => void,
162 ) {
163 super();
164
167 - this._reloadAndProfileConfigPersistence = reloadAndProfileConfigPersistence;
168 - const {getReloadAndProfileConfig, setReloadAndProfileConfig} =
169 - reloadAndProfileConfigPersistence;
170 - const reloadAndProfileConfig = getReloadAndProfileConfig();
171 - if (reloadAndProfileConfig.shouldReloadAndProfile) {
172 - this._recordChangeDescriptions =
173 - reloadAndProfileConfig.recordChangeDescriptions;
174 - this._isProfiling = true;
175 -
176 - setReloadAndProfileConfig({
177 - shouldReloadAndProfile: false,
178 - recordChangeDescriptions: false,
179 - });
180 - }
165 + this._isProfiling = isProfiling;
166 + this._onReloadAndProfile = onReloadAndProfile;
167
168 const persistedSelectionString = sessionStorageGetItem(
169 SESSION_STORAGE_LAST_SELECTION_KEY,
@@ -674,10 +660,9 @@ export default class Agent extends EventEmitter<{
660
661 reloadAndProfile: (recordChangeDescriptions: boolean) => void =
662 recordChangeDescriptions => {
677 - this._reloadAndProfileConfigPersistence.setReloadAndProfileConfig({
678 - shouldReloadAndProfile: true,
679 - recordChangeDescriptions,
680 - });
663 + if (typeof this._onReloadAndProfile === 'function') {
664 + this._onReloadAndProfile(recordChangeDescriptions);
665 + }
666
667 // This code path should only be hit if the shell has explicitly told the Store that it supports profiling.
668 // In that case, the shell must also listen for this specific message to know when it needs to reload the app.
@@ -757,7 +742,6 @@ export default class Agent extends EventEmitter<{
742
743 startProfiling: (recordChangeDescriptions: boolean) => void =
744 recordChangeDescriptions => {
760 - this._recordChangeDescriptions = recordChangeDescriptions;
745 this._isProfiling = true;
746 for (const rendererID in this._rendererInterfaces) {
747 const renderer = ((this._rendererInterfaces[
@@ -770,7 +754,6 @@ export default class Agent extends EventEmitter<{
754
755 stopProfiling: () => void = () => {
756 this._isProfiling = false;
773 - this._recordChangeDescriptions = false;
757 for (const rendererID in this._rendererInterfaces) {
758 const renderer = ((this._rendererInterfaces[
759 (rendererID: any)
packages/react-devtools-shared/src/backend/fiber/renderer.js
+5 -6
@@ -104,7 +104,6 @@ import {
104 supportsOwnerStacks,
105 supportsConsoleTasks,
106 } from './DevToolsFiberComponentStack';
107 -import type {ReloadAndProfileConfig} from '../types';
107
108 // $FlowFixMe[method-unbinding]
109 const toString = Object.prototype.toString;
@@ -136,6 +135,7 @@ import type {
135 WorkTagMap,
136 CurrentDispatcherRef,
137 LegacyDispatcherRef,
138 + ProfilingSettings,
139 } from '../types';
140 import type {
141 ComponentFilter,
@@ -864,7 +864,8 @@ export function attach(
864 rendererID: number,
865 renderer: ReactRenderer,
866 global: Object,
867 - reloadAndProfileConfig: ReloadAndProfileConfig,
867 + shouldStartProfilingNow: boolean,
868 + profilingSettings: ProfilingSettings,
869 ): RendererInterface {
870 // Newer versions of the reconciler package also specific reconciler version.
871 // If that version number is present, use it.
@@ -5225,10 +5226,8 @@ export function attach(
5226 }
5227
5228 // Automatically start profiling so that we don't miss timing info from initial "mount".
5228 - if (reloadAndProfileConfig.shouldReloadAndProfile) {
5229 - const shouldRecordChangeDescriptions =
5230 - reloadAndProfileConfig.recordChangeDescriptions;
5231 - startProfiling(shouldRecordChangeDescriptions);
5229 + if (shouldStartProfilingNow) {
5230 + startProfiling(profilingSettings.recordChangeDescriptions);
5231 }
5232
5233 function getNearestFiber(devtoolsInstance: DevToolsInstance): null | Fiber {
packages/react-devtools-shared/src/backend/types.js
+1 -11
@@ -485,20 +485,10 @@ export type DevToolsBackend = {
485 setupNativeStyleEditor?: SetupNativeStyleEditor,
486 };
487
488 -export type ReloadAndProfileConfig = {
489 - shouldReloadAndProfile: boolean,
488 +export type ProfilingSettings = {
489 recordChangeDescriptions: boolean,
490 };
491
493 -// Linter doesn't speak Flow's `Partial` type
494 -// eslint-disable-next-line no-undef
495 -type PartialReloadAndProfileConfig = Partial<ReloadAndProfileConfig>;
496 -
497 -export type ReloadAndProfileConfigPersistence = {
498 - setReloadAndProfileConfig: (config: PartialReloadAndProfileConfig) => void,
499 - getReloadAndProfileConfig: () => ReloadAndProfileConfig,
500 -};
501 -
492 export type DevToolsHook = {
493 listeners: {[key: string]: Array<Handler>, ...},
494 rendererInterfaces: Map<RendererID, RendererInterface>,
packages/react-devtools-shared/src/hook.js
+11 -4
@@ -16,7 +16,7 @@ import type {
16 RendererInterface,
17 DevToolsBackend,
18 DevToolsHookSettings,
19 - ReloadAndProfileConfig,
19 + ProfilingSettings,
20 } from './backend/types';
21
22 import {
@@ -27,7 +27,6 @@ import {
27 import attachRenderer from './attachRenderer';
28 import formatConsoleArguments from 'react-devtools-shared/src/backend/utils/formatConsoleArguments';
29 import formatWithStyles from 'react-devtools-shared/src/backend/utils/formatWithStyles';
30 -import {defaultReloadAndProfileConfigPersistence} from './utils';
30
31 // React's custom built component stack strings match "\s{4}in"
32 // Chrome's prefix matches "\s{4}at"
@@ -51,12 +50,17 @@ function areStackTracesEqual(a: string, b: string): boolean {
50
51 const targetConsole: Object = console;
52
53 +const defaultProfilingSettings: ProfilingSettings = {
54 + recordChangeDescriptions: false,
55 +};
56 +
57 export function installHook(
58 target: any,
59 maybeSettingsOrSettingsPromise?:
60 | DevToolsHookSettings
61 | Promise<DevToolsHookSettings>,
59 - reloadAndProfileConfig?: ReloadAndProfileConfig = defaultReloadAndProfileConfigPersistence.getReloadAndProfileConfig(),
62 + shouldStartProfilingNow: boolean = false,
63 + profilingSettings: ProfilingSettings = defaultProfilingSettings,
64 ): DevToolsHook | null {
65 if (target.hasOwnProperty('__REACT_DEVTOOLS_GLOBAL_HOOK__')) {
66 return null;
@@ -195,6 +199,8 @@ export function installHook(
199 } catch (err) {}
200 }
201
202 + // TODO: isProfiling should be stateful, and we should update it once profiling is finished
203 + const isProfiling = shouldStartProfilingNow;
204 let uidCounter = 0;
205 function inject(renderer: ReactRenderer): number {
206 const id = ++uidCounter;
@@ -215,7 +221,8 @@ export function installHook(
221 id,
222 renderer,
223 target,
218 - reloadAndProfileConfig,
224 + isProfiling,
225 + profilingSettings,
226 );
227 if (rendererInterface != null) {
228 hook.rendererInterfaces.set(id, rendererInterface);
packages/react-devtools-shared/src/utils.js
+32 -36
@@ -56,8 +56,9 @@ import {
56 localStorageGetItem,
57 localStorageSetItem,
58 sessionStorageGetItem,
59 + sessionStorageRemoveItem,
60 sessionStorageSetItem,
60 -} from './storage';
61 +} from 'react-devtools-shared/src/storage';
62 import {meta} from './hydration';
63 import isArray from './isArray';
64
@@ -67,12 +68,11 @@ import type {
68 SerializedElement as SerializedElementFrontend,
69 LRUCache,
70 } from 'react-devtools-shared/src/frontend/types';
70 -import type {SerializedElement as SerializedElementBackend} from 'react-devtools-shared/src/backend/types';
71 -import {isSynchronousXHRSupported} from './backend/utils';
71 import type {
73 - ReloadAndProfileConfig,
74 - ReloadAndProfileConfigPersistence,
75 -} from './backend/types';
72 + ProfilingSettings,
73 + SerializedElement as SerializedElementBackend,
74 +} from 'react-devtools-shared/src/backend/types';
75 +import {isSynchronousXHRSupported} from './backend/utils';
76
77 // $FlowFixMe[method-unbinding]
78 const hasOwnProperty = Object.prototype.hasOwnProperty;
@@ -990,34 +990,30 @@ export function getIsReloadAndProfileSupported(): boolean {
990 return isBackendStorageAPISupported && isSynchronousXHRSupported();
991 }
992
993 -export const defaultReloadAndProfileConfigPersistence: ReloadAndProfileConfigPersistence =
994 - {
995 - setReloadAndProfileConfig({
996 - shouldReloadAndProfile,
997 - recordChangeDescriptions,
998 - }): void {
999 - if (shouldReloadAndProfile != null) {
1000 - sessionStorageSetItem(
1001 - SESSION_STORAGE_RELOAD_AND_PROFILE_KEY,
1002 - shouldReloadAndProfile ? 'true' : 'false',
1003 - );
1004 - }
1005 - if (recordChangeDescriptions != null) {
1006 - sessionStorageSetItem(
1007 - SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY,
1008 - recordChangeDescriptions ? 'true' : 'false',
1009 - );
1010 - }
1011 - },
1012 - getReloadAndProfileConfig(): ReloadAndProfileConfig {
1013 - return {
1014 - shouldReloadAndProfile:
1015 - sessionStorageGetItem(SESSION_STORAGE_RELOAD_AND_PROFILE_KEY) ===
1016 - 'true',
1017 - recordChangeDescriptions:
1018 - sessionStorageGetItem(
1019 - SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY,
1020 - ) === 'true',
1021 - };
1022 - },
993 +// Expected to be used only by browser extension and react-devtools-inline
994 +export function getIfReloadedAndProfiling(): boolean {
995 + return (
996 + sessionStorageGetItem(SESSION_STORAGE_RELOAD_AND_PROFILE_KEY) === 'true'
997 + );
998 +}
999 +
1000 +export function getProfilingSettings(): ProfilingSettings {
1001 + return {
1002 + recordChangeDescriptions:
1003 + sessionStorageGetItem(SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY) ===
1004 + 'true',
1005 };
1006 +}
1007 +
1008 +export function onReloadAndProfile(recordChangeDescriptions: boolean): void {
1009 + sessionStorageSetItem(SESSION_STORAGE_RELOAD_AND_PROFILE_KEY, 'true');
1010 + sessionStorageSetItem(
1011 + SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY,
1012 + recordChangeDescriptions ? 'true' : 'false',
1013 + );
1014 +}
1015 +
1016 +export function onReloadAndProfileFlagsReset(): void {
1017 + sessionStorageRemoveItem(SESSION_STORAGE_RELOAD_AND_PROFILE_KEY);
1018 + sessionStorageRemoveItem(SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY);
1019 +}