Don't attempt to connect to React devtools every 2s

Summary:
When testing out the NetworkOverlay, I noticed that we were creating a lot of WebSocket connections for localhost:8097. Rick found that this is because we're trying to connect to React devtools every 2 seconds: https://github.com/facebook/react/blob/master/packages/react-devtools-core/src/backend.js#L67 and it appears we create a new WebSocket every time.

Dan suggested that we use opening the dev menu as a trigger for attempting to connect to React devtools. This diff uses RCTNativeAppEventEmitter to emit an event from native when the dev menu/dialog is shown, and listening to that event in JS to attempt to connect to devtools.

I'm also making the change of passing in a websocket instead of just passing in the host + port; this way it will only attempt to connect once on each call to `connectToDevTools` (otherwise, we would attempt to reconnect every 2 seconds as soon as the dev menu is opened, and then the next time the menu is opened we'd so start that *again*, and so on - I could have it keep track of whether it's already connecting and avoid doing it again, but this is easier and should be sufficient, I think).

We should probably also update the suggested troubleshooting tips on the devtools page to reflect this change, so that people don't get confused.

Changelog: [General] [Fixed] Fix issue where we attempt to connect to React devtools every 2 seconds

Reviewed By: mmmulani

Differential Revision: D17919808

fbshipit-source-id: 4658d995c274574d22f2f54ea06d7f29ef2f54dc

Libraries/Core/setUpDeveloperTools.js

@@ -24,40 +24,7 @@ if (__DEV__) {
   // TODO (T45803484) Enable devtools for bridgeless RN
   if (!global.RN$Bridgeless) {
     if (!global.__RCTProfileIsProfiling) {
-      // not when debugging in chrome
-      // TODO(t12832058) This check is broken
-      if (!window.document) {
-        const AppState = require('../AppState/AppState');
-        // $FlowFixMe Module is untyped
-        const reactDevTools = require('react-devtools-core');
-        const getDevServer = require('./Devtools/getDevServer');
-
-        // Don't steal the DevTools from currently active app.
-        // Note: if you add any AppState subscriptions to this file,
-        // you will also need to guard against `AppState.isAvailable`,
-        // or the code will throw for bundles that don't have it.
-        const isAppActive = () => AppState.currentState !== 'background';
-
-        // Get hostname from development server (packager)
-        const devServer = getDevServer();
-        const host = devServer.bundleLoadedFromServer
-          ? devServer.url.replace(/https?:\/\//, '').split(':')[0]
-          : 'localhost';
-
-        const viewConfig = require('../Components/View/ReactNativeViewViewConfig.js');
-
-        reactDevTools.connectToDevTools({
-          isAppActive,
-          host,
-          // Read the optional global variable for backward compatibility.
-          // It was added in https://github.com/facebook/react-native/commit/bf2b435322e89d0aeee8792b1c6e04656c2719a0.
-          port: window.__REACT_DEVTOOLS_PORT__,
-          resolveRNStyle: require('../StyleSheet/flattenStyle'),
-          nativeStyleEditorValidAttributes: Object.keys(
-            viewConfig.validAttributes.style,
-          ),
-        });
-      }
+      require('./setUpReactDevTools');
 
       // Set up inspector
       const JSInspector = require('../JSInspector/JSInspector');

Libraries/Core/setUpReactDevTools.js

@@ -0,0 +1,60 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ * @format
+ */
+
+'use strict';
+
+if (__DEV__) {
+  // $FlowFixMe Module is untyped
+  const reactDevTools = require('react-devtools-core');
+  const connectToDevTools = () => {
+    // not when debugging in chrome
+    // TODO(t12832058) This check is broken
+    if (!window.document) {
+      const AppState = require('../AppState/AppState');
+      const getDevServer = require('./Devtools/getDevServer');
+
+      // Don't steal the DevTools from currently active app.
+      // Note: if you add any AppState subscriptions to this file,
+      // you will also need to guard against `AppState.isAvailable`,
+      // or the code will throw for bundles that don't have it.
+      const isAppActive = () => AppState.currentState !== 'background';
+
+      // Get hostname from development server (packager)
+      const devServer = getDevServer();
+      const host = devServer.bundleLoadedFromServer
+        ? devServer.url.replace(/https?:\/\//, '').split(':')[0]
+        : 'localhost';
+
+      // Read the optional global variable for backward compatibility.
+      // It was added in https://github.com/facebook/react-native/commit/bf2b435322e89d0aeee8792b1c6e04656c2719a0.
+      const port =
+        window.__REACT_DEVTOOLS_PORT__ != null
+          ? window.__REACT_DEVTOOLS_PORT__
+          : 8097;
+
+      const WebSocket = require('../WebSocket/WebSocket');
+      const ws = new WebSocket('ws://' + host + ':' + port);
+
+      const viewConfig = require('../Components/View/ReactNativeViewViewConfig.js');
+      reactDevTools.connectToDevTools({
+        isAppActive,
+        resolveRNStyle: require('../StyleSheet/flattenStyle'),
+        nativeStyleEditorValidAttributes: Object.keys(
+          viewConfig.validAttributes.style,
+        ),
+        websocket: ws,
+      });
+    }
+  };
+
+  const RCTNativeAppEventEmitter = require('../EventEmitter/RCTNativeAppEventEmitter');
+  RCTNativeAppEventEmitter.addListener('RCTDevMenuShown', connectToDevTools);
+  connectToDevTools(); // Try connecting once on load
+}

React/DevSupport/RCTDevMenu.m

@@ -424,6 +424,11 @@ - (void)setDefaultJSBundle
 
   _presentedItems = items;
   [RCTPresentedViewController() presentViewController:_actionSheet animated:YES completion:nil];
+
+  [_bridge enqueueJSCall:@"RCTNativeAppEventEmitter"
+                  method:@"emit"
+                    args:@[@"RCTDevMenuShown"]
+              completion:NULL];
 }
 
 - (RCTDevMenuAlertActionHandler)alertActionHandlerForDevItem:(RCTDevMenuItem *__nullable)item

ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerImpl.java

@@ -47,6 +47,7 @@
 import com.facebook.react.devsupport.interfaces.ErrorCustomizer;
 import com.facebook.react.devsupport.interfaces.PackagerStatusCallback;
 import com.facebook.react.devsupport.interfaces.StackFrame;
+import com.facebook.react.modules.core.RCTNativeAppEventEmitter;
 import com.facebook.react.modules.debug.interfaces.DeveloperSettings;
 import com.facebook.react.packagerconnection.RequestHandler;
 import com.facebook.react.packagerconnection.Responder;
@@ -621,6 +622,9 @@ public void onCancel(DialogInterface dialog) {
                 })
             .create();
     mDevOptionsDialog.show();
+    if (mCurrentContext != null) {
+      mCurrentContext.getJSModule(RCTNativeAppEventEmitter.class).emit("RCTDevMenuShown", null);
+    }
   }
 
   /** Starts of stops the sampling profiler */