Skip to content

Commit

Permalink
Don't attempt to connect to React devtools every 2s
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Emily Janzer authored and facebook-github-bot committed Oct 23, 2019
1 parent c7ed398 commit e7f6210
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 34 deletions.
35 changes: 1 addition & 34 deletions Libraries/Core/setUpDeveloperTools.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
60 changes: 60 additions & 0 deletions Libraries/Core/setUpReactDevTools.js
Original file line number Diff line number Diff line change
@@ -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
}
5 changes: 5 additions & 0 deletions React/DevSupport/RCTDevMenu.m
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 */
Expand Down

0 comments on commit e7f6210

Please sign in to comment.