From 63b86e19955a3f68c3e6e4928f4e5b24fd8a8342 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 15 Mar 2022 10:48:26 -0700 Subject: [PATCH] Disable unsupported Bridge protocol version dialog and add workaround for old protocol operations format (#24093) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- .../react-devtools-core/src/standalone.js | 4 +- packages/react-devtools-shared/src/bridge.js | 3 ++ .../src/devtools/store.js | 42 ++++++++++++------- .../views/UnsupportedBridgeProtocolDialog.js | 8 ++-- .../src/devtools/views/hooks.js | 11 +++-- 5 files changed, 43 insertions(+), 25 deletions(-) diff --git a/packages/react-devtools-core/src/standalone.js b/packages/react-devtools-core/src/standalone.js index c04cad1ddea08..6b598efa22442 100644 --- a/packages/react-devtools-core/src/standalone.js +++ b/packages/react-devtools-core/src/standalone.js @@ -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'; @@ -106,9 +106,9 @@ function safeUnmount() { flushSync(() => { if (root !== null) { root.unmount(); + root = null; } }); - root = null; } function reload() { diff --git a/packages/react-devtools-shared/src/bridge.js b/packages/react-devtools-shared/src/bridge.js index ecb5e66672de8..0e5f837ecbc78 100644 --- a/packages/react-devtools-shared/src/bridge.js +++ b/packages/react-devtools-shared/src/bridge.js @@ -54,6 +54,9 @@ export const BRIDGE_PROTOCOL: Array = [ 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', diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js index ddf647cc210fa..f172cd6aa306b 100644 --- a/packages/react-devtools-shared/src/devtools/store.js +++ b/packages/react-devtools-shared/src/devtools/store.js @@ -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). @@ -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; } @@ -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 { @@ -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); @@ -1383,14 +1398,13 @@ 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 = () => { @@ -1398,7 +1412,7 @@ export default class Store extends EventEmitter<{| // 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'); }; diff --git a/packages/react-devtools-shared/src/devtools/views/UnsupportedBridgeProtocolDialog.js b/packages/react-devtools-shared/src/devtools/views/UnsupportedBridgeProtocolDialog.js index d48ab40aa3ea7..05f41c6211e75 100644 --- a/packages/react-devtools-shared/src/devtools/views/UnsupportedBridgeProtocolDialog.js +++ b/packages/react-devtools-shared/src/devtools/views/UnsupportedBridgeProtocolDialog.js @@ -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: ( - + ), }); } } else { - if (store.unsupportedBridgeProtocol === null) { + if (!store.unsupportedBridgeProtocolDetected) { dispatch({ type: 'HIDE', id: MODAL_DIALOG_ID, diff --git a/packages/react-devtools-shared/src/devtools/views/hooks.js b/packages/react-devtools-shared/src/devtools/views/hooks.js index 1a34373fc7e19..897bc928cb0a1 100644 --- a/packages/react-devtools-shared/src/devtools/views/hooks.js +++ b/packages/react-devtools-shared/src/devtools/views/hooks.js @@ -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);