Some Client related cleanups

Summary:
Client up `client.device` (which had no code references anymore) / `client.deviceSync`. Cleaned up feature code for old SDKs (pre 2, which is 3 years old).

This makes decapitating Client a little simpler in the rest of the stack.

Reviewed By: passy

Differential Revision: D31235436

fbshipit-source-id: 919679c1830e2b9368d0787d7b363c090305edb8

desktop/app/src/Client.tsx

@@ -103,12 +103,7 @@ export default class Client extends EventEmitter {
   store: Store;
   activePlugins: Set<string>;
 
-  /**
+  device: BaseDevice;
-   * @deprecated
-   * use plugin.deviceSync instead
-   */
-  device: Promise<BaseDevice>;
-  deviceSync: BaseDevice;
   logger: Logger;
   broadcastCallbacks: Map<string, Map<string, Set<Function>>>;
   messageBuffer: Record<
@@ -120,16 +115,6 @@ export default class Client extends EventEmitter {
   > = {};
   sandyPluginStates = new Map<string /*pluginID*/, _SandyPluginInstance>();
 
-  requestCallbacks: Map<
-    number,
-    {
-      resolve: (data: any) => void;
-      reject: (err: Error) => void;
-      metadata: RequestMetadata;
-      // eslint-disable-next-line prettier/prettier
-    }
-  >;
-
   constructor(
     id: string,
     query: ClientQuery,
@@ -151,11 +136,8 @@ export default class Client extends EventEmitter {
     this.logger = logger;
     this.store = store;
     this.broadcastCallbacks = new Map();
-    this.requestCallbacks = new Map();
     this.activePlugins = new Set();
-
+    this.device = device;
-    this.device = Promise.resolve(device);
-    this.deviceSync = device;
 
     const client = this;
     if (conn) {
@@ -345,14 +327,6 @@ export default class Client extends EventEmitter {
     this.emit('plugins-change');
   }
 
-  /**
-   * @deprecated
-   * use deviceSync.serial
-   */
-  async deviceSerial(): Promise<string> {
-    return this.deviceSync.serial;
-  }
-
   onMessage(msg: string) {
     if (typeof msg !== 'string') {
       return;
@@ -379,7 +353,7 @@ export default class Client extends EventEmitter {
 
       if (isFlipperMessageDebuggingEnabled()) {
         registerFlipperDebugMessage({
-          device: this.deviceSync?.displayTitle(),
+          device: this.device?.displayTitle(),
           app: this.query.app,
           flipperInternalMethod: method,
           plugin: data.params?.api,
@@ -398,7 +372,7 @@ export default class Client extends EventEmitter {
             }: ${error.message} + \nDevice Stack Trace: ${error.stacktrace}`,
             'deviceError',
           );
-          handleError(this.store, this.deviceSync, error);
+          handleError(this.store, this.device, error);
         } else if (method === 'refreshPlugins') {
           this.refreshPlugins();
         } else if (method === 'execute') {
@@ -458,16 +432,6 @@ export default class Client extends EventEmitter {
         }
         return; // method === 'execute'
       }
-
-      if (this.sdkVersion < 1) {
-        const callbacks = this.requestCallbacks.get(id);
-        if (!callbacks) {
-          return;
-        }
-        this.requestCallbacks.delete(id);
-        this.finishTimingRequestResponse(callbacks.metadata);
-        this.onResponse(data, callbacks.resolve, callbacks.reject);
-      }
     });
   }
 
@@ -485,7 +449,7 @@ export default class Client extends EventEmitter {
       reject(data.error);
       const {error} = data;
       if (error) {
-        handleError(this.store, this.deviceSync, error);
+        handleError(this.store, this.device, error);
       }
     } else {
       // ???
@@ -533,10 +497,6 @@ export default class Client extends EventEmitter {
         params,
       };
 
-      if (this.sdkVersion < 1) {
-        this.requestCallbacks.set(id, {reject, resolve, metadata});
-      }
-
       const data = {
         id,
         method,
@@ -547,14 +507,6 @@ export default class Client extends EventEmitter {
 
       console.debug(data, 'message:call');
 
-      if (this.sdkVersion < 1) {
-        this.startTimingRequestResponse({method, id, params});
-        if (this.connection) {
-          this.connection.send(data);
-        }
-        return;
-      }
-
       const mark = this.getPerformanceMark(metadata);
       performance.mark(mark);
       if (!this.connected.get()) {
@@ -578,7 +530,7 @@ export default class Client extends EventEmitter {
 
             if (isFlipperMessageDebuggingEnabled()) {
               registerFlipperDebugMessage({
-                device: this.deviceSync?.displayTitle(),
+                device: this.device?.displayTitle(),
                 app: this.query.app,
                 flipperInternalMethod: method,
                 payload: response,
@@ -604,7 +556,7 @@ export default class Client extends EventEmitter {
 
       if (isFlipperMessageDebuggingEnabled()) {
         registerFlipperDebugMessage({
-          device: this.deviceSync?.displayTitle(),
+          device: this.device?.displayTitle(),
           app: this.query.app,
           flipperInternalMethod: method,
           plugin: params?.api,
@@ -690,7 +642,7 @@ export default class Client extends EventEmitter {
 
     if (isFlipperMessageDebuggingEnabled()) {
       registerFlipperDebugMessage({
-        device: this.deviceSync?.displayTitle(),
+        device: this.device?.displayTitle(),
         app: this.query.app,
         flipperInternalMethod: method,
         payload: params,
@@ -749,9 +701,6 @@ export default class Client extends EventEmitter {
   }
 
   async supportsMethod(api: string, method: string): Promise<boolean> {
-    if (this.sdkVersion < 2) {
-      return Promise.resolve(false);
-    }
     const response = await this.rawCall<{
       isSupported: boolean;
     }>('isMethodSupported', true, {

desktop/app/src/dispatcher/flipperServer.tsx

@@ -173,8 +173,7 @@ export async function handleClientConnected(store: Store, client: Client) {
     payload: client,
   });
 
-  // eslint-disable-next-line node/no-sync
+  const device = client.device;
-  const device = client.deviceSync;
   if (device) {
     store.dispatch(selectDevice(device));
     store.dispatch(selectClient(client.id));

desktop/app/src/dispatcher/handleOpenPluginDeeplink.tsx

@@ -85,7 +85,7 @@ export async function handleOpenPluginDeeplink(store: Store, query: string) {
     : (deviceOrClient as Client);
   const device: BaseDevice = isDevicePlugin
     ? (deviceOrClient as BaseDevice)
-    : (deviceOrClient as Client).deviceSync;
+    : (deviceOrClient as Client).device;
 
   // verify plugin supported by selected device / client
   if (isDevicePlugin && !device.supportsPlugin(pluginDefinition)) {
@@ -442,7 +442,7 @@ async function selectDevicesAndClient(
             : c.plugins.has(params.pluginId),
       )
       .filter((c) => c.connected.get())
-      .filter((c) => availableDevices.includes(c.deviceSync));
+      .filter((c) => availableDevices.includes(c.device));
 
     if (validClients.length === 1) {
       return validClients[0];
@@ -558,7 +558,7 @@ async function selectClientDialog(
       'Multiple applications running this plugin were found, please select one:',
     options: clients.map((c) => ({
       value: c.id,
-      label: `${c.query.app} on ${c.deviceSync.displayTitle()}`,
+      label: `${c.query.app} on ${c.device.displayTitle()}`,
     })),
   });
   // might find nothing if id === false

desktop/app/src/plugin.tsx

@@ -277,16 +277,8 @@ export class FlipperPlugin<
   client: PluginClient;
   realClient: Client;
 
-  /**
-   * @deprecated use .device instead
-   */
-  getDevice(): Promise<BaseDevice> {
-    return this.realClient.device;
-  }
-
   get device() {
-    // eslint-disable-next-line node/no-sync
+    return this.realClient.device;
-    return this.realClient.deviceSync;
   }
 
   _teardown() {

desktop/app/src/sandy-chrome/appinspect/AppSelector.tsx

@@ -202,9 +202,7 @@ function computeEntries(
         // hide non default devices, unless they have a connected client or plugins
         canBeDefaultDevice(device) ||
         device.hasDevicePlugins ||
-        // Deliberate use of Sync.
+        clients.some((c) => c.device === device),
-        // eslint-disable-next-line node/no-sync
-        clients.some((c) => c.deviceSync === device),
     )
     .map((device) => {
       const deviceEntry = (

desktop/app/src/sandy-chrome/notification/Notification.tsx

@@ -217,8 +217,7 @@ export function Notification() {
       activeNotifications.map((noti) => {
         const client = getClientById(store, noti.client);
         const device = client
-          ? // eslint-disable-next-line node/no-sync
+          ? client.device
-            client.deviceSync
           : getDeviceById(store, noti.client);
         const plugin = getPlugin(noti.pluginId);
         return {
@@ -312,8 +311,7 @@ export function openNotification(store: Store, noti: PluginNotificationOrig) {
       selectPlugin({
         selectedPlugin: noti.pluginId,
         selectedApp: noti.client,
-        // eslint-disable-next-line node/no-sync
+        selectedDevice: client.device,
-        selectedDevice: client.deviceSync,
         deepLinkPayload: noti.notification.action,
       }),
     );

desktop/app/src/selectors/connections.tsx

@@ -53,7 +53,7 @@ export const getActiveDevice = createSelector(
     }
     // if there is an active app, use device owning the app
     if (client) {
-      return client.deviceSync;
+      return client.device;
     }
     // if no active app, use the preferred device
     if (userPreferredDevice) {

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

@@ -192,12 +192,6 @@ export default class MockFlipper {
       new Set(supportedPlugins),
       device,
     );
-    // yikes
-    client.device = {
-      then() {
-        return device;
-      },
-    } as any;
     client.rawCall = async (
       method: string,
       _fromPlugin: boolean,

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

@@ -107,7 +107,7 @@ export interface RealFlipperClient {
     device: string;
     device_id: string;
   };
-  deviceSync: Device;
+  device: Device;
   plugins: Set<string>;
   isBackgroundPlugin(pluginId: string): boolean;
   initPlugin(pluginId: string): void;
@@ -147,13 +147,7 @@ export class SandyPluginInstance extends BasePluginInstance {
     pluginKey: string,
     initialStates?: Record<string, any>,
   ) {
-    super(
+    super(flipperLib, definition, realClient.device, pluginKey, initialStates);
-      flipperLib,
-      definition,
-      realClient.deviceSync,
-      pluginKey,
-      initialStates,
-    );
     this.realClient = realClient;
     this.definition = definition;
     const self = this;
@@ -199,7 +193,7 @@ export class SandyPluginInstance extends BasePluginInstance {
       },
       selectPlugin(pluginId: string, deeplink?: unknown) {
         flipperLib.selectPlugin(
-          realClient.deviceSync,
+          realClient.device,
           realClient,
           pluginId,
           deeplink,

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

@@ -204,7 +204,7 @@ export function startPlugin<Module extends FlipperPluginModule<any>>(
       device_id: testDevice.serial,
       os: testDevice.serial,
     },
-    deviceSync: testDevice,
+    device: testDevice,
     isBackgroundPlugin(_pluginId: string) {
       return !!options?.isBackgroundPlugin;
     },