Support activate and deactivate in normal plugins

Summary:
Device plugins have an activate and deactivate hook, that reflects the plugin being selected in the UI. Added these same hooks to client plugins as well. In practice they are called at the same times as `onConnect` and `onDisconnect`, except for background plugins, which connect only once, so it is pretty useful to be still able to make the distinction.

Since there is now quite some common functionality between plugins and device plugins, will clean things a bit up in a next diff

[Interesting] as it explains the difference between the different lifecycle methods of plugins, and the impact of being a background plugin

LIfecycle summary:

1. app connects
2. for background plugins: connect them (`onConnect`)
3. user selects a plugin, triggers `onActivate`. will also trigger `onConnect` the plugin if it _isnt_ a bg plugin
4. user selects a different plugin, triggers `onDeactivate`. will also trigger `onDisconnect` if it isn't a bg plugin.
5. app is unloaded. Triggers `onDisconnect` for bg plugins. Triggers `onDestroy` for all plugins,

Reviewed By: jknoxville

Differential Revision: D22724791

fbshipit-source-id: 9fe2e666eb37fa2e0bd00fa61d78d2d4b1080137

desktop/app/src/PluginContainer.tsx

@@ -248,7 +248,6 @@ class PluginContainer extends PureComponent<Props, State> {
       pluginIsEnabled,
     } = this.props;
     if (!activePlugin || !target || !pluginKey) {
-      console.warn(`No selected plugin. Rendering empty!`);
       return null;
     }
 

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

