From c52f355e63608d6f724f093d21d871b840e62b96 Mon Sep 17 00:00:00 2001 From: Cedric van Putten Date: Sat, 27 May 2023 21:10:59 +0200 Subject: [PATCH 1/6] Allow string device ids in inspector proxy --- packages/metro-inspector-proxy/src/Device.js | 2 +- packages/metro-inspector-proxy/src/InspectorProxy.js | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/metro-inspector-proxy/src/Device.js b/packages/metro-inspector-proxy/src/Device.js index cffa4f345d..48a30faa1d 100644 --- a/packages/metro-inspector-proxy/src/Device.js +++ b/packages/metro-inspector-proxy/src/Device.js @@ -55,7 +55,7 @@ const REACT_NATIVE_RELOADABLE_PAGE_ID = '-1'; */ class Device { // ID of the device. - _id: number; + _id: string; // Name of the device. _name: string; diff --git a/packages/metro-inspector-proxy/src/InspectorProxy.js b/packages/metro-inspector-proxy/src/InspectorProxy.js index 96d4f0b19c..7373d17fbf 100644 --- a/packages/metro-inspector-proxy/src/InspectorProxy.js +++ b/packages/metro-inspector-proxy/src/InspectorProxy.js @@ -41,7 +41,7 @@ class InspectorProxy { _projectRoot: string; // Maps device ID to Device instance. - _devices: Map; + _devices: Map; // Internal counter for device IDs -- just gets incremented for each new device. _deviceCounter: number = 0; @@ -111,7 +111,7 @@ class InspectorProxy { // Converts page information received from device into PageDescription object // that is sent to debugger. _buildPageDescription( - deviceId: number, + deviceId: string, device: Device, page: Page, ): PageDescription { @@ -165,9 +165,9 @@ class InspectorProxy { const query = url.parse(req.url || '', true).query || {}; const deviceName = query.name || 'Unknown'; const appName = query.app || 'Unknown'; - const deviceId = this._deviceCounter++; + const deviceId = String(this._deviceCounter++); this._devices.set( - deviceId, + SdeviceId, new Device(deviceId, deviceName, appName, socket, this._projectRoot), ); @@ -206,7 +206,7 @@ class InspectorProxy { throw new Error('Incorrect URL - must provide device and page IDs'); } - const device = this._devices.get(parseInt(deviceId, 10)); + const device = this._devices.get(deviceId); if (device == null) { throw new Error('Unknown device with ID ' + deviceId); } From f2ca178b18ba9a168a43668635607d31a9c15cde Mon Sep 17 00:00:00 2001 From: Cedric van Putten Date: Sat, 27 May 2023 21:54:39 +0200 Subject: [PATCH 2/6] Allow device identifiers when registering inspectable device --- .../src/InspectorProxy.js | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/packages/metro-inspector-proxy/src/InspectorProxy.js b/packages/metro-inspector-proxy/src/InspectorProxy.js index 7373d17fbf..dc59a192be 100644 --- a/packages/metro-inspector-proxy/src/InspectorProxy.js +++ b/packages/metro-inspector-proxy/src/InspectorProxy.js @@ -162,16 +162,28 @@ class InspectorProxy { // $FlowFixMe[value-as-type] wss.on('connection', async (socket: WS, req) => { try { + const fallbackDeviceId = String(this._deviceCounter++); + const query = url.parse(req.url || '', true).query || {}; + const deviceId = query.device || fallbackDeviceId; const deviceName = query.name || 'Unknown'; const appName = query.app || 'Unknown'; - const deviceId = String(this._deviceCounter++); + + const oldDevice = this._devices.get(deviceId); + if (oldDevice) { + // Keep the debugger connection alive when disconnecting the device, if possible + if (oldDevice._name === deviceName && oldDevice._app === appName) { + oldDevice._debuggerConnection = null; + } + oldDevice._deviceSocket.close(); + } + this._devices.set( - SdeviceId, + deviceId, new Device(deviceId, deviceName, appName, socket, this._projectRoot), ); - debug(`Got new connection: device=${deviceName}, app=${appName}`); + debug(`Got new connection: device=${deviceName}, app=${appName}, id=${deviceId}`); socket.on('close', () => { this._devices.delete(deviceId); From 8c4a8278728e7eeb698191baec0eabbbe69b9bc4 Mon Sep 17 00:00:00 2001 From: Cedric van Putten Date: Sat, 27 May 2023 22:12:47 +0200 Subject: [PATCH 3/6] Fix some linting issue --- packages/metro-inspector-proxy/src/InspectorProxy.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/metro-inspector-proxy/src/InspectorProxy.js b/packages/metro-inspector-proxy/src/InspectorProxy.js index dc59a192be..bc179b3c8e 100644 --- a/packages/metro-inspector-proxy/src/InspectorProxy.js +++ b/packages/metro-inspector-proxy/src/InspectorProxy.js @@ -183,7 +183,9 @@ class InspectorProxy { new Device(deviceId, deviceName, appName, socket, this._projectRoot), ); - debug(`Got new connection: device=${deviceName}, app=${appName}, id=${deviceId}`); + debug( + `Got new connection: device=${deviceName}, app=${appName}, id=${deviceId}`, + ); socket.on('close', () => { this._devices.delete(deviceId); From 9bbf992fcf0afc0d9ce19eb97bf20e6b4f5e18c0 Mon Sep 17 00:00:00 2001 From: Cedric van Putten Date: Fri, 2 Jun 2023 13:08:24 +0200 Subject: [PATCH 4/6] Move debugger socket reuse logic to device --- packages/metro-inspector-proxy/src/Device.js | 33 +++++++++++++++++++ .../src/InspectorProxy.js | 19 ++++++----- 2 files changed, 43 insertions(+), 9 deletions(-) diff --git a/packages/metro-inspector-proxy/src/Device.js b/packages/metro-inspector-proxy/src/Device.js index 48a30faa1d..3860e94142 100644 --- a/packages/metro-inspector-proxy/src/Device.js +++ b/packages/metro-inspector-proxy/src/Device.js @@ -134,6 +134,10 @@ class Device { return this._name; } + getApp(): string { + return this._app; + } + getPagesList(): Array { if (this._lastConnectedReactNativePage) { const reactNativeReloadablePage = { @@ -212,6 +216,35 @@ class Device { }; } + // Handles cleaning up a duplicate device connection, by client-side device ID. + // 1. Checks if the same device is attempting to reconnect for the same app. + // 2. If not, close both the device and debugger socket. + // 3. If the debugger connection can be reused, close the device socket only. + // + // This allows users to reload the app, either as result of a crash, or manually + // reloading, without having to restart the debugger. + handleDuplicateDeviceConnection(newDevice: Device) { + if ( + this._app !== newDevice.getApp() || + this._name !== newDevice.getName() + ) { + this._deviceSocket.close(); + this._debuggerConnection?.socket.close(); + } + + const oldDebugger = this._debuggerConnection; + this._debuggerConnection = null; + + if (oldDebugger) { + oldDebugger.socket.removeAllListeners(); + this._deviceSocket.close(); + newDevice.handleDebuggerConnection( + oldDebugger.socket, + oldDebugger.pageId, + ); + } + } + // Handles messages received from device: // 1. For getPages responses updates local _pages list. // 2. All other messages are forwarded to debugger as wrappedEvent. diff --git a/packages/metro-inspector-proxy/src/InspectorProxy.js b/packages/metro-inspector-proxy/src/InspectorProxy.js index bc179b3c8e..3df941196a 100644 --- a/packages/metro-inspector-proxy/src/InspectorProxy.js +++ b/packages/metro-inspector-proxy/src/InspectorProxy.js @@ -170,18 +170,19 @@ class InspectorProxy { const appName = query.app || 'Unknown'; const oldDevice = this._devices.get(deviceId); + const newDevice = new Device( + deviceId, + deviceName, + appName, + socket, + this._projectRoot, + ); + if (oldDevice) { - // Keep the debugger connection alive when disconnecting the device, if possible - if (oldDevice._name === deviceName && oldDevice._app === appName) { - oldDevice._debuggerConnection = null; - } - oldDevice._deviceSocket.close(); + oldDevice.handleDuplicateDeviceConnection(newDevice); } - this._devices.set( - deviceId, - new Device(deviceId, deviceName, appName, socket, this._projectRoot), - ); + this._devices.set(deviceId, newDevice); debug( `Got new connection: device=${deviceName}, app=${appName}, id=${deviceId}`, From e4885b19353ba642af4b64d895a927775f45b67f Mon Sep 17 00:00:00 2001 From: Cedric van Putten Date: Fri, 2 Jun 2023 16:24:06 +0200 Subject: [PATCH 5/6] Update new device connection debug log Co-authored-by: Alex Hunt --- packages/metro-inspector-proxy/src/InspectorProxy.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/metro-inspector-proxy/src/InspectorProxy.js b/packages/metro-inspector-proxy/src/InspectorProxy.js index 3df941196a..d9ee0ce18e 100644 --- a/packages/metro-inspector-proxy/src/InspectorProxy.js +++ b/packages/metro-inspector-proxy/src/InspectorProxy.js @@ -185,7 +185,7 @@ class InspectorProxy { this._devices.set(deviceId, newDevice); debug( - `Got new connection: device=${deviceName}, app=${appName}, id=${deviceId}`, + `Got new connection: name=${deviceName}, app=${appName}, device=${deviceId}`, ); socket.on('close', () => { From 1d516da7f41e813caff84e6f59cdf9f041e68964 Mon Sep 17 00:00:00 2001 From: Cedric van Putten Date: Fri, 2 Jun 2023 16:23:29 +0200 Subject: [PATCH 6/6] Use docblock instead of line comments --- packages/metro-inspector-proxy/src/Device.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/metro-inspector-proxy/src/Device.js b/packages/metro-inspector-proxy/src/Device.js index 3860e94142..2123961fd9 100644 --- a/packages/metro-inspector-proxy/src/Device.js +++ b/packages/metro-inspector-proxy/src/Device.js @@ -216,13 +216,15 @@ class Device { }; } - // Handles cleaning up a duplicate device connection, by client-side device ID. - // 1. Checks if the same device is attempting to reconnect for the same app. - // 2. If not, close both the device and debugger socket. - // 3. If the debugger connection can be reused, close the device socket only. - // - // This allows users to reload the app, either as result of a crash, or manually - // reloading, without having to restart the debugger. + /** + * Handles cleaning up a duplicate device connection, by client-side device ID. + * 1. Checks if the same device is attempting to reconnect for the same app. + * 2. If not, close both the device and debugger socket. + * 3. If the debugger connection can be reused, close the device socket only. + * + * This allows users to reload the app, either as result of a crash, or manually + * reloading, without having to restart the debugger. + */ handleDuplicateDeviceConnection(newDevice: Device) { if ( this._app !== newDevice.getApp() ||