jens/flipper
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix connection and DOM management of React DevTools

Browse Source 
Summary:
This diff fixes several existing issues in the React DevTools:

Every time the user opened the plugin we re-instantiated the devtools, which has a few problems: 1) it looses all state (e.g. selection), and 2) this causes the tools to start a websocket listener on a new port, that was never cleaned up, or ever used, since React Native always connects to port 8097 anyway.

To preserve the state the idea of the original implementation was to move the devTools out of the current view, without disposing it. This actually didn't work in practice due to a faulty implementation, causing a full reinialization of the tools every time. Addressed this by reusing the mechanism that is used by the Hermes debugger tools as well.

By properly managing the port (e.g. closing it), there is no need to start (in vain) the devTools on a random port.

Port reversal for physical devices needs to happen only once, in principle upon connecting the device, so moved it to the device logic, which also avoids the need to hack into the global Flipper store.

Avoiding recreating the devTools makes plugin switching near instant, instead of needing to wait for a few seconds until the devTools connect.

When multiple apps are connected the behavior is now consistent: the application that refreshed last will be the one visible in the devTools. (That is still pretty suboptimal, but at least predicable and not a use case that is requested / supported in the DevTools themselves atm)

There is still ugly DOM business going on, did put that in a stand alone component for now.
Didn't extract the shared logic with Hermes plugin yet, but did verify both are still working correctly.

Changelog: [React DevTools] Several improvements that should improve the overal experience, the plugin should load much quicker and behave more predictably.

Reviewed By: bvaughn

Differential Revision: D28382587

fbshipit-source-id: 0f2787b24fa2afdf5014dbf1d79240606405199a
This commit is contained in:
Michel Weststrate
2021-05-12 14:20:57 -07:00
committed by Facebook GitHub Bot
parent ab17bbd555
commit 96cbc81e63
7 changed files with 184 additions and 147 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
desktop/app/src/devices/AndroidDevice.tsx
View File

