From 7bc38c17351b61b85ef100347b80b1370bec9de7 Mon Sep 17 00:00:00 2001 From: Anton Nikolaev Date: Tue, 29 Jun 2021 13:00:18 -0700 Subject: [PATCH] Allow selecting disabled, unsupported and not installed plugins Summary: This diffs removes restrictions on selecting disabled, uninstalled and unsupported plugins. Now it will be possible to select them, however for now empty view will be shown in such case. In next diffs I'll add Plugin Info view which will be shown in case disabled plugin is selected. It will show static info about plugin - readme, version, oncall, user feedback group etc. This will make it possible for users to browse info about all plugins, even unsupported by selected device/app. There is one problem that our analytics will be screwed by this change as even disabled plugins will be counted as used. I'll address this later in this stack. If you see other potential problems with removing restrictions on selecting disabled plugins - please let me know. Reviewed By: passy Differential Revision: D29186005 fbshipit-source-id: 2e55c5fd3bb402594f4dbace6e287725de65bc6f --- .../src/__tests__/PluginContainer.node.tsx | 10 ++------ desktop/app/src/devices/BaseDevice.tsx | 8 ------ desktop/app/src/devices/DummyDevice.tsx | 1 - desktop/app/src/devices/JSDevice.tsx | 1 - desktop/app/src/devices/MetroDevice.tsx | 1 - .../reducers/__tests__/connections.node.tsx | 25 ------------------- desktop/app/src/reducers/connections.tsx | 11 +++----- .../sandy-chrome/appinspect/PluginList.tsx | 5 +++- .../src/utils/__tests__/exportData.node.tsx | 1 - .../src/utils/flipperLibImplementation.tsx | 24 ------------------ .../src/plugin/DevicePlugin.tsx | 13 +--------- .../flipper-plugin/src/plugin/FlipperLib.tsx | 5 ---- desktop/flipper-plugin/src/plugin/Plugin.tsx | 20 +++------------ .../src/test-utils/test-utils.tsx | 3 --- 14 files changed, 13 insertions(+), 115 deletions(-) diff --git a/desktop/app/src/__tests__/PluginContainer.node.tsx b/desktop/app/src/__tests__/PluginContainer.node.tsx index 4beaf944c..3ccceb3bf 100644 --- a/desktop/app/src/__tests__/PluginContainer.node.tsx +++ b/desktop/app/src/__tests__/PluginContainer.node.tsx @@ -899,7 +899,6 @@ test('Sandy plugins support isPluginSupported + selectPlugin', async () => { return { activatedStub, deactivatedStub, - isPluginAvailable: client.isPluginAvailable, selectPlugin: client.selectPlugin, }; }; @@ -953,10 +952,6 @@ test('Sandy plugins support isPluginSupported + selectPlugin', async () => { const pluginInstance: ReturnType = client.sandyPluginStates.get(definition.id)!.instanceApi; - expect(pluginInstance.isPluginAvailable(definition.id)).toBeTruthy(); - expect(pluginInstance.isPluginAvailable('nonsense')).toBeFalsy(); - expect(pluginInstance.isPluginAvailable(definition2.id)).toBeFalsy(); // not enabled yet - expect(pluginInstance.isPluginAvailable(definition3.id)).toBeFalsy(); // not enabled yet expect(pluginInstance.activatedStub).toBeCalledTimes(1); expect(pluginInstance.deactivatedStub).toBeCalledTimes(0); expect(linksSeen).toEqual([]); @@ -964,7 +959,6 @@ test('Sandy plugins support isPluginSupported + selectPlugin', async () => { // star and navigate to a device plugin store.dispatch(switchPlugin({plugin: definition3})); pluginInstance.selectPlugin(definition3.id); - expect(pluginInstance.isPluginAvailable(definition3.id)).toBeTruthy(); expect(store.getState().connections.selectedPlugin).toBe(definition3.id); expect(renderer.baseElement.querySelector('h1')).toMatchInlineSnapshot(`

@@ -985,9 +979,9 @@ test('Sandy plugins support isPluginSupported + selectPlugin', async () => { `); expect(linksSeen).toEqual(['data']); - // try to go to plugin 2, fails (not enabled, so no-op) + // try to plugin 2 - it should be possible to select it even if it is not enabled pluginInstance.selectPlugin(definition2.id); - expect(store.getState().connections.selectedPlugin).toBe(definition.id); + expect(store.getState().connections.selectedPlugin).toBe(definition2.id); // star plugin 2 and navigate to plugin 2 store.dispatch( diff --git a/desktop/app/src/devices/BaseDevice.tsx b/desktop/app/src/devices/BaseDevice.tsx index 878e83e28..7bd14ebd7 100644 --- a/desktop/app/src/devices/BaseDevice.tsx +++ b/desktop/app/src/devices/BaseDevice.tsx @@ -84,9 +84,6 @@ export default class BaseDevice { // if imported, stores the original source location source = ''; - // sorted list of supported device plugins - devicePlugins: string[] = []; - sandyPluginStates: Map = new Map< string, _SandyDevicePluginInstance @@ -248,7 +245,6 @@ export default class BaseDevice { return; } this.hasDevicePlugins = true; - this.devicePlugins.push(plugin.id); if (plugin instanceof _SandyPluginDefinition) { this.sandyPluginStates.set( plugin.id, @@ -274,10 +270,6 @@ export default class BaseDevice { instance.destroy(); this.sandyPluginStates.delete(pluginId); } - const index = this.devicePlugins.indexOf(pluginId); - if (index >= 0) { - this.devicePlugins.splice(index, 1); - } } disconnect() { diff --git a/desktop/app/src/devices/DummyDevice.tsx b/desktop/app/src/devices/DummyDevice.tsx index e940cf649..6de7dfea3 100644 --- a/desktop/app/src/devices/DummyDevice.tsx +++ b/desktop/app/src/devices/DummyDevice.tsx @@ -15,6 +15,5 @@ import BaseDevice, {OS} from './BaseDevice'; export default class DummyDevice extends BaseDevice { constructor(serial: string, title: string, os: OS) { super(serial, 'dummy', title, os); - this.devicePlugins = []; } } diff --git a/desktop/app/src/devices/JSDevice.tsx b/desktop/app/src/devices/JSDevice.tsx index b5691739e..d5db6f260 100644 --- a/desktop/app/src/devices/JSDevice.tsx +++ b/desktop/app/src/devices/JSDevice.tsx @@ -14,7 +14,6 @@ export default class JSDevice extends BaseDevice { constructor(serial: string, title: string, webContentsId: number) { super(serial, 'emulator', title, 'JSWebApp'); - this.devicePlugins = []; this.webContentsId = webContentsId; } } diff --git a/desktop/app/src/devices/MetroDevice.tsx b/desktop/app/src/devices/MetroDevice.tsx index 848c0b747..85b839c7a 100644 --- a/desktop/app/src/devices/MetroDevice.tsx +++ b/desktop/app/src/devices/MetroDevice.tsx @@ -145,7 +145,6 @@ export default class MetroDevice extends BaseDevice { constructor(serial: string, ws: WebSocket | undefined) { super(serial, 'emulator', 'React Native', 'Metro'); - this.devicePlugins = []; if (ws) { this.ws = ws; ws.onmessage = this._handleWSMessage; diff --git a/desktop/app/src/reducers/__tests__/connections.node.tsx b/desktop/app/src/reducers/__tests__/connections.node.tsx index 846705462..62909048e 100644 --- a/desktop/app/src/reducers/__tests__/connections.node.tsx +++ b/desktop/app/src/reducers/__tests__/connections.node.tsx @@ -56,31 +56,6 @@ test('register, remove, re-register a metro device works correctly', () => { expect(state.devices[0]).not.toBe(device1); }); -test('triggering REGISTER_DEVICE before REGISTER_PLUGINS still registers device plugins', () => { - class TestDevicePlugin extends FlipperDevicePlugin { - static id = 'test'; - static supportsDevice() { - return true; - } - static details = TestUtils.createMockPluginDetails({ - id: 'test', - pluginType: 'device', - }); - } - - const stateWithDevice = reducer(undefined, { - type: 'REGISTER_DEVICE', - payload: new MacDevice(), - }); - - const endState = reducer(stateWithDevice, { - type: 'REGISTER_PLUGINS', - payload: [TestDevicePlugin], - }); - - expect(endState.devices[0].devicePlugins).toEqual(['test']); -}); - test('selectPlugin sets deepLinkPayload correctly', () => { const state = reducer( undefined, diff --git a/desktop/app/src/reducers/connections.tsx b/desktop/app/src/reducers/connections.tsx index 57cf5f1e9..d3bc49602 100644 --- a/desktop/app/src/reducers/connections.tsx +++ b/desktop/app/src/reducers/connections.tsx @@ -649,20 +649,15 @@ function updateSelection(state: Readonly): State { updates.metroDevice, ); - const availablePlugins: string[] = [ - ...(device?.devicePlugins || []), - ...(updates.activeClient?.plugins || []), - ]; - if ( // Try the preferred plugin first state.userPreferredPlugin && - availablePlugins.includes(state.userPreferredPlugin) + state.userPreferredPlugin !== state.selectedPlugin ) { updates.selectedPlugin = state.userPreferredPlugin; } else if ( - !state.selectedPlugin || - !availablePlugins.includes(state.selectedPlugin) + !state.selectedPlugin && + state.enabledDevicePlugins.has(DEFAULT_PLUGIN) ) { // currently selected plugin is not available in this state, // fall back to the default diff --git a/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx b/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx index 097a054fc..e867ed079 100644 --- a/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx +++ b/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx @@ -249,6 +249,7 @@ export const PluginList = memo(function PluginList({ plugin={plugin.details} scrollTo={plugin.id === connections.selectedPlugin} tooltip={getPluginTooltip(plugin.details)} + onClick={handleAppPluginClick} actions={ <> {reason}} /> @@ -396,7 +399,7 @@ const PluginEntry = function PluginEntry({ { const device2 = store.getState().connections.devices[1]; expect(device2).not.toBeFalsy(); expect(device2).not.toBe(device); - expect(device2.devicePlugins).toEqual([sandyDeviceTestPlugin.id]); const {counter} = device2.sandyPluginStates.get( sandyDeviceTestPlugin.id, diff --git a/desktop/app/src/utils/flipperLibImplementation.tsx b/desktop/app/src/utils/flipperLibImplementation.tsx index 670ecef88..ccf2c154a 100644 --- a/desktop/app/src/utils/flipperLibImplementation.tsx +++ b/desktop/app/src/utils/flipperLibImplementation.tsx @@ -35,30 +35,6 @@ export function initializeFlipperLibImplementation( GK(gatekeeper: string) { return GK.get(gatekeeper); }, - isPluginAvailable(device, client, pluginId) { - // supported device pluin - if (device.devicePlugins.includes(pluginId)) { - return true; - } - if (client) { - // plugin supported? - if (client.plugins.includes(pluginId)) { - // part of an archived device? - if (device.isArchived) { - return true; - } - // plugin enabled? - if ( - store - .getState() - .connections.enabledPlugins[client.query.app]?.includes(pluginId) - ) { - return true; - } - } - } - return false; - }, selectPlugin(device, client, pluginId, deeplink) { store.dispatch({ type: 'SELECT_PLUGIN', diff --git a/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx b/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx index 97bc53d49..071f8b19b 100644 --- a/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx +++ b/desktop/flipper-plugin/src/plugin/DevicePlugin.tsx @@ -51,11 +51,6 @@ export type DevicePluginPredicate = (device: Device) => boolean; export type DevicePluginFactory = (client: DevicePluginClient) => object; export interface DevicePluginClient extends BasePluginClient { - /** - * Checks if the provided plugin is available for the current device - */ - isPluginAvailable(pluginId: string): boolean; - /** * opens a different plugin by id, optionally providing a deeplink to bring the plugin to a certain state */ @@ -77,7 +72,6 @@ export interface RealFlipperDevice { addLogListener(callback: DeviceLogListener): Symbol; removeLogListener(id: Symbol): void; addLogEntry(entry: DeviceLogEntry): void; - devicePlugins: string[]; } export class SandyDevicePluginInstance extends BasePluginInstance { @@ -98,13 +92,8 @@ export class SandyDevicePluginInstance extends BasePluginInstance { super(flipperLib, definition, realDevice, pluginKey, initialStates); this.client = { ...this.createBasePluginClient(), - isPluginAvailable(pluginId: string) { - return flipperLib.isPluginAvailable(realDevice, null, pluginId); - }, selectPlugin(pluginId: string, deeplink?: unknown) { - if (this.isPluginAvailable(pluginId)) { - flipperLib.selectPlugin(realDevice, null, pluginId, deeplink); - } + flipperLib.selectPlugin(realDevice, null, pluginId, deeplink); }, get isConnected() { return realDevice.connected.get(); diff --git a/desktop/flipper-plugin/src/plugin/FlipperLib.tsx b/desktop/flipper-plugin/src/plugin/FlipperLib.tsx index c8b41d755..1712012cc 100644 --- a/desktop/flipper-plugin/src/plugin/FlipperLib.tsx +++ b/desktop/flipper-plugin/src/plugin/FlipperLib.tsx @@ -23,11 +23,6 @@ export interface FlipperLib { enableMenuEntries(menuEntries: NormalizedMenuEntry[]): void; createPaste(input: string): Promise; GK(gatekeeper: string): boolean; - isPluginAvailable( - device: RealFlipperDevice, - client: RealFlipperClient | null, - pluginId: string, - ): boolean; selectPlugin( device: RealFlipperDevice, client: RealFlipperClient | null, diff --git a/desktop/flipper-plugin/src/plugin/Plugin.tsx b/desktop/flipper-plugin/src/plugin/Plugin.tsx index d84422c8d..c1003918b 100644 --- a/desktop/flipper-plugin/src/plugin/Plugin.tsx +++ b/desktop/flipper-plugin/src/plugin/Plugin.tsx @@ -88,11 +88,6 @@ export interface PluginClient< */ supportsMethod(method: keyof Methods): Promise; - /** - * Checks if the provided plugin is available for the current device / application - */ - isPluginAvailable(pluginId: string): boolean; - /** * opens a different plugin by id, optionally providing a deeplink to bring the plugin to a certain state */ @@ -202,23 +197,14 @@ export class SandyPluginInstance extends BasePluginInstance { method as any, ); }, - isPluginAvailable(pluginId: string) { - return flipperLib.isPluginAvailable( + selectPlugin(pluginId: string, deeplink?: unknown) { + flipperLib.selectPlugin( realClient.deviceSync, realClient, pluginId, + deeplink, ); }, - selectPlugin(pluginId: string, deeplink?: unknown) { - if (this.isPluginAvailable(pluginId)) { - flipperLib.selectPlugin( - realClient.deviceSync, - realClient, - pluginId, - deeplink, - ); - } - }, }; this.initializePlugin(() => definition.asPluginModule().plugin(this.client), diff --git a/desktop/flipper-plugin/src/test-utils/test-utils.tsx b/desktop/flipper-plugin/src/test-utils/test-utils.tsx index 67b7cc514..df9968289 100644 --- a/desktop/flipper-plugin/src/test-utils/test-utils.tsx +++ b/desktop/flipper-plugin/src/test-utils/test-utils.tsx @@ -314,7 +314,6 @@ export function startDevicePlugin( const flipperLib = createMockFlipperLib(options); const testDevice = createMockDevice(options); - testDevice.devicePlugins.push(definition.id); const pluginInstance = new SandyDevicePluginInstance( flipperLib, definition, @@ -374,7 +373,6 @@ export function createMockFlipperLib(options?: StartPluginOptions): FlipperLib { return options?.GKs?.includes(gk) || false; }, selectPlugin: jest.fn(), - isPluginAvailable: jest.fn().mockImplementation(() => false), writeTextToClipboard: jest.fn(), showNotification: jest.fn(), }; @@ -495,7 +493,6 @@ function createMockDevice(options?: StartPluginOptions): RealFlipperDevice { serial: 'serial-000', isArchived: !!options?.isArchived, connected: createState(true), - devicePlugins: [], addLogListener(cb) { logListeners.push(cb); return (logListeners.length - 1) as any;