Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 31 additions & 10 deletions packages/react-devtools-core/src/backend.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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> =
Expand All @@ -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) {
Expand All @@ -86,7 +93,9 @@ export function connectToDevTools(options: ?ConnectOptions) {
isAppActive = () => true,
onSettingsUpdated,
isReloadAndProfileSupported = getIsReloadAndProfileSupported(),
reloadAndProfileConfigPersistence,
isProfiling,
onReloadAndProfile,
onReloadAndProfileFlagsReset,
} = options || {};

const protocol = useHttps ? 'wss' : 'ws';
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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({
Expand All @@ -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) {
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
Expand Down Expand Up @@ -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();
Comment on lines +153 to +155
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a safe assumption? Would it be possible that rendering/profiling start async/later somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because these are reload and profile flags. If Backend was reloaded for profiling, then we should've already read them, when Hook is initialized, or when Agent is constructed.

This doesn't affect basic profiling without reloading.


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.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import {installHook} from 'react-devtools-shared/src/hook';
import {
getIfReloadedAndProfiling,
getProfilingSettings,
} from 'react-devtools-shared/src/utils';

let resolveHookSettingsInjection;

Expand Down Expand Up @@ -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(
Expand Down
14 changes: 12 additions & 2 deletions packages/react-devtools-inline/src/backend.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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) {
Expand Down
8 changes: 5 additions & 3 deletions packages/react-devtools-shared/src/attachRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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)) {
Expand All @@ -55,7 +56,8 @@ export default function attachRenderer(
id,
renderer,
global,
reloadAndProfileConfig,
shouldStartProfilingNow,
profilingSettings,
);
} else if (renderer.ComponentTree) {
// react-dom v15
Expand Down
33 changes: 8 additions & 25 deletions packages/react-devtools-shared/src/backend/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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[
Expand All @@ -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)
Expand Down
11 changes: 5 additions & 6 deletions packages/react-devtools-shared/src/backend/fiber/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ import {
supportsOwnerStacks,
supportsConsoleTasks,
} from './DevToolsFiberComponentStack';
import type {ReloadAndProfileConfig} from '../types';

// $FlowFixMe[method-unbinding]
const toString = Object.prototype.toString;
Expand Down Expand Up @@ -136,6 +135,7 @@ import type {
WorkTagMap,
CurrentDispatcherRef,
LegacyDispatcherRef,
ProfilingSettings,
} from '../types';
import type {
ComponentFilter,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down
12 changes: 1 addition & 11 deletions packages/react-devtools-shared/src/backend/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down
Loading
Loading