@@ -101,6 +101,8 @@ test('PluginContainer can render Sandy plugins', async () => {
     expect(Object.keys(sandyApi)).toEqual([
       'connectedStub',
       'disconnectedStub',
+      'activatedStub',
+      'deactivatedStub',
     ]);
     expect(() => {
       // eslint-disable-next-line
@@ -114,9 +116,13 @@ test('PluginContainer can render Sandy plugins', async () => {
   const plugin = (client: FlipperClient) => {
     const connectedStub = jest.fn();
     const disconnectedStub = jest.fn();
+    const activatedStub = jest.fn();
+    const deactivatedStub = jest.fn();
     client.onConnect(connectedStub);
     client.onDisconnect(disconnectedStub);
-    return {connectedStub, disconnectedStub};
+    client.onActivate(activatedStub);
+    client.onDeactivate(deactivatedStub);
+    return {connectedStub, disconnectedStub, activatedStub, deactivatedStub};
   };
 
   const definition = new SandyPluginDefinition(
@@ -167,6 +173,8 @@ test('PluginContainer can render Sandy plugins', async () => {
   )!.instanceApi;
   expect(pluginInstance.connectedStub).toBeCalledTimes(1);
   expect(pluginInstance.disconnectedStub).toBeCalledTimes(0);
+  expect(pluginInstance.activatedStub).toBeCalledTimes(1);
+  expect(pluginInstance.deactivatedStub).toBeCalledTimes(0);
 
   // select non existing plugin
   act(() => {
@@ -187,6 +195,8 @@ test('PluginContainer can render Sandy plugins', async () => {
   `);
   expect(pluginInstance.connectedStub).toBeCalledTimes(1);
   expect(pluginInstance.disconnectedStub).toBeCalledTimes(1);
+  expect(pluginInstance.activatedStub).toBeCalledTimes(1);
+  expect(pluginInstance.deactivatedStub).toBeCalledTimes(1);
 
   // go back
   act(() => {
@@ -199,6 +209,8 @@ test('PluginContainer can render Sandy plugins', async () => {
   });
   expect(pluginInstance.connectedStub).toBeCalledTimes(2);
   expect(pluginInstance.disconnectedStub).toBeCalledTimes(1);
+  expect(pluginInstance.activatedStub).toBeCalledTimes(2);
+  expect(pluginInstance.deactivatedStub).toBeCalledTimes(1);
   expect(client.rawSend).toBeCalledWith('init', {plugin: 'TestPlugin'});
 
   // disable
@@ -212,6 +224,8 @@ test('PluginContainer can render Sandy plugins', async () => {
   });
   expect(pluginInstance.connectedStub).toBeCalledTimes(2);
   expect(pluginInstance.disconnectedStub).toBeCalledTimes(2);
+  expect(pluginInstance.activatedStub).toBeCalledTimes(2);
+  expect(pluginInstance.deactivatedStub).toBeCalledTimes(2);
   expect(client.rawSend).toBeCalledWith('deinit', {plugin: 'TestPlugin'});
 
   // re-enable
@@ -226,6 +240,8 @@ test('PluginContainer can render Sandy plugins', async () => {
   // note: this is the old pluginInstance, so that one is not reconnected!
   expect(pluginInstance.connectedStub).toBeCalledTimes(2);
   expect(pluginInstance.disconnectedStub).toBeCalledTimes(2);
+  expect(pluginInstance.activatedStub).toBeCalledTimes(2);
+  expect(pluginInstance.deactivatedStub).toBeCalledTimes(2);
 
   expect(
     client.sandyPluginStates.get('TestPlugin')!.instanceApi.connectedStub,
@@ -233,6 +249,150 @@ test('PluginContainer can render Sandy plugins', async () => {
   expect(client.rawSend).toBeCalledWith('init', {plugin: 'TestPlugin'});
 });
 
+test('PluginContainer triggers correct lifecycles for background plugin', async () => {
+  function MySandyPlugin() {
+    return <div>Hello from Sandy</div>;
+  }
+
+  const plugin = (client: FlipperClient) => {
+    const connectedStub = jest.fn();
+    const disconnectedStub = jest.fn();
+    const activatedStub = jest.fn();
+    const deactivatedStub = jest.fn();
+    client.onConnect(connectedStub);
+    client.onDisconnect(disconnectedStub);
+    client.onActivate(activatedStub);
+    client.onDeactivate(deactivatedStub);
+    return {connectedStub, disconnectedStub, activatedStub, deactivatedStub};
+  };
+
+  const definition = new SandyPluginDefinition(
+    TestUtils.createMockPluginDetails(),
+    {
+      plugin,
+      Component: MySandyPlugin,
+    },
+  );
+  const {act, client, store} = await renderMockFlipperWithPlugin(definition, {
+    onSend(method) {
+      if (method === 'getBackgroundPlugins') {
+        return {plugins: [definition.id]};
+      }
+    },
+  });
+
+  expect(client.rawSend).toBeCalledWith('init', {plugin: 'TestPlugin'});
+  (client.rawSend as jest.Mock).mockClear();
+  // make sure the plugin gets connected
+  const pluginInstance: ReturnType<typeof plugin> = client.sandyPluginStates.get(
+    definition.id,
+  )!.instanceApi;
+  expect(pluginInstance.connectedStub).toBeCalledTimes(1);
+  expect(pluginInstance.disconnectedStub).toBeCalledTimes(0);
+  expect(pluginInstance.activatedStub).toBeCalledTimes(1);
+  expect(pluginInstance.deactivatedStub).toBeCalledTimes(0);
+
+  // select non existing plugin
+  act(() => {
+    store.dispatch(
+      selectPlugin({
+        selectedPlugin: 'Logs',
+        deepLinkPayload: null,
+      }),
+    );
+  });
+  // bg plugin!
+  expect(client.rawSend).not.toBeCalled();
+  (client.rawSend as jest.Mock).mockClear();
+  expect(pluginInstance.connectedStub).toBeCalledTimes(1);
+  expect(pluginInstance.disconnectedStub).toBeCalledTimes(0);
+  expect(pluginInstance.activatedStub).toBeCalledTimes(1);
+  expect(pluginInstance.deactivatedStub).toBeCalledTimes(1);
+
+  // go back
+  act(() => {
+    store.dispatch(
+      selectPlugin({
+        selectedPlugin: definition.id,
+        deepLinkPayload: null,
+      }),
+    );
+  });
+  expect(pluginInstance.connectedStub).toBeCalledTimes(1);
+  expect(pluginInstance.disconnectedStub).toBeCalledTimes(0);
+  expect(pluginInstance.activatedStub).toBeCalledTimes(2);
+  expect(pluginInstance.deactivatedStub).toBeCalledTimes(1);
+  expect(client.rawSend).not.toBeCalled();
+  (client.rawSend as jest.Mock).mockClear();
+
+  // disable
+  act(() => {
+    store.dispatch(
+      starPlugin({
+        plugin: definition,
+        selectedApp: client.query.app,
+      }),
+    );
+  });
+  expect(pluginInstance.connectedStub).toBeCalledTimes(1);
+  expect(pluginInstance.disconnectedStub).toBeCalledTimes(1);
+  expect(pluginInstance.activatedStub).toBeCalledTimes(2);
+  expect(pluginInstance.deactivatedStub).toBeCalledTimes(2);
+  expect(client.rawSend).toBeCalledWith('deinit', {plugin: 'TestPlugin'});
+  (client.rawSend as jest.Mock).mockClear();
+
+  // select something else
+  act(() => {
+    store.dispatch(
+      selectPlugin({
+        selectedPlugin: 'Logs',
+        deepLinkPayload: null,
+      }),
+    );
+  });
+  // re-enable
+  act(() => {
+    store.dispatch(
+      starPlugin({
+        plugin: definition,
+        selectedApp: client.query.app,
+      }),
+    );
+  });
+  // note: this is the old pluginInstance, so that one is not reconnected!
+  expect(pluginInstance.connectedStub).toBeCalledTimes(1);
+  expect(pluginInstance.disconnectedStub).toBeCalledTimes(1);
+  expect(pluginInstance.activatedStub).toBeCalledTimes(2);
+  expect(pluginInstance.deactivatedStub).toBeCalledTimes(2);
+
+  const newPluginInstance: ReturnType<typeof plugin> = client.sandyPluginStates.get(
+    'TestPlugin',
+  )!.instanceApi;
+  expect(newPluginInstance.connectedStub).toBeCalledTimes(1);
+  expect(newPluginInstance.disconnectedStub).toBeCalledTimes(0);
+  expect(newPluginInstance.activatedStub).toBeCalledTimes(0);
+  expect(newPluginInstance.deactivatedStub).toBeCalledTimes(0);
+  expect(client.rawSend).toBeCalledWith('init', {plugin: 'TestPlugin'});
+  (client.rawSend as jest.Mock).mockClear();
+
+  // select new plugin
+  act(() => {
+    store.dispatch(
+      selectPlugin({
+        selectedPlugin: definition.id,
+        deepLinkPayload: null,
+      }),
+    );
+  });
+
+  expect(newPluginInstance.connectedStub).toBeCalledTimes(1);
+  expect(newPluginInstance.disconnectedStub).toBeCalledTimes(0);
+  expect(newPluginInstance.activatedStub).toBeCalledTimes(1);
+  expect(newPluginInstance.deactivatedStub).toBeCalledTimes(0);
+  expect(client.rawSend).not.toBeCalled();
+  (client.rawSend as jest.Mock).mockClear();
+});
+
 test('PluginContainer + Sandy plugin supports deeplink', async () => {
   const linksSeen: any[] = [];
 

desktop/app/src/index.tsx

@@ -116,7 +116,10 @@ export {default as Orderable} from './ui/components/Orderable';
 export {default as VirtualList} from './ui/components/VirtualList';
 export {Component, PureComponent} from 'react';
 export {default as ContextMenuProvider} from './ui/components/ContextMenuProvider';
-export {default as ContextMenu} from './ui/components/ContextMenu';
+export {
+  default as ContextMenu,
+  MenuTemplate,
+} from './ui/components/ContextMenu';
 export {FileListFile, FileListFiles} from './ui/components/FileList';
 export {default as FileList} from './ui/components/FileList';
 export {default as File} from './ui/components/File';

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

@@ -206,13 +206,14 @@ type Renderer = RenderResult<typeof queries>;
 
 export async function renderMockFlipperWithPlugin(
   pluginClazz: PluginDefinition,
+  options?: MockOptions,
 ): Promise<
   MockFlipperResult & {
     renderer: Renderer;
     act: (cb: () => void) => void;
   }
 > {
-  const args = await createMockFlipperWithPlugin(pluginClazz);
+  const args = await createMockFlipperWithPlugin(pluginClazz, options);
 
   function selectTestPlugin(store: Store, client: Client) {
     store.dispatch(

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

@@ -25,6 +25,8 @@ type Methods = {
 export function plugin(client: FlipperClient<Events, Methods>) {
   const connectStub = jest.fn();
   const disconnectStub = jest.fn();
+  const activateStub = jest.fn();
+  const deactivateStub = jest.fn();
   const destroyStub = jest.fn();
   const state = createState(
     {
@@ -37,6 +39,8 @@ export function plugin(client: FlipperClient<Events, Methods>) {
 
   client.onConnect(connectStub);
   client.onDisconnect(disconnectStub);
+  client.onActivate(activateStub);
+  client.onDeactivate(deactivateStub);
   client.onDestroy(destroyStub);
   client.onMessage('inc', ({delta}) => {
     state.update((draft) => {
@@ -64,6 +68,8 @@ export function plugin(client: FlipperClient<Events, Methods>) {
   }
 
   return {
+    activateStub,
+    deactivateStub,
     connectStub,
     destroyStub,
     disconnectStub,

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

@@ -24,11 +24,15 @@ test('it can start a plugin and lifecycle events', () => {
   // startPlugin starts connected
   expect(instance.connectStub).toBeCalledTimes(1);
   expect(instance.disconnectStub).toBeCalledTimes(0);
+  expect(instance.activateStub).toBeCalledTimes(1);
+  expect(instance.deactivateStub).toBeCalledTimes(0);
   expect(instance.destroyStub).toBeCalledTimes(0);
 
   p.connect(); // noop
   expect(instance.connectStub).toBeCalledTimes(1);
   expect(instance.disconnectStub).toBeCalledTimes(0);
+  expect(instance.activateStub).toBeCalledTimes(1);
+  expect(instance.deactivateStub).toBeCalledTimes(0);
   expect(instance.destroyStub).toBeCalledTimes(0);
 
   p.disconnect();
@@ -38,15 +42,59 @@ test('it can start a plugin and lifecycle events', () => {
   expect(instance.disconnectStub).toBeCalledTimes(1);
   expect(instance.destroyStub).toBeCalledTimes(0);
 
-  p.destroy();
-  expect(instance.connectStub).toBeCalledTimes(2);
+  p.deactivate(); // also disconnects
+  p.activate();
+  expect(instance.connectStub).toBeCalledTimes(3);
   expect(instance.disconnectStub).toBeCalledTimes(2);
+  expect(instance.activateStub).toBeCalledTimes(2);
+  expect(instance.deactivateStub).toBeCalledTimes(1);
+
+  p.destroy();
+  expect(instance.connectStub).toBeCalledTimes(3);
+  expect(instance.disconnectStub).toBeCalledTimes(3);
+  expect(instance.activateStub).toBeCalledTimes(2);
+  expect(instance.deactivateStub).toBeCalledTimes(2);
   expect(instance.destroyStub).toBeCalledTimes(1);
 
   // cannot interact with destroyed plugin
   expect(() => {
     p.connect();
   }).toThrowErrorMatchingInlineSnapshot(`"Plugin has been destroyed already"`);
+  expect(() => {
+    p.activate();
+  }).toThrowErrorMatchingInlineSnapshot(`"Plugin has been destroyed already"`);
+});
+
+test('it can start a plugin and lifecycle events for background plugins', () => {
+  const {instance, ...p} = TestUtils.startPlugin(testPlugin, {
+    isBackgroundPlugin: true,
+  });
+
+  // @ts-expect-error
+  p.bla;
+  // @ts-expect-error
+  instance.bla;
+
+  // startPlugin starts connected
+  expect(instance.connectStub).toBeCalledTimes(1);
+  expect(instance.disconnectStub).toBeCalledTimes(0);
+  expect(instance.activateStub).toBeCalledTimes(1);
+  expect(instance.deactivateStub).toBeCalledTimes(0);
+  expect(instance.destroyStub).toBeCalledTimes(0);
+
+  p.deactivate(); // bg, no disconnection
+  p.activate();
+  expect(instance.connectStub).toBeCalledTimes(1);
+  expect(instance.disconnectStub).toBeCalledTimes(0);
+  expect(instance.activateStub).toBeCalledTimes(2);
+  expect(instance.deactivateStub).toBeCalledTimes(1);
+
+  p.destroy();
+  expect(instance.connectStub).toBeCalledTimes(1);
+  expect(instance.disconnectStub).toBeCalledTimes(1);
+  expect(instance.activateStub).toBeCalledTimes(2);
+  expect(instance.deactivateStub).toBeCalledTimes(2);
+  expect(instance.destroyStub).toBeCalledTimes(1);
 });
 
 test('it can render a plugin', () => {

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

@@ -151,7 +151,10 @@ export class SandyDevicePluginInstance {
   }
 
   deactivate() {
-    if (!this.destroyed && this.activated) {
+    if (this.destroyed) {
+      return;
+    }
+    if (this.activated) {
       this.lastDeeplink = undefined;
       this.activated = false;
       this.events.emit('deactivate');

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

@@ -32,6 +32,16 @@ export interface FlipperClient<
    */
   onDestroy(cb: () => void): void;
 
+  /**
+   * the onActivate event is fired whenever the plugin is actived in the UI
+   */
+  onActivate(cb: () => void): void;
+
+  /**
+   * The counterpart of the `onActivate` handler.
+   */
+  onDeactivate(cb: () => void): void;
+
   /**
    * the onConnect event is fired whenever the plugin is connected to it's counter part on the device.
    * For most plugins this event is fired if the user selects the plugin,
@@ -124,6 +134,7 @@ export class SandyPluginInstance {
   /** the plugin instance api as used inside components and such  */
   instanceApi: any;
 
+  activated = false;
   connected = false;
   destroyed = false;
   events = new EventEmitter();
@@ -146,6 +157,12 @@ export class SandyPluginInstance {
       onDestroy: (cb) => {
         this.events.on('destroy', cb);
       },
+      onActivate: (cb) => {
+        this.events.on('activate', cb);
+      },
+      onDeactivate: (cb) => {
+        this.events.on('deactivate', cb);
+      },
       onConnect: (cb) => {
         this.events.on('connect', cb);
       },
@@ -181,22 +198,30 @@ export class SandyPluginInstance {
   // the plugin is selected in the UI
   activate() {
     this.assertNotDestroyed();
-    const pluginId = this.definition.id;
-    if (!this.realClient.isBackgroundPlugin(pluginId)) {
-      this.realClient.initPlugin(pluginId); // will call connect() if needed
+    if (!this.activated) {
+      this.activated = true;
+      this.events.emit('activate');
+      const pluginId = this.definition.id;
+      if (!this.realClient.isBackgroundPlugin(pluginId)) {
+        this.realClient.initPlugin(pluginId); // will call connect() if needed
+      }
     }
   }
 
   // the plugin is deselected in the UI
   deactivate() {
-    this.lastDeeplink = undefined;
     if (this.destroyed) {
       // this can happen if the plugin is disabled while active in the UI.
       // In that case deinit & destroy is already triggered from the STAR_PLUGIN action
       return;
     }
+    if (this.activated) {
+      this.lastDeeplink = undefined;
+      this.activated = false;
+      this.events.emit('deactivate');
+    }
     const pluginId = this.definition.id;
-    if (!this.realClient.isBackgroundPlugin(pluginId)) {
+    if (this.connected && !this.realClient.isBackgroundPlugin(pluginId)) {
       this.realClient.deinitPlugin(pluginId);
     }
   }
@@ -219,6 +244,7 @@ export class SandyPluginInstance {
 
   destroy() {
     this.assertNotDestroyed();
+    this.deactivate();
     if (this.connected) {
       this.realClient.deinitPlugin(this.definition.id);
     }

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

@@ -40,6 +40,7 @@ type Renderer = RenderResult<typeof queries>;
 interface StartPluginOptions {
   initialState?: Record<string, any>;
   isArchived?: boolean;
+  isBackgroundPlugin?: boolean;
 }
 
 type ExtractClientType<Module extends FlipperPluginModule<any>> = Parameters<
@@ -67,6 +68,15 @@ interface StartPluginResult<Module extends FlipperPluginModule<any>> {
    * module, from which any other exposed methods can be accessed during testing
    */
   module: Module;
+  /**
+   * Emulates the 'onActivate' event (when the user opens the plugin in the UI).
+   * Will also trigger the `onConnect` event for non-background plugins
+   */
+  activate(): void;
+  /**
+   * Emulatese the 'onDeactivate' event
+   */
+  deactivate(): void;
   /**
    * Emulates the 'onConnect' event
    */
@@ -127,7 +137,7 @@ interface StartDevicePluginResult<Module extends FlipperDevicePluginModule> {
    */
   activate(): void;
   /**
-   * Emulatese the 'onDeactivate' event
+   * Emulates the 'onDeactivate' event
    */
   deactivate(): void;
   /**
@@ -164,15 +174,13 @@ export function startPlugin<Module extends FlipperPluginModule<any>>(
 
   const sendStub = jest.fn();
   const fakeFlipper: RealFlipperClient = {
-    isBackgroundPlugin(_pluginId: string) {
-      // we only reason about non-background plugins,
-      // as from testing perspective the difference shouldn't matter
-      return false;
+    isBackgroundPlugin() {
+      return !!options?.isBackgroundPlugin;
     },
-    initPlugin(_pluginId: string) {
+    initPlugin() {
       pluginInstance.connect();
     },
-    deinitPlugin(_pluginId: string) {
+    deinitPlugin() {
       pluginInstance.disconnect();
     },
     call(
@@ -190,12 +198,25 @@ export function startPlugin<Module extends FlipperPluginModule<any>>(
     definition,
     options?.initialState,
   );
-  // we start connected
+  if (options?.isBackgroundPlugin) {
+    pluginInstance.connect(); // otherwise part of activate
+  }
+  // we start activated
   pluginInstance.activate();
 
   const res: StartPluginResult<Module> = {
     module,
     instance: pluginInstance.instanceApi,
+    activate() {
+      pluginInstance.activate();
+      pluginInstance.connect();
+    },
+    deactivate() {
+      pluginInstance.deactivate();
+      if (!fakeFlipper.isBackgroundPlugin) {
+        pluginInstance.disconnect();
+      }
+    },
     connect: () => pluginInstance.connect(),
     disconnect: () => pluginInstance.disconnect(),
     destroy: () => pluginInstance.destroy(),

desktop/plugins/logs/index.tsx

@@ -36,10 +36,10 @@ import {
   createPaste,
   textContent,
   KeyboardActions,
+  MenuTemplate,
 } from 'flipper';
 import LogWatcher from './LogWatcher';
 import React from 'react';
-import {MenuTemplate} from 'app/src/ui/components/ContextMenu';
 
 const LOG_WATCHER_LOCAL_STORAGE_KEY = 'LOG_WATCHER_LOCAL_STORAGE_KEY';