Expose React Devtools as a Metro plugin

Summary:
This diff turns the DevTools plugin from a normal plugin in a device plugin. The reason for that can be seen at the end of the test plan in the first stack of this diff: Regardless on which client you open the devtools, you are always looking at the react tree of the app that happens to listen at the appropriate port, unrelated to the actively selected app. This diff moves the plugin from being a client plugin to a device plugin, a Metro device plugin to be precisely, as of the latter there is only one and they should typically correspond (which is why we can trigger reload as done in the previous diff)

Currently we have a Flipper plugin inside the iOS / Android apps with one purpose: to select different ports to listen to on different devices. But this functionality was never implemented, nor seems there to be much demand for. So these plugin don't offer any actual value. The widely used standalone version of the react devtools (https://www.npmjs.com/package/react-devtools) doesn't offer port customization either, so this limitation seems to be acceptable.

To make sure that this change is backward compatible, we make sure to show the metro device if we find metro, regardless whether it is new enough to support log forwarding and reload commands (previously we only showed the device if it has the /events endpoint).

The only case I can think of we are killing with this approach is where people are debugging a RN app, but with having metro running. I doubt that is an actual case, but probably rickhanlonii knows more about that.

Furthermore this diff makes sure that the devTools can connect to physical android devices. Also, making it to the end of this explanation means that you have done most of the reviewing for this diff. The actual code diff is shorter.

Reviewed By: passy

Differential Revision: D19878605

fbshipit-source-id: 3f33e59d4f6e4cce39102420f38afee10018999f

android/src/main/java/com/facebook/flipper/plugins/react/ReactFlipperPlugin.java

@@ -7,17 +7,14 @@
 
 package com.facebook.flipper.plugins.react;
 
-import androidx.annotation.Nullable;
 import com.facebook.flipper.core.FlipperConnection;
-import com.facebook.flipper.core.FlipperObject;
 import com.facebook.flipper.core.FlipperPlugin;
-import com.facebook.flipper.core.FlipperReceiver;
-import com.facebook.flipper.core.FlipperResponder;
 
