Skip to content

Commit

Permalink
refactor[Agent/Store]: Store to send messages only after Agent is ini…
Browse files Browse the repository at this point in the history
…tialized (#30945)

Both for browser extension, and for React Native (as part of
`react-devtools-core`) `Store` is initialized before the Backend (and
`Agent` as a part of it):

https://github.com/facebook/react/blob/bac33d1f82d9094b45d6f662dd7fa895abab8bce/packages/react-devtools-extensions/src/main/index.js#L111-L113

Any messages that we send from `Store`'s constructor are ignored,
because there is nothing on the other end yet. With these changes,
`Agent` will send `backendInitialized` message to `Store`, after which
`getBackendVersion` and other events will be sent.

Note that `isBackendStorageAPISupported` and `isSynchronousXHRSupported`
are still sent from `Agent`'s constructor, because we don't explicitly
ask for it from `Store`, but these are used.

This the pre-requisite for fetching settings and unsupported renderers
reliably from the Frontend.
  • Loading branch information
hoxyq authored Sep 11, 2024
1 parent bac33d1 commit 344bc81
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 27 deletions.
17 changes: 6 additions & 11 deletions packages/react-devtools-shared/src/backend/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -230,18 +230,16 @@ export default class Agent extends EventEmitter<{
bridge.addListener('overrideProps', this.overrideProps);
bridge.addListener('overrideState', this.overrideState);

setupHighlighter(bridge, this);
setupTraceUpdates(this);

// By this time, Store should already be initialized and intercept events
bridge.send('backendInitialized');

if (this._isProfiling) {
bridge.send('profilingStatus', true);
}

// Send the Bridge protocol and backend versions, after initialization, in case the frontend has already requested it.
// The Store may be instantiated beore the agent.
const version = process.env.DEVTOOLS_VERSION;
if (version) {
this._bridge.send('backendVersion', version);
}
this._bridge.send('bridgeProtocol', currentBridgeProtocol);

// Notify the frontend if the backend supports the Storage API (e.g. localStorage).
// If not, features like reload-and-profile will not work correctly and must be disabled.
let isBackendStorageAPISupported = false;
Expand All @@ -251,9 +249,6 @@ export default class Agent extends EventEmitter<{
} catch (error) {}
bridge.send('isBackendStorageAPISupported', isBackendStorageAPISupported);
bridge.send('isSynchronousXHRSupported', isSynchronousXHRSupported());

setupHighlighter(bridge, this);
setupTraceUpdates(this);
}

get rendererInterfaces(): {[key: RendererID]: RendererInterface, ...} {
Expand Down
1 change: 1 addition & 0 deletions packages/react-devtools-shared/src/bridge.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ type SavedPreferencesParams = {
};

export type BackendEvents = {
backendInitialized: [],
backendVersion: [string],
bridgeProtocol: [BridgeProtocol],
extensionBackendInitialized: [],
Expand Down
41 changes: 25 additions & 16 deletions packages/react-devtools-shared/src/devtools/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,8 @@ export default class Store extends EventEmitter<{
// Used for windowing purposes.
_weightAcrossRoots: number = 0;

_shouldCheckBridgeProtocolCompatibility: boolean = false;

constructor(bridge: FrontendBridge, config?: Config) {
super();

Expand Down Expand Up @@ -218,6 +220,7 @@ export default class Store extends EventEmitter<{
supportsReloadAndProfile,
supportsTimeline,
supportsTraceUpdates,
checkBridgeProtocolCompatibility,
} = config;
if (supportsInspectMatchingDOMElement) {
this._supportsInspectMatchingDOMElement = true;
Expand All @@ -234,6 +237,9 @@ export default class Store extends EventEmitter<{
if (supportsTraceUpdates) {
this._supportsTraceUpdates = true;
}
if (checkBridgeProtocolCompatibility) {
this._shouldCheckBridgeProtocolCompatibility = true;
}
}

this._bridge = bridge;
Expand Down Expand Up @@ -262,24 +268,9 @@ export default class Store extends EventEmitter<{

this._profilerStore = new ProfilerStore(bridge, this, isProfiling);

// Verify that the frontend version is compatible with the connected backend.
// See github.com/facebook/react/issues/21326
if (config != null && config.checkBridgeProtocolCompatibility) {
// Older backends don't support an explicit bridge protocol,
// so we should timeout eventually and show a downgrade message.
this._onBridgeProtocolTimeoutID = setTimeout(
this.onBridgeProtocolTimeout,
10000,
);

bridge.addListener('bridgeProtocol', this.onBridgeProtocol);
bridge.send('getBridgeProtocol');
}

bridge.addListener('backendVersion', this.onBridgeBackendVersion);
bridge.send('getBackendVersion');

bridge.addListener('saveToClipboard', this.onSaveToClipboard);
bridge.addListener('backendInitialized', this.onBackendInitialized);
}

// This is only used in tests to avoid memory leaks.
Expand Down Expand Up @@ -1493,6 +1484,24 @@ export default class Store extends EventEmitter<{
copy(text);
};

onBackendInitialized: () => void = () => {
// Verify that the frontend version is compatible with the connected backend.
// See github.com/facebook/react/issues/21326
if (this._shouldCheckBridgeProtocolCompatibility) {
// Older backends don't support an explicit bridge protocol,
// so we should timeout eventually and show a downgrade message.
this._onBridgeProtocolTimeoutID = setTimeout(
this.onBridgeProtocolTimeout,
10000,
);

this._bridge.addListener('bridgeProtocol', this.onBridgeProtocol);
this._bridge.send('getBridgeProtocol');
}

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

// 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.
Expand Down

0 comments on commit 344bc81

Please sign in to comment.