From e1daa449ba95cb8347d8039f818606cd945c3092 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Mon, 1 Feb 2021 11:40:20 -0800 Subject: [PATCH] Unify computation of available plugins Summary: While trying to change something, discovered we have 3 different mechanisms in our code base to compute active plugins; the plugin list component, support form, and export flipper trace form had all their own, subtly different implementations of computing which plugins are available to the user. Also removed some hardcoded exceptions for e.g. Logs plugin, which in the next diff and onward will be just a vanilla plugin without special casing Unified that, which some how went a bit deeper than hoped, trough some hoops in in circular deps. Also unified to use the same testing utils, to avoid some gobbling objects manually together, with resulted in a bunch of unexpected NPEs. Found out that we actually still have unit tests using Flow in the process :-P. Converted one to TS. Reviewed By: nikoant Differential Revision: D26103172 fbshipit-source-id: 2fce2577d97d98543cb9312b3d013f24faee43aa --- .../src/__tests__/PluginContainer.node.tsx | 1 + .../app/src/chrome/ExportDataPluginSheet.tsx | 25 +- desktop/app/src/chrome/ListView.tsx | 3 + .../__tests__/ExportDataPluginSheet.node.tsx | 164 ++++---- .../ExportDataPluginSheet.node.tsx.snap | 45 +-- desktop/app/src/dispatcher/plugins.tsx | 9 +- desktop/app/src/reducers/connections.tsx | 8 +- desktop/app/src/reducers/pluginStates.tsx | 6 +- desktop/app/src/reducers/plugins.tsx | 34 +- desktop/app/src/reducers/supportForm.tsx | 32 +- .../sandy-chrome/appinspect/PluginList.tsx | 164 +------- .../appinspect/__tests__/PluginList.spec.tsx | 2 +- .../createMockFlipperWithPlugin.tsx | 35 +- .../src/utils/__tests__/pluginUtils.node.js | 293 -------------- .../src/utils/__tests__/pluginUtils.node.tsx | 144 +++++++ desktop/app/src/utils/clientUtils.tsx | 4 +- desktop/app/src/utils/pluginUtils.tsx | 379 +++++++++++------- 17 files changed, 556 insertions(+), 792 deletions(-) delete mode 100644 desktop/app/src/utils/__tests__/pluginUtils.node.js create mode 100644 desktop/app/src/utils/__tests__/pluginUtils.node.tsx diff --git a/desktop/app/src/__tests__/PluginContainer.node.tsx b/desktop/app/src/__tests__/PluginContainer.node.tsx index bf626cee2..92f358372 100644 --- a/desktop/app/src/__tests__/PluginContainer.node.tsx +++ b/desktop/app/src/__tests__/PluginContainer.node.tsx @@ -938,6 +938,7 @@ test('Sandy plugins support isPluginSupported + selectPlugin', async () => { definition, { additionalPlugins: [definition2, definition3], + dontEnableAdditionalPlugins: true, }, ); diff --git a/desktop/app/src/chrome/ExportDataPluginSheet.tsx b/desktop/app/src/chrome/ExportDataPluginSheet.tsx index ab360e38c..b316cd35f 100644 --- a/desktop/app/src/chrome/ExportDataPluginSheet.tsx +++ b/desktop/app/src/chrome/ExportDataPluginSheet.tsx @@ -13,7 +13,7 @@ import {ShareType} from '../reducers/application'; import {State as Store} from '../reducers'; import {ActiveSheet} from '../reducers/application'; import {selectedPlugins as actionForSelectedPlugins} from '../reducers/plugins'; -import {getActivePersistentPlugins} from '../utils/pluginUtils'; +import {getExportablePlugins} from '../utils/pluginUtils'; import { ACTIVE_SHEET_SHARE_DATA, setActiveSheet as getActiveSheetAction, @@ -103,25 +103,18 @@ class ExportDataPluginSheet extends Component { } export default connect( - ({ - application: {share}, - plugins, - pluginStates, - pluginMessageQueue, - connections: {selectedApp, clients}, - }) => { - const selectedClient = clients.find((o) => { - return o.id === selectedApp; + (state) => { + const selectedClient = state.connections.clients.find((o) => { + return o.id === state.connections.selectedApp; }); - const availablePluginsToExport = getActivePersistentPlugins( - pluginStates, - pluginMessageQueue, - plugins, + const availablePluginsToExport = getExportablePlugins( + state, + state.connections.selectedDevice ?? undefined, selectedClient, ); return { - share, - selectedPlugins: plugins.selectedPlugins, + share: state.application.share, + selectedPlugins: state.plugins.selectedPlugins, availablePluginsToExport, }; }, diff --git a/desktop/app/src/chrome/ListView.tsx b/desktop/app/src/chrome/ListView.tsx index 2ad7a1d65..75b645661 100644 --- a/desktop/app/src/chrome/ListView.tsx +++ b/desktop/app/src/chrome/ListView.tsx @@ -159,6 +159,9 @@ class RowComponent extends Component { } } +/** + * @deprecated use Ant Design instead + */ export default class ListView extends Component { state: State = {selectedElements: new Set([])}; static getDerivedStateFromProps(props: Props, _state: State) { diff --git a/desktop/app/src/chrome/__tests__/ExportDataPluginSheet.node.tsx b/desktop/app/src/chrome/__tests__/ExportDataPluginSheet.node.tsx index 141b953a7..6b1592497 100644 --- a/desktop/app/src/chrome/__tests__/ExportDataPluginSheet.node.tsx +++ b/desktop/app/src/chrome/__tests__/ExportDataPluginSheet.node.tsx @@ -8,96 +8,79 @@ */ import React from 'react'; -import Client from '../../Client'; import {create, act, ReactTestRenderer} from 'react-test-renderer'; -import configureStore from 'redux-mock-store'; import {Provider} from 'react-redux'; -import {default as BaseDevice} from '../../devices/BaseDevice'; import ExportDataPluginSheet from '../ExportDataPluginSheet'; import {FlipperPlugin, FlipperDevicePlugin} from '../../plugin'; +import {getExportablePlugins, getPluginKey} from '../../utils/pluginUtils'; +import {createMockFlipperWithPlugin} from '../../test-utils/createMockFlipperWithPlugin'; +import {setPluginState} from '../../reducers/pluginStates'; -function generateClientIdentifier(device: BaseDevice, app: string): string { - const {os, deviceType, serial} = device; - const identifier = `${app}#${os}#${deviceType}#${serial}`; - return identifier; +class TestPlugin extends FlipperPlugin { + static details = { + title: 'TestPlugin', + id: 'TestPlugin', + } as any; } - -class TestPlugin extends FlipperPlugin {} TestPlugin.title = 'TestPlugin'; TestPlugin.id = 'TestPlugin'; TestPlugin.defaultPersistedState = {msg: 'Test plugin'}; -class TestDevicePlugin extends FlipperDevicePlugin {} + +class TestDevicePlugin extends FlipperDevicePlugin { + static details = { + title: 'TestDevicePlugin', + id: 'TestDevicePlugin', + } as any; + + static supportsDevice() { + return true; + } +} TestDevicePlugin.title = 'TestDevicePlugin'; TestDevicePlugin.id = 'TestDevicePlugin'; TestDevicePlugin.defaultPersistedState = {msg: 'TestDevicePlugin'}; -function getStore(selectedPlugins: Array) { - const logger = { - track: () => {}, - info: () => {}, - warn: () => {}, - error: () => {}, - debug: () => {}, - trackTimeSince: () => {}, - }; - const selectedDevice = new BaseDevice( - 'serial', - 'emulator', - 'TestiPhone', - 'iOS', - ); - - const clientId = generateClientIdentifier(selectedDevice, 'app'); - const pluginStates: {[key: string]: any} = {}; - pluginStates[`${clientId}#TestDevicePlugin`] = { - msg: 'Test Device plugin', - }; - pluginStates[`${clientId}#TestPlugin`] = { - msg: 'Test plugin', - }; - const mockStore = configureStore([])({ - application: {share: {closeOnFinish: false, type: 'link'}}, - plugins: { - clientPlugins: new Map([['TestPlugin', TestPlugin]]), - devicePlugins: new Map([['TestDevicePlugin', TestDevicePlugin]]), - gatekeepedPlugins: [], - disabledPlugins: [], - failedPlugins: [], - selectedPlugins, - }, - pluginStates, - pluginMessageQueue: [], - connections: {selectedApp: clientId, clients: []}, - }); - - const client = new Client( - clientId, - { - app: 'app', - os: 'iOS', - device: 'TestiPhone', - device_id: 'serial', - }, - null, - logger, - // @ts-ignore - mockStore, - ['TestPlugin', 'TestDevicePlugin'], - selectedDevice, - ); - mockStore.dispatch({ - type: 'NEW_CLIENT', - payload: client, - }); - - return mockStore; -} - test('SettingsSheet snapshot with nothing enabled', async () => { let root: ReactTestRenderer; + const { + store, + togglePlugin, + client, + device, + pluginKey, + } = await createMockFlipperWithPlugin(TestPlugin, { + additionalPlugins: [TestDevicePlugin], + }); + + togglePlugin(); + + store.dispatch( + setPluginState({ + pluginKey, + state: {test: '1'}, + }), + ); + + expect(getExportablePlugins(store.getState(), device, client)).toEqual([]); + + // makes device plugin visible + store.dispatch( + setPluginState({ + pluginKey: getPluginKey(undefined, device, 'TestDevicePlugin'), + state: {test: '1'}, + }), + ); + + expect(getExportablePlugins(store.getState(), device, client)).toEqual([ + { + id: 'TestDevicePlugin', + label: 'TestDevicePlugin', + }, + ]); + await act(async () => { root = create( - + {}} /> , ); @@ -108,9 +91,42 @@ test('SettingsSheet snapshot with nothing enabled', async () => { test('SettingsSheet snapshot with one plugin enabled', async () => { let root: ReactTestRenderer; + const {store, device, client, pluginKey} = await createMockFlipperWithPlugin( + TestPlugin, + { + additionalPlugins: [TestDevicePlugin], + }, + ); + + // enabled, but no data + expect(getExportablePlugins(store.getState(), device, client)).toEqual([]); + + store.dispatch( + setPluginState({ + pluginKey, + state: {test: '1'}, + }), + ); + store.dispatch( + setPluginState({ + pluginKey: getPluginKey(undefined, device, 'TestDevicePlugin'), + state: {test: '1'}, + }), + ); + expect(getExportablePlugins(store.getState(), device, client)).toEqual([ + { + id: 'TestDevicePlugin', + label: 'TestDevicePlugin', + }, + { + id: 'TestPlugin', + label: 'TestPlugin', + }, + ]); + await act(async () => { root = create( - + {}} /> , ); diff --git a/desktop/app/src/chrome/__tests__/__snapshots__/ExportDataPluginSheet.node.tsx.snap b/desktop/app/src/chrome/__tests__/__snapshots__/ExportDataPluginSheet.node.tsx.snap index 504806c9f..54124ee09 100644 --- a/desktop/app/src/chrome/__tests__/__snapshots__/ExportDataPluginSheet.node.tsx.snap +++ b/desktop/app/src/chrome/__tests__/__snapshots__/ExportDataPluginSheet.node.tsx.snap @@ -59,47 +59,6 @@ exports[`SettingsSheet snapshot with nothing enabled 1`] = ` /> -
-
-
-
- - TestPlugin - -
- -
-
-
-
-