+// This plugin is not needed, but kept here for backward compatilibty
+@Deprecated
 public class ReactFlipperPlugin implements FlipperPlugin {
 
   public static final String ID = "React";
-  @Nullable private FlipperConnection mConnection;
 
   @Override
   public String getId() {
@@ -25,22 +22,10 @@ public class ReactFlipperPlugin implements FlipperPlugin {
   }
 
   @Override
-  public void onConnect(FlipperConnection connection) {
+  public void onConnect(FlipperConnection connection) {}
-    mConnection = connection;
-    connection.receive(
-        "config",
-        new FlipperReceiver() {
-          @Override
-          public void onReceive(final FlipperObject params, FlipperResponder responder) {
-            // set received host and port in react-native
-          }
-        });
-  }
 
   @Override
-  public void onDisconnect() {
+  public void onDisconnect() {}
-    mConnection = null;
-  }
 
   @Override
   public boolean runInBackground() {

iOS/Plugins/FlipperKitReactPlugin/FlipperKitReactPlugin/FlipperKitReactPlugin.m

@@ -13,6 +13,7 @@
 #import <FlipperKit/FlipperConnection.h>
 #import <FlipperKit/FlipperResponder.h>
 
+// This class is no longer needed, but kept here for backward compatibility
 @implementation FlipperKitReactPlugin
 
 - (NSString*)identifier {
@@ -20,10 +21,6 @@
 }
 
 - (void)didConnect:(id<FlipperConnection>)connection {
-  [connection receive:@"config"
-            withBlock:^(NSDictionary* params, id<FlipperResponder> responder){
-                // set received port and host to dev tools
-            }];
 }
 
 - (void)didDisconnect {

src/devices/MetroDevice.tsx

@@ -140,15 +140,17 @@ function getLoglevelFromMessageType(
 }
 
 export default class MetroDevice extends BaseDevice {
-  ws: WebSocket;
+  ws?: WebSocket;
   metroEventEmitter = new EventEmitter();
 
-  constructor(serial: string, ws: WebSocket) {
+  constructor(serial: string, ws: WebSocket | undefined) {
     super(serial, 'emulator', 'React Native', 'Metro');
-    this.ws = ws;
     this.devicePlugins = [];
+    if (ws) {
+      this.ws = ws;
       ws.onmessage = this._handleWSMessage;
     }
+  }
 
   private _handleWSMessage = ({data}: any) => {
     const message: MetroReportableEvent = JSON.parse(data);

src/dispatcher/metroDevice.tsx

@@ -31,7 +31,11 @@ async function isMetroRunning(): Promise<boolean> {
   }
 }
 
-async function registerDevice(ws: WebSocket, store: Store, logger: Logger) {
+async function registerDevice(
+  ws: WebSocket | undefined,
+  store: Store,
+  logger: Logger,
+) {
   const metroDevice = new MetroDevice(METRO_DEVICE_ID, ws);
   logger.track('usage', 'register-device', {
     os: 'Metro',
@@ -117,6 +121,7 @@ export default (store: Store, logger: Logger) => {
             urgent: true,
           },
         });
+        registerDevice(undefined, store, logger);
         // Note: no scheduleNext, we won't retry until restart
       }, 5000);
     } else {

src/plugins/reactdevtools/index.tsx

@@ -7,10 +7,9 @@
  * @format
  */
 
-// import {ReactReduxContext} from 'react-redux';
 import ReactDevToolsStandalone from 'react-devtools-core/standalone';
 import {
-  FlipperPlugin,
+  FlipperDevicePlugin,
   AndroidDevice,
   styled,
   View,
@@ -18,10 +17,10 @@ import {
   MetroDevice,
   ReduxState,
   connect,
+  Device,
 } from 'flipper';
 import React, {useEffect} from 'react';
 import getPort from 'get-port';
-import address from 'address';
 
 const Container = styled.div({
   display: 'flex',
@@ -87,17 +86,24 @@ const GrabMetroDevice = connect<
   return null;
 });
 
-export default class ReactDevTools extends FlipperPlugin<
+const SUPPORTED_OCULUS_DEVICE_TYPES = ['quest', 'go', 'pacific'];
+
+export default class ReactDevTools extends FlipperDevicePlugin<
   {
     status: string;
   },
   any,
   {}
 > {
+  static supportsDevice(device: Device) {
+    return !device.isArchived && device.os === 'Metro';
+  }
+
   pollHandle?: NodeJS.Timeout;
   containerRef: React.RefObject<HTMLDivElement> = React.createRef();
   triedToAutoConnect = false;
   metroDevice?: MetroDevice;
+  isMounted = true;
 
   state = {
     status: 'initializing',
@@ -124,6 +130,7 @@ export default class ReactDevTools extends FlipperPlugin<
   }
 
   componentWillUnmount() {
+    this.isMounted = false;
     if (this.pollHandle) {
       clearTimeout(this.pollHandle);
     }
@@ -132,7 +139,9 @@ export default class ReactDevTools extends FlipperPlugin<
   }
 
   setStatus(status: string) {
-    // console.log(`[ReactDevtoolsPlugin] ${status}`);
+    if (!this.isMounted) {
+      return;
+    }
     if (status.startsWith('The server is listening on')) {
       this.setState({status: status + ' Waiting for connection...'});
     } else {
@@ -142,6 +151,9 @@ export default class ReactDevTools extends FlipperPlugin<
 
   startPollForConnection() {
     this.pollHandle = setTimeout(() => {
+      if (!this.isMounted) {
+        return false;
+      }
       if (findDevToolsNode()?.innerHTML) {
         this.setStatus(CONNECTED);
       } else {
@@ -150,7 +162,7 @@ export default class ReactDevTools extends FlipperPlugin<
           this.setStatus(
             "The DevTools didn't connect yet. Please open the DevMenu in the React Native app, or Reload it to connect",
           );
-          if (this.metroDevice) {
+          if (this.metroDevice && this.metroDevice.ws) {
             this.setStatus(
               "Sending 'reload' to the Metro to force the DevTools to connect...",
             );
@@ -173,20 +185,13 @@ export default class ReactDevTools extends FlipperPlugin<
         })
         .startServer(port);
       this.setStatus('Waiting for device');
-      const device = await this.getDevice();
+      const device = this.device;
 
       if (device) {
-        const host =
+        if (
-          device.deviceType === 'physical'
+          device.deviceType === 'physical' ||
-            ? address.ip()
+          SUPPORTED_OCULUS_DEVICE_TYPES.includes(device.title.toLowerCase())
-            : device instanceof AndroidDevice
+        ) {
-            ? '10.0.2.2' // Host IP for Android emulator host system
-            : 'localhost';
-        this.setStatus(`Updating config to ${host}:${port}`);
-        this.client.call('config', {port, host});
-
-        if (['quest', 'go', 'pacific'].includes(device.title.toLowerCase())) {
-          const device = await this.getDevice();
           this.setStatus(`Setting up reverse port mapping: ${port}:${port}`);
           (device as AndroidDevice).reverse([port, port]);
         }