From ba01fa5bc9507e6d10f496b64c84c4bf2bf69dbd Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Wed, 1 Jul 2020 08:58:40 -0700 Subject: [PATCH] Introduce `onDestroy` hook Summary: This diff introduces the `onDestroy` hook that can be used by plugins to listen to the event where a plugin is cleaned up (either because it is disabled, or because the client is being cleaned up) Reviewed By: jknoxville Differential Revision: D22208121 fbshipit-source-id: 9c4951ae671be611f21da171c548d4054c481166 --- .../reducers/__tests__/sandyplugins.node.tsx | 49 ++++++-- desktop/flipper-plugin/src/index.ts | 1 + desktop/flipper-plugin/src/plugin/Plugin.tsx | 111 ++++-------------- .../src/plugin/SandyPluginDefinition.tsx | 97 +++++++++++++++ 4 files changed, 158 insertions(+), 100 deletions(-) create mode 100644 desktop/flipper-plugin/src/plugin/SandyPluginDefinition.tsx diff --git a/desktop/app/src/reducers/__tests__/sandyplugins.node.tsx b/desktop/app/src/reducers/__tests__/sandyplugins.node.tsx index 9cc5c2a5d..c548769e8 100644 --- a/desktop/app/src/reducers/__tests__/sandyplugins.node.tsx +++ b/desktop/app/src/reducers/__tests__/sandyplugins.node.tsx @@ -11,7 +11,11 @@ import {createMockFlipperWithPlugin} from '../../test-utils/createMockFlipperWit import {Store, Client} from '../../'; import {selectPlugin, starPlugin} from '../../reducers/connections'; import {registerPlugins} from '../../reducers/plugins'; -import {SandyPluginDefinition, SandyPluginInstance} from 'flipper-plugin'; +import { + SandyPluginDefinition, + SandyPluginInstance, + FlipperClient, +} from 'flipper-plugin'; interface PersistedState { count: 1; @@ -33,8 +37,17 @@ const pluginDetails = { let TestPlugin: SandyPluginDefinition; beforeEach(() => { + function plugin(client: FlipperClient) { + const destroyStub = jest.fn(); + + client.onDestroy(destroyStub); + + return { + destroyStub, + }; + } TestPlugin = new SandyPluginDefinition(pluginDetails, { - plugin: jest.fn().mockImplementation(() => ({})), + plugin: jest.fn().mockImplementation(plugin) as typeof plugin, Component: jest.fn().mockImplementation(() => null), }); }); @@ -76,14 +89,25 @@ test('it should cleanup a plugin if disabled', async () => { const {client, store} = await createMockFlipperWithPlugin(TestPlugin); expect(TestPlugin.module.plugin).toBeCalledTimes(1); - expect(client.sandyPluginStates.get(TestPlugin.id)).toBeInstanceOf( - SandyPluginInstance, - ); + const pluginInstance = client.sandyPluginStates.get(TestPlugin.id)!; + expect(pluginInstance).toBeInstanceOf(SandyPluginInstance); + expect(pluginInstance.instanceApi.destroyStub).toHaveBeenCalledTimes(0); // unstar starTestPlugin(store, client); - expect(client.sandyPluginStates.get(TestPlugin.id)).toBeUndefined(); - // TODO: make sure onDestroy is called T68683507 + expect(client.sandyPluginStates.has(TestPlugin.id)).toBeFalsy(); + expect(pluginInstance.instanceApi.destroyStub).toHaveBeenCalledTimes(1); +}); + +test('it should cleanup if client is removed', async () => { + const {client} = await createMockFlipperWithPlugin(TestPlugin); + const pluginInstance = client.sandyPluginStates.get(TestPlugin.id)!; + expect(pluginInstance.instanceApi.destroyStub).toHaveBeenCalledTimes(0); + + // close client + client.close(); + expect(client.sandyPluginStates.has(TestPlugin.id)).toBeFalsy(); + expect(pluginInstance.instanceApi.destroyStub).toHaveBeenCalledTimes(1); }); test('it should not initialize a sandy plugin if not enabled', async () => { @@ -96,7 +120,11 @@ test('it should not initialize a sandy plugin if not enabled', async () => { id: 'Plugin2', }, { - plugin: jest.fn().mockImplementation(() => ({})), + plugin: jest.fn().mockImplementation((client) => { + const destroyStub = jest.fn(); + client.onDestroy(destroyStub); + return {destroyStub}; + }), Component() { return null; }, @@ -121,10 +149,13 @@ test('it should not initialize a sandy plugin if not enabled', async () => { expect(client.sandyPluginStates.get(Plugin2.id)).toBeInstanceOf( SandyPluginInstance, ); + const destroyStub = client.sandyPluginStates.get(Plugin2.id)!.instanceApi + .destroyStub; expect(client.sandyPluginStates.get(TestPlugin.id)).toBe(pluginState1); // not reinitialized expect(TestPlugin.module.plugin).toBeCalledTimes(1); expect(Plugin2.module.plugin).toBeCalledTimes(1); + expect(destroyStub).toHaveBeenCalledTimes(0); // disable plugin again store.dispatch( @@ -135,7 +166,7 @@ test('it should not initialize a sandy plugin if not enabled', async () => { ); expect(client.sandyPluginStates.get(Plugin2.id)).toBeUndefined(); - // TODO: test destroy hook is called T68683507 + expect(destroyStub).toHaveBeenCalledTimes(1); }); // TODO: T68683449 state is persisted if a plugin connects and reconnects diff --git a/desktop/flipper-plugin/src/index.ts b/desktop/flipper-plugin/src/index.ts index b001bc7c8..3df328525 100644 --- a/desktop/flipper-plugin/src/index.ts +++ b/desktop/flipper-plugin/src/index.ts @@ -8,3 +8,4 @@ */ export * from './plugin/Plugin'; +export * from './plugin/SandyPluginDefinition'; diff --git a/desktop/flipper-plugin/src/plugin/Plugin.tsx b/desktop/flipper-plugin/src/plugin/Plugin.tsx index 65fe2ce48..1f401b6d4 100644 --- a/desktop/flipper-plugin/src/plugin/Plugin.tsx +++ b/desktop/flipper-plugin/src/plugin/Plugin.tsx @@ -7,7 +7,8 @@ * @format */ -import {PluginDetails} from 'flipper-plugin-lib'; +import {SandyPluginDefinition} from './SandyPluginDefinition'; +import {EventEmitter} from 'events'; type EventsContract = Record; type MethodsContract = Record Promise>; @@ -16,13 +17,18 @@ type MethodsContract = Record Promise>; * API available to a plugin factory */ export interface FlipperClient< - Events extends EventsContract, - Methods extends MethodsContract -> {} + Events extends EventsContract = {}, + Methods extends MethodsContract = {} +> { + /** + * the onDestroy event is fired whenever a client is unloaded from Flipper, or a plugin is disabled. + */ + onDestroy(cb: () => void): void; +} /** * Internal API exposed by Flipper, and wrapped by FlipperPluginInstance to be passed to the - * Plugin Factory + * Plugin Factory. For internal purposes only */ interface RealFlipperClient {} @@ -33,15 +39,6 @@ export type FlipperPluginFactory< export type FlipperPluginComponent = React.FC<{}>; -export type FlipperPluginModule = { - /** the factory function that initializes a plugin instance */ - plugin: FlipperPluginFactory; - /** the component type that can render this plugin */ - Component: FlipperPluginComponent; - // TODO: support device plugins T68738317 - // devicePlugin: FlipperPluginFactory -}; - export class SandyPluginInstance { /** base client provided by Flipper */ realClient: RealFlipperClient; @@ -50,7 +47,9 @@ export class SandyPluginInstance { /** the original plugin definition */ definition: SandyPluginDefinition; /** the plugin instance api as used inside components and such */ - instanceApi: object; + instanceApi: any; + + events = new EventEmitter(); constructor( realClient: RealFlipperClient, @@ -58,7 +57,11 @@ export class SandyPluginInstance { ) { this.realClient = realClient; this.definition = definition; - this.client = {}; + this.client = { + onDestroy: (cb) => { + this.events.on('destroy', cb); + }, + }; this.instanceApi = definition.module.plugin(this.client); } @@ -71,84 +74,10 @@ export class SandyPluginInstance { } destroy() { - // TODO: T68683507 + this.events.emit('destroy'); } toJSON() { // TODO: T68683449 } } - -/** - * A sandy plugin definitions represents a loaded plugin definition, storing two things: - * the loaded JS module, and the meta data (typically coming from package.json). - * - * Also delegates some of the standard plugin functionality to have a similar public static api as FlipperPlugin - */ -export class SandyPluginDefinition { - id: string; - module: FlipperPluginModule; - details: PluginDetails; - - // TODO: Implement T68683449 - exportPersistedState: - | (( - callClient: (method: string, params?: any) => Promise, - persistedState: any, // TODO: type StaticPersistedState | undefined, - store: any, // TODO: ReduxState | undefined, - idler?: any, // TODO: Idler, - statusUpdate?: (msg: string) => void, - supportsMethod?: (method: string) => Promise, - ) => Promise) - | undefined = undefined; - - constructor(details: PluginDetails, module: FlipperPluginModule) { - this.id = details.id; - this.details = details; - if (!module.plugin || typeof module.plugin !== 'function') { - throw new Error( - `Flipper plugin '${this.id}' should export named function called 'plugin'`, - ); - } - if (!module.Component || typeof module.Component !== 'function') { - throw new Error( - `Flipper plugin '${this.id}' should export named function called 'Component'`, - ); - } - this.module = module; - this.module.Component.displayName = `FlipperPlugin(${this.id})`; - } - - get packageName() { - return this.details.name; - } - - get title() { - return this.details.title; - } - - get icon() { - return this.details.icon; - } - - get category() { - return this.details.category; - } - - get gatekeeper() { - return this.details.gatekeeper; - } - - get version() { - return this.details.version; - } - - get isDefault() { - return this.details.isDefault; - } - - get keyboardActions() { - // TODO: T68882551 support keyboard actions - return []; - } -} diff --git a/desktop/flipper-plugin/src/plugin/SandyPluginDefinition.tsx b/desktop/flipper-plugin/src/plugin/SandyPluginDefinition.tsx new file mode 100644 index 000000000..42819ca86 --- /dev/null +++ b/desktop/flipper-plugin/src/plugin/SandyPluginDefinition.tsx @@ -0,0 +1,97 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + */ + +import {PluginDetails} from 'flipper-plugin-lib'; +import {FlipperPluginFactory, FlipperPluginComponent} from './Plugin'; + +/** + * FlipperPluginModule describe the exports that are provided by a typical Flipper Desktop plugin + */ +export type FlipperPluginModule = { + /** the factory function that initializes a plugin instance */ + plugin: FlipperPluginFactory; + /** the component type that can render this plugin */ + Component: FlipperPluginComponent; + // TODO: support device plugins T68738317 + // devicePlugin: FlipperPluginFactory +}; + +/** + * A sandy plugin definitions represents a loaded plugin definition, storing two things: + * the loaded JS module, and the meta data (typically coming from package.json). + * + * Also delegates some of the standard plugin functionality to have a similar public static api as FlipperPlugin + */ +export class SandyPluginDefinition { + id: string; + module: FlipperPluginModule; + details: PluginDetails; + + // TODO: Implement T68683449 + exportPersistedState: + | (( + callClient: (method: string, params?: any) => Promise, + persistedState: any, // TODO: type StaticPersistedState | undefined, + store: any, // TODO: ReduxState | undefined, + idler?: any, // TODO: Idler, + statusUpdate?: (msg: string) => void, + supportsMethod?: (method: string) => Promise, + ) => Promise) + | undefined = undefined; + + constructor(details: PluginDetails, module: FlipperPluginModule) { + this.id = details.id; + this.details = details; + if (!module.plugin || typeof module.plugin !== 'function') { + throw new Error( + `Flipper plugin '${this.id}' should export named function called 'plugin'`, + ); + } + if (!module.Component || typeof module.Component !== 'function') { + throw new Error( + `Flipper plugin '${this.id}' should export named function called 'Component'`, + ); + } + this.module = module; + this.module.Component.displayName = `FlipperPlugin(${this.id})`; + } + + get packageName() { + return this.details.name; + } + + get title() { + return this.details.title; + } + + get icon() { + return this.details.icon; + } + + get category() { + return this.details.category; + } + + get gatekeeper() { + return this.details.gatekeeper; + } + + get version() { + return this.details.version; + } + + get isDefault() { + return this.details.isDefault; + } + + get keyboardActions() { + // TODO: T68882551 support keyboard actions + return []; + } +}