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

hoxyq · web-flow · commit bfe91fbecf18 · 2024-10-09T13:57:02.000+01:00
Stacked on #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.
diff --git a/packages/react-devtools-core/src/backend.js b/packages/react-devtools-core/src/backend.js
@@ -26,8 +26,7 @@ import type {
 import type {
   DevToolsHook,
   DevToolsHookSettings,
-  ReloadAndProfileConfig,
-  ReloadAndProfileConfigPersistence,
+  ProfilingSettings,
 } from 'react-devtools-shared/src/backend/types';
 import type {ResolveNativeStyle} from 'react-devtools-shared/src/backend/NativeStyleEditor/setupNativeStyleEditor';
 
@@ -42,7 +41,9 @@ type ConnectOptions = {
   websocket?: ?WebSocket,
   onSettingsUpdated?: (settings: $ReadOnly<DevToolsHookSettings>) => void,
   isReloadAndProfileSupported?: boolean,
-  reloadAndProfileConfigPersistence?: ReloadAndProfileConfigPersistence,
+  isProfiling?: boolean,
+  onReloadAndProfile?: (recordChangeDescriptions: boolean) => void,
+  onReloadAndProfileFlagsReset?: () => void,
 };
 
 let savedComponentFilters: Array<ComponentFilter> =
@@ -63,9 +64,15 @@ export function initialize(
   maybeSettingsOrSettingsPromise?:
     | DevToolsHookSettings
     | Promise<DevToolsHookSettings>,
-  reloadAndProfileConfig?: ReloadAndProfileConfig,
+  shouldStartProfilingNow: boolean = false,
+  profilingSettings?: ProfilingSettings,
 ) {
-  installHook(window, maybeSettingsOrSettingsPromise, reloadAndProfileConfig);
+  installHook(
+    window,
+    maybeSettingsOrSettingsPromise,
+    shouldStartProfilingNow,
+    profilingSettings,
+  );
 }
 
 export function connectToDevTools(options: ?ConnectOptions) {
@@ -86,7 +93,9 @@ export function connectToDevTools(options: ?ConnectOptions) {
     isAppActive = () => true,
     onSettingsUpdated,
     isReloadAndProfileSupported = getIsReloadAndProfileSupported(),
-    reloadAndProfileConfigPersistence,
+    isProfiling,
+    onReloadAndProfile,
+    onReloadAndProfileFlagsReset,
   } = options || {};
 
   const protocol = useHttps ? 'wss' : 'ws';
@@ -180,7 +189,11 @@ export function connectToDevTools(options: ?ConnectOptions) {
 
     // TODO (npm-packages) Warn if "isBackendStorageAPISupported"
     // $FlowFixMe[incompatible-call] found when upgrading Flow
-    const agent = new Agent(bridge, reloadAndProfileConfigPersistence);
+    const agent = new Agent(bridge, isProfiling, onReloadAndProfile);
+    if (typeof onReloadAndProfileFlagsReset === 'function') {
+      onReloadAndProfileFlagsReset();
+    }
+
     if (onSettingsUpdated != null) {
       agent.addListener('updateHookSettings', onSettingsUpdated);
     }
@@ -320,7 +333,9 @@ type ConnectWithCustomMessagingOptions = {
   resolveRNStyle?: ResolveNativeStyle,
   onSettingsUpdated?: (settings: $ReadOnly<DevToolsHookSettings>) => void,
   isReloadAndProfileSupported?: boolean,
-  reloadAndProfileConfigPersistence?: ReloadAndProfileConfigPersistence,
+  isProfiling?: boolean,
+  onReloadAndProfile?: (recordChangeDescriptions: boolean) => void,
+  onReloadAndProfileFlagsReset?: () => void,
 };
 
 export function connectWithCustomMessagingProtocol({
@@ -331,7 +346,9 @@ export function connectWithCustomMessagingProtocol({
   resolveRNStyle,
   onSettingsUpdated,
   isReloadAndProfileSupported = getIsReloadAndProfileSupported(),
-  reloadAndProfileConfigPersistence,
+  isProfiling,
+  onReloadAndProfile,
+  onReloadAndProfileFlagsReset,
 }: ConnectWithCustomMessagingOptions): Function {
   const hook: ?DevToolsHook = window.__REACT_DEVTOOLS_GLOBAL_HOOK__;
   if (hook == null) {
@@ -368,7 +385,11 @@ export function connectWithCustomMessagingProtocol({
     bridge.send('overrideComponentFilters', savedComponentFilters);
   }
 
-  const agent = new Agent(bridge, reloadAndProfileConfigPersistence);
+  const agent = new Agent(bridge, isProfiling, onReloadAndProfile);
+  if (typeof onReloadAndProfileFlagsReset === 'function') {
+    onReloadAndProfileFlagsReset();
+  }
+
   if (onSettingsUpdated != null) {
     agent.addListener('updateHookSettings', onSettingsUpdated);
   }
diff --git a/packages/react-devtools-extensions/src/contentScripts/backendManager.js b/packages/react-devtools-extensions/src/contentScripts/backendManager.js
@@ -14,6 +14,11 @@ import type {
 import {hasAssignedBackend} from 'react-devtools-shared/src/backend/utils';
 import {COMPACT_VERSION_NAME} from 'react-devtools-extensions/src/utils';
 import {getIsReloadAndProfileSupported} from 'react-devtools-shared/src/utils';
+import {
+  getIfReloadedAndProfiling,
+  onReloadAndProfile,
+  onReloadAndProfileFlagsReset,
+} from 'react-devtools-shared/src/utils';
 
 let welcomeHasInitialized = false;
 const requiredBackends = new Set<string>();
@@ -140,7 +145,15 @@ function activateBackend(version: string, hook: DevToolsHook) {
     },
   });
 
-  const agent = new Agent(bridge);
+  const agent = new Agent(
+    bridge,
+    getIfReloadedAndProfiling(),
+    onReloadAndProfile,
+  );
+  // Agent read flags successfully, we can count it as successful launch
+  // Clean up flags, so that next reload won't start profiling
+  onReloadAndProfileFlagsReset();
+
   agent.addListener('shutdown', () => {
     // If we received 'shutdown' from `agent`, we assume the `bridge` is already shutting down,
     // and that caused the 'shutdown' event on the `agent`, so we don't need to call `bridge.shutdown()` here.
diff --git a/packages/react-devtools-extensions/src/contentScripts/installHook.js b/packages/react-devtools-extensions/src/contentScripts/installHook.js
@@ -1,4 +1,8 @@
 import {installHook} from 'react-devtools-shared/src/hook';
+import {
+  getIfReloadedAndProfiling,
+  getProfilingSettings,
+} from 'react-devtools-shared/src/utils';
 
 let resolveHookSettingsInjection;
 
@@ -34,8 +38,15 @@ if (!window.hasOwnProperty('__REACT_DEVTOOLS_GLOBAL_HOOK__')) {
     payload: {handshake: true},
   });
 
+  const shouldStartProfiling = getIfReloadedAndProfiling();
+  const profilingSettings = getProfilingSettings();
   // Can't delay hook installation, inject settings lazily
-  installHook(window, hookSettingsPromise);
+  installHook(
+    window,
+    hookSettingsPromise,
+    shouldStartProfiling,
+    profilingSettings,
+  );
 
   // Detect React
   window.__REACT_DEVTOOLS_GLOBAL_HOOK__.on(
diff --git a/packages/react-devtools-inline/src/backend.js b/packages/react-devtools-inline/src/backend.js
@@ -8,7 +8,12 @@ import setupNativeStyleEditor from 'react-devtools-shared/src/backend/NativeStyl
 
 import type {BackendBridge} from 'react-devtools-shared/src/bridge';
 import type {Wall} from 'react-devtools-shared/src/frontend/types';
-import {getIsReloadAndProfileSupported} from 'react-devtools-shared/src/utils';
+import {
+  getIfReloadedAndProfiling,
+  getIsReloadAndProfileSupported,
+  onReloadAndProfile,
+  onReloadAndProfileFlagsReset,
+} from 'react-devtools-shared/src/utils';
 
 function startActivation(contentWindow: any, bridge: BackendBridge) {
   const onSavedPreferences = (data: $FlowFixMe) => {
@@ -63,7 +68,12 @@ function startActivation(contentWindow: any, bridge: BackendBridge) {
 }
 
 function finishActivation(contentWindow: any, bridge: BackendBridge) {
-  const agent = new Agent(bridge);
+  const agent = new Agent(
+    bridge,
+    getIfReloadedAndProfiling(),
+    onReloadAndProfile,
+  );
+  onReloadAndProfileFlagsReset();
 
   const hook = contentWindow.__REACT_DEVTOOLS_GLOBAL_HOOK__;
   if (hook) {
diff --git a/packages/react-devtools-shared/src/attachRenderer.js b/packages/react-devtools-shared/src/attachRenderer.js
@@ -12,8 +12,8 @@ import type {
   RendererInterface,
   DevToolsHook,
   RendererID,
+  ProfilingSettings,
 } from 'react-devtools-shared/src/backend/types';
-import type {ReloadAndProfileConfig} from './backend/types';
 
 import {attach as attachFlight} from 'react-devtools-shared/src/backend/flight/renderer';
 import {attach as attachFiber} from 'react-devtools-shared/src/backend/fiber/renderer';
@@ -30,7 +30,8 @@ export default function attachRenderer(
   id: RendererID,
   renderer: ReactRenderer,
   global: Object,
-  reloadAndProfileConfig: ReloadAndProfileConfig,
+  shouldStartProfilingNow: boolean,
+  profilingSettings: ProfilingSettings,
 ): RendererInterface | void {
   // only attach if the renderer is compatible with the current version of the backend
   if (!isMatchingRender(renderer.reconcilerVersion || renderer.version)) {
@@ -55,7 +56,8 @@ export default function attachRenderer(
         id,
         renderer,
         global,
-        reloadAndProfileConfig,
+        shouldStartProfilingNow,
+        profilingSettings,
       );
     } else if (renderer.ComponentTree) {
       // react-dom v15
diff --git a/packages/react-devtools-shared/src/backend/agent.js b/packages/react-devtools-shared/src/backend/agent.js
@@ -26,11 +26,9 @@ import type {
   RendererID,
   RendererInterface,
   DevToolsHookSettings,
-  ReloadAndProfileConfigPersistence,
 } from './types';
 import type {ComponentFilter} from 'react-devtools-shared/src/frontend/types';
 import {isReactNativeEnvironment} from './utils';
-import {defaultReloadAndProfileConfigPersistence} from '../utils';
 import {
   sessionStorageGetItem,
   sessionStorageRemoveItem,
@@ -151,33 +149,21 @@ export default class Agent extends EventEmitter<{
 }> {
   _bridge: BackendBridge;
   _isProfiling: boolean = false;
-  _recordChangeDescriptions: boolean = false;
   _rendererInterfaces: {[key: RendererID]: RendererInterface, ...} = {};
   _persistedSelection: PersistedSelection | null = null;
   _persistedSelectionMatch: PathMatch | null = null;
   _traceUpdatesEnabled: boolean = false;
-  _reloadAndProfileConfigPersistence: ReloadAndProfileConfigPersistence;
+  _onReloadAndProfile: ((recordChangeDescriptions: boolean) => void) | void;
 
   constructor(
     bridge: BackendBridge,
-    reloadAndProfileConfigPersistence?: ReloadAndProfileConfigPersistence = defaultReloadAndProfileConfigPersistence,
+    isProfiling: boolean = false,
+    onReloadAndProfile?: (recordChangeDescriptions: boolean) => void,
   ) {
     super();
 
-    this._reloadAndProfileConfigPersistence = reloadAndProfileConfigPersistence;
-    const {getReloadAndProfileConfig, setReloadAndProfileConfig} =
-      reloadAndProfileConfigPersistence;
-    const reloadAndProfileConfig = getReloadAndProfileConfig();
-    if (reloadAndProfileConfig.shouldReloadAndProfile) {
-      this._recordChangeDescriptions =
-        reloadAndProfileConfig.recordChangeDescriptions;
-      this._isProfiling = true;
-
-      setReloadAndProfileConfig({
-        shouldReloadAndProfile: false,
-        recordChangeDescriptions: false,
-      });
-    }
+    this._isProfiling = isProfiling;
+    this._onReloadAndProfile = onReloadAndProfile;
 
     const persistedSelectionString = sessionStorageGetItem(
       SESSION_STORAGE_LAST_SELECTION_KEY,
@@ -674,10 +660,9 @@ export default class Agent extends EventEmitter<{
 
   reloadAndProfile: (recordChangeDescriptions: boolean) => void =
     recordChangeDescriptions => {
-      this._reloadAndProfileConfigPersistence.setReloadAndProfileConfig({
-        shouldReloadAndProfile: true,
-        recordChangeDescriptions,
-      });
+      if (typeof this._onReloadAndProfile === 'function') {
+        this._onReloadAndProfile(recordChangeDescriptions);
+      }
 
       // This code path should only be hit if the shell has explicitly told the Store that it supports profiling.
       // 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<{
 
   startProfiling: (recordChangeDescriptions: boolean) => void =
     recordChangeDescriptions => {
-      this._recordChangeDescriptions = recordChangeDescriptions;
       this._isProfiling = true;
       for (const rendererID in this._rendererInterfaces) {
         const renderer = ((this._rendererInterfaces[
@@ -770,7 +754,6 @@ export default class Agent extends EventEmitter<{
 
   stopProfiling: () => void = () => {
     this._isProfiling = false;
-    this._recordChangeDescriptions = false;
     for (const rendererID in this._rendererInterfaces) {
       const renderer = ((this._rendererInterfaces[
         (rendererID: any)
diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js
@@ -104,7 +104,6 @@ import {
   supportsOwnerStacks,
   supportsConsoleTasks,
 } from './DevToolsFiberComponentStack';
-import type {ReloadAndProfileConfig} from '../types';
 
 // $FlowFixMe[method-unbinding]
 const toString = Object.prototype.toString;
@@ -136,6 +135,7 @@ import type {
   WorkTagMap,
   CurrentDispatcherRef,
   LegacyDispatcherRef,
+  ProfilingSettings,
 } from '../types';
 import type {
   ComponentFilter,
@@ -864,7 +864,8 @@ export function attach(
   rendererID: number,
   renderer: ReactRenderer,
   global: Object,
-  reloadAndProfileConfig: ReloadAndProfileConfig,
+  shouldStartProfilingNow: boolean,
+  profilingSettings: ProfilingSettings,
 ): RendererInterface {
   // Newer versions of the reconciler package also specific reconciler version.
   // If that version number is present, use it.
@@ -5225,10 +5226,8 @@ export function attach(
   }
 
   // Automatically start profiling so that we don't miss timing info from initial "mount".
-  if (reloadAndProfileConfig.shouldReloadAndProfile) {
-    const shouldRecordChangeDescriptions =
-      reloadAndProfileConfig.recordChangeDescriptions;
-    startProfiling(shouldRecordChangeDescriptions);
+  if (shouldStartProfilingNow) {
+    startProfiling(profilingSettings.recordChangeDescriptions);
   }
 
   function getNearestFiber(devtoolsInstance: DevToolsInstance): null | Fiber {
diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js
@@ -485,20 +485,10 @@ export type DevToolsBackend = {
   setupNativeStyleEditor?: SetupNativeStyleEditor,
 };
 
-export type ReloadAndProfileConfig = {
-  shouldReloadAndProfile: boolean,
+export type ProfilingSettings = {
   recordChangeDescriptions: boolean,
 };
 
-// Linter doesn't speak Flow's `Partial` type
-// eslint-disable-next-line no-undef
-type PartialReloadAndProfileConfig = Partial<ReloadAndProfileConfig>;
-
-export type ReloadAndProfileConfigPersistence = {
-  setReloadAndProfileConfig: (config: PartialReloadAndProfileConfig) => void,
-  getReloadAndProfileConfig: () => ReloadAndProfileConfig,
-};
-
 export type DevToolsHook = {
   listeners: {[key: string]: Array<Handler>, ...},
   rendererInterfaces: Map<RendererID, RendererInterface>,
diff --git a/packages/react-devtools-shared/src/hook.js b/packages/react-devtools-shared/src/hook.js
diff --git a/packages/react-devtools-shared/src/utils.js b/packages/react-devtools-shared/src/utils.js
