diff --git a/desktop/app/src/Client.tsx b/desktop/app/src/Client.tsx index 122efd522..65de1bd0e 100644 --- a/desktop/app/src/Client.tsx +++ b/desktop/app/src/Client.tsx @@ -32,7 +32,7 @@ import {sideEffect} from './utils/sideEffect'; import {emitBytesReceived} from './dispatcher/tracking'; import {debounce} from 'lodash'; import {batch} from 'react-redux'; -import {SandyPluginDefinition} from 'flipper-plugin'; +import {SandyPluginDefinition, SandyPluginInstance} from 'flipper-plugin'; type Plugins = Array; @@ -135,6 +135,7 @@ export default class Client extends EventEmitter { messages: Params[]; } > = {}; + sandyPluginStates = new Map(); requestCallbacks: Map< number, @@ -253,18 +254,26 @@ export default class Client extends EventEmitter { return this.backgroundPlugins.includes(pluginId); } + isEnabledPlugin(pluginId: string) { + return this.store + .getState() + .connections.userStarredPlugins[this.query.app]?.includes(pluginId); + } + shouldConnectAsBackgroundPlugin(pluginId: string) { return ( defaultEnabledBackgroundPlugins.includes(pluginId) || - this.store - .getState() - .connections.userStarredPlugins[this.query.app]?.includes(pluginId) + this.isEnabledPlugin(pluginId) ); } async init() { this.setMatchingDevice(); await this.loadPlugins(); + // this starts all sandy enabled plugin + this.plugins.forEach((pluginId) => + this.startPluginIfNeeded(this.getPlugin(pluginId)), + ); this.backgroundPlugins = await this.getBackgroundPlugins(); this.backgroundPlugins.forEach((plugin) => { if (this.shouldConnectAsBackgroundPlugin(plugin)) { @@ -298,6 +307,47 @@ export default class Client extends EventEmitter { return plugins; } + startPluginIfNeeded( + plugin: PluginDefinition | undefined, + isEnabled = plugin ? this.isEnabledPlugin(plugin.id) : false, + ) { + // start a plugin on start if it is a SandyPlugin, which is starred, and doesn't have persisted state yet + if ( + plugin instanceof SandyPluginDefinition && + isEnabled && + !this.sandyPluginStates.has(plugin.id) + ) { + // TODO: needs to be wrapped in error tracking T68955280 + // TODO: pick up any existing persisted state T68683449 + this.sandyPluginStates.set( + plugin.id, + new SandyPluginInstance(this, plugin), + ); + } + } + + stopPluginIfNeeded(pluginId: string) { + const instance = this.sandyPluginStates.get(pluginId); + if (instance) { + instance.destroy(); + // TODO: make sure persisted state is writtenT68683449 + this.sandyPluginStates.delete(pluginId); + } + } + + close() { + this.emit('close'); + this.plugins.forEach((pluginId) => this.stopPluginIfNeeded(pluginId)); + } + + // gets a plugin by pluginId + getPlugin(pluginId: string): PluginDefinition | undefined { + const plugins = this.store.getState().plugins; + return ( + plugins.clientPlugins.get(pluginId) || plugins.devicePlugins.get(pluginId) + ); + } + // get the supported background plugins async getBackgroundPlugins(): Promise { if (this.sdkVersion < 4) { @@ -313,6 +363,9 @@ export default class Client extends EventEmitter { async refreshPlugins() { const oldBackgroundPlugins = this.backgroundPlugins; await this.loadPlugins(); + this.plugins.forEach((pluginId) => + this.startPluginIfNeeded(this.getPlugin(pluginId)), + ); const newBackgroundPlugins = await this.getBackgroundPlugins(); this.backgroundPlugins = newBackgroundPlugins; // diff the background plugin list, disconnect old, connect new ones @@ -616,9 +669,11 @@ export default class Client extends EventEmitter { initPlugin(pluginId: string) { this.activePlugins.add(pluginId); this.rawSend('init', {plugin: pluginId}); + // TODO: call sandyOnConnect } deinitPlugin(pluginId: string) { + // TODO: call sandyOnDisconnect this.activePlugins.delete(pluginId); this.rawSend('deinit', {plugin: pluginId}); } diff --git a/desktop/app/src/PluginContainer.tsx b/desktop/app/src/PluginContainer.tsx index 765611996..82fdd275b 100644 --- a/desktop/app/src/PluginContainer.tsx +++ b/desktop/app/src/PluginContainer.tsx @@ -274,7 +274,7 @@ class PluginContainer extends PureComponent { toggled={false} onClick={() => { this.props.starPlugin({ - selectedPlugin: activePlugin.id, + plugin: activePlugin, selectedApp: (this.props.target as Client).query.app, }); }} diff --git a/desktop/app/src/chrome/mainsidebar/MainSidebar2.tsx b/desktop/app/src/chrome/mainsidebar/MainSidebar2.tsx index 510893960..053c8bc44 100644 --- a/desktop/app/src/chrome/mainsidebar/MainSidebar2.tsx +++ b/desktop/app/src/chrome/mainsidebar/MainSidebar2.tsx @@ -486,10 +486,10 @@ const PluginList = memo(function PluginList({ }, [client]); const onFavorite = useCallback( - (plugin: string) => { + (plugin: PluginDefinition) => { starPlugin({ selectedApp: client.query.app, - selectedPlugin: plugin, + plugin, }); }, [client], @@ -589,7 +589,7 @@ const PluginsByCategory = memo(function PluginsByCategory({ starred: boolean; selectedPlugin?: null | string; selectedApp?: null | string; - onFavorite: (pluginId: string) => void; + onFavorite: (plugin: PluginDefinition) => void; selectPlugin: SelectPlugin; }) { return ( @@ -617,7 +617,7 @@ const PluginsByCategory = memo(function PluginsByCategory({ } plugin={plugin} app={client.query.app} - onFavorite={() => onFavorite(plugin.id)} + onFavorite={() => onFavorite(plugin)} starred={device.isArchived ? undefined : starred} /> ))} diff --git a/desktop/app/src/reducers/__tests__/sandyplugins.node.tsx b/desktop/app/src/reducers/__tests__/sandyplugins.node.tsx new file mode 100644 index 000000000..9cc5c2a5d --- /dev/null +++ b/desktop/app/src/reducers/__tests__/sandyplugins.node.tsx @@ -0,0 +1,141 @@ +/** + * 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 {createMockFlipperWithPlugin} from '../../test-utils/createMockFlipperWithPlugin'; +import {Store, Client} from '../../'; +import {selectPlugin, starPlugin} from '../../reducers/connections'; +import {registerPlugins} from '../../reducers/plugins'; +import {SandyPluginDefinition, SandyPluginInstance} from 'flipper-plugin'; + +interface PersistedState { + count: 1; +} + +const pluginDetails = { + id: 'TestPlugin', + dir: '', + name: 'TestPlugin', + specVersion: 0, + entry: '', + isDefault: false, + main: '', + source: '', + title: 'Testing Plugin', + version: '', +} as const; + +let TestPlugin: SandyPluginDefinition; + +beforeEach(() => { + TestPlugin = new SandyPluginDefinition(pluginDetails, { + plugin: jest.fn().mockImplementation(() => ({})), + Component: jest.fn().mockImplementation(() => null), + }); +}); + +function starTestPlugin(store: Store, client: Client) { + store.dispatch( + starPlugin({ + plugin: TestPlugin, + selectedApp: client.query.app, + }), + ); +} + +function selectTestPlugin(store: Store, client: Client) { + store.dispatch( + selectPlugin({ + selectedPlugin: TestPlugin.id, + selectedApp: client.query.app, + deepLinkPayload: null, + selectedDevice: store.getState().connections.selectedDevice!, + }), + ); +} + +test('it should initialize starred sandy plugins', async () => { + const {client, store} = await createMockFlipperWithPlugin(TestPlugin); + + // already started, so initialized immediately + expect(TestPlugin.module.plugin).toBeCalledTimes(1); + expect(client.sandyPluginStates.get(TestPlugin.id)).toBeInstanceOf( + SandyPluginInstance, + ); + + selectTestPlugin(store, client); + // TODO: make sure lifecycle 'activated' or something is triggered T68683507 +}); + +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, + ); + + // unstar + starTestPlugin(store, client); + expect(client.sandyPluginStates.get(TestPlugin.id)).toBeUndefined(); + // TODO: make sure onDestroy is called T68683507 +}); + +test('it should not initialize a sandy plugin if not enabled', async () => { + const {client, store} = await createMockFlipperWithPlugin(TestPlugin); + + const Plugin2 = new SandyPluginDefinition( + { + ...pluginDetails, + name: 'Plugin2', + id: 'Plugin2', + }, + { + plugin: jest.fn().mockImplementation(() => ({})), + Component() { + return null; + }, + }, + ); + + const pluginState1 = client.sandyPluginStates.get(TestPlugin.id); + expect(pluginState1).toBeInstanceOf(SandyPluginInstance); + store.dispatch(registerPlugins([Plugin2])); + await client.refreshPlugins(); + // not yet enabled, so not yet started + expect(client.sandyPluginStates.get(Plugin2.id)).toBeUndefined(); + expect(Plugin2.module.plugin).toBeCalledTimes(0); + + store.dispatch( + starPlugin({ + plugin: Plugin2, + selectedApp: client.query.app, + }), + ); + + expect(client.sandyPluginStates.get(Plugin2.id)).toBeInstanceOf( + SandyPluginInstance, + ); + expect(client.sandyPluginStates.get(TestPlugin.id)).toBe(pluginState1); // not reinitialized + + expect(TestPlugin.module.plugin).toBeCalledTimes(1); + expect(Plugin2.module.plugin).toBeCalledTimes(1); + + // disable plugin again + store.dispatch( + starPlugin({ + plugin: Plugin2, + selectedApp: client.query.app, + }), + ); + + expect(client.sandyPluginStates.get(Plugin2.id)).toBeUndefined(); + // TODO: test destroy hook is called T68683507 +}); + +// TODO: T68683449 state is persisted if a plugin connects and reconnects diff --git a/desktop/app/src/reducers/connections.tsx b/desktop/app/src/reducers/connections.tsx index e9d6a7980..8398f736f 100644 --- a/desktop/app/src/reducers/connections.tsx +++ b/desktop/app/src/reducers/connections.tsx @@ -28,7 +28,7 @@ import { defaultEnabledBackgroundPlugins, } from '../utils/pluginUtils'; import {deconstructClientId} from '../utils/clientUtils'; -import {FlipperDevicePlugin} from '../plugin'; +import {FlipperDevicePlugin, PluginDefinition} from '../plugin'; import {RegisterPluginAction} from './plugins'; import {SandyPluginDefinition} from 'flipper-plugin'; @@ -138,8 +138,8 @@ export type Action = | { type: 'STAR_PLUGIN'; payload: { - selectedPlugin: string; selectedApp: string; + plugin: PluginDefinition; }; } | { @@ -279,33 +279,46 @@ export default (state: State = INITAL_STATE, action: Actions): State => { } case 'STAR_PLUGIN': { - const {selectedPlugin, selectedApp} = action.payload; - const client = state.clients.find( + const {plugin, selectedApp} = action.payload; + const selectedPlugin = plugin.id; + const clients = state.clients.filter( (client) => client.query.app === selectedApp, ); return produce(state, (draft) => { if (!draft.userStarredPlugins[selectedApp]) { - draft.userStarredPlugins[selectedApp] = [selectedPlugin]; - } else { - const plugins = draft.userStarredPlugins[selectedApp]; - const idx = plugins.indexOf(selectedPlugin); - if (idx === -1) { - plugins.push(selectedPlugin); + draft.userStarredPlugins[selectedApp] = []; + } + const plugins = draft.userStarredPlugins[selectedApp]; + const idx = plugins.indexOf(selectedPlugin); + if (idx === -1) { + plugins.push(selectedPlugin); + // enabling a plugin on one device enables it on all... + clients.forEach((client) => { + // sandy plugin? initialize it + client.startPluginIfNeeded(plugin, true); + // background plugin? connect it needed if ( !defaultEnabledBackgroundPlugins.includes(selectedPlugin) && client?.isBackgroundPlugin(selectedPlugin) ) { client.initPlugin(selectedPlugin); } - } else { - plugins.splice(idx, 1); + }); + } else { + plugins.splice(idx, 1); + // enabling a plugin on one device disables it on all... + clients.forEach((client) => { + // disconnect background plugins if ( !defaultEnabledBackgroundPlugins.includes(selectedPlugin) && client?.isBackgroundPlugin(selectedPlugin) ) { client.deinitPlugin(selectedPlugin); } - } + // stop sandy plugins + // TODO: forget any persisted state as well T68683449 + client.stopPluginIfNeeded(plugin.id); + }); } }); } @@ -500,7 +513,7 @@ export const selectPlugin = (payload: { }); export const starPlugin = (payload: { - selectedPlugin: string; + plugin: PluginDefinition; selectedApp: string; }): Action => ({ type: 'STAR_PLUGIN', diff --git a/desktop/app/src/reducers/supportForm.tsx b/desktop/app/src/reducers/supportForm.tsx index 4ef7be6c8..56bb9a9c0 100644 --- a/desktop/app/src/reducers/supportForm.tsx +++ b/desktop/app/src/reducers/supportForm.tsx @@ -134,10 +134,13 @@ export class Group { selectedClient.plugins.includes(requiredPlugin) && !requiredPluginEnabled ) { + const plugin = + store.getState().plugins.clientPlugins.get(requiredPlugin) || + store.getState().plugins.devicePlugins.get(requiredPlugin)!; store.dispatch( setStarPlugin({ selectedApp: app, - selectedPlugin: requiredPlugin, + plugin, }), ); } else if ( diff --git a/desktop/app/src/server.tsx b/desktop/app/src/server.tsx index 90431bf08..64a4b776c 100644 --- a/desktop/app/src/server.tsx +++ b/desktop/app/src/server.tsx @@ -507,7 +507,7 @@ class Server extends EventEmitter { removeConnection(id: string) { const info = this.connections.get(id); if (info) { - info.client.emit('close'); + info.client.close(); this.connections.delete(id); this.emit('clients-change'); this.emit('removed-client', id); diff --git a/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx b/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx index c24d322db..cb25754a8 100644 --- a/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx +++ b/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx @@ -29,33 +29,28 @@ import Client, {ClientQuery} from '../Client'; import {buildClientId} from '../utils/clientUtils'; import {Logger} from '../fb-interfaces/Logger'; -import {FlipperPlugin} from '../plugin'; +import {FlipperPlugin, PluginDefinition} from '../plugin'; import {registerPlugins} from '../reducers/plugins'; import PluginContainer from '../PluginContainer'; - -export function createStubLogger(): Logger { - return { - ...console, - track: console.info, - trackTimeSince: console.info, - }; -} +import {getPluginKey} from '../utils/pluginUtils'; +import {getInstance} from '../fb-stubs/Logger'; type MockFlipperResult = { client: Client; device: BaseDevice; store: Store; - sendMessage(method: string, params: any): void; + pluginKey: string; + sendMessage(method: string, params: any, client?: Client): void; createDevice(serial: string): BaseDevice; - createClient(device: BaseDevice, name: string): Client; + createClient(device: BaseDevice, name: string): Promise; logger: Logger; }; export async function createMockFlipperWithPlugin( - pluginClazz: typeof FlipperPlugin, + pluginClazz: PluginDefinition, ): Promise { const store = createStore(reducers); - const logger = createStubLogger(); + const logger = getInstance(); store.dispatch(registerPlugins([pluginClazz])); function createDevice(serial: string): BaseDevice { @@ -72,7 +67,10 @@ export async function createMockFlipperWithPlugin( return device; } - function createClient(device: BaseDevice, name: string): Client { + async function createClient( + device: BaseDevice, + name: string, + ): Promise { const query: ClientQuery = { app: name, os: 'Android', @@ -102,6 +100,38 @@ export async function createMockFlipperWithPlugin( return device; }, } as any; + client.rawCall = async (method, _fromPlugin, _params): Promise => { + // TODO: could use an interceptor here + switch (method) { + case 'getPlugins': + // assuming this plugin supports all plugins for now + return { + plugins: [ + ...store.getState().plugins.clientPlugins.keys(), + ...store.getState().plugins.devicePlugins.keys(), + ], + }; + default: + throw new Error(`Test client doesn't supoprt rawCall to ${method}`); + } + }; + + // enable the plugin + if ( + !store + .getState() + .connections.userStarredPlugins[client.query.app]?.includes( + pluginClazz.id, + ) + ) { + store.dispatch( + starPlugin({ + plugin: pluginClazz, + selectedApp: client.query.app, + }), + ); + } + await client.init(); // As convenience, by default we select the new client, star the plugin, and select it store.dispatch({ @@ -112,18 +142,11 @@ export async function createMockFlipperWithPlugin( } const device = createDevice('serial'); - const client = createClient(device, 'TestApp'); + const client = await createClient(device, 'TestApp'); store.dispatch(selectDevice(device)); store.dispatch(selectClient(client.id)); - store.dispatch( - starPlugin({ - selectedPlugin: pluginClazz.id, - selectedApp: client.query.app, - }), - ); - store.dispatch( selectPlugin({ selectedPlugin: pluginClazz.id, @@ -137,8 +160,8 @@ export async function createMockFlipperWithPlugin( client, device: device as any, store, - sendMessage(method, params) { - client.onMessage( + sendMessage(method, params, actualClient = client) { + actualClient.onMessage( JSON.stringify({ method: 'execute', params: { @@ -152,6 +175,7 @@ export async function createMockFlipperWithPlugin( createDevice, createClient, logger, + pluginKey: getPluginKey(client.id, device, pluginClazz.id), }; } diff --git a/desktop/app/src/utils/__tests__/messageQueue.node.tsx b/desktop/app/src/utils/__tests__/messageQueue.node.tsx index ab9f21c75..585605ae9 100644 --- a/desktop/app/src/utils/__tests__/messageQueue.node.tsx +++ b/desktop/app/src/utils/__tests__/messageQueue.node.tsx @@ -57,7 +57,7 @@ class TestPlugin extends FlipperPlugin { function starTestPlugin(store: Store, client: Client) { store.dispatch( starPlugin({ - selectedPlugin: TestPlugin.id, + plugin: TestPlugin, selectedApp: client.query.app, }), ); @@ -214,7 +214,7 @@ test('queue - events are queued for plugins that are favorite when app is not se selectDeviceLogs(store); expect(store.getState().connections.selectedPlugin).not.toBe('TestPlugin'); - const client2 = createClient(device, 'TestApp2'); + const client2 = await createClient(device, 'TestApp2'); store.dispatch(selectClient(client2.id)); // Now we send a message to the second client, it should arrive, @@ -248,27 +248,39 @@ test('queue - events are queued for plugins that are favorite when app is select expect(store.getState().connections.selectedPlugin).not.toBe('TestPlugin'); const device2 = createDevice('serial2'); - const client2 = createClient(device2, client.query.app); // same app id + const client2 = await createClient(device2, client.query.app); // same app id store.dispatch(selectDevice(device2)); store.dispatch(selectClient(client2.id)); - // Now we send a message to the second client, it should arrive, + // Now we send a message to the first and second client, it should arrive, // as the plugin was enabled already on the first client as well sendMessage('inc', {delta: 2}); + sendMessage('inc', {delta: 3}, client2); + client.flushMessageBuffer(); + client2.flushMessageBuffer(); expect(store.getState().pluginStates).toMatchInlineSnapshot(`Object {}`); expect(store.getState().pluginMessageQueue).toMatchInlineSnapshot(` - Object { - "TestApp#Android#MockAndroidDevice#serial#TestPlugin": Array [ - Object { - "api": "TestPlugin", - "method": "inc", - "params": Object { - "delta": 2, - }, - }, - ], - } - `); + Object { + "TestApp#Android#MockAndroidDevice#serial#TestPlugin": Array [ + Object { + "api": "TestPlugin", + "method": "inc", + "params": Object { + "delta": 2, + }, + }, + ], + "TestApp#Android#MockAndroidDevice#serial2#TestPlugin": Array [ + Object { + "api": "TestPlugin", + "method": "inc", + "params": Object { + "delta": 3, + }, + }, + ], + } + `); }); test('queue - events processing will be paused', async () => { diff --git a/desktop/flipper-plugin/src/plugin/Plugin.tsx b/desktop/flipper-plugin/src/plugin/Plugin.tsx index 4bfc6f1c0..65fe2ce48 100644 --- a/desktop/flipper-plugin/src/plugin/Plugin.tsx +++ b/desktop/flipper-plugin/src/plugin/Plugin.tsx @@ -42,25 +42,40 @@ export type FlipperPluginModule = { // devicePlugin: FlipperPluginFactory }; -export class FlipperPluginInstance { +export class SandyPluginInstance { /** base client provided by Flipper */ realClient: RealFlipperClient; /** client that is bound to this instance */ client: FlipperClient; /** the original plugin definition */ - definition: FlipperPluginModule; + definition: SandyPluginDefinition; /** the plugin instance api as used inside components and such */ instanceApi: object; - constructor(realClient: RealFlipperClient, definition: FlipperPluginModule) { + constructor( + realClient: RealFlipperClient, + definition: SandyPluginDefinition, + ) { this.realClient = realClient; this.definition = definition; this.client = {}; - this.instanceApi = definition.plugin(this.client); + this.instanceApi = definition.module.plugin(this.client); + } + + activate() { + // TODO: T68683507 } deactivate() { - // TODO: + // TODO: T68683507 + } + + destroy() { + // TODO: T68683507 + } + + toJSON() { + // TODO: T68683449 } }