jens/flipper
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Introduce onConnect / onDisconnect hooks

Browse Source 
Summary:
Introduced hooks that are called whenever the plugin is connected / disconnected to it's counter part on the device.

There is some logic duplication between `PluginContainer` for old plugins, and `PluginRenderer` for new plugins, mostly caused by the fact that those lifecycles are triggered from the UI rather than from the reducers, but I figured refactoring that to be too risky.

Reviewed By: jknoxville

Differential Revision: D22232337

fbshipit-source-id: a384c45731a4c8d9b8b532a83e2becf49ce807c2
This commit is contained in:
Michel Weststrate
2020-07-01 08:58:40 -07:00
committed by Facebook GitHub Bot
parent dd0d957d8b
commit bde112bf85
10 changed files with 222 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
desktop/app/src/Client.tsx
View File

@@ -673,14 +673,16 @@ export default class Client extends EventEmitter {
initPlugin(pluginId: string) { initPlugin(pluginId: string) {
this.activePlugins.add(pluginId); this.activePlugins.add(pluginId);
this.rawSend('init', {plugin: pluginId}); this.rawSend('init', {plugin: pluginId});
// TODO: call sandyOnConnect this.sandyPluginStates.get(pluginId)?.connect();
} }
deinitPlugin(pluginId: string) { deinitPlugin(pluginId: string) {
// TODO: call sandyOnDisconnect
this.activePlugins.delete(pluginId); this.activePlugins.delete(pluginId);
this.sandyPluginStates.get(pluginId)?.disconnect();
if (this.connected) {
this.rawSend('deinit', {plugin: pluginId}); this.rawSend('deinit', {plugin: pluginId});
} }
}
rawSend(method: string, params?: Object): void { rawSend(method: string, params?: Object): void {
const data = { const data = {

1
desktop/app/src/PluginContainer.tsx
View File

@@ -142,6 +142,7 @@ class PluginContainer extends PureComponent<Props, State> {
| null | null
| undefined, | undefined,
) => { ) => {
// N.B. for Sandy plugins this lifecycle is managed by PluginRenderer
if (this.plugin) { if (this.plugin) {
this.plugin._teardown(); this.plugin._teardown();
this.plugin = null; this.plugin = null;

64
desktop/app/src/__tests__/PluginContainer.node.tsx
View File

@@ -14,7 +14,12 @@ import {
renderMockFlipperWithPlugin, renderMockFlipperWithPlugin,
createMockPluginDetails, createMockPluginDetails,
} from '../test-utils/createMockFlipperWithPlugin'; } from '../test-utils/createMockFlipperWithPlugin';
import {SandyPluginContext, SandyPluginDefinition} from 'flipper-plugin'; import {
SandyPluginContext,
SandyPluginDefinition,
FlipperClient,
} from 'flipper-plugin';
import {selectPlugin} from '../reducers/connections';
interface PersistedState { interface PersistedState {
count: 1; count: 1;
@@ -94,7 +99,13 @@ test('PluginContainer can render Sandy plugins', async () => {
return <div>Hello from Sandy</div>; return <div>Hello from Sandy</div>;
} }
const plugin = () => ({}); const plugin = (client: FlipperClient) => {
const connectedStub = jest.fn();
const disconnectedStub = jest.fn();
client.onConnect(connectedStub);
client.onDisconnect(disconnectedStub);
return {connectedStub, disconnectedStub};
};
const definition = new SandyPluginDefinition(createMockPluginDetails(), { const definition = new SandyPluginDefinition(createMockPluginDetails(), {
plugin, plugin,
@@ -103,9 +114,13 @@ test('PluginContainer can render Sandy plugins', async () => {
// any cast because this plugin is not enriched with the meta data that the plugin loader // any cast because this plugin is not enriched with the meta data that the plugin loader
// normally adds. Our further sandy plugin test infra won't need this, but // normally adds. Our further sandy plugin test infra won't need this, but
// for this test we do need to act a s a loaded plugin, to make sure PluginContainer itself can handle it // for this test we do need to act a s a loaded plugin, to make sure PluginContainer itself can handle it
const {renderer, act, sendMessage} = await renderMockFlipperWithPlugin( const {
definition, renderer,
); act,
sendMessage,
client,
store,
} = await renderMockFlipperWithPlugin(definition);
expect(renderer.baseElement).toMatchInlineSnapshot(` expect(renderer.baseElement).toMatchInlineSnapshot(`
<body> <body>
<div> <div>
@@ -129,9 +144,44 @@ test('PluginContainer can render Sandy plugins', async () => {
act(() => { act(() => {
sendMessage('inc', {delta: 2}); sendMessage('inc', {delta: 2});
}); });
expect(renders).toBe(1);
// 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);
// TODO: check that onConnect is called T68683507
// TODO: check that messages have arrived T68683442 // TODO: check that messages have arrived T68683442
expect(renders).toBe(1); // select non existing plugin
act(() => {
store.dispatch(
selectPlugin({
selectedPlugin: 'Logs',
deepLinkPayload: null,
}),
);
});
expect(renderer.baseElement).toMatchInlineSnapshot(`
<body>
<div />
</body>
`);
expect(pluginInstance.connectedStub).toBeCalledTimes(1);
expect(pluginInstance.disconnectedStub).toBeCalledTimes(1);
// go back
act(() => {
store.dispatch(
selectPlugin({
selectedPlugin: definition.id,
deepLinkPayload: null,
}),
);
});
expect(pluginInstance.connectedStub).toBeCalledTimes(2);
expect(pluginInstance.disconnectedStub).toBeCalledTimes(1);
}); });

1
desktop/app/src/__tests__/__snapshots__/createMockFlipperWithPlugin.node.tsx.snap
View File

@@ -11,6 +11,7 @@ Object {
"device": "MockAndroidDevice", "device": "MockAndroidDevice",
"device_id": "serial", "device_id": "serial",
"os": "Android", "os": "Android",
"sdk_version": 4,
}, },
}, },
], ],

5
desktop/app/src/plugin.tsx
View File

@@ -297,10 +297,7 @@ export class FlipperPlugin<
} }
// run plugin teardown // run plugin teardown
this.teardown(); this.teardown();
if ( if (!this.realClient.isBackgroundPlugin(pluginId)) {
this.realClient.connected &&
!this.realClient.isBackgroundPlugin(pluginId)
) {
this.realClient.deinitPlugin(pluginId); this.realClient.deinitPlugin(pluginId);
} }
} }

91
desktop/app/src/reducers/__tests__/sandyplugins.node.tsx
View File

@@ -26,24 +26,36 @@ interface PersistedState {
const pluginDetails = createMockPluginDetails(); const pluginDetails = createMockPluginDetails();
let TestPlugin: SandyPluginDefinition; let initialized = false;
beforeEach(() => { beforeEach(() => {
function plugin(client: FlipperClient) { initialized = false;
});
function plugin(client: FlipperClient) {
const connectStub = jest.fn();
const disconnectStub = jest.fn();
const destroyStub = jest.fn(); const destroyStub = jest.fn();
client.onConnect(connectStub);
client.onDisconnect(disconnectStub);
client.onDestroy(destroyStub); client.onDestroy(destroyStub);
initialized = true;
return { return {
connectStub,
disconnectStub,
destroyStub, destroyStub,
}; };
} }
TestPlugin = new SandyPluginDefinition(pluginDetails, { const TestPlugin = new SandyPluginDefinition(pluginDetails, {
plugin: jest.fn().mockImplementation(plugin) as typeof plugin, plugin: jest.fn().mockImplementation(plugin) as typeof plugin,
Component: jest.fn().mockImplementation(() => null), Component: jest.fn().mockImplementation(() => null),
});
}); });
type PluginApi = ReturnType<typeof plugin>;
function starTestPlugin(store: Store, client: Client) { function starTestPlugin(store: Store, client: Client) {
store.dispatch( store.dispatch(
starPlugin({ starPlugin({
@@ -68,27 +80,40 @@ test('it should initialize starred sandy plugins', async () => {
const {client, store} = await createMockFlipperWithPlugin(TestPlugin); const {client, store} = await createMockFlipperWithPlugin(TestPlugin);
// already started, so initialized immediately // already started, so initialized immediately
expect(TestPlugin.module.plugin).toBeCalledTimes(1); expect(initialized).toBe(true);
expect(client.sandyPluginStates.get(TestPlugin.id)).toBeInstanceOf( expect(client.sandyPluginStates.get(TestPlugin.id)).toBeInstanceOf(
SandyPluginInstance, SandyPluginInstance,
); );
const instanceApi: PluginApi = client.sandyPluginStates.get(TestPlugin.id)!
.instanceApi;
expect(instanceApi.connectStub).toBeCalledTimes(0);
selectTestPlugin(store, client); selectTestPlugin(store, client);
// TODO: make sure lifecycle 'activated' or something is triggered T68683507
// without rendering, non-bg plugins won't connect automatically,
// so this isn't the best test, but PluginContainer tests do test that part of the lifecycle
client.initPlugin(TestPlugin.id);
expect(instanceApi.connectStub).toBeCalledTimes(1);
client.deinitPlugin(TestPlugin.id);
expect(instanceApi.disconnectStub).toBeCalledTimes(1);
expect(instanceApi.destroyStub).toBeCalledTimes(0);
}); });
test('it should cleanup a plugin if disabled', async () => { test('it should cleanup a plugin if disabled', async () => {
const {client, store} = await createMockFlipperWithPlugin(TestPlugin); const {client, store} = await createMockFlipperWithPlugin(TestPlugin);
expect(TestPlugin.module.plugin).toBeCalledTimes(1); expect(TestPlugin.module.plugin).toBeCalledTimes(1);
const pluginInstance = client.sandyPluginStates.get(TestPlugin.id)!; const pluginInstance: PluginApi = client.sandyPluginStates.get(TestPlugin.id)!
expect(pluginInstance).toBeInstanceOf(SandyPluginInstance); .instanceApi;
expect(pluginInstance.instanceApi.destroyStub).toHaveBeenCalledTimes(0); expect(pluginInstance.destroyStub).toHaveBeenCalledTimes(0);
client.initPlugin(TestPlugin.id);
expect(pluginInstance.connectStub).toHaveBeenCalledTimes(1);
// unstar // unstar
starTestPlugin(store, client); starTestPlugin(store, client);
expect(client.sandyPluginStates.has(TestPlugin.id)).toBeFalsy(); expect(client.sandyPluginStates.has(TestPlugin.id)).toBeFalsy();
expect(pluginInstance.instanceApi.destroyStub).toHaveBeenCalledTimes(1); expect(pluginInstance.disconnectStub).toHaveBeenCalledTimes(1);
expect(pluginInstance.destroyStub).toHaveBeenCalledTimes(1);
}); });
test('it should cleanup if client is removed', async () => { test('it should cleanup if client is removed', async () => {
@@ -99,7 +124,9 @@ test('it should cleanup if client is removed', async () => {
// close client // close client
client.close(); client.close();
expect(client.sandyPluginStates.has(TestPlugin.id)).toBeFalsy(); expect(client.sandyPluginStates.has(TestPlugin.id)).toBeFalsy();
expect(pluginInstance.instanceApi.destroyStub).toHaveBeenCalledTimes(1); expect(
(pluginInstance.instanceApi as PluginApi).destroyStub,
).toHaveBeenCalledTimes(1);
}); });
test('it should not initialize a sandy plugin if not enabled', async () => { test('it should not initialize a sandy plugin if not enabled', async () => {
@@ -111,11 +138,7 @@ test('it should not initialize a sandy plugin if not enabled', async () => {
id: 'Plugin2', id: 'Plugin2',
}), }),
{ {
plugin: jest.fn().mockImplementation((client) => { plugin: jest.fn().mockImplementation(plugin),
const destroyStub = jest.fn();
client.onDestroy(destroyStub);
return {destroyStub};
}),
Component() { Component() {
return null; return null;
}, },
@@ -140,13 +163,13 @@ test('it should not initialize a sandy plugin if not enabled', async () => {
expect(client.sandyPluginStates.get(Plugin2.id)).toBeInstanceOf( expect(client.sandyPluginStates.get(Plugin2.id)).toBeInstanceOf(
SandyPluginInstance, SandyPluginInstance,
); );
const destroyStub = client.sandyPluginStates.get(Plugin2.id)!.instanceApi const instance = client.sandyPluginStates.get(Plugin2.id)!
.destroyStub; .instanceApi as PluginApi;
expect(client.sandyPluginStates.get(TestPlugin.id)).toBe(pluginState1); // not reinitialized expect(client.sandyPluginStates.get(TestPlugin.id)).toBe(pluginState1); // not reinitialized
expect(TestPlugin.module.plugin).toBeCalledTimes(1); expect(TestPlugin.module.plugin).toBeCalledTimes(1);
expect(Plugin2.module.plugin).toBeCalledTimes(1); expect(Plugin2.module.plugin).toBeCalledTimes(1);
expect(destroyStub).toHaveBeenCalledTimes(0); expect(instance.destroyStub).toHaveBeenCalledTimes(0);
// disable plugin again // disable plugin again
store.dispatch( store.dispatch(
@@ -157,7 +180,33 @@ test('it should not initialize a sandy plugin if not enabled', async () => {
); );
expect(client.sandyPluginStates.get(Plugin2.id)).toBeUndefined(); expect(client.sandyPluginStates.get(Plugin2.id)).toBeUndefined();
expect(destroyStub).toHaveBeenCalledTimes(1); expect(instance.connectStub).toHaveBeenCalledTimes(0);
// disconnect wasn't called because connect was never called
expect(instance.disconnectStub).toHaveBeenCalledTimes(0);
expect(instance.destroyStub).toHaveBeenCalledTimes(1);
});
test('it trigger hooks for background plugins', async () => {
const {client} = await createMockFlipperWithPlugin(TestPlugin, {
onSend(method) {
if (method === 'getBackgroundPlugins') {
return {plugins: [TestPlugin.id]};
}
},
});
const pluginInstance: PluginApi = client.sandyPluginStates.get(TestPlugin.id)!
.instanceApi;
expect(client.isBackgroundPlugin(TestPlugin.id)).toBeTruthy();
expect(pluginInstance.destroyStub).toHaveBeenCalledTimes(0);
expect(pluginInstance.connectStub).toHaveBeenCalledTimes(1);
expect(pluginInstance.disconnectStub).toHaveBeenCalledTimes(0);
// close client
client.close();
expect(client.sandyPluginStates.has(TestPlugin.id)).toBeFalsy();
expect(pluginInstance.destroyStub).toHaveBeenCalledTimes(1);
expect(pluginInstance.connectStub).toHaveBeenCalledTimes(1);
expect(pluginInstance.disconnectStub).toHaveBeenCalledTimes(1);
}); });
// TODO: T68683449 state is persisted if a plugin connects and reconnects // TODO: T68683449 state is persisted if a plugin connects and reconnects

21
desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx
View File

@@ -47,8 +47,17 @@ type MockFlipperResult = {
logger: Logger; logger: Logger;
}; };
type MockOptions = Partial<{
/**
* can be used to intercept outgoing calls. If it returns undefined
* the base implementation will be used
*/
onSend(method: string, params?: object): object | undefined;
}>;
export async function createMockFlipperWithPlugin( export async function createMockFlipperWithPlugin(
pluginClazz: PluginDefinition, pluginClazz: PluginDefinition,
options?: MockOptions,
): Promise<MockFlipperResult> { ): Promise<MockFlipperResult> {
const store = createStore(reducers); const store = createStore(reducers);
const logger = getInstance(); const logger = getInstance();
@@ -77,6 +86,7 @@ export async function createMockFlipperWithPlugin(
os: 'Android', os: 'Android',
device: device.title, device: device.title,
device_id: device.serial, device_id: device.serial,
sdk_version: 4,
}; };
const id = buildClientId({ const id = buildClientId({
app: query.app, app: query.app,
@@ -101,8 +111,11 @@ export async function createMockFlipperWithPlugin(
return device; return device;
}, },
} as any; } as any;
client.rawCall = async (method, _fromPlugin, _params): Promise<any> => { client.rawCall = async (method, _fromPlugin, params): Promise<any> => {
// TODO: could use an interceptor here const intercepted = options?.onSend?.(method, params);
if (intercepted !== undefined) {
return intercepted;
}
switch (method) { switch (method) {
case 'getPlugins': case 'getPlugins':
// assuming this plugin supports all plugins for now // assuming this plugin supports all plugins for now
@@ -112,8 +125,10 @@ export async function createMockFlipperWithPlugin(
...store.getState().plugins.devicePlugins.keys(), ...store.getState().plugins.devicePlugins.keys(),
], ],
}; };
case 'getBackgroundPlugins':
return {plugins: []};
default: default:
throw new Error(`Test client doesn't supoprt rawCall to ${method}`); throw new Error(`Test client doesn't support rawCall to ${method}`);
} }
}; };

55
desktop/flipper-plugin/src/plugin/Plugin.tsx
View File

@@ -24,13 +24,32 @@ export interface FlipperClient<
* the onDestroy event is fired whenever a client is unloaded from Flipper, or a plugin is disabled. * the onDestroy event is fired whenever a client is unloaded from Flipper, or a plugin is disabled.
*/ */
onDestroy(cb: () => void): void; onDestroy(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,
* for background plugins when the initial connection is made.
*/
onConnect(cb: () => void): void;
/**
* The counterpart of the `onConnect` handler.
* Will also be fired before the plugin is cleaned up if the connection is currently active:
* - when the client disconnects
* - when the plugin is disabled
*/
onDisconnect(cb: () => void): void;
} }
/** /**
* Internal API exposed by Flipper, and wrapped by FlipperPluginInstance to be passed to the * Internal API exposed by Flipper, and wrapped by FlipperPluginInstance to be passed to the
* Plugin Factory. For internal purposes only * Plugin Factory. For internal purposes only
*/ */
interface RealFlipperClient {} interface RealFlipperClient {
isBackgroundPlugin(pluginId: string): boolean;
initPlugin(pluginId: string): void;
deinitPlugin(pluginId: string): void;
}
export type FlipperPluginFactory< export type FlipperPluginFactory<
Events extends EventsContract, Events extends EventsContract,
@@ -49,6 +68,7 @@ export class SandyPluginInstance {
/** the plugin instance api as used inside components and such */ /** the plugin instance api as used inside components and such */
instanceApi: any; instanceApi: any;
connected = false;
events = new EventEmitter(); events = new EventEmitter();
constructor( constructor(
@@ -61,19 +81,48 @@ export class SandyPluginInstance {
onDestroy: (cb) => { onDestroy: (cb) => {
this.events.on('destroy', cb); this.events.on('destroy', cb);
}, },
onConnect: (cb) => {
this.events.on('connect', cb);
},
onDisconnect: (cb) => {
this.events.on('disconnect', cb);
},
}; };
this.instanceApi = definition.module.plugin(this.client); this.instanceApi = definition.module.plugin(this.client);
} }
// the plugin is selected in the UI
activate() { activate() {
// TODO: T68683507 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() { deactivate() {
// TODO: T68683507 const pluginId = this.definition.id;
if (!this.realClient.isBackgroundPlugin(pluginId)) {
this.realClient.deinitPlugin(pluginId);
}
}
connect() {
if (!this.connected) {
this.connected = true;
this.events.emit('connect');
}
}
disconnect() {
if (this.connected) {
this.connected = false;
this.events.emit('disconnect');
}
} }
destroy() { destroy() {
this.disconnect();
this.events.emit('destroy'); this.events.emit('destroy');
} }

16
desktop/flipper-plugin/src/plugin/PluginContext.tsx
View File

@@ -8,16 +8,8 @@
*/ */
import {createContext} from 'react'; import {createContext} from 'react';
import {SandyPluginInstance} from './Plugin';
export type SandyPluginContext = { export const SandyPluginContext = createContext<
deactivate(): void; SandyPluginInstance | undefined
}; >(undefined);
// TODO: to be filled in later with testing and such
const stubPluginContext: SandyPluginContext = {
deactivate() {},
};
export const SandyPluginContext = createContext<SandyPluginContext>(
stubPluginContext,
);

3
desktop/flipper-plugin/src/plugin/PluginRenderer.tsx
View File

@@ -21,7 +21,10 @@ type Props = {
export const SandyPluginRenderer = memo( export const SandyPluginRenderer = memo(
({plugin}: Props) => { ({plugin}: Props) => {
useEffect(() => { useEffect(() => {
plugin.activate();
return () => {
plugin.deactivate(); plugin.deactivate();
};
}, [plugin]); }, [plugin]);
return ( return (