Factor out realDevice [7/n]

Summary: `device.realDevice` was the escape hatch used in Sandy plugins to give access to device specific features like taking screenshots, clearing logs or accessing `adb`. Since in decapitated Flipper that won't be possible anymore (since plugins run in the client but device implementations on the server), all escape hatches have been bridged in this stack, and we can get of the `realDevice` interaction, by explicitly exposing those cases, which makes it type safe as well.

Reviewed By: passy

Differential Revision: D31079509

fbshipit-source-id: c9ec2e044d0dec0ccb1de287cf424907b198f818

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

@@ -687,7 +687,7 @@ test('PluginContainer can render Sandy device plugins', async () => {
     const deactivatedStub = jest.fn();
     client.onActivate(activatedStub);
     client.onDeactivate(deactivatedStub);
-    client.device.onLogEntry((e) => {
+    client.onDeviceLogEntry((e) => {
       lastLogMessage.set(e);
     });
     return {activatedStub, deactivatedStub, lastLogMessage};

desktop/app/src/devices/BaseDevice.tsx

@@ -20,6 +20,7 @@ import {
   DeviceOS,
   DeviceDescription,
   FlipperServer,
+  Device,
 } from 'flipper-plugin';
 import {DeviceSpec, PluginDetails} from 'flipper-plugin-lib';
 import {getPluginKey} from '../utils/pluginKey';
@@ -42,7 +43,7 @@ export type DeviceExport = {
   pluginStates: Record<string, any>;
 };
 
-export default class BaseDevice {
+export default class BaseDevice implements Device {
   description: DeviceDescription;
   flipperServer: FlipperServer;
   isArchived = false;
@@ -53,6 +54,10 @@ export default class BaseDevice {
     this.description = description;
   }
 
+  get isConnected(): boolean {
+    return this.connected.get();
+  }
+
   // operating system of this device
   get os() {
     return this.description.os;
@@ -181,8 +186,8 @@ export default class BaseDevice {
     }
   }
 
-  navigateToLocation(_location: string) {
-    throw new Error('unimplemented');
+  async navigateToLocation(location: string) {
+    return this.flipperServer.exec('device-navigate', this.serial, location);
   }
 
   async screenshotAvailable(): Promise<boolean> {
@@ -233,7 +238,7 @@ export default class BaseDevice {
     return this.flipperServer.exec('metro-command', this.serial, command);
   }
 
-  async forwardPort(local: string, remote: string): Promise<void> {
+  async forwardPort(local: string, remote: string): Promise<boolean> {
     return this.flipperServer.exec(
       'device-forward-port',
       this.serial,
@@ -242,6 +247,10 @@ export default class BaseDevice {
     );
   }
 
+  async clearLogs() {
+    return this.flipperServer.exec('device-clear-logs', this.serial);
+  }
+
   supportsPlugin(plugin: PluginDefinition | PluginDetails) {
     let pluginDetails: PluginDetails;
     if (plugin instanceof _SandyPluginDefinition) {

desktop/app/src/server/FlipperServerImpl.tsx

@@ -251,6 +251,11 @@ export class FlipperServerImpl implements FlipperServer {
       this.getDevice(serial).stopScreenCapture(),
     'device-shell-exec': async (serial: string, command: string) =>
       this.getDevice(serial).executeShell(command),
+    'device-forward-port': async (serial, local, remote) =>
+      this.getDevice(serial).forwardPort(local, remote),
+    'device-clear-logs': async (serial) => this.getDevice(serial).clearLogs(),
+    'device-navigate': async (serial, loc) =>
+      this.getDevice(serial).navigateToLocation(loc),
     'metro-command': async (serial: string, command: string) => {
       const device = this.getDevice(serial);
       if (!(device instanceof MetroDevice)) {
@@ -258,8 +263,6 @@ export class FlipperServerImpl implements FlipperServer {
       }
       device.sendCommand(command);
     },
-    'device-forward-port': async (serial, local, remote) =>
-      this.getDevice(serial).forwardPort(local, remote),
   };
 
   registerDevice(device: ServerDevice) {

desktop/app/src/server/devices/ServerDevice.tsx

@@ -70,7 +70,15 @@ export abstract class ServerDevice {
     throw new Error('executeShell not implemented on BaseDevice');
   }
 
-  async forwardPort(_local: string, _remote: string): Promise<void> {
+  async forwardPort(_local: string, _remote: string): Promise<boolean> {
     throw new Error('forwardPort not implemented on BaseDevice');
   }
+
+  async clearLogs(): Promise<void> {
+    // no-op on most devices
+  }
+
+  async navigateToLocation(_location: string) {
+    throw new Error('navigateLocation not implemented on BaseDevice');
+  }
 }

desktop/app/src/server/devices/__tests__/BaseDevice.node.tsx

@@ -197,7 +197,7 @@ test('log listeners are resumed and suspended automatically - 1', async () => {
       let disposer: any;
 
       function start() {
-        disposer = client.device.onLogEntry((entry) => {
+        disposer = client.onDeviceLogEntry((entry) => {
           entries.push(entry);
         });
       }
@@ -258,7 +258,7 @@ test('log listeners are resumed and suspended automatically - 2', async () => {
 
   const DevicePlugin = TestUtils.createTestDevicePlugin({
     devicePlugin(client) {
-      client.device.onLogEntry((entry) => {
+      client.onDeviceLogEntry((entry) => {
         entries.push(entry);
       });
       return {};
@@ -268,7 +268,7 @@ test('log listeners are resumed and suspended automatically - 2', async () => {
   const Plugin = TestUtils.createTestPlugin(
     {
       plugin(client) {
-        client.device.onLogEntry((entry) => {
+        client.onDeviceLogEntry((entry) => {
           entries.push(entry);
         });
         return {};

desktop/app/src/server/devices/android/AndroidDevice.tsx

@@ -126,7 +126,7 @@ export default class AndroidDevice extends ServerDevice {
     return this.executeShellOrDie(['logcat', '-c']);
   }
 
-  navigateToLocation(location: string) {
+  async navigateToLocation(location: string) {
     const shellCommand = `am start ${encodeURI(location)}`;
     this.adb.shell(this.serial, shellCommand);
   }
@@ -264,7 +264,7 @@ export default class AndroidDevice extends ServerDevice {
     return destination;
   }
 
-  async forwardPort(local: string, remote: string): Promise<void> {
+  async forwardPort(local: string, remote: string): Promise<boolean> {
     return this.adb.forward(this.serial, local, remote);
   }
 

desktop/app/src/server/devices/ios/IOSDevice.tsx

@@ -71,7 +71,7 @@ export default class IOSDevice extends ServerDevice {
     return await this.iOSBridge.screenshot(this.serial);
   }
 
-  navigateToLocation(location: string) {
+  async navigateToLocation(location: string) {
     return this.iOSBridge.navigate(this.serial, location).catch((err) => {
       console.warn(`Failed to navigate to location ${location}:`, err);
       return err;

desktop/app/src/test-utils/DeviceTestPlugin.tsx

@@ -34,7 +34,7 @@ export function devicePlugin(client: DevicePluginClient) {
     },
   );
 
-  client.device.onLogEntry((entry) => {
+  client.onDeviceLogEntry((entry) => {
     state.update((d) => {
       d.count++;
     });

desktop/app/src/utils/__tests__/exportData.node.tsx

@@ -1333,7 +1333,7 @@ const sandyDeviceTestPlugin = new _SandyPluginDefinition(
       const _somethingElse = createState(0);
       const anotherState = createState({testCount: 0}, {persist: 'otherState'});
 
-      client.device.onLogEntry(() => {
+      client.onDeviceLogEntry(() => {
         counter.set(counter.get() + 1);
         anotherState.update((draft) => {
           draft.testCount -= 1;

desktop/app/src/utils/createSandyPluginWrapper.tsx

@@ -28,6 +28,7 @@ import {useStore} from './useStore';
 import {setStaticView, StaticView} from '../reducers/connections';
 import {getStore} from '../store';
 import {setActiveNotifications} from '../reducers/notifications';
+import BaseDevice from '../devices/BaseDevice';
 
 export type SandyPluginModule = ConstructorParameters<
   typeof _SandyPluginDefinition
@@ -159,7 +160,7 @@ export function createSandyPluginWrapper<S, A extends BaseAction, P>(
 
     return {
       instanceRef,
-      device: client.device.realDevice,
+      device: client.device,
       persistedState,
       deeplink,
       selectPlugin: client.selectPlugin,
@@ -189,7 +190,8 @@ export function createSandyPluginWrapper<S, A extends BaseAction, P>(
     const settingsState = useStore((state) => state.settingsState);
 
     const target = isDevicePlugin
-      ? instance.device
+      ? // in the client, all Device's are BaseDevice, so this is safe..
+        (instance.device as BaseDevice)
       : // eslint-disable-next-line
           useStore((state) =>
           state.connections.clients.find((c) => c.id === instance.appId),

desktop/flipper-plugin/src/__tests__/DeviceTestPlugin.tsx

@@ -30,7 +30,7 @@ export function devicePlugin(client: DevicePluginClient) {
     },
   );
 
-  client.device.onLogEntry((entry) => {
+  client.onDeviceLogEntry((entry) => {
     state.update((d) => {
       d.count++;
     });

desktop/flipper-plugin/src/plugin/DevicePlugin.tsx

@@ -35,14 +35,21 @@ export type LogLevel =
   | 'fatal';
 
 export interface Device {
-  readonly realDevice: any; // TODO: temporarily, clean up T70688226
   readonly isArchived: boolean;
   readonly isConnected: boolean;
   readonly os: DeviceOS;
   readonly serial: string;
   readonly deviceType: DeviceType;
-  onLogEntry(cb: DeviceLogListener): () => void;
+  readonly connected: Atom<boolean>;
   executeShell(command: string): Promise<string>;
+  addLogListener(callback: DeviceLogListener): Symbol;
+  removeLogListener(id: Symbol): void;
+  executeShell(command: string): Promise<string>;
+  forwardPort(local: string, remote: string): Promise<boolean>;
+  clearLogs(): Promise<void>;
+  sendMetroCommand(command: string): Promise<void>;
+  navigateToLocation(location: string): Promise<void>;
+  screenshot(): Promise<Buffer>;
 }
 
 export type DevicePluginPredicate = (device: Device) => boolean;
@@ -59,20 +66,6 @@ export interface DevicePluginClient extends BasePluginClient {
   readonly connected: ReadOnlyAtom<boolean>;
 }
 
-/**
- * Wrapper interface around BaseDevice in Flipper
- */
-export interface RealFlipperDevice {
-  os: DeviceOS;
-  serial: string;
-  isArchived: boolean;
-  connected: Atom<boolean>;
-  deviceType: DeviceType;
-  addLogListener(callback: DeviceLogListener): Symbol;
-  removeLogListener(id: Symbol): void;
-  executeShell(command: string): Promise<string>;
-}
-
 export class SandyDevicePluginInstance extends BasePluginInstance {
   static is(thing: any): thing is SandyDevicePluginInstance {
     return thing instanceof SandyDevicePluginInstance;
@@ -84,20 +77,20 @@ export class SandyDevicePluginInstance extends BasePluginInstance {
   constructor(
     flipperLib: FlipperLib,
     definition: SandyPluginDefinition,
-    realDevice: RealFlipperDevice,
+    device: Device,
     pluginKey: string,
     initialStates?: Record<string, any>,
   ) {
-    super(flipperLib, definition, realDevice, pluginKey, initialStates);
+    super(flipperLib, definition, device, pluginKey, initialStates);
     this.client = {
       ...this.createBasePluginClient(),
       selectPlugin(pluginId: string, deeplink?: unknown) {
-        flipperLib.selectPlugin(realDevice, null, pluginId, deeplink);
+        flipperLib.selectPlugin(device, null, pluginId, deeplink);
       },
       get isConnected() {
-        return realDevice.connected.get();
+        return device.connected.get();
       },
-      connected: realDevice.connected,
+      connected: device.connected,
     };
     this.initializePlugin(() =>
       definition.asDevicePluginModule().devicePlugin(this.client),

desktop/flipper-plugin/src/plugin/FlipperLib.tsx

@@ -8,7 +8,7 @@
  */
 
 import {Logger} from '../utils/Logger';
-import {RealFlipperDevice} from './DevicePlugin';
+import {Device} from './DevicePlugin';
 import {NormalizedMenuEntry} from './MenuEntry';
 import {RealFlipperClient} from './Plugin';
 import {Notification} from './Notification';
@@ -24,7 +24,7 @@ export interface FlipperLib {
   createPaste(input: string): Promise<string | undefined>;
   GK(gatekeeper: string): boolean;
   selectPlugin(
-    device: RealFlipperDevice,
+    device: Device,
     client: RealFlipperClient | null,
     pluginId: string,
     deeplink: unknown,

desktop/flipper-plugin/src/plugin/Plugin.tsx

@@ -10,7 +10,7 @@
 import {SandyPluginDefinition} from './SandyPluginDefinition';
 import {BasePluginInstance, BasePluginClient} from './PluginBase';
 import {FlipperLib} from './FlipperLib';
-import {RealFlipperDevice} from './DevicePlugin';
+import {Device} from './DevicePlugin';
 import {batched} from '../state/batch';
 import {Atom, createState, ReadOnlyAtom} from '../state/atom';
 
@@ -107,7 +107,7 @@ export interface RealFlipperClient {
     device: string;
     device_id: string;
   };
-  deviceSync: RealFlipperDevice;
+  deviceSync: Device;
   plugins: Set<string>;
   isBackgroundPlugin(pluginId: string): boolean;
   initPlugin(pluginId: string): void;

desktop/flipper-plugin/src/plugin/PluginBase.tsx

@@ -12,7 +12,7 @@ import {EventEmitter} from 'events';
 import {SandyPluginDefinition} from './SandyPluginDefinition';
 import {MenuEntry, NormalizedMenuEntry, normalizeMenuEntry} from './MenuEntry';
 import {FlipperLib} from './FlipperLib';
-import {Device, RealFlipperDevice} from './DevicePlugin';
+import {Device, DeviceLogListener} from './DevicePlugin';
 import {batched} from '../state/batch';
 import {Idler} from '../utils/Idler';
 import {Notification} from './Notification';
@@ -78,6 +78,12 @@ export interface BasePluginClient {
    */
   addMenuEntry(...entry: MenuEntry[]): void;
 
+  /**
+   * Listener that is triggered if the underlying device emits a log message.
+   * Listeners established with this mechanism will automatically be cleaned up during destroy
+   */
+  onDeviceLogEntry(cb: DeviceLogListener): () => void;
+
   /**
    * Creates a Paste (similar to a Github Gist).
    * Facebook only function. Resolves to undefined if creating a paste failed.
@@ -186,7 +192,7 @@ export abstract class BasePluginInstance {
   constructor(
     flipperLib: FlipperLib,
     definition: SandyPluginDefinition,
-    realDevice: RealFlipperDevice,
+    device: Device,
     pluginKey: string,
     initialStates?: Record<string, any>,
   ) {
@@ -194,32 +200,10 @@ export abstract class BasePluginInstance {
     this.definition = definition;
     this.initialStates = initialStates;
     this.pluginKey = pluginKey;
-    if (!realDevice) {
+    if (!device) {
       throw new Error('Illegal State: Device has not yet been loaded');
     }
-    this.device = {
-      realDevice, // TODO: temporarily, clean up T70688226
-      // N.B. we model OS as string, not as enum, to make custom device types possible in the future
-      os: realDevice.os,
-      serial: realDevice.serial,
-      get isArchived() {
-        return realDevice.isArchived;
-      },
-      get isConnected() {
-        return realDevice.connected.get();
-      },
-      deviceType: realDevice.deviceType,
-      onLogEntry: (cb) => {
-        const handle = realDevice.addLogListener(cb);
-        this.logListeners.push(handle);
-        return () => {
-          realDevice.removeLogListener(handle);
-        };
-      },
-      executeShell(command: string): Promise<string> {
-        return realDevice.executeShell(command);
-      },
-    };
+    this.device = device;
   }
 
   protected initializePlugin(factory: () => any) {
@@ -325,6 +309,13 @@ export abstract class BasePluginInstance {
           }
         }
       },
+      onDeviceLogEntry: (cb: DeviceLogListener): (() => void) => {
+        const handle = this.device.addLogListener(cb);
+        this.logListeners.push(handle);
+        return () => {
+          this.device.removeLogListener(handle);
+        };
+      },
       writeTextToClipboard: this.flipperLib.writeTextToClipboard,
       createPaste: this.flipperLib.createPaste,
       isFB: this.flipperLib.isFB,
@@ -364,7 +355,7 @@ export abstract class BasePluginInstance {
     this.assertNotDestroyed();
     this.deactivate();
     this.logListeners.splice(0).forEach((handle) => {
-      this.device.realDevice.removeLogListener(handle);
+      this.device.removeLogListener(handle);
     });
     this.events.emit('destroy');
     this.destroyed = true;

desktop/flipper-plugin/src/test-utils/test-utils.tsx

@@ -32,7 +32,7 @@ import {act} from '@testing-library/react';
 import {
   DeviceLogEntry,
   SandyDevicePluginInstance,
-  RealFlipperDevice,
+  Device,
   DeviceLogListener,
 } from '../plugin/DevicePlugin';
 import {BasePluginInstance} from '../plugin/PluginBase';
@@ -486,7 +486,7 @@ export function createMockBundledPluginDetails(
   };
 }
 
-function createMockDevice(options?: StartPluginOptions): RealFlipperDevice & {
+function createMockDevice(options?: StartPluginOptions): Device & {
   addLogEntry(entry: DeviceLogEntry): void;
 } {
   const logListeners: (undefined | DeviceLogListener)[] = [];
@@ -507,6 +507,14 @@ function createMockDevice(options?: StartPluginOptions): RealFlipperDevice & {
       logListeners.forEach((f) => f?.(entry));
     },
     executeShell: jest.fn(),
+    clearLogs: jest.fn(),
+    forwardPort: jest.fn(),
+    get isConnected() {
+      return this.connected.get();
+    },
+    navigateToLocation: jest.fn(),
+    screenshot: jest.fn(),
+    sendMetroCommand: jest.fn(),
   };
 }
 

desktop/flipper-plugin/src/types/server-types.tsx

@@ -87,7 +87,9 @@ export type FlipperServerCommands = {
     serial: string,
     local: string,
     remote: string,
-  ) => Promise<void>;
+  ) => Promise<boolean>;
+  'device-clear-logs': (serial: string) => Promise<void>;
+  'device-navigate': (serial: string, location: string) => Promise<void>;
   'metro-command': (serial: string, command: string) => Promise<void>;
 };
 

desktop/plugins/public/crash_reporter/android-crash-utils.tsx

@@ -59,7 +59,7 @@ export function startAndroidCrashWatcher(
   let androidLog: string = '';
   let androidLogUnderProcess = false;
   let timer: null | NodeJS.Timeout = null;
-  client.device.onLogEntry((entry: DeviceLogEntry) => {
+  client.onDeviceLogEntry((entry: DeviceLogEntry) => {
     if (shouldParseAndroidLog(entry, referenceDate)) {
       if (androidLogUnderProcess) {
         androidLog += '\n' + entry.message;

desktop/plugins/public/logs/index.tsx

@@ -156,7 +156,7 @@ export function devicePlugin(client: DevicePluginClient) {
     if (isPaused.get() && client.device.isConnected) {
       // start listening to the logs
       isPaused.set(false);
-      logDisposer = client.device.onLogEntry((entry: DeviceLogEntry) => {
+      logDisposer = client.onDeviceLogEntry((entry: DeviceLogEntry) => {
         const lastIndex = rows.size - 1;
         const previousRow = rows.get(lastIndex);
         if (
@@ -182,9 +182,8 @@ export function devicePlugin(client: DevicePluginClient) {
     }
   }
 
-  function clearLogs() {
-    // Non public Android specific api
-    (client.device.realDevice as any)?.clearLogs?.();
+  async function clearLogs() {
+    await client.device.clearLogs();
     rows.clear();
     tableManagerRef.current?.clearSelection();
   }

desktop/plugins/public/navigation/plugin.tsx

@@ -73,7 +73,7 @@ export function plugin(client: PluginClient<Events, Methods>) {
       draft.unshift(navigationEvent);
     });
 
-    const screenshot: Buffer = await client.device.realDevice.screenshot();
+    const screenshot: Buffer = await client.device.screenshot();
     const blobURL = URL.createObjectURL(bufferToBlob(screenshot));
     // this process is async, make sure we update the correct one..
     const navigationEventIndex = navigationEvents
@@ -86,7 +86,7 @@ export function plugin(client: PluginClient<Events, Methods>) {
     }
   });
 
-  getAppMatchPatterns(client.appId, client.device.realDevice)
+  getAppMatchPatterns(client.appId, client.device)
     .then((patterns) => {
       appMatchPatterns.set(patterns);
     })
@@ -111,7 +111,7 @@ export function plugin(client: PluginClient<Events, Methods>) {
           url: filterOptionalParameters(filteredQuery),
         });
       } else {
-        client.device.realDevice.navigateToLocation(
+        client.device.navigateToLocation(
           filterOptionalParameters(filteredQuery),
         );
       }

desktop/plugins/public/navigation/util/appMatchPatterns.tsx

@@ -9,8 +9,9 @@
 
 import fs from 'fs';
 import path from 'path';
-import {BaseDevice, getAppPath} from 'flipper';
+import {getAppPath} from 'flipper';
 import {AppMatchPattern} from '../types';
+import {Device} from 'flipper-plugin';
 
 let patternsPath: string | undefined;
 
@@ -28,7 +29,7 @@ const extractAppNameFromSelectedApp = (selectedApp: string | null) => {
 
 export const getAppMatchPatterns = (
   selectedApp: string | null,
-  device: BaseDevice,
+  device: Device,
 ) => {
   return new Promise<Array<AppMatchPattern>>((resolve, reject) => {
     const appName = extractAppNameFromSelectedApp(selectedApp);

desktop/plugins/public/reactdevtools/index.tsx

@@ -29,10 +29,6 @@ const DEV_TOOLS_NODE_ID = 'reactdevtools-out-of-react-node';
 const CONNECTED = 'DevTools connected';
 const DEV_TOOLS_PORT = 8097; // hardcoded in RN
 
-interface MetroDevice {
-  sendMetroCommand(command: string, params?: any): void;
-}
-
 function findGlobalDevTools(): Promise<string | undefined> {
   return new Promise((resolve) => {
     child_process.exec('npm root -g', (error, basePath) => {
@@ -64,10 +60,7 @@ enum ConnectionStatus {
 }
 
 export function devicePlugin(client: DevicePluginClient) {
-  const metroDevice: MetroDevice = client.device.realDevice;
-  if (!metroDevice.sendMetroCommand) {
-    throw new Error('Invalid metroDevice');
-  }
+  const metroDevice = client.device;
 
   const statusMessage = createState('initializing');
   const connectionStatus = createState<ConnectionStatus>(

desktop/types/adbkit.d.ts

@@ -69,7 +69,11 @@ declare module 'adbkit' {
     getProperties: (serial: string) => Promise<{[key: string]: string}>;
     trackDevices: () => Promise<DeviceTracker>;
     kill: () => Promise<boolean>;
-    forward: (serial: string, local: string, remote: string) => Promise<void>;
+    forward: (
+      serial: string,
+      local: string,
+      remote: string,
+    ) => Promise<boolean>; // TODO: verify correctness of signature
   }
   export function createClient(config: {port: number; host: string}): Client;
 }