Improve device destruction code [8/n]

Summary: Some cleanup in device destruction code. This diff wraps up the stack that decouples device management from the UI. Next steps are client management, and system management (file access / command execution)

Reviewed By: passy

Differential Revision: D31084036

fbshipit-source-id: 93efee7dba2193589d3c08c51128ce03de5eff7f

desktop/app/src/__tests__/disconnect.node.tsx

@@ -14,7 +14,6 @@ import {
   createState,
   DevicePluginClient,
   PluginClient,
-  sleep,
 } from 'flipper-plugin';
 import {handleClientConnected} from '../dispatcher/flipperServer';
 import {TestDevice} from '../test-utils/TestDevice';
@@ -106,6 +105,13 @@ test('New device with same serial removes & cleans the old one', async () => {
     'MockAndroidDevice',
     'Android',
   );
+  expect(() => {
+    store.dispatch({
+      type: 'REGISTER_DEVICE',
+      payload: device2,
+    });
+  }).toThrow('still connected');
+  device.destroy();
   store.dispatch({
     type: 'REGISTER_DEVICE',
     payload: device2,
@@ -115,7 +121,6 @@ test('New device with same serial removes & cleans the old one', async () => {
     store.getState().connections.enabledDevicePlugins,
   );
 
-  await sleep(100);
   expect(device.isArchived).toBe(false);
   expect(device.connected.get()).toBe(false);
   expect(instance.instanceApi.destroy).toBeCalledTimes(1);

desktop/app/src/dispatcher/flipperServer.tsx

@@ -76,6 +76,21 @@ export default async (store: Store, logger: Logger) => {
       serial: deviceInfo.serial,
     });
 
+    const existing = store
+      .getState()
+      .connections.devices.find(
+        (device) => device.serial === deviceInfo.serial,
+      );
+    // handled outside reducer, as it might emit new redux actions...
+    if (existing) {
+      if (existing.connected.get()) {
+        console.warn(
+          `Tried to replace still connected device '${existing.serial}' with a new instance.`,
+        );
+      }
+      existing.destroy();
+    }
+
     const device = new BaseDevice(server, deviceInfo);
     device.loadDevicePlugins(
       store.getState().plugins.devicePlugins,

desktop/app/src/reducers/__tests__/connections.node.tsx

@@ -21,7 +21,7 @@ afterEach(() => {
   _setFlipperLibImplementation(undefined);
 });
 
-test('doing a double REGISTER_DEVICE keeps the last', () => {
+test('doing a double REGISTER_DEVICE fails', () => {
   const device1 = new TestDevice('serial', 'physical', 'title', 'Android');
   const device2 = new TestDevice('serial', 'physical', 'title2', 'Android');
   const initialState: State = reducer(undefined, {
@@ -31,12 +31,12 @@ test('doing a double REGISTER_DEVICE keeps the last', () => {
   expect(initialState.devices.length).toBe(1);
   expect(initialState.devices[0]).toBe(device1);
 
-  const endState = reducer(initialState, {
+  expect(() => {
-    type: 'REGISTER_DEVICE',
+    reducer(initialState, {
-    payload: device2,
+      type: 'REGISTER_DEVICE',
-  });
+      payload: device2,
-  expect(endState.devices.length).toBe(1);
+    });
-  expect(endState.devices[0]).toBe(device2);
+  }).toThrow('still connected');
 });
 
 test('register, remove, re-register a metro device works correctly', () => {

desktop/app/src/reducers/connections.tsx

@@ -246,13 +246,8 @@ export default (state: State = INITAL_STATE, action: Actions): State => {
       if (existing !== -1) {
         const d = newDevices[existing];
         if (d.connected.get()) {
-          console.warn(
+          throw new Error(`Cannot register, '${d.serial}' is still connected`);
-            `Tried to replace still connected device '${d.serial}' with a new instance`,
-          );
         }
-        setImmediate(() => {
-          d.destroy();
-        });
         newDevices[existing] = payload;
       } else {
         newDevices.push(payload);