From 8d30cc968b6715b4d2aa8b5aca1503b5e2854c6d Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin <28902667+hoxyq@users.noreply.github.com> Date: Thu, 23 Jul 2026 10:39:15 +0100 Subject: [PATCH 1/5] [DevTools] Type EventEmitter error handling (#37048) Tightens `EventEmitter` listener types and fixes error handling so the first thrown value is preserved while subsequent errors are reported instead of swallowed. Adds regression coverage for listener failures. --- .../src/__tests__/events-test.js | 47 ++++++++++++++++--- packages/react-devtools-shared/src/events.js | 15 ++++-- 2 files changed, 52 insertions(+), 10 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/events-test.js b/packages/react-devtools-shared/src/__tests__/events-test.js index 1d68acaab588..6377aa03199e 100644 --- a/packages/react-devtools-shared/src/__tests__/events-test.js +++ b/packages/react-devtools-shared/src/__tests__/events-test.js @@ -16,12 +16,10 @@ describe('events', () => { dispatcher = new EventEmitter(); }); - // @reactVersion >=16 it('can dispatch an event with no listeners', () => { dispatcher.emit('event', 123); }); - // @reactVersion >=16 it('handles a listener being attached multiple times', () => { const callback = jest.fn(); @@ -33,7 +31,6 @@ describe('events', () => { expect(callback).toHaveBeenCalledWith(123); }); - // @reactVersion >=16 it('notifies all attached listeners of events', () => { const callback1 = jest.fn(); const callback2 = jest.fn(); @@ -51,7 +48,6 @@ describe('events', () => { expect(callback3).not.toHaveBeenCalled(); }); - // @reactVersion >= 16.0 it('calls later listeners before re-throwing if an earlier one throws', () => { const callbackThatThrows = jest.fn(() => { throw Error('expected'); @@ -71,7 +67,46 @@ describe('events', () => { expect(callback).toHaveBeenCalledWith(123); }); - // @reactVersion >= 16.0 + it('preserves the first thrown value and reports later errors', () => { + const laterError = new Error('later error'); + const errorHandler = jest.fn(event => { + event.preventDefault(); + }); + const firstCallback = jest.fn(() => { + // This verifies that the emitter preserves any legal thrown value. + // eslint-disable-next-line no-throw-literal + throw null; + }); + const secondCallback = jest.fn(() => { + throw laterError; + }); + const thirdCallback = jest.fn(); + + dispatcher.addListener('event', firstCallback); + dispatcher.addListener('event', secondCallback); + dispatcher.addListener('event', thirdCallback); + + let caughtValue = undefined; + window.addEventListener('error', errorHandler); + try { + dispatcher.emit('event', 123); + } catch (error) { + caughtValue = error; + } finally { + window.removeEventListener('error', errorHandler); + } + + expect(caughtValue).toBe(null); + expect(errorHandler).toHaveBeenCalledTimes(1); + expect(errorHandler.mock.calls[0][0]).toEqual( + expect.objectContaining({ + error: laterError, + message: 'later error', + }), + ); + expect(thirdCallback).toHaveBeenCalledWith(123); + }); + it('removes attached listeners', () => { const callback1 = jest.fn(); const callback2 = jest.fn(); @@ -86,7 +121,6 @@ describe('events', () => { expect(callback2).toHaveBeenCalledWith(123); }); - // @reactVersion >= 16.0 it('removes all listeners', () => { const callback1 = jest.fn(); const callback2 = jest.fn(); @@ -104,7 +138,6 @@ describe('events', () => { expect(callback3).not.toHaveBeenCalled(); }); - // @reactVersion >= 16.0 it('should call the initial listeners even if others are added or removed during a dispatch', () => { const callback1 = jest.fn(() => { dispatcher.removeListener('event', callback2); diff --git a/packages/react-devtools-shared/src/events.js b/packages/react-devtools-shared/src/events.js index 2442ec20f155..d7be6d6ebe0b 100644 --- a/packages/react-devtools-shared/src/events.js +++ b/packages/react-devtools-shared/src/events.js @@ -7,12 +7,14 @@ * @flow */ +import reportGlobalError from 'shared/reportGlobalError'; + export default class EventEmitter { listenersMap: Map> = new Map(); addListener>( event: Event, - listener: (...Events[Event]) => any, + listener: (...Events[Event]) => mixed, ): void { const listeners = this.listenersMap.get(event); if (listeners === undefined) { @@ -42,9 +44,13 @@ export default class EventEmitter { try { listener.apply(null, args); } catch (error) { - if (caughtError === null) { + if (!didThrow) { didThrow = true; caughtError = error; + } else { + // Continue notifying the remaining listeners, but do not hide + // additional failures behind the first one. + reportGlobalError(error); } } } @@ -60,7 +66,10 @@ export default class EventEmitter { this.listenersMap.clear(); } - removeListener(event: $Keys, listener: Function): void { + removeListener>( + event: Event, + listener: (...Events[Event]) => mixed, + ): void { const listeners = this.listenersMap.get(event); if (listeners !== undefined) { const index = listeners.indexOf(listener); From 218efce013b05907e05ee80ab5d848f990fcf6b9 Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin <28902667+hoxyq@users.noreply.github.com> Date: Thu, 23 Jul 2026 10:39:15 +0100 Subject: [PATCH 2/5] [DevTools] Shut down standalone Bridge on socket close (#37076) Ensures the standalone DevTools Bridge fully shuts down when its WebSocket closes, with re-entrancy protection. Adds tests confirming event-only shutdown leaves the Bridge active while socket closure shuts it down. --- packages/react-devtools-core/src/backend.js | 20 ++++--- .../src/__tests__/backend-test.js | 58 +++++++++++++++++++ 2 files changed, 70 insertions(+), 8 deletions(-) create mode 100644 packages/react-devtools-shared/src/__tests__/backend-test.js diff --git a/packages/react-devtools-core/src/backend.js b/packages/react-devtools-core/src/backend.js index 7367e7bd1bc2..338828b5c95b 100644 --- a/packages/react-devtools-core/src/backend.js +++ b/packages/react-devtools-core/src/backend.js @@ -132,6 +132,16 @@ export function connectToDevTools(options: ?ConnectOptions) { let bridge: BackendBridge | null = null; + function shutdownBridge(): void { + const bridgeToShutdown = bridge; + if (bridgeToShutdown !== null) { + // Clear the active reference before shutdown flushes its final message + // through a potentially closed socket. + bridge = null; + bridgeToShutdown.shutdown(); + } + } + const messageListeners = []; const uri = protocol + '://' + host + ':' + port + prefixedPath; @@ -170,10 +180,7 @@ export function connectToDevTools(options: ?ConnectOptions) { ); } - if (bridge !== null) { - bridge.shutdown(); - } - + shutdownBridge(); scheduleRetry(); } }, @@ -273,10 +280,7 @@ export function connectToDevTools(options: ?ConnectOptions) { debug('WebSocket.onclose'); } - if (bridge !== null) { - bridge.emit('shutdown'); - } - + shutdownBridge(); scheduleRetry(); } diff --git a/packages/react-devtools-shared/src/__tests__/backend-test.js b/packages/react-devtools-shared/src/__tests__/backend-test.js new file mode 100644 index 000000000000..2e1b8634fbd0 --- /dev/null +++ b/packages/react-devtools-shared/src/__tests__/backend-test.js @@ -0,0 +1,58 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +describe('connectToDevTools', () => { + let connectToDevTools; + let hook; + + beforeEach(() => { + jest.resetModules(); + delete window.__REACT_DEVTOOLS_GLOBAL_HOOK__; + + const backend = require('react-devtools-core/src/backend'); + backend.initialize(); + connectToDevTools = backend.connectToDevTools; + hook = window.__REACT_DEVTOOLS_GLOBAL_HOOK__; + }); + + afterEach(() => { + jest.clearAllTimers(); + delete window.__REACT_DEVTOOLS_GLOBAL_HOOK__; + }); + + function createWebSocket(): WebSocket { + return { + CLOSED: 3, + OPEN: 1, + readyState: 1, + send: jest.fn(), + } as any as WebSocket; + } + + it('shuts down cleanly when the WebSocket closes', () => { + const websocket = createWebSocket(); + const onShutdown = jest.fn(); + const unsubscribe = hook.sub('shutdown', onShutdown); + + try { + connectToDevTools({websocket}); + websocket.onopen(); + websocket.readyState = websocket.CLOSED; + websocket.onclose(); + jest.runAllTimers(); + + expect(onShutdown).toHaveBeenCalledTimes(1); + expect(global.consoleWarnMock).not.toHaveBeenCalledWith( + 'Bridge was already shutdown.', + ); + } finally { + unsubscribe(); + } + }); +}); From fc08438abd8fe11f819f609fefc39f06a279ef5b Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin <28902667+hoxyq@users.noreply.github.com> Date: Thu, 23 Jul 2026 10:39:15 +0100 Subject: [PATCH 3/5] [DevTools] Harden Bridge and Wall lifecycle types (#37049) Builds on #37048 by replacing `any`-based Bridge and Wall boundaries with typed `mixed` values and explicit runtime validation. Invalid messages and post-shutdown operations now throw, while shutdown reliably flushes queued messages even if cleanup fails. Strengthens the DevTools Bridge and Wall contracts: - Models event dictionaries as event-to-payload maps, using `void` for events without payloads. - Types `send(event, payload?)` directly, eliminating runtime payload-arity handling. - Replaces broad `any` transport types with `mixed` and boundary validation. - Throws on invalid lifecycle usage instead of warning or silently returning. - Ensures shutdown flushes queued messages even when Wall cleanup fails. - Updates Wall implementations and adds Bridge lifecycle coverage. --- packages/react-devtools-core/src/backend.js | 16 +- .../react-devtools-core/src/standalone.js | 4 +- .../src/background/index.js | 4 +- .../src/contentScripts/backendManager.js | 2 +- .../src/contentScripts/proxy.js | 2 +- .../src/main/index.js | 6 +- .../react-devtools-fusebox/src/frontend.d.ts | 6 +- .../react-devtools-fusebox/src/frontend.js | 2 +- packages/react-devtools-inline/src/backend.js | 6 +- .../react-devtools-inline/src/frontend.js | 6 +- .../src/__tests__/bridge-test.js | 82 ++++- .../src/__tests__/setupTests.js | 2 +- packages/react-devtools-shared/src/bridge.js | 304 ++++++++++-------- .../src/devtools/views/DevTools.js | 8 +- .../views/WarnIfLegacyBackendDetected.js | 14 +- .../src/frontend/types.js | 16 +- .../src/multi/devtools.js | 6 + scripts/flow/react-devtools.js | 6 +- 18 files changed, 321 insertions(+), 171 deletions(-) diff --git a/packages/react-devtools-core/src/backend.js b/packages/react-devtools-core/src/backend.js index 338828b5c95b..93d5296b2ba9 100644 --- a/packages/react-devtools-core/src/backend.js +++ b/packages/react-devtools-core/src/backend.js @@ -163,7 +163,11 @@ export function connectToDevTools(options: ?ConnectOptions) { } }; }, - send(event: string, payload: any, transferable?: Array) { + send( + event: string, + payload: mixed, + transferable?: $ReadOnlyArray, + ) { if (ws.readyState === ws.OPEN) { // $FlowFixMe[constant-condition] if (__DEBUG__) { @@ -327,9 +331,9 @@ export function connectToDevTools(options: ?ConnectOptions) { } type ConnectWithCustomMessagingOptions = { - onSubscribe: (cb: Function) => void, - onUnsubscribe: (cb: Function) => void, - onMessage: (event: string, payload: any) => void, + onSubscribe: (cb: (message: mixed) => void) => void, + onUnsubscribe: (cb: (message: mixed) => void) => void, + onMessage: (event: string, payload: mixed) => void, nativeStyleEditorValidAttributes?: $ReadOnlyArray, resolveRNStyle?: ResolveNativeStyle, onSettingsUpdated?: (settings: $ReadOnly) => void, @@ -358,14 +362,14 @@ export function connectWithCustomMessagingProtocol({ } const wall: Wall = { - listen(fn: Function) { + listen(fn: (message: mixed) => void) { onSubscribe(fn); return () => { onUnsubscribe(fn); }; }, - send(event: string, payload: any) { + send(event: string, payload: mixed) { onMessage(event, payload); }, }; diff --git a/packages/react-devtools-core/src/standalone.js b/packages/react-devtools-core/src/standalone.js index 32b9566bf626..0c78a15ac0b8 100644 --- a/packages/react-devtools-core/src/standalone.js +++ b/packages/react-devtools-core/src/standalone.js @@ -209,7 +209,7 @@ function onError({code, message}: $FlowFixMe) { function openProfiler() { // Mocked up bridge and store to allow the DevTools to be rendered - bridge = new Bridge({listen: () => {}, send: () => {}}); + bridge = new Bridge({listen: () => () => {}, send: () => {}}); store = new Store(bridge, {}); // Ensure the Profiler tab is shown initially. @@ -260,7 +260,7 @@ function initialize(socket: WebSocket) { } }; }, - send(event: string, payload: any, transferable?: Array) { + send(event: string, payload: mixed, transferable?: $ReadOnlyArray) { if (socket.readyState === socket.OPEN) { socket.send(JSON.stringify({event, payload})); } diff --git a/packages/react-devtools-extensions/src/background/index.js b/packages/react-devtools-extensions/src/background/index.js index 34c337a59e25..9c2fc8ab84a8 100644 --- a/packages/react-devtools-extensions/src/background/index.js +++ b/packages/react-devtools-extensions/src/background/index.js @@ -163,7 +163,7 @@ function connectExtensionAndProxyPorts( ); } - function extensionPortMessageListener(message: any) { + function extensionPortMessageListener(message: mixed) { try { proxyPort.postMessage(message); } catch (e) { @@ -175,7 +175,7 @@ function connectExtensionAndProxyPorts( } } - function proxyPortMessageListener(message: any) { + function proxyPortMessageListener(message: mixed) { try { extensionPort.postMessage(message); } catch (e) { diff --git a/packages/react-devtools-extensions/src/contentScripts/backendManager.js b/packages/react-devtools-extensions/src/contentScripts/backendManager.js index 402a13785762..5589bcc1cb61 100644 --- a/packages/react-devtools-extensions/src/contentScripts/backendManager.js +++ b/packages/react-devtools-extensions/src/contentScripts/backendManager.js @@ -133,7 +133,7 @@ function activateBackend(version: string, hook: DevToolsHook) { window.removeEventListener('message', listener); }; }, - send(event: string, payload: any, transferable?: Array) { + send(event: string, payload: mixed, transferable?: $ReadOnlyArray) { window.postMessage( { source: 'react-devtools-bridge', diff --git a/packages/react-devtools-extensions/src/contentScripts/proxy.js b/packages/react-devtools-extensions/src/contentScripts/proxy.js index caa4c6d99fec..637e52c8cc63 100644 --- a/packages/react-devtools-extensions/src/contentScripts/proxy.js +++ b/packages/react-devtools-extensions/src/contentScripts/proxy.js @@ -71,7 +71,7 @@ function sayHelloToBackendManager() { ); } -function handleMessageFromDevtools(message: any) { +function handleMessageFromDevtools(message: mixed) { window.postMessage( { source: 'react-devtools-content-script', diff --git a/packages/react-devtools-extensions/src/main/index.js b/packages/react-devtools-extensions/src/main/index.js index 1edbbf5e1cd3..b5434098bbf8 100644 --- a/packages/react-devtools-extensions/src/main/index.js +++ b/packages/react-devtools-extensions/src/main/index.js @@ -2,7 +2,7 @@ /** @flow */ import type {RootType} from 'react-dom/src/client/ReactDOMRoot'; -import type {FrontendBridge, Message} from 'react-devtools-shared/src/bridge'; +import type {FrontendBridge} from 'react-devtools-shared/src/bridge'; import type { TabID, ViewElementSource, @@ -59,7 +59,7 @@ const hookNamesModuleLoaderFunction = () => resolvedParseHookNames; function createBridge() { bridge = new Bridge({ listen(fn) { - const bridgeListener = (message: Message) => fn(message); + const bridgeListener = (message: mixed) => fn(message); // Store the reference so that we unsubscribe from the same object. const portOnMessage = port.onMessage; portOnMessage.addListener(bridgeListener); @@ -72,7 +72,7 @@ function createBridge() { }; }, - send(event: string, payload: any, transferable?: Array) { + send(event: string, payload: mixed, transferable?: $ReadOnlyArray) { port?.postMessage({event, payload}, transferable); }, }); diff --git a/packages/react-devtools-fusebox/src/frontend.d.ts b/packages/react-devtools-fusebox/src/frontend.d.ts index 988197e89ca4..4950049b805e 100644 --- a/packages/react-devtools-fusebox/src/frontend.d.ts +++ b/packages/react-devtools-fusebox/src/frontend.d.ts @@ -14,14 +14,14 @@ export type MessagePayload = | MessagePayload[]; export type Message = {event: string; payload?: MessagePayload}; -export type WallListener = (message: Message) => void; +export type WallListener = (message: unknown) => void; export type Wall = { - listen: (fn: WallListener) => Function; + listen: (fn: WallListener) => () => void; send: (event: string, payload?: MessagePayload) => void; }; export type Bridge = { - addListener(event: string, listener: (params: unknown) => any): void; + addListener(event: string, listener: (params: unknown) => unknown): void; removeListener(event: string, listener: Function): void; shutdown: () => void; }; diff --git a/packages/react-devtools-fusebox/src/frontend.js b/packages/react-devtools-fusebox/src/frontend.js index 7d09e280233a..82c6ac1f4023 100644 --- a/packages/react-devtools-fusebox/src/frontend.js +++ b/packages/react-devtools-fusebox/src/frontend.js @@ -32,7 +32,7 @@ export function createBridge(wall?: Wall): FrontendBridge { return new Bridge(wall); } - return new Bridge({listen: () => {}, send: () => {}}); + return new Bridge({listen: () => () => {}, send: () => {}}); } export function createStore(bridge: FrontendBridge, config?: Config): Store { diff --git a/packages/react-devtools-inline/src/backend.js b/packages/react-devtools-inline/src/backend.js index 8b0ab3584325..9b8a7ae05397 100644 --- a/packages/react-devtools-inline/src/backend.js +++ b/packages/react-devtools-inline/src/backend.js @@ -103,7 +103,11 @@ export function createBridge(contentWindow: any, wall?: Wall): BackendBridge { contentWindow.removeEventListener('message', onMessage); }; }, - send(event: string, payload: any, transferable?: Array) { + send( + event: string, + payload: mixed, + transferable?: $ReadOnlyArray, + ) { parent.postMessage({event, payload}, '*', transferable); }, }; diff --git a/packages/react-devtools-inline/src/frontend.js b/packages/react-devtools-inline/src/frontend.js index 28fe70b1b299..48ffe43c5271 100644 --- a/packages/react-devtools-inline/src/frontend.js +++ b/packages/react-devtools-inline/src/frontend.js @@ -34,7 +34,11 @@ export function createBridge(contentWindow: any, wall?: Wall): FrontendBridge { window.removeEventListener('message', onMessage); }; }, - send(event: string, payload: any, transferable?: Array) { + send( + event: string, + payload: mixed, + transferable?: $ReadOnlyArray, + ) { contentWindow.postMessage({event, payload}, '*', transferable); }, }; diff --git a/packages/react-devtools-shared/src/__tests__/bridge-test.js b/packages/react-devtools-shared/src/__tests__/bridge-test.js index 1cc47fbe5d8a..0633de7c99eb 100644 --- a/packages/react-devtools-shared/src/__tests__/bridge-test.js +++ b/packages/react-devtools-shared/src/__tests__/bridge-test.js @@ -40,14 +40,86 @@ describe('Bridge', () => { expect(wall.send).toHaveBeenCalledWith('shutdown', undefined); expect(shutdownCallback).toHaveBeenCalledTimes(1); - // Verify that the Bridge doesn't send messages after shutdown. - jest.spyOn(console, 'warn').mockImplementation(() => {}); + // Using a Bridge after shutdown is a lifecycle error. wall.send.mockClear(); - bridge.send('should not send'); + expect(() => bridge.send('should not send')).toThrow( + 'Cannot send a message through a Bridge that has been shut down.', + ); + expect(() => bridge.addListener('event', () => {})).toThrow( + 'Cannot add a listener through a Bridge that has been shut down.', + ); + expect(() => bridge.emit('event')).toThrow( + 'Cannot emit an event through a Bridge that has been shut down.', + ); + expect(() => bridge.shutdown()).toThrow( + 'Cannot shut down through a Bridge that has been shut down.', + ); jest.runAllTimers(); expect(wall.send).not.toHaveBeenCalled(); - expect(console.warn).toHaveBeenCalledWith( - 'Cannot send message "should not send" through a Bridge that has been shutdown.', + }); + + // @reactVersion >=16.0 + it('validates messages received from the wall', () => { + let wallListener: ((message: mixed) => void) | null = null; + const wall = { + listen: jest.fn(listener => { + wallListener = listener; + return () => {}; + }), + send: jest.fn(), + }; + const bridge = new Bridge(wall); + const listener = jest.fn(); + bridge.addListener('event', listener); + + const dispatch = (message: mixed) => { + if (wallListener === null) { + throw new Error('Expected the Bridge to subscribe to the wall.'); + } + wallListener(message); + }; + + // Walls may share their transport with unrelated or legacy messages. + dispatch(null); + dispatch({type: 'event'}); + expect(listener).not.toHaveBeenCalled(); + + expect(() => dispatch({event: 123})).toThrow( + 'Bridge event names must be non-empty strings.', ); + expect(() => dispatch({event: ''})).toThrow( + 'Bridge event names must be non-empty strings.', + ); + + dispatch({event: 'event', payload: 123}); + expect(listener).toHaveBeenCalledWith(123); + }); + + // @reactVersion >=16.0 + it('requires Wall.listen to return a cleanup function', () => { + expect( + () => + new Bridge({ + listen: () => undefined, + send: jest.fn(), + }), + ).toThrow('Wall.listen() must return an unlisten function.'); + }); + + // @reactVersion >=16.0 + it('flushes pending messages when wall cleanup throws', () => { + const expectedError = new Error('Failed to unsubscribe'); + const wall = { + listen: jest.fn(() => () => { + throw expectedError; + }), + send: jest.fn(), + }; + const bridge = new Bridge(wall); + + bridge.send('update', 'value'); + expect(() => bridge.shutdown()).toThrow(expectedError); + expect(wall.send).toHaveBeenCalledWith('update', 'value'); + expect(wall.send).toHaveBeenCalledWith('shutdown', undefined); }); }); diff --git a/packages/react-devtools-shared/src/__tests__/setupTests.js b/packages/react-devtools-shared/src/__tests__/setupTests.js index 37b07ea7c35a..197e76c9e2ea 100644 --- a/packages/react-devtools-shared/src/__tests__/setupTests.js +++ b/packages/react-devtools-shared/src/__tests__/setupTests.js @@ -258,7 +258,7 @@ beforeEach(() => { } }; }, - send(event: string, payload: any, transferable?: Array) { + send(event: string, payload: mixed, transferable?: $ReadOnlyArray) { bridgeListeners.forEach(callback => callback({event, payload})); }, }); diff --git a/packages/react-devtools-shared/src/bridge.js b/packages/react-devtools-shared/src/bridge.js index a568c22c392d..162fa8eabea2 100644 --- a/packages/react-devtools-shared/src/bridge.js +++ b/packages/react-devtools-shared/src/bridge.js @@ -9,7 +9,7 @@ import EventEmitter from './events'; -import type {ComponentFilter, Wall} from './frontend/types'; +import type {ComponentFilter, Wall, WallMessage} from './frontend/types'; import type { InspectedElementPayload, OwnersList, @@ -74,9 +74,17 @@ export const currentBridgeProtocol: BridgeProtocol = type ElementAndRendererID = {id: number, rendererID: RendererID}; -export type Message = { +export type Message = WallMessage; + +type QueuedMessage = { event: string, - payload: any, + payload: mixed, +}; + +type EventArguments = Payload extends void ? [] : [Payload]; + +type EventEmitterEvents = { + [Event in keyof Events]: EventArguments, }; type HighlightHostInstance = { @@ -101,7 +109,7 @@ type OverrideValue = { ...ElementAndRendererID, path: Array, wasForwarded?: boolean, - value: any, + value: mixed, }; type OverrideHookState = { @@ -131,7 +139,7 @@ type OverrideValueAtPath = { type: PathType, hookID?: ?number, path: Array, - value: any, + value: mixed, }; type OverrideError = { @@ -197,95 +205,96 @@ export type SavedPreferencesParams = { }; export type BackendEvents = { - backendInitialized: [], - backendVersion: [string], - bridgeProtocol: [BridgeProtocol], - extensionBackendInitialized: [], - fastRefreshScheduled: [], - getSavedPreferences: [], - inspectedElement: [InspectedElementPayload], - inspectedScreen: [InspectedElementPayload], - isReloadAndProfileSupportedByBackend: [boolean], - operations: [Array], - ownersList: [OwnersList], - environmentNames: [Array], - profilingData: [ProfilingDataBackend], - profilingStatus: [boolean], - reloadAppForProfiling: [], - saveToClipboard: [string], - selectElement: [number | null], - shutdown: [], - stopInspectingHost: [boolean], - scrollTo: [{left: number, top: number, right: number, bottom: number}], - syncSelectionToBuiltinElementsPanel: [], - unsupportedRendererVersion: [], - - extensionComponentsPanelShown: [], - extensionComponentsPanelHidden: [], - - resumeElementPolling: [], - pauseElementPolling: [], + backendInitialized: void, + backendVersion: string, + bridgeProtocol: BridgeProtocol, + extensionBackendInitialized: void, + fastRefreshScheduled: void, + getSavedPreferences: void, + inspectedElement: InspectedElementPayload, + inspectedScreen: InspectedElementPayload, + isReloadAndProfileSupportedByBackend: boolean, + operations: Array, + ownersList: OwnersList, + environmentNames: Array, + profilingData: ProfilingDataBackend, + profilingStatus: boolean, + reloadAppForProfiling: void, + saveToClipboard: string, + selectElement: number | null, + shutdown: void, + stopInspectingHost: boolean, + scrollTo: {left: number, top: number, right: number, bottom: number}, + syncSelectionToBuiltinElementsPanel: void, + unsupportedRendererVersion: void, + + extensionComponentsPanelShown: void, + extensionComponentsPanelHidden: void, + + resumeElementPolling: void, + pauseElementPolling: void, // React Native style editor plug-in. - isNativeStyleEditorSupported: [ - {isSupported: boolean, validAttributes: ?$ReadOnlyArray}, - ], - NativeStyleEditor_styleAndLayout: [StyleAndLayoutPayload], + isNativeStyleEditorSupported: { + isSupported: boolean, + validAttributes: ?$ReadOnlyArray, + }, + NativeStyleEditor_styleAndLayout: StyleAndLayoutPayload, - hookSettings: [$ReadOnly], + hookSettings: $ReadOnly, }; type StartProfilingParams = ProfilingSettings; type ReloadAndProfilingParams = ProfilingSettings; export type FrontendEvents = { - clearErrorsAndWarnings: [{rendererID: RendererID}], - clearErrorsForElementID: [ElementAndRendererID], - clearHostInstanceHighlight: [], - clearWarningsForElementID: [ElementAndRendererID], - copyElementPath: [CopyElementPathParams], - deletePath: [DeletePath], - getBackendVersion: [], - getBridgeProtocol: [], - getIfHasUnsupportedRendererVersion: [], - getOwnersList: [ElementAndRendererID], - getProfilingData: [{rendererID: RendererID}], - getProfilingStatus: [], - highlightHostInstance: [HighlightHostInstance], - highlightHostInstances: [HighlightHostInstances], - inspectElement: [InspectElementParams], - inspectScreen: [InspectScreenParams], - logElementToConsole: [ElementAndRendererID], - overrideError: [OverrideError], - overrideSuspense: [OverrideSuspense], - overrideSuspenseMilestone: [OverrideSuspenseMilestone], - overrideValueAtPath: [OverrideValueAtPath], - profilingData: [ProfilingDataBackend], - reloadAndProfile: [ReloadAndProfilingParams], - renamePath: [RenamePath], - savedPreferences: [SavedPreferencesParams], - setTraceUpdatesEnabled: [boolean], - shutdown: [], - startInspectingHost: [boolean], - startProfiling: [StartProfilingParams], - stopInspectingHost: [], - scrollToHostInstance: [ScrollToHostInstance], - scrollTo: [{left: number, top: number, right: number, bottom: number}], - requestScrollPosition: [], - stopProfiling: [], - storeAsGlobal: [StoreAsGlobalParams], - updateComponentFilters: [Array], - getEnvironmentNames: [], - updateHookSettings: [$ReadOnly], - viewAttributeSource: [ViewAttributeSourceParams], - viewElementSource: [ElementAndRendererID], - - syncSelectionFromBuiltinElementsPanel: [], + clearErrorsAndWarnings: {rendererID: RendererID}, + clearErrorsForElementID: ElementAndRendererID, + clearHostInstanceHighlight: void, + clearWarningsForElementID: ElementAndRendererID, + copyElementPath: CopyElementPathParams, + deletePath: DeletePath, + getBackendVersion: void, + getBridgeProtocol: void, + getIfHasUnsupportedRendererVersion: void, + getOwnersList: ElementAndRendererID, + getProfilingData: {rendererID: RendererID}, + getProfilingStatus: void, + highlightHostInstance: HighlightHostInstance, + highlightHostInstances: HighlightHostInstances, + inspectElement: InspectElementParams, + inspectScreen: InspectScreenParams, + logElementToConsole: ElementAndRendererID, + overrideError: OverrideError, + overrideSuspense: OverrideSuspense, + overrideSuspenseMilestone: OverrideSuspenseMilestone, + overrideValueAtPath: OverrideValueAtPath, + profilingData: ProfilingDataBackend, + reloadAndProfile: ReloadAndProfilingParams, + renamePath: RenamePath, + savedPreferences: SavedPreferencesParams, + setTraceUpdatesEnabled: boolean, + shutdown: void, + startInspectingHost: boolean, + startProfiling: StartProfilingParams, + stopInspectingHost: void, + scrollToHostInstance: ScrollToHostInstance, + scrollTo: {left: number, top: number, right: number, bottom: number}, + requestScrollPosition: void, + stopProfiling: void, + storeAsGlobal: StoreAsGlobalParams, + updateComponentFilters: Array, + getEnvironmentNames: void, + updateHookSettings: $ReadOnly, + viewAttributeSource: ViewAttributeSourceParams, + viewElementSource: ElementAndRendererID, + + syncSelectionFromBuiltinElementsPanel: void, // React Native style editor plug-in. - NativeStyleEditor_measure: [ElementAndRendererID], - NativeStyleEditor_renameAttribute: [NativeStyleEditor_RenameAttributeParams], - NativeStyleEditor_setValue: [NativeStyleEditor_SetValueParams], + NativeStyleEditor_measure: ElementAndRendererID, + NativeStyleEditor_renameAttribute: NativeStyleEditor_RenameAttributeParams, + NativeStyleEditor_setValue: NativeStyleEditor_SetValueParams, // Temporarily support newer standalone front-ends sending commands to older embedded backends. // We do this because React Native embeds the React DevTools backend, @@ -297,35 +306,34 @@ export type FrontendEvents = { // Note that this approach does no support the combination of a newer backend with an older frontend. // It would be more work to support both approaches (and not run handlers twice) // so I chose to support the more likely/common scenario (and the one more difficult for an end user to "fix"). - overrideContext: [OverrideValue], - overrideHookState: [OverrideHookState], - overrideProps: [OverrideValue], - overrideState: [OverrideValue], + overrideContext: OverrideValue, + overrideHookState: OverrideHookState, + overrideProps: OverrideValue, + overrideState: OverrideValue, - getHookSettings: [], + getHookSettings: void, }; class Bridge< OutgoingEvents: Object, IncomingEvents: Object, -> extends EventEmitter { +> extends EventEmitter> { _isShutdown: boolean = false; - _messageQueue: Array = []; + _messageQueue: Array = []; _scheduledFlush: boolean = false; _wall: Wall; - _wallUnlisten: Function | null = null; + _wallUnlisten: (() => void) | null = null; constructor(wall: Wall) { super(); this._wall = wall; - this._wallUnlisten = - wall.listen((message: Message) => { - if (message && message.event) { - (this as any).emit(message.event, message.payload); - } - }) || null; + const wallUnlisten = wall.listen(this._handleMessage); + if (typeof wallUnlisten !== 'function') { + throw new TypeError('Wall.listen() must return an unlisten function.'); + } + this._wallUnlisten = wallUnlisten; // Temporarily support older standalone front-ends sending commands to newer embedded backends. // We do this because React Native embeds the React DevTools backend, @@ -339,15 +347,30 @@ class Bridge< return this._wall; } + addListener>>( + event: Event, + listener: (...EventEmitterEvents[Event]) => mixed, + ): void { + this._assertNotShutdown('add a listener'); + super.addListener(event, listener); + } + + emit>>( + event: Event, + ...args: EventEmitterEvents[Event] + ): void { + this._assertNotShutdown('emit an event'); + super.emit(event, ...args); + } + send>( event: EventName, - ...payload: OutgoingEvents[EventName] - ) { - if (this._isShutdown) { - console.warn( - `Cannot send message "${event}" through a Bridge that has been shutdown.`, - ); - return; + payload?: OutgoingEvents[EventName], + ): void { + this._assertNotShutdown('send a message'); + + if (typeof event !== 'string' || event.length === 0) { + throw new TypeError('Bridge event names must be non-empty strings.'); } // When we receive a message: @@ -358,7 +381,10 @@ class Bridge< // - if there *has* been a message flushed in the last BATCH_DURATION ms // (or we're waiting for our setTimeout-0 to fire), then _timeoutID will // be set, and we'll simply add to the queue and wait for that - this._messageQueue.push(event, payload); + this._messageQueue.push({ + event, + payload, + }); if (!this._scheduledFlush) { this._scheduledFlush = true; // $FlowFixMe[cannot-resolve-name] @@ -375,11 +401,8 @@ class Bridge< } } - shutdown() { - if (this._isShutdown) { - console.warn('Bridge was already shutdown.'); - return; - } + shutdown(): void { + this._assertNotShutdown('shut down'); // Queue the shutdown outgoing message for subscribers. this.emit('shutdown'); @@ -388,27 +411,23 @@ class Bridge< // Mark this bridge as destroyed, i.e. disable its public API. this._isShutdown = true; - // Disable the API inherited from EventEmitter that can add more listeners and send more messages. - // $FlowFixMe[cannot-write] This property is not writable. - this.addListener = function () {}; - // $FlowFixMe[cannot-write] This property is not writable. - this.emit = function () {}; - // NOTE: There's also EventEmitter API like `on` and `prependListener` that we didn't add to our Flow type of EventEmitter. - // Unsubscribe this bridge incoming message listeners to be sure, and so they don't have to do that. this.removeAllListeners(); // Stop accepting and emitting incoming messages from the wall. const wallUnlisten = this._wallUnlisten; - if (wallUnlisten) { - wallUnlisten(); + this._wallUnlisten = null; + try { + if (wallUnlisten !== null) { + wallUnlisten(); + } + } finally { + // Synchronously flush all queued outgoing messages. + // At this step the subscribers' code may run in this call stack. + do { + this._flush(); + } while (this._messageQueue.length); } - - // Synchronously flush all queued outgoing messages. - // At this step the subscribers' code may run in this call stack. - do { - this._flush(); - } while (this._messageQueue.length); } _flush: () => void = () => { @@ -417,9 +436,9 @@ class Bridge< // It is a private method that the bridge ensures is only called at the right times. try { if (this._messageQueue.length) { - for (let i = 0; i < this._messageQueue.length; i += 2) { - // This only supports one argument in practice but the types suggests it should support multiple. - this._wall.send(this._messageQueue[i], this._messageQueue[i + 1][0]); + for (let i = 0; i < this._messageQueue.length; i++) { + const {event, payload} = this._messageQueue[i]; + this._wall.send(event, payload); } this._messageQueue.length = 0; } @@ -430,6 +449,35 @@ class Bridge< } }; + _assertNotShutdown(action: string): void { + if (this._isShutdown) { + throw new Error( + `Cannot ${action} through a Bridge that has been shut down.`, + ); + } + } + + _handleMessage: (message: mixed) => void = message => { + // Some Walls share a transport with unrelated messages or legacy DevTools + // protocols. A message without an event field does not belong to this Bridge. + if ( + message === null || + typeof message !== 'object' || + !('event' in message) + ) { + return; + } + + const event = message.event; + if (typeof event !== 'string' || event.length === 0) { + throw new TypeError('Bridge event names must be non-empty strings.'); + } + + this._assertNotShutdown('receive a message'); + // The wire event name cannot be statically refined to a key of IncomingEvents. + (this as any).emit(event, message.payload); + }; + // Temporarily support older standalone backends by forwarding "overrideValueAtPath" commands // to the older message types they may be listening to. overrideValueAtPath: OverrideValueAtPath => void = ({ diff --git a/packages/react-devtools-shared/src/devtools/views/DevTools.js b/packages/react-devtools-shared/src/devtools/views/DevTools.js index 1e36e43c9139..9f3f8a41b04e 100644 --- a/packages/react-devtools-shared/src/devtools/views/DevTools.js +++ b/packages/react-devtools-shared/src/devtools/views/DevTools.js @@ -275,12 +275,8 @@ export default function DevTools({ useLayoutEffect(() => { return () => { - try { - // Shut the Bridge down synchronously (during unmount). - bridge.shutdown(); - } catch (error) { - // Attempting to use a disconnected port. - } + // Shut the Bridge down synchronously (during unmount). + bridge.shutdown(); }; }, [bridge]); diff --git a/packages/react-devtools-shared/src/devtools/views/WarnIfLegacyBackendDetected.js b/packages/react-devtools-shared/src/devtools/views/WarnIfLegacyBackendDetected.js index 13f093d2e762..a3714aa13f4e 100644 --- a/packages/react-devtools-shared/src/devtools/views/WarnIfLegacyBackendDetected.js +++ b/packages/react-devtools-shared/src/devtools/views/WarnIfLegacyBackendDetected.js @@ -22,8 +22,12 @@ export default function WarnIfLegacyBackendDetected(_: {}): null { // We do this by listening to a message that it broadcasts but the v4 backend doesn't. // In this case the frontend should show upgrade instructions. useEffect(() => { - // Wall.listen returns a cleanup function - let unlisten: $FlowFixMe = bridge.wall.listen(message => { + let unlisten: (() => void) | null = null; + unlisten = bridge.wall.listen(message => { + if (message === null || typeof message !== 'object') { + return; + } + switch (message.type) { case 'call': case 'event': @@ -38,7 +42,7 @@ export default function WarnIfLegacyBackendDetected(_: {}): null { }); // Once we've identified the backend version, it's safe to unsubscribe. - if (typeof unlisten === 'function') { + if (unlisten !== null) { unlisten(); unlisten = null; } @@ -54,7 +58,7 @@ export default function WarnIfLegacyBackendDetected(_: {}): null { case 'overrideComponentFilters': // Any of these is sufficient to indicate a v4 backend. // Once we've identified the backend version, it's safe to unsubscribe. - if (typeof unlisten === 'function') { + if (unlisten !== null) { unlisten(); unlisten = null; } @@ -65,7 +69,7 @@ export default function WarnIfLegacyBackendDetected(_: {}): null { }); return () => { - if (typeof unlisten === 'function') { + if (unlisten !== null) { unlisten(); unlisten = null; } diff --git a/packages/react-devtools-shared/src/frontend/types.js b/packages/react-devtools-shared/src/frontend/types.js index 97f8cf073b86..195b520cb061 100644 --- a/packages/react-devtools-shared/src/frontend/types.js +++ b/packages/react-devtools-shared/src/frontend/types.js @@ -23,10 +23,20 @@ import type {UnknownSuspendersReason} from '../constants'; export type BrowserTheme = 'dark' | 'light'; +export type WallMessage = { + event: string, + payload?: mixed, +}; + export type Wall = { - // `listen` returns the "unlisten" function. - listen: (fn: Function) => Function, - send: (event: string, payload: any, transferable?: Array) => void, + // A Wall may share its transport with unrelated or legacy messages, so the + // Bridge must refine incoming values at the boundary. + listen: (fn: (message: mixed) => void) => () => void, + send: ( + event: string, + payload: mixed, + transferable?: $ReadOnlyArray, + ) => void, }; // WARNING diff --git a/packages/react-devtools-shell/src/multi/devtools.js b/packages/react-devtools-shell/src/multi/devtools.js index 0ba45bc6c15f..5549663cf6ad 100644 --- a/packages/react-devtools-shell/src/multi/devtools.js +++ b/packages/react-devtools-shell/src/multi/devtools.js @@ -43,6 +43,12 @@ function init(appIframe, devtoolsContainer, appSource) { } wall._listeners.push(listener); + return () => { + const index = wall._listeners.indexOf(listener); + if (index !== -1) { + wall._listeners.splice(index, 1); + } + }; }, send(event, payload) { if (__DEBUG__) { diff --git a/scripts/flow/react-devtools.js b/scripts/flow/react-devtools.js index ddd925c2443d..a7acfbf3592d 100644 --- a/scripts/flow/react-devtools.js +++ b/scripts/flow/react-devtools.js @@ -58,9 +58,11 @@ interface ExtensionRuntimeSender { interface ExtensionRuntimePort { disconnect(): void; name: string; - onMessage: ExtensionEvent<(message: any, port: ExtensionRuntimePort) => void>; + onMessage: ExtensionEvent< + (message: mixed, port: ExtensionRuntimePort) => void, + >; onDisconnect: ExtensionEvent<(port: ExtensionRuntimePort) => void>; - postMessage(message: mixed, transferable?: Array): void; + postMessage(message: mixed, transferable?: $ReadOnlyArray): void; sender?: ExtensionRuntimeSender; } From d87711f8d0a45e6950fef15183ad6f9ec827b8b0 Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin <28902667+hoxyq@users.noreply.github.com> Date: Thu, 23 Jul 2026 10:39:16 +0100 Subject: [PATCH 4/5] [DevTools] Validate Store operation invariants (#37050) Builds on #37049 by validating Store operation invariants before mutation. Missing nodes, invalid element types, inconsistent parent-child relationships, and invalid reorder operations now emit and throw explicit errors instead of silently continuing with corrupted state. Adds a canonical-render regression test for invalid child removal. --- .../src/__tests__/store-test.js | 70 ++++ .../src/devtools/store.js | 320 +++++++++++------- 2 files changed, 272 insertions(+), 118 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js index 0a62c089dd13..aecdfe93c854 100644 --- a/packages/react-devtools-shared/src/__tests__/store-test.js +++ b/packages/react-devtools-shared/src/__tests__/store-test.js @@ -129,6 +129,76 @@ describe('Store', () => { expect(store).toMatchInlineSnapshot(`[root]`); }); + it('throws when a transition timeline is requested during initial paint', () => { + const errorListener = jest.fn(); + store.addListener('error', errorListener); + + expect(() => + store.getSuspendableDocumentOrderSuspenseTransition(false, 1), + ).toThrow( + 'Cannot get a transition timeline during the initial paint. This is a bug in React DevTools.', + ); + expect(errorListener).toHaveBeenCalledTimes(1); + + store.removeListener('error', errorListener); + }); + + // @reactVersion >= 18.0 + it('throws before removing a node that is not a child of its parent', () => { + function FirstChild() { + return null; + } + function SecondChild() { + return null; + } + function Parent({showFirstChild}) { + return ( + <> + {showFirstChild && } + + + ); + } + + act(() => render()); + + const parent = store.getElementAtIndex(0); + expect(parent.displayName).toBe('Parent'); + const firstChildIndex = parent.children.findIndex(id => { + const child = store.getElementByID(id); + return child !== null && child.displayName === 'FirstChild'; + }); + expect(firstChildIndex).not.toBe(-1); + const firstChildID = parent.children[firstChildIndex]; + + // Corrupt only the frontend relationship. The removal operation below is + // still produced canonically by rendering React. + parent.children.splice(firstChildIndex, 1); + + const errorListener = jest.fn(); + store.addListener('error', errorListener); + let caughtError = null; + try { + act(() => render()); + } catch (error) { + caughtError = error; + } finally { + // The test Bridge invokes listeners synchronously, so discard the batch + // whose Store listener intentionally threw. + bridge._messageQueue.length = 0; + } + + const expectedMessage = + `Cannot remove node "${firstChildID}" from parent "${parent.id}" ` + + `because it is not a child of the parent.`; + expect(caughtError).toMatchObject({message: expectedMessage}); + expect(errorListener).toHaveBeenCalledWith(caughtError); + expect(store.containsElement(firstChildID)).toBe(true); + + parent.children.splice(firstChildIndex, 0, firstChildID); + store.removeListener('error', errorListener); + }); + // This test is not the same cause as what's reported on GitHub, // but the resulting behavior (owner mounting after descendant) is the same. // Thec ase below is admittedly contrived and relies on side effects. diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js index 91b3586b816b..67836086f498 100644 --- a/packages/react-devtools-shared/src/devtools/store.js +++ b/packages/react-devtools-shared/src/devtools/store.js @@ -28,7 +28,20 @@ import { SUSPENSE_TREE_OPERATION_SUSPENDERS, } from '../constants'; import { + ElementTypeClass, + ElementTypeContext, + ElementTypeFunction, + ElementTypeForwardRef, + ElementTypeHostComponent, + ElementTypeMemo, + ElementTypeOtherOrUnknown, + ElementTypeProfiler, ElementTypeRoot, + ElementTypeSuspense, + ElementTypeSuspenseList, + ElementTypeTracingMarker, + ElementTypeVirtual, + ElementTypeViewTransition, ElementTypeActivity, ComponentFilterActivitySlice, } from '../frontend/types'; @@ -140,6 +153,32 @@ function isNonZeroRect(rect: Rect) { return rect.width > 0 || rect.height > 0 || rect.x > 0 || rect.y > 0; } +function parseElementType(value: number): ElementType | null { + // Cast before switching so Flow checks exhaustiveness while the default rejects unknown bridge values. + const type = value as any as ElementType; + switch (type) { + case ElementTypeClass: + case ElementTypeContext: + case ElementTypeFunction: + case ElementTypeForwardRef: + case ElementTypeHostComponent: + case ElementTypeMemo: + case ElementTypeOtherOrUnknown: + case ElementTypeProfiler: + case ElementTypeRoot: + case ElementTypeSuspense: + case ElementTypeSuspenseList: + case ElementTypeTracingMarker: + case ElementTypeVirtual: + case ElementTypeViewTransition: + case ElementTypeActivity: + return type; + default: + (type) as empty; + return null; + } +} + /** * The store is the single source of truth for updates from the backend. * ContextProviders can subscribe to the Store for specific things they want to provide. @@ -366,7 +405,7 @@ export default class Store extends EventEmitter<{ } // This is only used in tests to avoid memory leaks. - assertMapSizeMatchesRootCount(map: Map, mapName: string) { + assertMapSizeMatchesRootCount(map: Map, mapName: string) { const expectedSize = this.roots.length; if (map.size !== expectedSize) { this._throwAndEmitError( @@ -609,13 +648,11 @@ export default class Store extends EventEmitter<{ if (root === undefined) { // We should never reach this. This is a bug in the backend renderer. - this._throwAndEmitError( + return this._throwAndEmitError( Error( `Couldn't find root with id "${rootID}": no matching node was found in the Store.`, ), ); - - return null; } if (root.children.length === 0) { @@ -630,7 +667,9 @@ export default class Store extends EventEmitter<{ } if (root === undefined) { - return null; + return this._throwAndEmitError( + Error(`Could not find an element at index "${index}" in the Store.`), + ); } // Find the element in the tree using the weight of each node... @@ -640,19 +679,18 @@ export default class Store extends EventEmitter<{ while (index !== currentWeight) { const numChildren = currentElement.children.length; + let didFindChild = false; for (let i = 0; i < numChildren; i++) { const childID = currentElement.children[i]; const child = this._idToElement.get(childID); if (child === undefined) { // We should never reach this. This is a bug in the backend renderer. - this._throwAndEmitError( + return this._throwAndEmitError( Error( `Couldn't child element with id "${childID}": no matching node was found in the Store.`, ), ); - - return null; } const childWeight = child.isCollapsed ? 1 : child.weight; @@ -660,14 +698,23 @@ export default class Store extends EventEmitter<{ if (index <= currentWeight + childWeight) { currentWeight++; currentElement = child; + didFindChild = true; break; } else { currentWeight += childWeight; } } + + if (!didFindChild) { + return this._throwAndEmitError( + Error( + `Could not find an element at index "${index}" because the Store tree weights are invalid.`, + ), + ); + } } - return currentElement || null; + return currentElement; } getElementIDAtIndex(index: number): number | null { @@ -685,6 +732,26 @@ export default class Store extends EventEmitter<{ return element; } + _getElementByIDOrThrow(id: Element['id']): Element { + const element = this._idToElement.get(id); + if (element === undefined) { + return this._throwAndEmitError( + Error( + `Could not find element with id "${id}": no matching node was found in the Store.`, + ), + ); + } + return element; + } + + _recalculateWeightAcrossRoots(): void { + let weightAcrossRoots = 0; + this._roots.forEach(rootID => { + weightAcrossRoots += this._getElementByIDOrThrow(rootID).weight; + }); + this._weightAcrossRoots = weightAcrossRoots; + } + containsSuspense(id: SuspenseNode['id']): boolean { return this._idToSuspense.has(id); } @@ -893,8 +960,15 @@ export default class Store extends EventEmitter<{ let depth = 0; while (parentID > 0) { if (parentID === ownerID || unsortedIDs.has(parentID)) { - // $FlowFixMe[unsafe-addition] addition with possible null/undefined value - depth = depthMap.get(parentID) + 1; + const parentDepth = depthMap.get(parentID); + if (parentDepth === undefined) { + return this._throwAndEmitError( + Error( + `Invalid owners list: owner depth for element "${parentID}" was not found.`, + ), + ); + } + depth = parentDepth + 1; depthMap.set(id, depth); break; } @@ -961,14 +1035,13 @@ export default class Store extends EventEmitter<{ let rootStep: null | SuspenseTimelineStep = null; for (let i = 0; i < roots.length; i++) { const rootID = roots[i]; - const root = this.getElementByID(rootID); - if (root === null) { - continue; - } + this._getElementByIDOrThrow(rootID); const rendererID = this._rootIDToRendererID.get(rootID); if (rendererID === undefined) { - throw new Error( - 'Failed to find renderer ID for root. This is a bug in React DevTools.', + return this._throwAndEmitError( + Error( + 'Failed to find renderer ID for root. This is a bug in React DevTools.', + ), ); } // TODO: This includes boundaries that can't be suspended due to no support from the renderer. @@ -1059,9 +1132,12 @@ export default class Store extends EventEmitter<{ ): Array { const target: Array = []; const focusedTransitionID = this._focusedTransition; - // $FlowFixMe[invalid-compare] - if (focusedTransitionID === null) { - return target; + if (focusedTransitionID === 0) { + return this._throwAndEmitError( + Error( + 'Cannot get a transition timeline during the initial paint. This is a bug in React DevTools.', + ), + ); } target.push({ @@ -1155,14 +1231,18 @@ export default class Store extends EventEmitter<{ this._focusedTransition, ); if (focusedTransitionRootID === null) { - throw new Error( - 'Failed to find root ID for focused transition. This is a bug in React DevTools.', + return this._throwAndEmitError( + Error( + 'Failed to find root ID for focused transition. This is a bug in React DevTools.', + ), ); } const rendererID = this._rootIDToRendererID.get(focusedTransitionRootID); if (rendererID === undefined) { - throw new Error( - 'Failed to find renderer ID for focused transition root. This is a bug in React DevTools.', + return this._throwAndEmitError( + Error( + 'Failed to find renderer ID for focused transition root. This is a bug in React DevTools.', + ), ); } timeline = this.getSuspendableDocumentOrderSuspenseTransition( @@ -1313,12 +1393,7 @@ export default class Store extends EventEmitter<{ // Only re-calculate weights and emit an "update" event if the store was mutated. if (didMutate) { - let weightAcrossRoots = 0; - this._roots.forEach(rootID => { - const {weight} = this.getElementByID(rootID) as any as Element; - weightAcrossRoots += weight; - }); - this._weightAcrossRoots = weightAcrossRoots; + this._recalculateWeightAcrossRoots(); // The Tree context's search reducer expects an explicit list of ids for nodes that were added or removed. // In this case, we can pass it empty arrays since nodes in a collapsed tree are still there (just hidden). @@ -1428,13 +1503,22 @@ export default class Store extends EventEmitter<{ switch (operation) { case TREE_OPERATION_ADD: { const id = operations[i + 1]; - const type = operations[i + 2] as any as ElementType; + const rawType = operations[i + 2]; + const type = parseElementType(rawType); + + if (type === null) { + return this._throwAndEmitError( + Error( + `Cannot add node "${id}" because "${rawType}" is not a valid element type.`, + ), + ); + } i += 3; if (this._idToElement.has(id)) { // We should never reach this. This is a bug in the backend renderer. - this._throwAndEmitError( + return this._throwAndEmitError( Error( `Cannot add node "${id}" because a node with that id is already in the Store.`, ), @@ -1545,13 +1629,11 @@ export default class Store extends EventEmitter<{ const parentElement = this._idToElement.get(parentID); if (parentElement === undefined) { // We should never reach this. This is a bug in the backend renderer. - this._throwAndEmitError( + return this._throwAndEmitError( Error( `Cannot add child "${id}" to parent "${parentID}" because parent node was not found in the Store.`, ), ); - - break; } parentElement.children.push(id); @@ -1622,13 +1704,11 @@ export default class Store extends EventEmitter<{ if (element === undefined) { // We should never reach this. This is a bug in the backend renderer. - this._throwAndEmitError( + return this._throwAndEmitError( Error( `Cannot remove node "${id}" because no matching node was found in the Store.`, ), ); - - break; } i += 1; @@ -1636,13 +1716,11 @@ export default class Store extends EventEmitter<{ const {children, ownerID, parentID, weight} = element; if (children.length > 0) { // We should never reach this. This is a bug in the backend renderer. - this._throwAndEmitError( + return this._throwAndEmitError( Error(`Node "${id}" was removed before its children.`), ); } - this._idToElement.delete(id); - let parentElement: ?Element = null; if (parentID === 0) { // $FlowFixMe[constant-condition] @@ -1664,19 +1742,26 @@ export default class Store extends EventEmitter<{ parentElement = this._idToElement.get(parentID); if (parentElement === undefined) { // We should never reach this. This is a bug in the backend renderer. - this._throwAndEmitError( + return this._throwAndEmitError( Error( `Cannot remove node "${id}" from parent "${parentID}" because no matching node was found in the Store.`, ), ); - - break; } const index = parentElement.children.indexOf(id); + if (index === -1) { + return this._throwAndEmitError( + Error( + `Cannot remove node "${id}" from parent "${parentID}" because it is not a child of the parent.`, + ), + ); + } parentElement.children.splice(index, 1); } + this._idToElement.delete(id); + this._adjustParentTreeWeight(parentElement, -weight); removedElementIDs.set(id, parentID); @@ -1704,37 +1789,42 @@ export default class Store extends EventEmitter<{ const element = this._idToElement.get(id); if (element === undefined) { // We should never reach this. This is a bug in the backend renderer. - this._throwAndEmitError( + return this._throwAndEmitError( Error( `Cannot reorder children for node "${id}" because no matching node was found in the Store.`, ), ); - - break; } const children = element.children; if (children.length !== numChildren) { // We should never reach this. This is a bug in the backend renderer. - this._throwAndEmitError( + return this._throwAndEmitError( Error( `Children cannot be added or removed during a reorder operation.`, ), ); } + const reorderedChildIDs: Set = new Set(); for (let j = 0; j < numChildren; j++) { const childID = operations[i + j]; - children[j] = childID; - if (__DEV__) { - // This check is more expensive so it's gated by __DEV__. - const childElement = this._idToElement.get(childID); - if (childElement == null || childElement.parentID !== id) { - console.error( + const childElement = this._idToElement.get(childID); + if ( + childElement === undefined || + childElement.parentID !== id || + reorderedChildIDs.has(childID) + ) { + return this._throwAndEmitError( + Error( `Children cannot be added or removed during a reorder operation.`, - ); - } + ), + ); } + reorderedChildIDs.add(childID); + } + for (let j = 0; j < numChildren; j++) { + children[j] = operations[i + j]; } i += numChildren; @@ -1829,12 +1919,12 @@ export default class Store extends EventEmitter<{ const parentID = operations[i + 2]; const nameStringID = operations[i + 3]; const isSuspended = operations[i + 4] === 1; - const numRects = operations[i + 5] as any as number; + const numRects = operations[i + 5]; let name = stringTable[nameStringID]; if (this._idToSuspense.has(id)) { // We should never reach this. This is a bug in the backend renderer. - this._throwAndEmitError( + return this._throwAndEmitError( Error( `Cannot add suspense node "${id}" because a suspense node with that id is already in the Store.`, ), @@ -1887,13 +1977,11 @@ export default class Store extends EventEmitter<{ const parentSuspense = this._idToSuspense.get(parentID); if (parentSuspense === undefined) { // We should never reach this. This is a bug in the backend renderer. - this._throwAndEmitError( + return this._throwAndEmitError( Error( `Cannot add suspense child "${id}" to parent suspense "${parentID}" because parent suspense node was not found in the Store.`, ), ); - - break; } parentSuspense.children.push(id); @@ -1924,13 +2012,11 @@ export default class Store extends EventEmitter<{ if (suspense === undefined) { // We should never reach this. This is a bug in the backend renderer. - this._throwAndEmitError( + return this._throwAndEmitError( Error( `Cannot remove suspense node "${id}" because no matching node was found in the Store.`, ), ); - - break; } i += 1; @@ -1938,11 +2024,33 @@ export default class Store extends EventEmitter<{ const {children, parentID, rects} = suspense; if (children.length > 0) { // We should never reach this. This is a bug in the backend renderer. - this._throwAndEmitError( + return this._throwAndEmitError( Error(`Suspense node "${id}" was removed before its children.`), ); } + let parentSuspense: SuspenseNode | null = null; + let parentIndex = -1; + if (parentID !== 0) { + parentSuspense = this._idToSuspense.get(parentID) || null; + if (parentSuspense === null) { + return this._throwAndEmitError( + Error( + `Cannot remove suspense node "${id}" from parent "${parentID}" because no matching node was found in the Store.`, + ), + ); + } + + parentIndex = parentSuspense.children.indexOf(id); + if (parentIndex === -1) { + return this._throwAndEmitError( + Error( + `Cannot remove suspense node "${id}" from parent "${parentID}" because it is not a child of the parent.`, + ), + ); + } + } + if (rects !== null && parentID !== 0) { // Delete all the existing rects from the R-tree for (let j = 0; j < rects.length; j++) { @@ -1953,8 +2061,7 @@ export default class Store extends EventEmitter<{ this._idToSuspense.delete(id); removedSuspenseIDs.set(id, parentID); - let parentSuspense: ?SuspenseNode = null; - if (parentID === 0) { + if (parentSuspense === null) { // $FlowFixMe[constant-condition] if (__DEBUG__) { debug('Suspense remove', `node ${id} root`); @@ -1965,28 +2072,7 @@ export default class Store extends EventEmitter<{ debug('Suspense Remove', `node ${id} from parent ${parentID}`); } - parentSuspense = this._idToSuspense.get(parentID); - if (parentSuspense === undefined) { - // We should never reach this. This is a bug in the backend renderer. - this._throwAndEmitError( - Error( - `Cannot remove suspense node "${id}" from parent "${parentID}" because no matching node was found in the Store.`, - ), - ); - - break; - } - - const index = parentSuspense.children.indexOf(id); - if (index === -1) { - // We should never reach this. This is a bug in the backend renderer. - this._throwAndEmitError( - Error( - `Cannot remove suspense node "${id}" from parent "${parentID}" because it is not a child of the parent.`, - ), - ); - } - parentSuspense.children.splice(index, 1); + parentSuspense.children.splice(parentIndex, 1); } } @@ -2001,37 +2087,42 @@ export default class Store extends EventEmitter<{ const suspense = this._idToSuspense.get(id); if (suspense === undefined) { // We should never reach this. This is a bug in the backend renderer. - this._throwAndEmitError( + return this._throwAndEmitError( Error( `Cannot reorder children for suspense node "${id}" because no matching node was found in the Store.`, ), ); - - break; } const children = suspense.children; if (children.length !== numChildren) { // We should never reach this. This is a bug in the backend renderer. - this._throwAndEmitError( + return this._throwAndEmitError( Error( `Suspense children cannot be added or removed during a reorder operation.`, ), ); } + const reorderedChildIDs: Set = new Set(); for (let j = 0; j < numChildren; j++) { const childID = operations[i + j]; - children[j] = childID; - if (__DEV__) { - // This check is more expensive so it's gated by __DEV__. - const childSuspense = this._idToSuspense.get(childID); - if (childSuspense == null || childSuspense.parentID !== id) { - console.error( + const childSuspense = this._idToSuspense.get(childID); + if ( + childSuspense === undefined || + childSuspense.parentID !== id || + reorderedChildIDs.has(childID) + ) { + return this._throwAndEmitError( + Error( `Suspense children cannot be added or removed during a reorder operation.`, - ); - } + ), + ); } + reorderedChildIDs.add(childID); + } + for (let j = 0; j < numChildren; j++) { + children[j] = operations[i + j]; } i += numChildren; @@ -2047,20 +2138,18 @@ export default class Store extends EventEmitter<{ break; } case SUSPENSE_TREE_OPERATION_RESIZE: { - const id = operations[i + 1] as any as number; - const numRects = operations[i + 2] as any as number; + const id = operations[i + 1]; + const numRects = operations[i + 2]; i += 3; const suspense = this._idToSuspense.get(id); if (suspense === undefined) { // We should never reach this. This is a bug in the backend renderer. - this._throwAndEmitError( + return this._throwAndEmitError( Error( `Cannot set rects for suspense node "${id}" because no matching node was found in the Store.`, ), ); - - break; } const prevRects = suspense.rects; @@ -2142,13 +2231,11 @@ export default class Store extends EventEmitter<{ if (suspense === undefined) { // We should never reach this. This is a bug in the backend renderer. - this._throwAndEmitError( + return this._throwAndEmitError( Error( `Cannot update suspenders of suspense node "${id}" because no matching node was found in the Store.`, ), ); - - break; } // $FlowFixMe[constant-condition] @@ -2178,7 +2265,7 @@ export default class Store extends EventEmitter<{ break; } default: - this._throwAndEmitError( + return this._throwAndEmitError( new UnsupportedBridgeOperationError( `Unsupported Bridge operation "${operation}"`, ), @@ -2283,7 +2370,9 @@ export default class Store extends EventEmitter<{ // the Activities that are descendants of the next Activity slice. const nextActivitySlice = this._idToElement.get(nextActivitySliceID); if (nextActivitySlice === undefined) { - throw new Error('Next Activity slice not found in Store.'); + return this._throwAndEmitError( + Error('Next Activity slice not found in Store.'), + ); } for (let j = 0; j < nextActivitySlice.children.length; j++) { @@ -2293,12 +2382,7 @@ export default class Store extends EventEmitter<{ } if (didCollapse) { - let weightAcrossRoots = 0; - this._roots.forEach(rootID => { - const {weight} = this.getElementByID(rootID) as any as Element; - weightAcrossRoots += weight; - }); - this._weightAcrossRoots = weightAcrossRoots; + this._recalculateWeightAcrossRoots(); } } @@ -2329,7 +2413,7 @@ export default class Store extends EventEmitter<{ let didMutate = false; const element = this._idToElement.get(elementID); if (element === undefined) { - throw new Error('Element not found in Store.'); + return this._throwAndEmitError(Error('Element not found in Store.')); } if (element.type === ElementTypeActivity) { From 28cd4bb08f1b66808bede284fca978cc9b065154 Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin <28902667+hoxyq@users.noreply.github.com> Date: Thu, 23 Jul 2026 10:39:16 +0100 Subject: [PATCH 5/5] [DevTools] Buffer Bridge messages during extension reconnects (#37075) Buffers Bridge messages during extension port reconnects and adds a readiness handshake for ordered queue flushing. Includes regression coverage for reconnect delivery and listener cleanup. Potential scenario could be a long user session, where Chrome kills one of the extension ports to save resources and then user re-connects by navigating back to the DevTools UI. --- .../src/background/index.js | 40 ++++++ .../src/constants.js | 45 ++++++ .../src/contentScripts/proxy.js | 103 ++++++++++++-- .../src/main/index.js | 128 +++++++++++++++--- .../src/__tests__/setupTests.js | 61 ++++++++- .../src/__tests__/store-test.js | 52 +++++++ 6 files changed, 393 insertions(+), 36 deletions(-) create mode 100644 packages/react-devtools-extensions/src/constants.js diff --git a/packages/react-devtools-extensions/src/background/index.js b/packages/react-devtools-extensions/src/background/index.js index 9c2fc8ab84a8..641ae605b5e9 100644 --- a/packages/react-devtools-extensions/src/background/index.js +++ b/packages/react-devtools-extensions/src/background/index.js @@ -19,6 +19,11 @@ import { handleReactDevToolsHookMessage, handleFetchResourceContentScriptMessage, } from './messageHandlers'; +import { + EXTENSION_BRIDGE_CONNECTION_DISCONNECTED, + EXTENSION_BRIDGE_CONNECTION_READY, +} from '../constants'; +import type {ExtensionBridgeConnectionType} from '../constants'; const ports: { // TODO: Check why we convert tab IDs to strings, and if we can avoid it @@ -156,6 +161,20 @@ function connectExtensionAndProxyPorts( } const proxyPort = maybeProxyPort; + function sendBridgeConnectionMessage( + port: ExtensionRuntimePort, + type: ExtensionBridgeConnectionType, + ) { + try { + port.postMessage({ + source: 'react-devtools-background', + payload: {type}, + }); + } catch (error) { + // The port disconnected before the status update could be delivered. + } + } + // $FlowFixMe[incompatible-type] if (ports[tabId].disconnectPipe) { throw new Error( @@ -163,6 +182,8 @@ function connectExtensionAndProxyPorts( ); } + let didDisconnect = false; + function extensionPortMessageListener(message: mixed) { try { proxyPort.postMessage(message); @@ -188,9 +209,23 @@ function connectExtensionAndProxyPorts( } function disconnectListener() { + if (didDisconnect) { + return; + } + didDisconnect = true; + extensionPort.onMessage.removeListener(extensionPortMessageListener); proxyPort.onMessage.removeListener(proxyPortMessageListener); + sendBridgeConnectionMessage( + extensionPort, + EXTENSION_BRIDGE_CONNECTION_DISCONNECTED, + ); + sendBridgeConnectionMessage( + proxyPort, + EXTENSION_BRIDGE_CONNECTION_DISCONNECTED, + ); + // We handle disconnect() calls manually, based on each specific case // No need to disconnect other port here @@ -205,6 +240,11 @@ function connectExtensionAndProxyPorts( extensionPort.onDisconnect.addListener(disconnectListener); proxyPort.onDisconnect.addListener(disconnectListener); + + // The proxy owns the backend message queue. Once both forwarding listeners + // are installed, tell it to flush that queue through this pipe. It echoes the + // message to the frontend after the queued backend messages have been sent. + sendBridgeConnectionMessage(proxyPort, EXTENSION_BRIDGE_CONNECTION_READY); } chrome.runtime.onMessage.addListener((message, sender) => { diff --git a/packages/react-devtools-extensions/src/constants.js b/packages/react-devtools-extensions/src/constants.js new file mode 100644 index 000000000000..901e9e3ed755 --- /dev/null +++ b/packages/react-devtools-extensions/src/constants.js @@ -0,0 +1,45 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +export type ExtensionBridgeConnectionType = + | 'react-devtools-extension-bridge-connection-ready' + | 'react-devtools-extension-bridge-connection-disconnected'; + +export const EXTENSION_BRIDGE_CONNECTION_READY: ExtensionBridgeConnectionType = + 'react-devtools-extension-bridge-connection-ready'; +export const EXTENSION_BRIDGE_CONNECTION_DISCONNECTED: ExtensionBridgeConnectionType = + 'react-devtools-extension-bridge-connection-disconnected'; + +export function getExtensionBridgeConnectionType( + message: mixed, +): ExtensionBridgeConnectionType | null { + if ( + message === null || + typeof message !== 'object' || + !('source' in message) || + message.source !== 'react-devtools-background' || + !('payload' in message) + ) { + return null; + } + + const payload = message.payload; + if (payload === null || typeof payload !== 'object' || !('type' in payload)) { + return null; + } + + const type = payload.type; + if (type === EXTENSION_BRIDGE_CONNECTION_READY) { + return EXTENSION_BRIDGE_CONNECTION_READY; + } + if (type === EXTENSION_BRIDGE_CONNECTION_DISCONNECTED) { + return EXTENSION_BRIDGE_CONNECTION_DISCONNECTED; + } + return null; +} diff --git a/packages/react-devtools-extensions/src/contentScripts/proxy.js b/packages/react-devtools-extensions/src/contentScripts/proxy.js index 637e52c8cc63..99e4080a3445 100644 --- a/packages/react-devtools-extensions/src/contentScripts/proxy.js +++ b/packages/react-devtools-extensions/src/contentScripts/proxy.js @@ -6,16 +6,23 @@ * * @flow */ -/* global chrome */ +/* global chrome, ExtensionRuntimePort */ 'use strict'; +import { + EXTENSION_BRIDGE_CONNECTION_DISCONNECTED, + EXTENSION_BRIDGE_CONNECTION_READY, + getExtensionBridgeConnectionType, +} from '../constants'; + function injectProxy() { // Firefox's behaviour for injecting this content script can be unpredictable // While navigating the history, some content scripts might not be re-injected and still be alive if (!window.__REACT_DEVTOOLS_PROXY_INJECTED__) { window.__REACT_DEVTOOLS_PROXY_INJECTED__ = true; + listenToMessagesFromBackend(); connectPort(); sayHelloToBackendManager(); @@ -58,8 +65,42 @@ window.addEventListener('pagehide', function ({target}) { delete window.__REACT_DEVTOOLS_PROXY_INJECTED__; }); -let port = null; +let port: ExtensionRuntimePort | null = null; let backendInitialized: boolean = false; +let isBridgeConnected: boolean = false; +let isListeningToMessagesFromBackend: boolean = false; +const pendingMessages: Array = []; + +function listenToMessagesFromBackend() { + if (!isListeningToMessagesFromBackend) { + window.addEventListener('message', handleMessageFromPage); + isListeningToMessagesFromBackend = true; + } +} + +function flushPendingMessages(): boolean { + const currentPort = port; + if (!isBridgeConnected || currentPort === null) { + return false; + } + + let sentCount = 0; + while (sentCount < pendingMessages.length) { + try { + currentPort.postMessage(pendingMessages[sentCount]); + sentCount++; + } catch (error) { + isBridgeConnected = false; + break; + } + } + + if (sentCount > 0) { + pendingMessages.splice(0, sentCount); + } + + return isBridgeConnected && pendingMessages.length === 0; +} function sayHelloToBackendManager() { window.postMessage( @@ -71,7 +112,37 @@ function sayHelloToBackendManager() { ); } -function handleMessageFromDevtools(message: mixed) { +function handleMessageFromDevtools( + sourcePort: ExtensionRuntimePort, + message: mixed, +) { + if (port !== sourcePort) { + return; + } + + switch (getExtensionBridgeConnectionType(message)) { + case EXTENSION_BRIDGE_CONNECTION_READY: + isBridgeConnected = true; + if (flushPendingMessages()) { + const currentPort = port; + if (currentPort === null) { + // The port may disconnect synchronously while its queue is flushed. + return; + } + try { + // This travels through the forwarding pipe after all queued backend + // messages, so the frontend can safely flush its command queue. + currentPort.postMessage(message); + } catch (error) { + isBridgeConnected = false; + } + } + return; + case EXTENSION_BRIDGE_CONNECTION_DISCONNECTED: + isBridgeConnected = false; + return; + } + window.postMessage( { source: 'react-devtools-content-script', @@ -91,8 +162,8 @@ function handleMessageFromPage(event: any) { case 'react-devtools-bridge': { backendInitialized = true; - // $FlowFixMe[incompatible-use] - port.postMessage(event.data.payload); + pendingMessages.push(event.data.payload); + flushPendingMessages(); break; } @@ -110,8 +181,12 @@ function handleMessageFromPage(event: any) { } } -function handleDisconnect() { - window.removeEventListener('message', handleMessageFromPage); +function handleDisconnect(disconnectedPort: ExtensionRuntimePort) { + if (port !== disconnectedPort) { + return; + } + + isBridgeConnected = false; port = null; // Mirrors the guard in handlePageShow(): the background script can evict/ @@ -131,16 +206,18 @@ function handleDisconnect() { // Creates port from application page to the React DevTools' service worker // Which then connects it with extension port function connectPort() { - port = chrome.runtime.connect({ + isBridgeConnected = false; + const nextPort = chrome.runtime.connect({ name: 'proxy', }); + port = nextPort; - window.addEventListener('message', handleMessageFromPage); + listenToMessagesFromBackend(); - // $FlowFixMe[incompatible-use] - port.onMessage.addListener(handleMessageFromDevtools); - // $FlowFixMe[incompatible-use] - port.onDisconnect.addListener(handleDisconnect); + nextPort.onMessage.addListener(message => + handleMessageFromDevtools(nextPort, message), + ); + nextPort.onDisconnect.addListener(() => handleDisconnect(nextPort)); } let evalRequestId = 0; diff --git a/packages/react-devtools-extensions/src/main/index.js b/packages/react-devtools-extensions/src/main/index.js index b5434098bbf8..d8373090366f 100644 --- a/packages/react-devtools-extensions/src/main/index.js +++ b/packages/react-devtools-extensions/src/main/index.js @@ -48,6 +48,11 @@ import injectBackendManager from './injectBackendManager'; import registerEventsLogger from './registerEventsLogger'; import getProfilingFlags from './getProfilingFlags'; import debounce from './debounce'; +import { + EXTENSION_BRIDGE_CONNECTION_DISCONNECTED, + EXTENSION_BRIDGE_CONNECTION_READY, + getExtensionBridgeConnectionType, +} from '../constants'; import './requestAnimationFramePolyfill'; const resolvedParseHookNames = Promise.resolve(parseHookNames); @@ -56,24 +61,98 @@ const resolvedParseHookNames = Promise.resolve(parseHookNames); // wrapper around calling the worker. const hookNamesModuleLoaderFunction = () => resolvedParseHookNames; +type PendingBridgeMessage = { + event: string, + payload: mixed, + transferable?: $ReadOnlyArray, +}; + +function flushPendingBridgeMessages(): void { + const currentPort = port; + if (!isBridgeConnected || currentPort === null) { + return; + } + + let sentCount = 0; + while (sentCount < pendingBridgeMessages.length) { + const {event, payload, transferable} = pendingBridgeMessages[sentCount]; + try { + currentPort.postMessage({event, payload}, transferable); + sentCount++; + } catch (error) { + isBridgeConnected = false; + break; + } + } + + if (sentCount > 0) { + pendingBridgeMessages.splice(0, sentCount); + } +} + +function handleBridgeConnectionMessage(message: mixed): void { + switch (getExtensionBridgeConnectionType(message)) { + case EXTENSION_BRIDGE_CONNECTION_READY: + isBridgeConnected = true; + flushPendingBridgeMessages(); + break; + case EXTENSION_BRIDGE_CONNECTION_DISCONNECTED: + isBridgeConnected = false; + break; + } +} + +function removeBridgePortListener(): void { + if (subscribedBridgePort !== null && bridgePortListener !== null) { + subscribedBridgePort.onMessage.removeListener(bridgePortListener); + } + subscribedBridgePort = null; + bridgePortListener = null; +} + +function addBridgePortListener(nextPort: ExtensionRuntimePort): void { + const bridgeListener = lastSubscribedBridgeListener; + if (bridgeListener === null) { + return; + } + + removeBridgePortListener(); + + const nextBridgePortListener = (message: mixed) => { + if (port === nextPort) { + bridgeListener(message); + } + }; + nextPort.onMessage.addListener(nextBridgePortListener); + subscribedBridgePort = nextPort; + bridgePortListener = nextBridgePortListener; +} + function createBridge() { bridge = new Bridge({ listen(fn) { - const bridgeListener = (message: mixed) => fn(message); - // Store the reference so that we unsubscribe from the same object. - const portOnMessage = port.onMessage; - portOnMessage.addListener(bridgeListener); + const currentPort = port; + if (currentPort === null) { + throw new Error('DevTools port is not connected.'); + } + if (lastSubscribedBridgeListener !== null) { + throw new Error('The Bridge already has a Wall listener.'); + } - lastSubscribedBridgeListener = bridgeListener; + lastSubscribedBridgeListener = fn; + addBridgePortListener(currentPort); return () => { - port?.onMessage.removeListener(bridgeListener); - lastSubscribedBridgeListener = null; + if (lastSubscribedBridgeListener === fn) { + lastSubscribedBridgeListener = null; + removeBridgePortListener(); + } }; }, send(event: string, payload: mixed, transferable?: $ReadOnlyArray) { - port?.postMessage({event, payload}, transferable); + pendingBridgeMessages.push({event, payload, transferable}); + flushPendingBridgeMessages(); }, }); @@ -490,6 +569,7 @@ function performInTabNavigationCleanup() { bridge = null as $FlowFixMe; render = null as $FlowFixMe; root = null as $FlowFixMe; + pendingBridgeMessages.length = 0; } function performFullCleanup() { @@ -517,9 +597,10 @@ function performFullCleanup() { store = null as $FlowFixMe; bridge = null as $FlowFixMe; render = null as $FlowFixMe; + pendingBridgeMessages.length = 0; port?.disconnect(); - port = null as $FlowFixMe; + port = null; } function connectExtensionPort(): void { @@ -528,25 +609,34 @@ function connectExtensionPort(): void { } const tabId = chrome.devtools.inspectedWindow.tabId; - port = chrome.runtime.connect({ + isBridgeConnected = false; + const nextPort = chrome.runtime.connect({ name: String(tabId), }); + port = nextPort; + nextPort.onMessage.addListener(message => { + if (port === nextPort) { + handleBridgeConnectionMessage(message); + } + }); // If DevTools port was reconnected and Bridge was already created // We should subscribe bridge to this port events // This could happen if service worker dies and all ports are disconnected, // but later user continues the session and Chrome reconnects all ports // Bridge object is still in-memory, though - if (lastSubscribedBridgeListener) { - port.onMessage.addListener(lastSubscribedBridgeListener); - } + addBridgePortListener(nextPort); // This port may be disconnected by Chrome at some point, this callback // will be executed only if this port was disconnected from the other end // so, when we call `port.disconnect()` from this script, // this should not trigger this callback and port reconnection - port.onDisconnect.addListener(() => { - port = null as $FlowFixMe; + nextPort.onDisconnect.addListener(() => { + if (port !== nextPort) { + return; + } + isBridgeConnected = false; + port = null; connectExtensionPort(); }); } @@ -600,7 +690,9 @@ function mountReactDevToolsWhenReactHasLoaded() { } let bridge: FrontendBridge = null as $FlowFixMe; -let lastSubscribedBridgeListener = null; +let lastSubscribedBridgeListener: ((message: mixed) => void) | null = null; +let subscribedBridgePort: ExtensionRuntimePort | null = null; +let bridgePortListener: ((message: mixed) => void) | null = null; let store: Store = null as $FlowFixMe; let profilingData = null; @@ -622,7 +714,9 @@ let root: RootType = null as $FlowFixMe; let currentSelectedSource: null | SourceSelection = null; -let port: ExtensionRuntimePort = null as $FlowFixMe; +let port: ExtensionRuntimePort | null = null; +let isBridgeConnected: boolean = false; +const pendingBridgeMessages: Array = []; // In case when multiple navigation events emitted in a short period of time // This debounced callback primarily used to avoid mounting React DevTools multiple times, which results diff --git a/packages/react-devtools-shared/src/__tests__/setupTests.js b/packages/react-devtools-shared/src/__tests__/setupTests.js index 197e76c9e2ea..085390814f52 100644 --- a/packages/react-devtools-shared/src/__tests__/setupTests.js +++ b/packages/react-devtools-shared/src/__tests__/setupTests.js @@ -14,6 +14,18 @@ import type { FrontendBridge, } from 'react-devtools-shared/src/bridge'; +type TestBridgeMessage = {event: string, payload: mixed}; +type TestBridgeWall = { + disconnect: () => void, + reconnect: () => void, + listen: (callback: (message: mixed) => void) => () => void, + send: ( + event: string, + payload: mixed, + transferable?: $ReadOnlyArray, + ) => void, +}; + const {getTestFlags} = require('../../../../scripts/jest/TestFlags'); // Argument is serialized when passed from jest-cli script through to setupTests. @@ -247,21 +259,58 @@ beforeEach(() => { disableSecondConsoleLogDimmingInStrictMode: false, }); - const bridgeListeners = []; - const bridge = new Bridge({ + let bridgeListeners: Array<(message: mixed) => void> = []; + let disconnectedBridgeListeners: Array<(message: mixed) => void> | null = + null; + let pendingBridgeMessages: Array = []; + const bridgeWall: TestBridgeWall = { + disconnect() { + if (disconnectedBridgeListeners === null) { + disconnectedBridgeListeners = bridgeListeners; + bridgeListeners = []; + } + }, + reconnect() { + if (disconnectedBridgeListeners !== null) { + bridgeListeners = disconnectedBridgeListeners; + disconnectedBridgeListeners = null; + + const messages = pendingBridgeMessages; + pendingBridgeMessages = []; + messages.forEach(message => { + bridgeListeners.forEach(callback => callback(message)); + }); + } + }, listen(callback) { - bridgeListeners.push(callback); + const listeners = + disconnectedBridgeListeners !== null + ? disconnectedBridgeListeners + : bridgeListeners; + listeners.push(callback); return () => { - const index = bridgeListeners.indexOf(callback); + let index = bridgeListeners.indexOf(callback); if (index >= 0) { bridgeListeners.splice(index, 1); } + if (disconnectedBridgeListeners !== null) { + index = disconnectedBridgeListeners.indexOf(callback); + if (index >= 0) { + disconnectedBridgeListeners.splice(index, 1); + } + } }; }, send(event: string, payload: mixed, transferable?: $ReadOnlyArray) { - bridgeListeners.forEach(callback => callback({event, payload})); + const message = {event, payload}; + if (disconnectedBridgeListeners === null) { + bridgeListeners.forEach(callback => callback(message)); + } else { + pendingBridgeMessages.push(message); + } }, - }); + }; + const bridge = new Bridge(bridgeWall); const store = new Store(((bridge: any): FrontendBridge), { supportsTimeline: true, diff --git a/packages/react-devtools-shared/src/__tests__/store-test.js b/packages/react-devtools-shared/src/__tests__/store-test.js index aecdfe93c854..aef5ed14b9be 100644 --- a/packages/react-devtools-shared/src/__tests__/store-test.js +++ b/packages/react-devtools-shared/src/__tests__/store-test.js @@ -199,6 +199,58 @@ describe('Store', () => { store.removeListener('error', errorListener); }); + // @reactVersion >= 18.0 + it('receives operations queued while the frontend transport reconnects', () => { + const App = ({children}) => children ?? null; + const Parent = ({children}) => children ?? null; + const Child = () => null; + + act(() => render()); + + const bridgeWall = (bridge.wall: any); + + bridgeWall.disconnect(); + try { + act(() => + render( + + + , + ), + ); + + expect(store).toMatchInlineSnapshot(` + [root] + + `); + } finally { + bridgeWall.reconnect(); + } + + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + + `); + + act(() => + render( + + + + + , + ), + ); + + expect(store).toMatchInlineSnapshot(` + [root] + ▾ + ▾ + + `); + }); + // This test is not the same cause as what's reported on GitHub, // but the resulting behavior (owner mounting after descendant) is the same. // Thec ase below is admittedly contrived and relies on side effects.