From f2c39aed55ecdc484a12e2ef2cce39ae8c515e85 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Wed, 1 Jul 2020 08:58:40 -0700 Subject: [PATCH] Introduce PluginRenderer to render plugins Summary: PluginContainer will now wrap Sandy plugins in PluginRenderer. PluginRenderer will also be used by plugin unit tests in the future Reviewed By: jknoxville Differential Revision: D22159359 fbshipit-source-id: 69f9c8f4bec9392022c1d7a14957f5aca0339d97 --- desktop/app/src/PluginContainer.tsx | 113 +++++++++++------- .../src/__tests__/PluginContainer.node.tsx | 60 +++++++++- .../reducers/__tests__/sandyplugins.node.tsx | 23 ++-- .../createMockFlipperWithPlugin.tsx | 23 +++- desktop/flipper-plugin/src/index.ts | 2 + .../src/plugin/PluginContext.tsx | 23 ++++ .../src/plugin/PluginRenderer.tsx | 38 ++++++ 7 files changed, 216 insertions(+), 66 deletions(-) create mode 100644 desktop/flipper-plugin/src/plugin/PluginContext.tsx create mode 100644 desktop/flipper-plugin/src/plugin/PluginRenderer.tsx diff --git a/desktop/app/src/PluginContainer.tsx b/desktop/app/src/PluginContainer.tsx index 3e45dc4cf..34f8c181a 100644 --- a/desktop/app/src/PluginContainer.tsx +++ b/desktop/app/src/PluginContainer.tsx @@ -47,6 +47,7 @@ import {Message} from './reducers/pluginMessageQueue'; import {Idler} from './utils/Idler'; import {processMessageQueue} from './utils/messageQueue'; import {ToggleButton, SmallText} from './ui'; +import {SandyPluginRenderer} from 'flipper-plugin'; const Container = styled(FlexColumn)({ width: 0, @@ -332,53 +333,73 @@ class PluginContainer extends PureComponent { console.warn(`No selected plugin. Rendering empty!`); return null; } + let pluginElement: null | React.ReactElement; if (isSandyPlugin(activePlugin)) { - // TODO: - return null; - } - const props: PluginProps & { - key: string; - ref: ( - ref: - | FlipperPlugin - | FlipperDevicePlugin - | null - | undefined, - ) => void; - } = { - key: pluginKey, - logger: this.props.logger, - selectedApp, - settingsState, - persistedState: activePlugin.defaultPersistedState - ? { - ...activePlugin.defaultPersistedState, - ...pluginState, + if (target instanceof Client) { + // Make sure we throw away the container for different pluginKey! + pluginElement = ( + + ); + } else { + // TODO: target might be a device as well, support that T68738317 + pluginElement = null; + } + } else { + const props: PluginProps & { + key: string; + ref: ( + ref: + | FlipperPlugin + | FlipperDevicePlugin + | null + | undefined, + ) => void; + } = { + key: pluginKey, + logger: this.props.logger, + selectedApp, + persistedState: activePlugin.defaultPersistedState + ? { + ...activePlugin.defaultPersistedState, + ...pluginState, + } + : pluginState, + setStaticView: (payload: StaticView) => + this.props.setStaticView(payload), + setPersistedState: (state) => setPluginState({pluginKey, state}), + target, + deepLinkPayload: this.props.deepLinkPayload, + selectPlugin: (pluginID: string, deepLinkPayload: string | null) => { + const {target} = this.props; + // check if plugin will be available + if ( + target instanceof Client && + target.plugins.some((p) => p === pluginID) + ) { + this.props.selectPlugin({ + selectedPlugin: pluginID, + deepLinkPayload, + }); + return true; + } else if (target instanceof BaseDevice) { + this.props.selectPlugin({ + selectedPlugin: pluginID, + deepLinkPayload, + }); + return true; + } else { + return false; } - : pluginState, - setStaticView: (payload: StaticView) => this.props.setStaticView(payload), - setPersistedState: (state) => setPluginState({pluginKey, state}), - target, - deepLinkPayload: this.props.deepLinkPayload, - selectPlugin: (pluginID: string, deepLinkPayload: string | null) => { - const {target} = this.props; - // check if plugin will be available - if ( - target instanceof Client && - target.plugins.some((p) => p === pluginID) - ) { - this.props.selectPlugin({selectedPlugin: pluginID, deepLinkPayload}); - return true; - } else if (target instanceof BaseDevice) { - this.props.selectPlugin({selectedPlugin: pluginID, deepLinkPayload}); - return true; - } else { - return false; - } - }, - ref: this.refChanged, - isArchivedDevice, - }; + }, + ref: this.refChanged, + isArchivedDevice, + settingsState, + }; + pluginElement = React.createElement(activePlugin, props); + } return ( @@ -386,7 +407,7 @@ class PluginContainer extends PureComponent { heading={`Plugin "${ activePlugin.title || 'Unknown' }" encountered an error during render`}> - {React.createElement(activePlugin, props)} + {pluginElement} diff --git a/desktop/app/src/__tests__/PluginContainer.node.tsx b/desktop/app/src/__tests__/PluginContainer.node.tsx index 233ad24e4..1e6cbfb9a 100644 --- a/desktop/app/src/__tests__/PluginContainer.node.tsx +++ b/desktop/app/src/__tests__/PluginContainer.node.tsx @@ -7,10 +7,14 @@ * @format */ -import React from 'react'; +import React, {useContext} from 'react'; import produce from 'immer'; import {FlipperPlugin} from '../plugin'; -import {renderMockFlipperWithPlugin} from '../test-utils/createMockFlipperWithPlugin'; +import { + renderMockFlipperWithPlugin, + createMockPluginDetails, +} from '../test-utils/createMockFlipperWithPlugin'; +import {SandyPluginContext, SandyPluginDefinition} from 'flipper-plugin'; interface PersistedState { count: 1; @@ -79,3 +83,55 @@ test('Plugin container can render plugin and receive updates', async () => { expect((await renderer.findByTestId('counter')).textContent).toBe('2'); }); + +test('PluginContainer can render Sandy plugins', async () => { + let renders = 0; + + function MySandyPlugin() { + renders++; + const sandyContext = useContext(SandyPluginContext); + expect(sandyContext).not.toBe(null); + return
Hello from Sandy
; + } + + const plugin = () => ({}); + + const definition = new SandyPluginDefinition(createMockPluginDetails(), { + plugin, + Component: MySandyPlugin, + }); + // 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 + // 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( + definition, + ); + expect(renderer.baseElement).toMatchInlineSnapshot(` + +
+
+
+ Hello from Sandy +
+
+
+
+ + `); + expect(renders).toBe(1); + + // sending a new message doesn't cause a re-render + act(() => { + sendMessage('inc', {delta: 2}); + }); + + // TODO: check that onConnect is called T68683507 + // TODO: check that messages have arrived T68683442 + + expect(renders).toBe(1); +}); diff --git a/desktop/app/src/reducers/__tests__/sandyplugins.node.tsx b/desktop/app/src/reducers/__tests__/sandyplugins.node.tsx index c548769e8..83f841f66 100644 --- a/desktop/app/src/reducers/__tests__/sandyplugins.node.tsx +++ b/desktop/app/src/reducers/__tests__/sandyplugins.node.tsx @@ -7,7 +7,10 @@ * @format */ -import {createMockFlipperWithPlugin} from '../../test-utils/createMockFlipperWithPlugin'; +import { + createMockFlipperWithPlugin, + createMockPluginDetails, +} from '../../test-utils/createMockFlipperWithPlugin'; import {Store, Client} from '../../'; import {selectPlugin, starPlugin} from '../../reducers/connections'; import {registerPlugins} from '../../reducers/plugins'; @@ -21,18 +24,7 @@ interface PersistedState { count: 1; } -const pluginDetails = { - id: 'TestPlugin', - dir: '', - name: 'TestPlugin', - specVersion: 0, - entry: '', - isDefault: false, - main: '', - source: '', - title: 'Testing Plugin', - version: '', -} as const; +const pluginDetails = createMockPluginDetails(); let TestPlugin: SandyPluginDefinition; @@ -114,11 +106,10 @@ test('it should not initialize a sandy plugin if not enabled', async () => { const {client, store} = await createMockFlipperWithPlugin(TestPlugin); const Plugin2 = new SandyPluginDefinition( - { - ...pluginDetails, + createMockPluginDetails({ name: 'Plugin2', id: 'Plugin2', - }, + }), { plugin: jest.fn().mockImplementation((client) => { const destroyStub = jest.fn(); diff --git a/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx b/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx index cb25754a8..229010624 100644 --- a/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx +++ b/desktop/app/src/test-utils/createMockFlipperWithPlugin.tsx @@ -29,11 +29,12 @@ import Client, {ClientQuery} from '../Client'; import {buildClientId} from '../utils/clientUtils'; import {Logger} from '../fb-interfaces/Logger'; -import {FlipperPlugin, PluginDefinition} from '../plugin'; +import {PluginDefinition} from '../plugin'; import {registerPlugins} from '../reducers/plugins'; import PluginContainer from '../PluginContainer'; import {getPluginKey} from '../utils/pluginUtils'; import {getInstance} from '../fb-stubs/Logger'; +import {PluginDetails} from 'flipper-plugin-lib'; type MockFlipperResult = { client: Client; @@ -182,7 +183,7 @@ export async function createMockFlipperWithPlugin( type Renderer = RenderResult; export async function renderMockFlipperWithPlugin( - pluginClazz: typeof FlipperPlugin, + pluginClazz: PluginDefinition, ): Promise< MockFlipperResult & { renderer: Renderer; @@ -219,3 +220,21 @@ export async function renderMockFlipperWithPlugin( }, }; } + +export function createMockPluginDetails( + details?: Partial, +): PluginDetails { + return { + id: 'TestPlugin', + dir: '', + name: 'TestPlugin', + specVersion: 0, + entry: '', + isDefault: false, + main: '', + source: '', + title: 'Testing Plugin', + version: '', + ...details, + }; +} diff --git a/desktop/flipper-plugin/src/index.ts b/desktop/flipper-plugin/src/index.ts index 3df328525..c1d1a1d46 100644 --- a/desktop/flipper-plugin/src/index.ts +++ b/desktop/flipper-plugin/src/index.ts @@ -9,3 +9,5 @@ export * from './plugin/Plugin'; export * from './plugin/SandyPluginDefinition'; +export * from './plugin/PluginRenderer'; +export * from './plugin/PluginContext'; diff --git a/desktop/flipper-plugin/src/plugin/PluginContext.tsx b/desktop/flipper-plugin/src/plugin/PluginContext.tsx new file mode 100644 index 000000000..01e9be2f7 --- /dev/null +++ b/desktop/flipper-plugin/src/plugin/PluginContext.tsx @@ -0,0 +1,23 @@ +/** + * 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 {createContext} from 'react'; + +export type SandyPluginContext = { + deactivate(): void; +}; + +// TODO: to be filled in later with testing and such +const stubPluginContext: SandyPluginContext = { + deactivate() {}, +}; + +export const SandyPluginContext = createContext( + stubPluginContext, +); diff --git a/desktop/flipper-plugin/src/plugin/PluginRenderer.tsx b/desktop/flipper-plugin/src/plugin/PluginRenderer.tsx new file mode 100644 index 000000000..dd72e8434 --- /dev/null +++ b/desktop/flipper-plugin/src/plugin/PluginRenderer.tsx @@ -0,0 +1,38 @@ +/** + * 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 React, {memo, useEffect, createElement} from 'react'; +import {SandyPluginContext} from './PluginContext'; +import {SandyPluginInstance} from './Plugin'; + +type Props = { + plugin: SandyPluginInstance; +}; + +/** + * Component to render a Sandy plugin container + */ +export const SandyPluginRenderer = memo( + ({plugin}: Props) => { + useEffect(() => { + 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; + }, +);