diff --git a/desktop/app/src/__tests__/PluginContainer.node.tsx b/desktop/app/src/__tests__/PluginContainer.node.tsx index a0fe4cbf0..a6e124dd9 100644 --- a/desktop/app/src/__tests__/PluginContainer.node.tsx +++ b/desktop/app/src/__tests__/PluginContainer.node.tsx @@ -7,18 +7,17 @@ * @format */ -import React, {useContext} from 'react'; +import React from 'react'; import produce from 'immer'; import {FlipperPlugin} from '../plugin'; import {renderMockFlipperWithPlugin} from '../test-utils/createMockFlipperWithPlugin'; import { - SandyPluginContext, SandyPluginDefinition, FlipperClient, TestUtils, usePlugin, } from 'flipper-plugin'; -import {selectPlugin} from '../reducers/connections'; +import {selectPlugin, starPlugin} from '../reducers/connections'; interface PersistedState { count: 1; @@ -132,6 +131,9 @@ test('PluginContainer can render Sandy plugins', async () => { client, store, } = await renderMockFlipperWithPlugin(definition); + + expect(client.rawSend).toBeCalledWith('init', {plugin: 'TestPlugin'}); + expect(renderer.baseElement).toMatchInlineSnapshot(`
@@ -164,8 +166,6 @@ test('PluginContainer can render Sandy plugins', async () => { expect(pluginInstance.connectedStub).toBeCalledTimes(1); expect(pluginInstance.disconnectedStub).toBeCalledTimes(0); - // TODO: check that messages have arrived T68683442 - // select non existing plugin act(() => { store.dispatch( @@ -176,6 +176,8 @@ test('PluginContainer can render Sandy plugins', async () => { ); }); + expect(client.rawSend).toBeCalledWith('deinit', {plugin: 'TestPlugin'}); + expect(renderer.baseElement).toMatchInlineSnapshot(`
@@ -195,4 +197,36 @@ test('PluginContainer can render Sandy plugins', async () => { }); expect(pluginInstance.connectedStub).toBeCalledTimes(2); expect(pluginInstance.disconnectedStub).toBeCalledTimes(1); + expect(client.rawSend).toBeCalledWith('init', {plugin: 'TestPlugin'}); + + // disable + act(() => { + store.dispatch( + starPlugin({ + plugin: definition, + selectedApp: client.query.app, + }), + ); + }); + expect(pluginInstance.connectedStub).toBeCalledTimes(2); + expect(pluginInstance.disconnectedStub).toBeCalledTimes(2); + expect(client.rawSend).toBeCalledWith('deinit', {plugin: 'TestPlugin'}); + + // 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(2); + expect(pluginInstance.disconnectedStub).toBeCalledTimes(2); + + expect( + client.sandyPluginStates.get('TestPlugin')!.instanceApi.connectedStub, + ).toBeCalledTimes(1); + expect(client.rawSend).toBeCalledWith('init', {plugin: 'TestPlugin'}); }); diff --git a/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx b/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx index d9af3ad90..2d67f36f9 100644 --- a/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx +++ b/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx @@ -137,6 +137,7 @@ export async function createMockFlipperWithPlugin( ); } }; + client.rawSend = jest.fn(); // enable the plugin if ( diff --git a/desktop/flipper-plugin/src/index.ts b/desktop/flipper-plugin/src/index.ts index a92139618..a7b6cf478 100644 --- a/desktop/flipper-plugin/src/index.ts +++ b/desktop/flipper-plugin/src/index.ts @@ -13,7 +13,7 @@ export {SandyPluginInstance, FlipperClient} from './plugin/Plugin'; export {SandyPluginDefinition} from './plugin/SandyPluginDefinition'; export {SandyPluginRenderer} from './plugin/PluginRenderer'; export {SandyPluginContext, usePlugin} from './plugin/PluginContext'; -export {createState as createValue, useValue, Atom} from './state/atom'; +export {createState, useValue, Atom} from './state/atom'; // It's not ideal that this exists in flipper-plugin sources directly, // but is the least pain for plugin authors. diff --git a/desktop/flipper-plugin/src/plugin/Plugin.tsx b/desktop/flipper-plugin/src/plugin/Plugin.tsx index 2c0c29183..b71a6bad3 100644 --- a/desktop/flipper-plugin/src/plugin/Plugin.tsx +++ b/desktop/flipper-plugin/src/plugin/Plugin.tsx @@ -149,7 +149,11 @@ export class SandyPluginInstance { // the plugin is deselected in the UI deactivate() { - this.assertNotDestroyed(); + 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; + } const pluginId = this.definition.id; if (!this.realClient.isBackgroundPlugin(pluginId)) { this.realClient.deinitPlugin(pluginId); @@ -174,7 +178,9 @@ export class SandyPluginInstance { destroy() { this.assertNotDestroyed(); - this.disconnect(); + if (this.connected) { + this.realClient.deinitPlugin(this.definition.id); + } this.events.emit('destroy'); this.destroyed = true; } diff --git a/desktop/flipper-plugin/src/plugin/PluginRenderer.tsx b/desktop/flipper-plugin/src/plugin/PluginRenderer.tsx index 8880e03cd..2a16d8d17 100644 --- a/desktop/flipper-plugin/src/plugin/PluginRenderer.tsx +++ b/desktop/flipper-plugin/src/plugin/PluginRenderer.tsx @@ -18,24 +18,17 @@ type Props = { /** * Component to render a Sandy plugin container */ -export const SandyPluginRenderer = memo( - ({plugin}: Props) => { - useEffect(() => { - plugin.activate(); - return () => { - plugin.deactivate(); - }; - }, [plugin]); +export const SandyPluginRenderer = memo(({plugin}: Props) => { + useEffect(() => { + plugin.activate(); + return () => { + plugin.deactivate(); + }; + }, [plugin]); - return ( - - {createElement(plugin.definition.module.Component)} - - ); - }, - () => { - // One of the goals of the ModernPluginContainer is that we want to prevent it from rendering - // for any outside change. Whatever happens outside of us, we don't care. If it is relevant for use, we take care about it from the insde - return true; - }, -); + return ( + + {createElement(plugin.definition.module.Component)} + + ); +}); diff --git a/desktop/flipper-plugin/src/test-utils/test-utils.tsx b/desktop/flipper-plugin/src/test-utils/test-utils.tsx index e1e55dd70..990cb295e 100644 --- a/desktop/flipper-plugin/src/test-utils/test-utils.tsx +++ b/desktop/flipper-plugin/src/test-utils/test-utils.tsx @@ -117,8 +117,12 @@ export function startPlugin>( // as from testing perspective the difference shouldn't matter return false; }, - initPlugin(_pluginId: string) {}, - deinitPlugin(_pluginId: string) {}, + initPlugin(_pluginId: string) { + pluginInstance.connect(); + }, + deinitPlugin(_pluginId: string) { + pluginInstance.disconnect(); + }, call( api: string, method: string, @@ -131,7 +135,7 @@ export function startPlugin>( const pluginInstance = new SandyPluginInstance(fakeFlipper, definition); // we start connected - pluginInstance.connect(); + pluginInstance.activate(); const res: StartPluginResult = { module,