Separate the concepts of archived and disconnected devices

Summary: Minor code cleanup to avoid future confusion: - archived: a device that was imported from a Flipper trace, and only has persisted state - (dis)connected: a real stateful device that might or might not have an active connection Reviewed By: nikoant Differential Revision: D26275459 fbshipit-source-id: eba554b37c39711e367c3795ff4456329a303c22
2021-02-09 04:12:09 -08:00
parent 1bb1cae167
commit 43c68c0e7c
19 changed files with 69 additions and 66 deletions
--- a/desktop/app/src/PluginContainer.tsx
+++ b/desktop/app/src/PluginContainer.tsx
@@ -588,7 +588,7 @@ export default connect<StateFromProps, DispatchFromProps, OwnProps, Store>(
    }
    const isArchivedDevice = !selectedDevice
      ? false
-      : selectedDevice instanceof ArchivedDevice;
+      : selectedDevice.isArchived;
    if (isArchivedDevice) {
      pluginIsEnabled = true;
    }
--- a/desktop/app/src/tests/disconnect.node.tsx
+++ b/desktop/app/src/tests/disconnect.node.tsx
@@ -50,10 +50,12 @@ test('Devices can disconnect', async () => {
  ).toBe(true);

  expect(device.isArchived).toBe(false);
+  expect(device.connected.get()).toBe(true);

  device.disconnect();

-  expect(device.isArchived).toBe(true);
+  expect(device.isArchived).toBe(false);
+  expect(device.connected.get()).toBe(false);
  const instance = device.sandyPluginStates.get(deviceplugin.id)!;
  expect(instance.instanceApi.isConnected).toBe(false);
  expect(instance).toBeTruthy();
@@ -61,7 +63,8 @@ test('Devices can disconnect', async () => {
  expect(instance.instanceApi.destroy).toBeCalledTimes(0);

  device.destroy();
-  expect(device.isArchived).toBe(true);
+  expect(device.isArchived).toBe(false);
+  expect(device.connected.get()).toBe(false);
  expect(instance.instanceApi.destroy).toBeCalledTimes(1);

  expect(device.sandyPluginStates.get(deviceplugin.id)).toBeUndefined();
@@ -91,6 +94,7 @@ test('New device with same serial removes & cleans the old one', async () => {
  const instance = device.sandyPluginStates.get(deviceplugin.id)!;

  expect(device.isArchived).toBe(false);
+  expect(device.connected.get()).toBe(true);
  expect(instance.instanceApi.destroy).toBeCalledTimes(0);
  expect(store.getState().connections.devices).toEqual([device]);

@@ -107,7 +111,8 @@ test('New device with same serial removes & cleans the old one', async () => {
  });
  device2.loadDevicePlugins(store.getState().plugins.devicePlugins);

-  expect(device.isArchived).toBe(true);
+  expect(device.isArchived).toBe(false);
+  expect(device.connected.get()).toBe(false);
  expect(instance.instanceApi.destroy).toBeCalledTimes(1);
  expect(
    device2.sandyPluginStates.get(deviceplugin.id)!.instanceApi.destroy,
--- a/desktop/app/src/chrome/MetroButton.tsx
+++ b/desktop/app/src/chrome/MetroButton.tsx
@@ -12,13 +12,12 @@ import MetroDevice, {MetroReportableEvent} from '../devices/MetroDevice';
 import {useStore} from '../utils/useStore';
 import {Button as AntButton} from 'antd';
 import {MenuOutlined, ReloadOutlined} from '@ant-design/icons';
-
-type LogEntry = {};
+import {theme} from 'flipper-plugin';

 export default function MetroButton() {
  const device = useStore((state) =>
    state.connections.devices.find(
-      (device) => device.os === 'Metro' && !device.isArchived,
+      (device) => device.os === 'Metro' && device.connected.get(),
    ),
  ) as MetroDevice | undefined;

@@ -69,6 +68,7 @@ export default function MetroButton() {
          sendCommand('reload');
        }}
        loading={progress < 1}
+        style={{color: _hasBuildError ? theme.errorColor : undefined}}
      />
      <AntButton
        icon={<MenuOutlined />}
--- a/desktop/app/src/devices/ArchivedDevice.tsx
+++ b/desktop/app/src/devices/ArchivedDevice.tsx
@@ -13,6 +13,8 @@ import {OS, DeviceShell} from './BaseDevice';
 import {SupportFormRequestDetailsState} from '../reducers/supportForm';

 export default class ArchivedDevice extends BaseDevice {
+  isArchived = true;
+
  constructor(options: {
    serial: string;
    deviceType: DeviceType;
@@ -23,7 +25,7 @@ export default class ArchivedDevice extends BaseDevice {
    supportRequestDetails?: SupportFormRequestDetailsState;
  }) {
    super(options.serial, options.deviceType, options.title, options.os);
-    this.archivedState.set(true);
+    this.connected.set(false);
    this.source = options.source || '';
    this.supportRequestDetails = options.supportRequestDetails;
    this.archivedScreenshotHandle = options.screenshotHandle;
--- a/desktop/app/src/devices/BaseDevice.tsx
+++ b/desktop/app/src/devices/BaseDevice.tsx
@@ -38,6 +38,8 @@ export type DeviceExport = {
 };

 export default class BaseDevice {
+  isArchived = false;
+
  constructor(
    serial: string,
    deviceType: DeviceType,
@@ -72,10 +74,7 @@ export default class BaseDevice {

  logListeners: Map<Symbol, DeviceLogListener> = new Map();

-  archivedState = createState(false);
-  get isArchived() {
-    return this.archivedState.get();
-  }
+  readonly connected = createState(true);

  // if imported, stores the original source location
  source = '';
@@ -93,7 +92,7 @@ export default class BaseDevice {
  }

  displayTitle(): string {
-    return this.title;
+    return this.connected.get() ? this.title : `${this.title} (Offline)`;
  }

  async exportState(
@@ -226,7 +225,7 @@ export default class BaseDevice {
  }

  disconnect() {
-    this.archivedState.set(true);
+    this.connected.set(false);
  }

  destroy() {
--- a/desktop/app/src/devices/IOSDevice.tsx
+++ b/desktop/app/src/devices/IOSDevice.tsx
@@ -53,7 +53,7 @@ export default class IOSDevice extends BaseDevice {
  }

  async screenshot(): Promise<Buffer> {
-    if (this.isArchived) {
+    if (!this.connected.get()) {
      return Buffer.from([]);
    }
    const tmpImageName = uuid() + '.png';
@@ -192,7 +192,7 @@ export default class IOSDevice extends BaseDevice {
  }

  async screenCaptureAvailable() {
-    return this.deviceType === 'emulator' && !this.isArchived;
+    return this.deviceType === 'emulator' && this.connected.get();
  }

  async startScreenCapture(destination: string) {
--- a/desktop/app/src/devices/MetroDevice.tsx
+++ b/desktop/app/src/devices/MetroDevice.tsx
@@ -9,7 +9,6 @@

 import {LogLevel} from 'flipper-plugin';
 import BaseDevice from './BaseDevice';
-import ArchivedDevice from './ArchivedDevice';
 import {EventEmitter} from 'events';

 // From xplat/js/metro/packages/metro/src/lib/reporting.js
@@ -198,14 +197,4 @@ export default class MetroDevice extends BaseDevice {
      console.warn('Cannot send command, no connection', command);
    }
  }
-
-  archive() {
-    return new ArchivedDevice({
-      serial: this.serial,
-      deviceType: this.deviceType,
-      title: this.title,
-      os: this.os,
-      screenshotHandle: null,
-    });
-  }
 }
--- a/desktop/app/src/dispatcher/androidDevice.tsx
+++ b/desktop/app/src/dispatcher/androidDevice.tsx
@@ -231,7 +231,7 @@ export default (store: Store, logger: Logger) => {
      .getState()
      .connections.devices.filter(
        (device: BaseDevice) =>
-          device.serial === androidDevice.serial && device.isArchived,
+          device.serial === androidDevice.serial && !device.connected.get(),
      )
      .map((device) => device.serial);

--- a/desktop/app/src/dispatcher/iOSDevice.tsx
+++ b/desktop/app/src/dispatcher/iOSDevice.tsx
@@ -118,7 +118,7 @@ function processDevices(
        (device) =>
          device instanceof IOSDevice &&
          device.deviceType === type &&
-          !device.isArchived,
+          device.connected.get(),
      )
      .map((device) => device.serial),
  );
--- a/desktop/app/src/reducers/tests/connections.node.tsx
+++ b/desktop/app/src/reducers/tests/connections.node.tsx
@@ -41,17 +41,8 @@ test('register, remove, re-register a metro device works correctly', () => {
  expect(state.devices.length).toBe(1);
  expect(state.devices[0].displayTitle()).toBe('React Native');

-  const archived = device1.archive();
-  state = reducer(state, {
-    type: 'UNREGISTER_DEVICES',
-    payload: new Set([device1.serial]),
-  });
-  expect(state.devices.length).toBe(0);
+  device1.disconnect();

-  state = reducer(state, {
-    type: 'REGISTER_DEVICE',
-    payload: archived,
-  });
  expect(state.devices.length).toBe(1);
  expect(state.devices[0].displayTitle()).toBe('React Native (Offline)');

@@ -61,6 +52,7 @@ test('register, remove, re-register a metro device works correctly', () => {
  });
  expect(state.devices.length).toBe(1);
  expect(state.devices[0].displayTitle()).toBe('React Native');
+  expect(state.devices[0]).not.toBe(device1);
 });

 test('triggering REGISTER_DEVICE before REGISTER_PLUGINS still registers device plugins', () => {
--- a/desktop/app/src/sandy-chrome/appinspect/AppInspect.tsx
+++ b/desktop/app/src/sandy-chrome/appinspect/AppInspect.tsx
@@ -23,7 +23,6 @@ import Client from '../../Client';
 import {State} from '../../reducers';
 import BaseDevice from '../../devices/BaseDevice';
 import MetroDevice from '../../devices/MetroDevice';
-import ArchivedDevice from '../../devices/ArchivedDevice';
 import {ExclamationCircleOutlined, FieldTimeOutlined} from '@ant-design/icons';

 const {Text} = Typography;
@@ -56,7 +55,7 @@ export function AppInspect() {
    metroDevice,
    connections.userPreferredDevice,
  ]);
-  const isDeviceArchived = useValue(activeDevice?.archivedState, false);
+  const isDeviceConnected = useValue(activeDevice?.connected, false);
  const isAppConnected = useValue(client?.connected, false);

  return (
@@ -69,13 +68,13 @@ export function AppInspect() {
          <Layout.Container padv="small" padh="medium" gap={theme.space.large}>
            <AppSelector />
            {renderStatusMessage(
-              isDeviceArchived,
+              isDeviceConnected,
              activeDevice,
              client,
              isAppConnected,
            )}
-            {!isDeviceArchived && isAppConnected && <BookmarkSection />}
-            {!isDeviceArchived && activeDevice && (
+            {isDeviceConnected && isAppConnected && <BookmarkSection />}
+            {isDeviceConnected && activeDevice && (
              <Toolbar gap>
                <MetroButton />
                <ScreenCaptureButtons />
@@ -145,13 +144,28 @@ export function findBestDevice(
 }

 function renderStatusMessage(
-  isDeviceArchived: boolean,
+  isDeviceConnected: boolean,
  activeDevice: BaseDevice | undefined,
  client: Client | undefined,
  isAppConnected: boolean,
 ): React.ReactNode {
-  return isDeviceArchived ? (
-    activeDevice instanceof ArchivedDevice ? (
+  if (!activeDevice) {
+    return (
+      <Layout.Horizontal gap center>
+        <ExclamationCircleOutlined style={{color: theme.warningColor}} />
+        <Text
+          type="secondary"
+          style={{
+            textTransform: 'uppercase',
+            fontSize: '0.8em',
+          }}>
+          Device disconnected
+        </Text>
+      </Layout.Horizontal>
+    );
+  }
+  return !isDeviceConnected ? (
+    activeDevice.isArchived ? (
      <Layout.Horizontal gap center>
        <FieldTimeOutlined style={{color: theme.primaryColor}} />
        <Text
@@ -160,7 +174,7 @@ function renderStatusMessage(
            textTransform: 'uppercase',
            fontSize: '0.8em',
          }}>
-          Device loaded from file
+          No device selected
        </Text>
      </Layout.Horizontal>
    ) : (
--- a/desktop/app/src/sandy-chrome/appinspect/AppSelector.tsx
+++ b/desktop/app/src/sandy-chrome/appinspect/AppSelector.tsx
@@ -57,7 +57,7 @@ export function AppSelector() {
    uninitializedClients,
    selectedApp,
  } = useStore((state) => state.connections);
-  useValue(selectedDevice?.archivedState, false); // subscribe to future archived state changes
+  useValue(selectedDevice?.connected, false); // subscribe to future archived state changes

  const onSelectDevice = useTrackedCallback(
    'select-device',
@@ -223,8 +223,8 @@ function computeEntries(
 }

 function DeviceTitle({device}: {device: BaseDevice}) {
-  const connected = !useValue(device.archivedState);
-  const isImported = device instanceof ArchivedDevice;
+  const connected = useValue(device.connected);
+  const isImported = device.isArchived;
  return (
    <span>
      <>{device.title} </>
--- a/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx
+++ b/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx
@@ -85,7 +85,7 @@ export const PluginList = memo(function PluginList({
    connections.userStarredPlugins,
    pluginsChanged,
  ]);
-  const isArchived = useValue(activeDevice?.archivedState, false);
+  const isConnected = useValue(activeDevice?.connected, false);

  const annotatedDownloadablePlugins = useMemoize<
    [
@@ -198,7 +198,7 @@ export const PluginList = memo(function PluginList({
            ))}
          </PluginGroup>

-          {!isArchived && (
+          {isConnected && (
            <PluginGroup
              key="metro"
              title="React Native"
@@ -226,7 +226,7 @@ export const PluginList = memo(function PluginList({
                onClick={handleAppPluginClick}
                tooltip={getPluginTooltip(plugin.details)}
                actions={
-                  isArchived ? null : (
+                  isConnected ? (
                    <ActionButton
                      id={plugin.id}
                      onClick={handleStarPlugin}
@@ -235,12 +235,12 @@ export const PluginList = memo(function PluginList({
                        <MinusOutlined size={16} style={{marginRight: 0}} />
                      }
                    />
-                  )
+                  ) : null
                }
              />
            ))}
          </PluginGroup>
-          {!isArchived && (
+          {isConnected && (
            <PluginGroup
              key="disabled"
              title="Disabled"
@@ -305,7 +305,7 @@ export const PluginList = memo(function PluginList({
              />
            ))}
          </PluginGroup>
-          {!isArchived && (
+          {isConnected && (
            <PluginGroup
              key="unavailable"
              title="Unavailable plugins"
--- a/desktop/app/src/utils/exportData.tsx
+++ b/desktop/app/src/utils/exportData.tsx
@@ -431,14 +431,14 @@ export async function processStore(
      statusUpdate = () => {};
    }
    statusUpdate('Capturing screenshot...');
-    const deviceScreenshot = device.isArchived
-      ? null
-      : await capture(device).catch((e) => {
+    const deviceScreenshot = device.connected.get()
+      ? await capture(device).catch((e) => {
          console.warn(
            'Failed to capture device screenshot when exporting. ' + e,
          );
          return null;
-        });
+        })
+      : null;
    const processedClients = processClients(clients, serial, statusUpdate);
    const processedPluginStates = processPluginStates({
      clients: processedClients,
--- a/desktop/app/src/utils/pluginUtils.tsx
+++ b/desktop/app/src/utils/pluginUtils.tsx
@@ -301,7 +301,7 @@ function getFavoritePlugins(
  starredPlugins: undefined | string[],
  returnFavoredPlugins: boolean, // if false, unfavoried plugins are returned
 ): PluginDefinition[] {
-  if (device instanceof ArchivedDevice) {
+  if (device.isArchived) {
    if (!returnFavoredPlugins) {
      return [];
    }
--- a/desktop/app/src/utils/screenshot.tsx
+++ b/desktop/app/src/utils/screenshot.tsx
@@ -27,8 +27,8 @@ export function getFileName(extension: 'png' | 'mp4'): string {
 }

 export async function capture(device: BaseDevice): Promise<string> {
-  if (device.isArchived) {
-    console.log('Skipping screenshot for archived device');
+  if (!device.connected.get()) {
+    console.log('Skipping screenshot for disconnected device');
    return '';
  }
  const pngPath = path.join(CAPTURE_LOCATION, getFileName('png'));
--- a/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx
+++ b/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx
@@ -11,6 +11,7 @@ import {SandyPluginDefinition} from './SandyPluginDefinition';
 import {BasePluginInstance, BasePluginClient} from './PluginBase';
 import {FlipperLib} from './FlipperLib';
 import {DeviceType as PluginDeviceType} from 'flipper-plugin-lib';
+import {Atom} from '../state/atom';

 export type DeviceLogListener = (entry: DeviceLogEntry) => void;

@@ -67,6 +68,7 @@ export interface RealFlipperDevice {
  os: string;
  serial: string;
  isArchived: boolean;
+  connected: Atom<boolean>;
  deviceType: DeviceType;
  addLogListener(callback: DeviceLogListener): Symbol;
  removeLogListener(id: Symbol): void;
--- a/desktop/flipper-plugin/src/plugin/PluginBase.tsx
+++ b/desktop/flipper-plugin/src/plugin/PluginBase.tsx
@@ -136,8 +136,7 @@ export abstract class BasePluginInstance {
        return realDevice.isArchived;
      },
      get isConnected() {
-        // for now same as isArchived, in the future we might distinguish between archived/imported and disconnected/offline devices
-        return !realDevice.isArchived;
+        return realDevice.connected.get();
      },
      deviceType: realDevice.deviceType,

--- a/desktop/flipper-plugin/src/test-utils/test-utils.tsx
+++ b/desktop/flipper-plugin/src/test-utils/test-utils.tsx
@@ -421,6 +421,7 @@ function createMockDevice(options?: StartPluginOptions): RealFlipperDevice {
    deviceType: 'emulator',
    serial: 'serial-000',
    isArchived: !!options?.isArchived,
+    connected: createState(true),
    devicePlugins: [],
    addLogListener(cb) {
      logListeners.push(cb);