Skip to content

Commit

Permalink
Disable unsupported Bridge protocol version dialog and add workaround…
Browse files Browse the repository at this point in the history
… for old protocol operations format (#24093)

Rationale: The only case where the unsupported dialog really matters is React Naive. That's the case where the frontend and backend versions are most likely to mismatch. In React Native, the backend is likely to send the bridge protocol version before sending operations– since the agent does this proactively during initialization.

I've tested the React Native starter app– after forcefully downgrading the backend version to 4.19.1 (see #23307 (comment)) and verified that this change "fixes" things. Not only does DevTools no longer throw an error that causes the UI to be hidden– it works (meaning that the Components tree can be inspected and interacted with).
  • Loading branch information
Brian Vaughn committed Mar 15, 2022
1 parent 43eb283 commit 63b86e1
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 25 deletions.
4 changes: 2 additions & 2 deletions packages/react-devtools-core/src/standalone.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {createElement} from 'react';
import {
// $FlowFixMe Flow does not yet know about flushSync()
flushSync,
} from 'react-dom/client';
} from 'react-dom';
import {createRoot} from 'react-dom/client';
import Bridge from 'react-devtools-shared/src/bridge';
import Store from 'react-devtools-shared/src/devtools/store';
Expand Down Expand Up @@ -106,9 +106,9 @@ function safeUnmount() {
flushSync(() => {
if (root !== null) {
root.unmount();
root = null;
}
});
root = null;
}

function reload() {
Expand Down
3 changes: 3 additions & 0 deletions packages/react-devtools-shared/src/bridge.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ export const BRIDGE_PROTOCOL: Array<BridgeProtocol> = [
minNpmVersion: '"<4.11.0"',
maxNpmVersion: '"<4.11.0"',
},
// Versions 4.11.x – 4.12.x contained the backwards breaking change,
// but we didn't add the "fix" of checking the protocol version until 4.13,
// so we don't recommend downgrading to 4.11 or 4.12.
{
version: 1,
minNpmVersion: '4.13.0',
Expand Down
42 changes: 28 additions & 14 deletions packages/react-devtools-shared/src/devtools/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ export default class Store extends EventEmitter<{|
_rootSupportsBasicProfiling: boolean = false;
_rootSupportsTimelineProfiling: boolean = false;

_unsupportedBridgeProtocol: BridgeProtocol | null = null;
_bridgeProtocol: BridgeProtocol | null = null;
_unsupportedBridgeProtocolDetected: boolean = false;
_unsupportedRendererVersionDetected: boolean = false;

// Total number of visible elements (within all roots).
Expand Down Expand Up @@ -375,6 +376,10 @@ export default class Store extends EventEmitter<{|
this.emit('componentFilters');
}

get bridgeProtocol(): BridgeProtocol | null {
return this._bridgeProtocol;
}

get errorCount(): number {
return this._cachedErrorCount;
}
Expand Down Expand Up @@ -466,8 +471,8 @@ export default class Store extends EventEmitter<{|
return this._supportsTraceUpdates;
}

get unsupportedBridgeProtocol(): BridgeProtocol | null {
return this._unsupportedBridgeProtocol;
get unsupportedBridgeProtocolDetected(): boolean {
return this._unsupportedBridgeProtocolDetected;
}

get unsupportedRendererVersionDetected(): boolean {
Expand Down Expand Up @@ -942,11 +947,21 @@ export default class Store extends EventEmitter<{|
(operations[i] & PROFILING_FLAG_TIMELINE_SUPPORT) !== 0;
i++;

const supportsStrictMode = operations[i] > 0;
i++;
let supportsStrictMode = false;
let hasOwnerMetadata = false;

const hasOwnerMetadata = operations[i] > 0;
i++;
// If we don't know the bridge protocol, guess that we're dealing with the latest.
// If we do know it, we can take it into consideration when parsing operations.
if (
this._bridgeProtocol === null ||
this._bridgeProtocol.version >= 2
) {
supportsStrictMode = operations[i] > 0;
i++;

hasOwnerMetadata = operations[i] > 0;
i++;
}

this._roots = this._roots.concat(id);
this._rootIDToRendererID.set(id, rendererID);
Expand Down Expand Up @@ -1383,22 +1398,21 @@ export default class Store extends EventEmitter<{|
this._onBridgeProtocolTimeoutID = null;
}

this._bridgeProtocol = bridgeProtocol;

if (bridgeProtocol.version !== currentBridgeProtocol.version) {
this._unsupportedBridgeProtocol = bridgeProtocol;
} else {
// If we should happen to get a response after timing out...
this._unsupportedBridgeProtocol = null;
// Technically newer versions of the frontend can, at least for now,
// gracefully handle older versions of the backend protocol.
// So for now we don't need to display the unsupported dialog.
}

this.emit('unsupportedBridgeProtocolDetected');
};

onBridgeProtocolTimeout = () => {
this._onBridgeProtocolTimeoutID = null;

// If we timed out, that indicates the backend predates the bridge protocol,
// so we can set a fake version (0) to trigger the downgrade message.
this._unsupportedBridgeProtocol = BRIDGE_PROTOCOL[0];
this._bridgeProtocol = BRIDGE_PROTOCOL[0];

this.emit('unsupportedBridgeProtocolDetected');
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,18 @@ export default function UnsupportedBridgeProtocolDialog(_: {||}) {
useEffect(() => {
const updateDialog = () => {
if (!isVisible) {
if (store.unsupportedBridgeProtocol !== null) {
if (store.unsupportedBridgeProtocolDetected) {
dispatch({
canBeDismissed: false,
id: MODAL_DIALOG_ID,
type: 'SHOW',
content: (
<DialogContent
unsupportedBridgeProtocol={store.unsupportedBridgeProtocol}
/>
<DialogContent unsupportedBridgeProtocol={store.bridgeProtocol} />
),
});
}
} else {
if (store.unsupportedBridgeProtocol === null) {
if (!store.unsupportedBridgeProtocolDetected) {
dispatch({
type: 'HIDE',
id: MODAL_DIALOG_ID,
Expand Down
11 changes: 7 additions & 4 deletions packages/react-devtools-shared/src/devtools/views/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -239,10 +239,13 @@ export function useModalDismissSignal(
// It's important to listen to the ownerDocument to support the browser extension.
// Here we use portals to render individual tabs (e.g. Profiler),
// and the root document might belong to a different window.
ownerDocument = ((modalRef.current: any): HTMLDivElement).ownerDocument;
ownerDocument.addEventListener('keydown', handleDocumentKeyDown);
if (dismissOnClickOutside) {
ownerDocument.addEventListener('click', handleDocumentClick, true);
const div = modalRef.current;
if (div != null) {
ownerDocument = div.ownerDocument;
ownerDocument.addEventListener('keydown', handleDocumentKeyDown);
if (dismissOnClickOutside) {
ownerDocument.addEventListener('click', handleDocumentClick, true);
}
}
}, 0);

Expand Down

0 comments on commit 63b86e1

Please sign in to comment.