From d8f507dba0be5e4bcd7e163ccf1c49126c7df37e Mon Sep 17 00:00:00 2001 From: Anton Kastritskiy Date: Mon, 27 Nov 2023 15:33:57 -0800 Subject: [PATCH] Back out "remove all mentions of metro from flipper-ui-core" Summary: Original commit changeset: edc33c2b989e Original Phabricator Diff: D51200129 Reviewed By: lblasa Differential Revision: D51586736 fbshipit-source-id: e81355f7a0f0533ebc47f4cfc6c40af4865dbad0 --- .../reducers/__tests__/connections.node.tsx | 71 ++++- .../sandy-chrome/appinspect/AppInspect.tsx | 13 +- .../sandy-chrome/appinspect/PluginList.tsx | 45 +++- .../appinspect/__tests__/PluginList.spec.tsx | 244 +++++++++++++++++- .../src/selectors/connections.tsx | 15 +- .../flipper-ui-core/src/utils/pluginUtils.tsx | 31 ++- 6 files changed, 406 insertions(+), 13 deletions(-) diff --git a/desktop/flipper-ui-core/src/reducers/__tests__/connections.node.tsx b/desktop/flipper-ui-core/src/reducers/__tests__/connections.node.tsx index e4339f836..02ff3095a 100644 --- a/desktop/flipper-ui-core/src/reducers/__tests__/connections.node.tsx +++ b/desktop/flipper-ui-core/src/reducers/__tests__/connections.node.tsx @@ -56,6 +56,39 @@ test('doing a double REGISTER_DEVICE fails', () => { }).toThrow('still connected'); }); +test('register, remove, re-register a metro device works correctly', () => { + const device1 = new TestDevice( + 'http://localhost:8081', + 'emulator', + 'React Native', + 'Metro', + ); + let state: State = reducer(undefined, { + type: 'REGISTER_DEVICE', + payload: device1, + }); + expect(state.devices.length).toBe(1); + expect(state.devices[0].displayTitle()).toBe('React Native'); + + device1.disconnect(); + + expect(state.devices.length).toBe(1); + expect(state.devices[0].displayTitle()).toBe('React Native (Offline)'); + + state = reducer(state, { + type: 'REGISTER_DEVICE', + payload: new TestDevice( + 'http://localhost:8081', + 'emulator', + 'React Native', + 'Metro', + ), + }); + expect(state.devices.length).toBe(1); + expect(state.devices[0].displayTitle()).toBe('React Native'); + expect(state.devices[0]).not.toBe(device1); +}); + test('selectPlugin sets deepLinkPayload correctly', () => { const device1 = new TestDevice( 'http://localhost:8081', @@ -200,8 +233,10 @@ describe('selection changes', () => { let device1: BaseDevice; let device2: BaseDevice; + let metroDevice: BaseDevice; let d1app1: Client; let d1app2: Client; + let d2app1: Client; let d2app2: Client; let store: Store; let mockFlipper: MockFlipperResult; @@ -214,8 +249,13 @@ describe('selection changes', () => { device1 = mockFlipper.device; device2 = mockFlipper.createDevice({}); + metroDevice = mockFlipper.createDevice({ + os: 'Metro', + serial: 'http://localhost:8081', + }); d1app1 = mockFlipper.client; d1app2 = await mockFlipper.createClient(device1, 'd1app2'); + d2app1 = await mockFlipper.createClient(device2, 'd2app1'); d2app2 = await mockFlipper.createClient(device2, 'd2app2'); store = mockFlipper.store; }); @@ -279,6 +319,31 @@ describe('selection changes', () => { expect(getActiveDevice(store.getState())).toBe(device1); }); + test('select a metro device', async () => { + store.dispatch( + selectPlugin({ + selectedPlugin: DevicePlugin1.id, + selectedDevice: metroDevice, + selectedAppId: d2app1.id, // this app will determine the active device + }), + ); + + const state = store.getState(); + expect(state.connections).toMatchObject({ + selectedDevice: metroDevice, + selectedPlugin: DevicePlugin1.id, + selectedAppId: d2app1.id, + userPreferredDevice: metroDevice.title, + // other prefs not updated + userPreferredPlugin: DevicePlugin1.id, + userPreferredApp: d2app1.query.app, + }); + + // used by plugin list, to keep main device / app selection correct + expect(getActiveClient(state)).toBe(d2app1); + expect(getActiveDevice(state)).toBe(device2); + }); + test('introducing new client does not select it', async () => { await mockFlipper.createClient(device2, 'd2app3'); expect(store.getState().connections).toMatchObject({ @@ -336,12 +401,12 @@ describe('selection changes', () => { }); test('select device', () => { - store.dispatch(selectDevice(device1)); + store.dispatch(selectDevice(metroDevice)); expect(store.getState().connections).toMatchObject({ - selectedDevice: device1, + selectedDevice: metroDevice, selectedPlugin: TestPlugin1.id, selectedAppId: null, - userPreferredDevice: device1.title, + userPreferredDevice: metroDevice.title, // other prefs not updated userPreferredPlugin: TestPlugin1.id, userPreferredApp: d1app1.query.app, diff --git a/desktop/flipper-ui-core/src/sandy-chrome/appinspect/AppInspect.tsx b/desktop/flipper-ui-core/src/sandy-chrome/appinspect/AppInspect.tsx index ce9aa6586..72dcbb6dd 100644 --- a/desktop/flipper-ui-core/src/sandy-chrome/appinspect/AppInspect.tsx +++ b/desktop/flipper-ui-core/src/sandy-chrome/appinspect/AppInspect.tsx @@ -19,13 +19,18 @@ import Client from '../../Client'; import {BaseDevice} from 'flipper-frontend-core'; import {ExclamationCircleOutlined, FieldTimeOutlined} from '@ant-design/icons'; import {useSelector} from 'react-redux'; -import {getActiveClient, getActiveDevice} from '../../selectors/connections'; +import { + getActiveClient, + getActiveDevice, + getMetroDevice, +} from '../../selectors/connections'; import * as connections from '../../selectors/connections'; import {PluginActionsMenu} from '../../chrome/PluginActionsMenu'; const {Text} = Typography; export function AppInspect() { + const metroDevice = useSelector(getMetroDevice); const client = useSelector(getActiveClient); const activeDevice = useSelector(getActiveDevice); const isDeviceConnected = useValue(activeDevice?.connected, false); @@ -51,7 +56,11 @@ export function AppInspect() { {activeDevice ? ( - + ) : null} diff --git a/desktop/flipper-ui-core/src/sandy-chrome/appinspect/PluginList.tsx b/desktop/flipper-ui-core/src/sandy-chrome/appinspect/PluginList.tsx index 0d74fcee1..7f2b2bd20 100644 --- a/desktop/flipper-ui-core/src/sandy-chrome/appinspect/PluginList.tsx +++ b/desktop/flipper-ui-core/src/sandy-chrome/appinspect/PluginList.tsx @@ -42,9 +42,11 @@ const {Text} = Typography; export const PluginList = memo(function PluginList({ client, activeDevice, + metroDevice, }: { client: Client | null; activeDevice: BaseDevice | null; + metroDevice: BaseDevice | null; }) { const dispatch = useDispatch(); const connections = useStore((state) => state.connections); @@ -52,9 +54,11 @@ export const PluginList = memo(function PluginList({ const pluginLists = useSelector(getPluginLists); const downloads = useStore((state) => state.pluginDownloads); const isConnected = useValue(activeDevice?.connected, false); + const metroConnected = useValue(metroDevice?.connected, false); const { devicePlugins, + metroPlugins, enabledPlugins, disabledPlugins, unavailablePlugins, @@ -92,6 +96,19 @@ export const PluginList = memo(function PluginList({ }, [dispatch, activeDevice, connections.selectedAppId], ); + const handleMetroPluginClick = useCallback( + (pluginId) => { + dispatch( + selectPlugin({ + selectedPlugin: pluginId, + selectedAppId: connections.selectedAppId, + deepLinkPayload: null, + selectedDevice: metroDevice, + }), + ); + }, + [dispatch, metroDevice, connections.selectedAppId], + ); const handleEnablePlugin = useCallback( (id: string) => { const plugin = (plugins.clientPlugins.get(id) ?? @@ -190,15 +207,39 @@ export const PluginList = memo(function PluginList({ {}} - defaultOpenKeys={['enabled']} + defaultOpenKeys={['enabled', 'metro']} selectedKeys={ - connections.selectedPlugin ? [connections.selectedPlugin] : [] + connections.selectedPlugin + ? [ + (connections.selectedDevice === metroDevice ? 'metro:' : '') + + connections.selectedPlugin, + ] + : [] } mode="inline"> {allEnabledPlugins} + {!isArchived && metroConnected && ( + + {metroPlugins.map((plugin) => ( + + ))} + + )} {isConnected && ( {} @@ -39,3 +63,221 @@ describe('basic getActiveDevice', () => { expect(getActiveDevice(store.getState())).toBe(device); }); }); + +describe('basic getActiveDevice with metro present', () => { + let flipper: MockFlipperResult; + let metro: BaseDevice; + let testDevice: BaseDevice; + + beforeEach(async () => { + flipper = await createMockFlipperWithPlugin(logsPlugin); + flipper.device.supportsPlugin = (p) => { + return p.id !== 'unsupportedDevicePlugin'; + }; + testDevice = flipper.device; + // flipper.store.dispatch(registerPlugins([LogsPlugin])) + flipper.store.dispatch({ + type: 'REGISTER_DEVICE', + payload: new TestDevice( + 'http://localhost:8081', + 'physical', + 'metro', + 'Metro', + ), + }); + metro = getMetroDevice(flipper.store.getState())!; + metro.supportsPlugin = (p) => { + return p.id !== 'unsupportedDevicePlugin'; + }; + }); + + test('findMetroDevice', () => { + expect(metro.os).toBe('Metro'); + }); + + test('correct base selection state', () => { + const state = flipper.store.getState(); + const {connections} = state; + expect(connections).toMatchObject({ + devices: [testDevice, metro], + selectedDevice: testDevice, + selectedPlugin: 'DeviceLogs', + userPreferredDevice: 'MockAndroidDevice', + userPreferredPlugin: 'DeviceLogs', + userPreferredApp: 'TestApp', + }); + expect(getActiveClient(state)).toBe(flipper.client); + }); + + test('selecting Metro Logs works but keeps normal device preferred', () => { + expect(getActiveClient(flipper.store.getState())).toBe(flipper.client); + flipper.store.dispatch( + selectPlugin({ + selectedPlugin: logsPlugin.id, + selectedAppId: flipper.client.id, + selectedDevice: metro, + deepLinkPayload: null, + }), + ); + expect(flipper.store.getState().connections).toMatchObject({ + devices: [testDevice, metro], + selectedAppId: 'TestApp#Android#MockAndroidDevice#serial', + selectedDevice: metro, + selectedPlugin: 'DeviceLogs', + userPreferredDevice: 'MockAndroidDevice', // Not metro! + userPreferredPlugin: 'DeviceLogs', + userPreferredApp: 'TestApp', + }); + const state = flipper.store.getState(); + // find best device is still metro + expect(getActiveDevice(state)).toBe(testDevice); + // find best client still returns app + expect(getActiveClient(state)).toBe(flipper.client); + }); + + test('computePluginLists', () => { + const state = flipper.store.getState(); + expect(getPluginLists(state)).toEqual({ + downloadablePlugins: [], + devicePlugins: [logsPlugin], + metroPlugins: [logsPlugin], + enabledPlugins: [], + disabledPlugins: [], + unavailablePlugins: [], + }); + }); + + test('computePluginLists with problematic plugins', () => { + const noopPlugin = { + plugin() {}, + Component() { + return null; + }, + }; + + const unsupportedDevicePlugin = new _SandyPluginDefinition( + createMockPluginDetails({ + id: 'unsupportedDevicePlugin', + title: 'Unsupported Device Plugin', + }), + { + devicePlugin() { + return {}; + }, + supportsDevice() { + return false; + }, + Component() { + return null; + }, + }, + ); + const unsupportedPlugin = new _SandyPluginDefinition( + createMockPluginDetails({ + id: 'unsupportedPlugin', + title: 'Unsupported Plugin', + }), + noopPlugin, + ); + + const gateKeepedPlugin = createMockPluginDetails({ + id: 'gateKeepedPlugin', + title: 'Gatekeeped Plugin', + gatekeeper: 'not for you', + }); + + const plugin1 = new _SandyPluginDefinition( + createMockPluginDetails({ + id: 'plugin1', + title: 'Plugin 1', + }), + { + plugin() {}, + Component() { + return null; + }, + }, + ); + + const plugin2 = new _SandyPluginDefinition( + createMockPluginDetails({ + id: 'plugin2', + title: 'Plugin 2', + }), + noopPlugin, + ); + + const supportedDownloadablePlugin = createMockDownloadablePluginDetails({ + id: 'supportedUninstalledPlugin', + title: 'Supported Uninstalled Plugin', + }); + + const unsupportedDownloadablePlugin = createMockDownloadablePluginDetails({ + id: 'unsupportedUninstalledPlugin', + title: 'Unsupported Uninstalled Plugin', + }); + + flipper.store.dispatch( + registerPlugins([ + unsupportedDevicePlugin, + unsupportedPlugin, + plugin1, + plugin2, + ]), + ); + flipper.store.dispatch(addGatekeepedPlugins([gateKeepedPlugin])); + flipper.store.dispatch( + registerMarketplacePlugins([ + supportedDownloadablePlugin, + unsupportedDownloadablePlugin, + ]), + ); + + // ok, this is a little hackish + flipper.client.plugins = new Set([ + 'plugin1', + 'plugin2', + 'supportedUninstalledPlugin', + ]); + + let state = flipper.store.getState(); + const pluginLists = getPluginLists(state); + expect(pluginLists).toEqual({ + devicePlugins: [logsPlugin], + metroPlugins: [logsPlugin], + enabledPlugins: [], + disabledPlugins: [plugin1, plugin2], + unavailablePlugins: [ + [ + gateKeepedPlugin, + "Plugin 'Gatekeeped Plugin' is only available to members of gatekeeper 'not for you'", + ], + [ + unsupportedDevicePlugin.details, + "Device plugin 'Unsupported Device Plugin' is not supported by the selected device 'MockAndroidDevice' (Android)", + ], + [ + unsupportedPlugin.details, + "Plugin 'Unsupported Plugin' is not supported by the selected application 'TestApp' (Android)", + ], + [ + unsupportedDownloadablePlugin, + "Plugin 'Unsupported Uninstalled Plugin' is not supported by the selected application 'TestApp' (Android) and not installed in Flipper", + ], + ], + downloadablePlugins: [supportedDownloadablePlugin], + }); + + flipper.store.dispatch( + switchPlugin({ + plugin: plugin2, + selectedApp: flipper.client.query.app, + }), + ); + state = flipper.store.getState(); + expect(getPluginLists(state)).toMatchObject({ + enabledPlugins: [plugin2], + disabledPlugins: [plugin1], + }); + }); +}); diff --git a/desktop/flipper-ui-core/src/selectors/connections.tsx b/desktop/flipper-ui-core/src/selectors/connections.tsx index 99ca68c45..e5a5a7e8f 100644 --- a/desktop/flipper-ui-core/src/selectors/connections.tsx +++ b/desktop/flipper-ui-core/src/selectors/connections.tsx @@ -26,6 +26,13 @@ const getPluginDownloads = (state: State) => state.pluginDownloads; export const getActiveClient = (state: State) => state.connections.clients.get(state.connections.selectedAppId!) ?? null; +export const getMetroDevice = createSelector(getDevices, (devices) => { + return ( + devices.find((device) => device.os === 'Metro' && !device.isArchived) ?? + null + ); +}); + export const getSelectableDevices = createSelector( getDevices, getClients, @@ -48,7 +55,12 @@ export const hasSelectableDevices = createSelector( export const getActiveDevice = createSelector( getSelectedDevice, getActiveClient, - (selectedDevice, client) => { + getMetroDevice, + (selectedDevice, client, metroDevice) => { + // if not Metro device, use the selected device as metro device + if (selectedDevice !== metroDevice) { + return selectedDevice; + } // if there is an active app, use device owning the app if (client) { // TODO: Will be fixed later in the stack @@ -90,6 +102,7 @@ export const getPluginLists = createSelector( failedPlugins, }), getActiveDevice, + getMetroDevice, getActiveClient, computePluginLists, ); diff --git a/desktop/flipper-ui-core/src/utils/pluginUtils.tsx b/desktop/flipper-ui-core/src/utils/pluginUtils.tsx index b702e0205..3f53ff33e 100644 --- a/desktop/flipper-ui-core/src/utils/pluginUtils.tsx +++ b/desktop/flipper-ui-core/src/utils/pluginUtils.tsx @@ -25,6 +25,7 @@ import {getAppVersion} from './info'; export type PluginLists = { devicePlugins: PluginDefinition[]; + metroPlugins: PluginDefinition[]; enabledPlugins: PluginDefinition[]; disabledPlugins: PluginDefinition[]; unavailablePlugins: [plugin: PluginDetails, reason: string][]; @@ -169,9 +170,11 @@ export function computePluginLists( | 'clientPlugins' >, device: BaseDevice | null, + metroDevice: BaseDevice | null, client: Client | null, ): { devicePlugins: PluginDefinition[]; + metroPlugins: PluginDefinition[]; enabledPlugins: PluginDefinition[]; disabledPlugins: PluginDefinition[]; unavailablePlugins: [plugin: PluginDetails, reason: string][]; @@ -186,19 +189,26 @@ export function computePluginLists( const devicePlugins: PluginDefinition[] = [...plugins.devicePlugins.values()] .filter((p) => device?.supportsPlugin(p)) .filter((p) => enabledDevicePluginsState.has(p.id)); + const metroPlugins: PluginDefinition[] = [...plugins.devicePlugins.values()] + .filter((p) => metroDevice?.supportsPlugin(p)) + .filter((p) => enabledDevicePluginsState.has(p.id)); const enabledPlugins: PluginDefinition[] = []; const disabledPlugins: PluginDefinition[] = [ ...plugins.devicePlugins.values(), ] - .filter((p) => device?.supportsPlugin(p.details)) + .filter( + (p) => + device?.supportsPlugin(p.details) || + metroDevice?.supportsPlugin(p.details), + ) .filter((p) => !enabledDevicePluginsState.has(p.id)); const unavailablePlugins: [plugin: PluginDetails, reason: string][] = []; const downloadablePlugins: DownloadablePluginDetails[] = []; if (device) { - // find all device plugins that aren't part of the current device + // find all device plugins that aren't part of the current device / metro for (const p of plugins.devicePlugins.values()) { - if (!device.supportsPlugin(p)) { + if (!device.supportsPlugin(p) && !metroDevice?.supportsPlugin(p)) { unavailablePlugins.push([ p.details, `Device plugin '${getPluginTitle( @@ -212,7 +222,10 @@ export function computePluginLists( for (const plugin of uninstalledMarketplacePlugins.filter( (d) => d.pluginType === 'device', )) { - if (device.supportsPlugin(plugin)) { + if ( + device.supportsPlugin(plugin) || + metroDevice?.supportsPlugin(plugin) + ) { downloadablePlugins.push(plugin); } } @@ -314,6 +327,7 @@ export function computePluginLists( enabledPlugins.sort(sortPluginsByName); devicePlugins.sort(sortPluginsByName); disabledPlugins.sort(sortPluginsByName); + metroPlugins.sort(sortPluginsByName); unavailablePlugins.sort(([a], [b]) => { return getPluginTitle(a) > getPluginTitle(b) ? 1 : -1; }); @@ -323,6 +337,7 @@ export function computePluginLists( return { devicePlugins, + metroPlugins, enabledPlugins, disabledPlugins, unavailablePlugins, @@ -356,6 +371,7 @@ function getFavoritePlugins( export function computeActivePluginList({ enabledPlugins, devicePlugins, + metroPlugins, disabledPlugins, downloadablePlugins, unavailablePlugins, @@ -375,6 +391,13 @@ export function computeActivePluginList({ definition: plugin, }; } + for (const plugin of metroPlugins) { + pluginList[plugin.id] = { + status: 'enabled', + details: plugin.details, + definition: plugin, + }; + } for (const plugin of disabledPlugins) { pluginList[plugin.id] = { status: 'disabled',