@@ -106,7 +106,7 @@ export default class AndroidDevice extends BaseDevice {
this.reader = undefined;
}
reverse(ports: [number, number]): Promise<void> {
reverse(ports: number[]): Promise<void> {
return Promise.all(
ports.map((port) =>
this.adb.reverse(this.serial, `tcp:${port}`, `tcp:${port}`),

9
desktop/app/src/dispatcher/androidDevice.tsx
View File

@@ -61,6 +61,15 @@ function createDevice(
);
});
}
if (type === 'physical') {
// forward port for React DevTools, which is fixed on React Native
await androidLikeDevice.reverse([8097]).catch((e) => {
console.warn(
`Failed to reverse-proxy React DevTools port 8097 on ${androidLikeDevice.serial}`,
e,
);
});
}
resolve(androidLikeDevice);
} catch (e) {
reject(e);

2
desktop/app/src/sandy-chrome/SandyApp.tsx
View File

@@ -150,7 +150,6 @@ export function SandyApp() {
</_Sidebar>
</Layout.Horizontal>
<MainContainer>
{outOfContentsContainer}
{staticView ? (
<TrackingScope
scope={
@@ -172,6 +171,7 @@ export function SandyApp() {
) : (
<PluginContainer logger={logger} isSandy />
)}
{outOfContentsContainer}
</MainContainer>
</Layout.Left>
</Layout.Top>

4
desktop/app/src/store.tsx
View File

@@ -37,7 +37,3 @@ if (!isProduction()) {
// @ts-ignore
window.flipperStore = store;
}
// Escape hatch during Sandy conversion;
// Some plugins directly interact with the Store and need further abstractions
// @ts-ignore
window.__SECRET_FLIPPER_STORE_DONT_USE_OR_YOU_WILL_BE_FIRED__ = store;

3
desktop/plugins/public/hermesdebuggerrn/ChromeDevTools.tsx
View File

@@ -16,6 +16,7 @@ const devToolsNodeId = (url: string) =>
`hermes-chromedevtools-out-of-react-node-${url.replace(/\W+/g, '-')}`;
// TODO: build abstraction of this: T62306732
// TODO: reuse reactdevtools/DevToolsEmbedder for this
const TARGET_CONTAINER_ID = 'flipper-out-of-contents-container'; // should be a hook in the future
function createDevToolsNode(
@@ -62,6 +63,8 @@ function attachDevTools(devToolsNode: HTMLElement) {
document.getElementById(TARGET_CONTAINER_ID)!.style.display = 'block';
document.getElementById(TARGET_CONTAINER_ID)!.parentElement!.style.display =
'block';
document.getElementById(TARGET_CONTAINER_ID)!.parentElement!.style.height =
'100%';
}
function detachDevTools(devToolsNode: HTMLElement | null) {

68
desktop/plugins/public/reactdevtools/DevToolsEmbedder.tsx Normal file
View File

@@ -0,0 +1,68 @@
/**
* 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.
*
* @format
*/
import {useEffect} from 'react';
// TODO: build abstraction of this: T62306732
const TARGET_CONTAINER_ID = 'flipper-out-of-contents-container'; // should be a hook in the future
export function DevToolsEmbedder({
offset,
nodeId,
}: {
offset: number;
nodeId: string;
}) {
useEffect(() => {
attachDevTools(createDevToolsNode(nodeId), offset);
return () => {
detachDevTools(findDevToolsNode(nodeId));
};
}, [offset, nodeId]);
return null;
}
function createDevToolsNode(nodeId: string): HTMLElement {
const existing = findDevToolsNode(nodeId);
if (existing) {
return existing;
}
const wrapper = document.createElement('div');
wrapper.id = nodeId;
wrapper.style.height = '100%';
wrapper.style.width = '100%';
document.getElementById(TARGET_CONTAINER_ID)!.appendChild(wrapper);
return wrapper;
}
function findDevToolsNode(nodeId: string): HTMLElement | null {
return document.querySelector('#' + nodeId);
}
function attachDevTools(devToolsNode: HTMLElement, offset: number = 0) {
devToolsNode.style.display = 'block';
const container = document.getElementById(TARGET_CONTAINER_ID)!;
container.style.display = 'block';
container.parentElement!.style.display = 'block';
container.parentElement!.style.height = `calc(100% - ${offset}px)`;
container.parentElement!.style.marginTop = '0px';
}
function detachDevTools(devToolsNode: HTMLElement | null) {
document.getElementById(TARGET_CONTAINER_ID)!.style.display = 'none';
document.getElementById(TARGET_CONTAINER_ID)!.parentElement!.style.display =
'none';
if (devToolsNode) {
devToolsNode.style.display = 'none';
}
}

217
desktop/plugins/public/reactdevtools/index.tsx
View File

@@ -7,25 +7,27 @@
* @format
*/
import ReactDevToolsStandalone from 'react-devtools-core/standalone';
import ReactDevToolsStandaloneEmbedded from 'react-devtools-core/standalone';
import {
Layout,
usePlugin,
DevicePluginClient,
createState,
useValue,
theme,
sleep,
Toolbar,
} from 'flipper-plugin';
import React, {createRef, useEffect} from 'react';
import React from 'react';
import getPort from 'get-port';
import {Alert, Button, Switch} from 'antd';
import {Button, Switch, Typography} from 'antd';
import child_process from 'child_process';
import fs from 'fs';
import path from 'path';
import {DevToolsEmbedder} from './DevToolsEmbedder';
const DEV_TOOLS_NODE_ID = 'reactdevtools-out-of-react-node';
const CONNECTED = 'DevTools connected';
const DEV_TOOLS_PORT = 8097; // hardcoded in RN
interface MetroDevice {
ws?: WebSocket;
@@ -55,38 +57,6 @@ function findGlobalDevTools(): Promise<string | undefined> {
});
}
function createDevToolsNode(): HTMLElement {
const div = document.createElement('div');
div.id = DEV_TOOLS_NODE_ID;
div.style.display = 'none';
div.style.width = '100%';
div.style.height = '100%';
div.style.flex = '1 1 0%';
div.style.justifyContent = 'center';
div.style.alignItems = 'stretch';
document.body && document.body.appendChild(div);
return div;
}
function findDevToolsNode(): HTMLElement | null {
return document.querySelector('#' + DEV_TOOLS_NODE_ID);
}
function attachDevTools(target: Element | Text, devToolsNode: HTMLElement) {
target.appendChild(devToolsNode);
devToolsNode.style.display = 'flex';
}
function detachDevTools(devToolsNode: HTMLElement) {
devToolsNode.style.display = 'none';
document.body && document.body.appendChild(devToolsNode);
}
const CONNECTED = 'DevTools connected';
const SUPPORTED_OCULUS_DEVICE_TYPES = ['quest', 'go', 'pacific'];
enum ConnectionStatus {
Initializing = 'Initializing...',
WaitingForReload = 'Waiting for connection from device...',
@@ -106,48 +76,92 @@ export function devicePlugin(client: DevicePluginClient) {
);
const globalDevToolsPath = createState<string>();
const useGlobalDevTools = createState(false); // TODO: store in local storage T69989583
let devToolsInstance: typeof ReactDevToolsStandalone =
ReactDevToolsStandalone;
let devToolsInstance: typeof ReactDevToolsStandaloneEmbedded =
ReactDevToolsStandaloneEmbedded;
let startResult: {close(): void} | undefined = undefined;
const containerRef = createRef<HTMLDivElement>();
let pollHandle: NodeJS.Timeout | undefined = undefined;
let isMounted = false;
async function toggleUseGlobalDevTools() {
if (!globalDevToolsPath.get()) {
return;
}
useGlobalDevTools.update((v) => !v);
// Load right library
if (useGlobalDevTools.get()) {
console.log('Loading ' + globalDevToolsPath.get());
devToolsInstance = global.electronRequire(
globalDevToolsPath.get()!,
).default;
} else {
devToolsInstance = ReactDevToolsStandalone;
devToolsInstance = ReactDevToolsStandaloneEmbedded;
}
statusMessage.set('Switching devTools');
connectionStatus.set(ConnectionStatus.Initializing);
// clean old instance
if (pollHandle) {
clearTimeout(pollHandle);
}
startResult?.close();
stopDevtools();
findDevToolsNode()!.remove();
await sleep(1000); // wait for port to close
startResult = undefined;
await bootDevTools();
}
async function bootDevTools() {
isMounted = true;
let devToolsNode = findDevToolsNode();
const devToolsNode = document.getElementById(DEV_TOOLS_NODE_ID);
if (!devToolsNode) {
devToolsNode = createDevToolsNode();
setStatus(ConnectionStatus.Error, 'Failed to find target DOM Node');
return;
}
// React DevTools were initilized before
if (startResult) {
if (devtoolsHaveStarted()) {
setStatus(ConnectionStatus.Connected, CONNECTED);
} else {
startPollForConnection();
}
return;
}
// They're new!
try {
setStatus(
ConnectionStatus.Initializing,
'Waiting for port ' + DEV_TOOLS_PORT,
);
const port = await getPort({port: DEV_TOOLS_PORT}); // default port for dev tools
if (port !== DEV_TOOLS_PORT) {
setStatus(
ConnectionStatus.Error,
`Port ${DEV_TOOLS_PORT} is already taken`,
);
return;
}
setStatus(
ConnectionStatus.Initializing,
'Starting DevTools server on ' + port,
);
startResult = devToolsInstance
.setContentDOMNode(devToolsNode)
.setStatusListener((status) => {
// TODO: since devToolsInstance is an instance, we are probably leaking memory here
setStatus(ConnectionStatus.Initializing, status);
})
.startServer(port) as any;
setStatus(ConnectionStatus.Initializing, 'Waiting for device');
} catch (e) {
console.error('Failed to initalize React DevTools' + e);
setStatus(ConnectionStatus.Error, 'Failed to initialize DevTools: ' + e);
}
attachDevTools(containerRef.current!, devToolsNode);
initializeDevTools(devToolsNode);
setStatus(
ConnectionStatus.Initializing,
'DevTools have been initialized, waiting for connection...',
);
await sleep(5); // give node time to move
if (devtoolsHaveStarted()) {
setStatus(ConnectionStatus.Connected, CONNECTED);
} else {
@@ -157,9 +171,6 @@ export function devicePlugin(client: DevicePluginClient) {
function setStatus(cs: ConnectionStatus, status: string) {
connectionStatus.set(cs);
if (!isMounted) {
return;
}
if (status.startsWith('The server is listening on')) {
statusMessage.set(status + ' Waiting for connection...');
} else {
@@ -170,9 +181,6 @@ export function devicePlugin(client: DevicePluginClient) {
function startPollForConnection(delay = 3000) {
pollHandle = setTimeout(async () => {
switch (true) {
// Closed already, ignore
case !isMounted:
return;
// Found DevTools!
case devtoolsHaveStarted():
setStatus(ConnectionStatus.Connected, CONNECTED);
@@ -191,7 +199,7 @@ export function devicePlugin(client: DevicePluginClient) {
case connectionStatus.get() === ConnectionStatus.Initializing:
setStatus(
ConnectionStatus.WaitingForReload,
"The DevTools didn't connect yet. Please trigger the DevMenu in the React Native app, or Reload it to connect",
"The DevTools didn't connect yet. Please trigger the DevMenu in the React Native app, or Reload it to connect.",
);
startPollForConnection(10000);
return;
@@ -199,7 +207,7 @@ export function devicePlugin(client: DevicePluginClient) {
case connectionStatus.get() === ConnectionStatus.WaitingForReload:
setStatus(
ConnectionStatus.WaitingForReload,
"The DevTools didn't connect yet. Please verify your React Native app is in development mode, and that no other instance of the React DevTools are attached to the app already.",
"The DevTools didn't connect yet. Check if no other instances are running.",
);
startPollForConnection();
return;
@@ -208,64 +216,12 @@ export function devicePlugin(client: DevicePluginClient) {
}
function devtoolsHaveStarted() {
return (findDevToolsNode()?.childElementCount ?? 0) > 0;
}
async function initializeDevTools(devToolsNode: HTMLElement) {
try {
setStatus(ConnectionStatus.Initializing, 'Waiting for port 8097');
const port = await getPort({port: 8097}); // default port for dev tools
setStatus(
ConnectionStatus.Initializing,
'Starting DevTools server on ' + port,
return (
(document.getElementById(DEV_TOOLS_NODE_ID)?.childElementCount ?? 0) > 0
);
// Currently a new port is negotatiated every time the plugin is opened.
// This can be potentially optimized by keeping the devTools instance around
startResult = devToolsInstance
.setContentDOMNode(devToolsNode)
.setStatusListener((status) => {
setStatus(ConnectionStatus.Initializing, status);
})
.startServer(port) as any;
setStatus(ConnectionStatus.Initializing, 'Waiting for device');
// This is a hack that should be cleaned up. Instead of setting up port forwarding
// for any physical android device, we should introduce a mechanism to detect all connected
// metro apps, and connect to one of them.
// Since this is not how we want (or can) reliably detect the device we intend to interact with,
// leaving this here until we can get a list of connected applications & ports from Metro or Flipper
(window as any).__SECRET_FLIPPER_STORE_DONT_USE_OR_YOU_WILL_BE_FIRED__
.getState()
.connections.devices.forEach((d: any) => {
if (
(d.deviceType === 'physical' && d.os === 'Android') ||
SUPPORTED_OCULUS_DEVICE_TYPES.includes(d.title.toLowerCase())
) {
console.log(
`[React DevTools] Forwarding port ${port} for device ${d.title}`,
);
d.reverse([port]);
}
});
} catch (e) {
console.error('Failed to initalize React DevTools' + e);
setStatus(ConnectionStatus.Error, 'Failed to initialize DevTools: ' + e);
}
}
function stopDevtools() {
isMounted = false;
if (pollHandle) {
clearTimeout(pollHandle);
}
const devToolsNode = findDevToolsNode();
if (devToolsNode) {
detachDevTools(devToolsNode);
}
}
client.onReady(() => {
console.log('searching');
findGlobalDevTools().then((path) => {
globalDevToolsPath.set(path + '/standalone');
if (path) {
@@ -274,14 +230,26 @@ export function devicePlugin(client: DevicePluginClient) {
});
});
client.onDestroy(() => {
startResult?.close();
});
client.onActivate(() => {
bootDevTools();
});
client.onDeactivate(() => {
if (pollHandle) {
clearTimeout(pollHandle);
}
});
return {
devtoolsHaveStarted,
connectionStatus,
statusMessage,
bootDevTools,
metroDevice,
containerRef,
stopDevtools,
globalDevToolsPath,
useGlobalDevTools,
toggleUseGlobalDevTools,
@@ -295,11 +263,6 @@ export function Component() {
const globalDevToolsPath = useValue(instance.globalDevToolsPath);
const useGlobalDevTools = useValue(instance.useGlobalDevTools);
useEffect(() => {
instance.bootDevTools();
return instance.stopDevtools;
}, [instance]);
return (
<Layout.Container grow>
{globalDevToolsPath ? (
@@ -309,21 +272,20 @@ export function Component() {
<Switch
checked={useGlobalDevTools}
onChange={instance.toggleUseGlobalDevTools}
size="small"
/>
Use globally installed DevTools
</>
}
/>
wash>
{connectionStatus !== ConnectionStatus.Connected ? (
<Typography.Text type="secondary">{statusMessage}</Typography.Text>
) : null}
{!instance.devtoolsHaveStarted() ? (
<Layout.Container
style={{width: 400, margin: `${theme.space.large}px auto`}}>
<Alert message={statusMessage} type="warning" showIcon>
{(connectionStatus === ConnectionStatus.WaitingForReload &&
instance.metroDevice?.ws) ||
connectionStatus === ConnectionStatus.Error ? (
<Button
style={{width: 200, margin: '10px auto 0 auto'}}
size="small"
onClick={() => {
instance.metroDevice?.sendCommand('reload');
instance.bootDevTools();
@@ -331,10 +293,9 @@ export function Component() {
Retry
</Button>
) : null}
</Alert>
</Layout.Container>
</Toolbar>
) : null}
<Layout.Container grow ref={instance.containerRef} />
<DevToolsEmbedder offset={40} nodeId={DEV_TOOLS_NODE_ID} />
</Layout.Container>
);
}