diff --git a/desktop/app/src/PluginContainer.tsx b/desktop/app/src/PluginContainer.tsx index 23b1c8334..d075288c4 100644 --- a/desktop/app/src/PluginContainer.tsx +++ b/desktop/app/src/PluginContainer.tsx @@ -248,7 +248,6 @@ class PluginContainer extends PureComponent { pluginIsEnabled, } = this.props; if (!activePlugin || !target || !pluginKey) { - console.warn(`No selected plugin. Rendering empty!`); return null; } diff --git a/desktop/app/src/__tests__/PluginContainer.node.tsx b/desktop/app/src/__tests__/PluginContainer.node.tsx index d09b0646a..e139838e2 100644 --- a/desktop/app/src/__tests__/PluginContainer.node.tsx +++ b/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
Hello from Sandy
; + } + + 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 = 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 = 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[] = []; diff --git a/desktop/app/src/index.tsx b/desktop/app/src/index.tsx index 0b70c5ccd..570215781 100644 --- a/desktop/app/src/index.tsx +++ b/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'; diff --git a/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx b/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx index 3b5594aa1..a2b33492d 100644 --- a/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx +++ b/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx @@ -206,13 +206,14 @@ type Renderer = RenderResult; 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( diff --git a/desktop/flipper-plugin/src/__tests__/TestPlugin.tsx b/desktop/flipper-plugin/src/__tests__/TestPlugin.tsx index 314c0736c..32a0f49f3 100644 --- a/desktop/flipper-plugin/src/__tests__/TestPlugin.tsx +++ b/desktop/flipper-plugin/src/__tests__/TestPlugin.tsx @@ -25,6 +25,8 @@ type Methods = { export function plugin(client: FlipperClient) { 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) { 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) { } return { + activateStub, + deactivateStub, connectStub, destroyStub, disconnectStub, diff --git a/desktop/flipper-plugin/src/__tests__/test-utils.node.tsx b/desktop/flipper-plugin/src/__tests__/test-utils.node.tsx index 018dc056f..f56cbfdac 100644 --- a/desktop/flipper-plugin/src/__tests__/test-utils.node.tsx +++ b/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', () => { diff --git a/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx b/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx index 7315ace1a..70336869d 100644 --- a/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx +++ b/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'); diff --git a/desktop/flipper-plugin/src/plugin/Plugin.tsx b/desktop/flipper-plugin/src/plugin/Plugin.tsx index 3304daf56..f7f967fca 100644 --- a/desktop/flipper-plugin/src/plugin/Plugin.tsx +++ b/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); } diff --git a/desktop/flipper-plugin/src/test-utils/test-utils.tsx b/desktop/flipper-plugin/src/test-utils/test-utils.tsx index 0d17ed1bd..687a2e899 100644 --- a/desktop/flipper-plugin/src/test-utils/test-utils.tsx +++ b/desktop/flipper-plugin/src/test-utils/test-utils.tsx @@ -40,6 +40,7 @@ type Renderer = RenderResult; interface StartPluginOptions { initialState?: Record; isArchived?: boolean; + isBackgroundPlugin?: boolean; } type ExtractClientType> = Parameters< @@ -67,6 +68,15 @@ interface StartPluginResult> { * 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 { */ activate(): void; /** - * Emulatese the 'onDeactivate' event + * Emulates the 'onDeactivate' event */ deactivate(): void; /** @@ -164,15 +174,13 @@ export function startPlugin>( 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>( definition, options?.initialState, ); - // we start connected + if (options?.isBackgroundPlugin) { + pluginInstance.connect(); // otherwise part of activate + } + // we start activated pluginInstance.activate(); const res: StartPluginResult = { 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(), diff --git a/desktop/plugins/logs/index.tsx b/desktop/plugins/logs/index.tsx index c173e12b6..56b21dcf6 100644 --- a/desktop/plugins/logs/index.tsx +++ b/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';