Refactor plugin lists computations

Summary: This is purely refactoring change. Before that we computed plugin lists in-place in PluginList component. Now we will be re-computing them as side effect and will keep computed lists in redux. This makes it easier to re-use plugin lists in other places outside of PluginList component, e.g. in the upcoming Marketplace UI.

Reviewed By: mweststrate

Differential Revision: D29161719

fbshipit-source-id: 5cb06d4d8a553aa856101c78b2311fbc078c6bd7
This commit is contained in:
Anton Nikolaev
2021-06-17 07:38:51 -07:00
committed by Facebook GitHub Bot
parent 0d6262aa5e
commit ac9ef7620a
10 changed files with 325 additions and 190 deletions

View File

@@ -2,6 +2,22 @@
exports[`can create a Fake flipper 1`] = `
Object {
"activeClient": Object {
"id": "TestApp#Android#MockAndroidDevice#serial",
"query": Object {
"app": "TestApp",
"device": "MockAndroidDevice",
"device_id": "serial",
"os": "Android",
"sdk_version": 4,
},
},
"activeDevice": Object {
"deviceType": "physical",
"os": "Android",
"serial": "serial",
"title": "MockAndroidDevice",
},
"androidEmulators": Array [],
"clients": Array [
Object {
@@ -36,6 +52,7 @@ Object {
"TestPlugin",
],
},
"metroDevice": null,
"selectedApp": "TestApp#Android#MockAndroidDevice#serial",
"selectedDevice": Object {
"deviceType": "physical",

View File

@@ -23,6 +23,7 @@ import reactNative from './reactNative';
import pluginMarketplace from './fb-stubs/pluginMarketplace';
import pluginDownloads from './pluginDownloads';
import info from '../utils/info';
import pluginLists from './pluginLists';
import {Logger} from '../fb-interfaces/Logger';
import {Store} from '../reducers/index';
@@ -51,6 +52,7 @@ export default function (store: Store, logger: Logger): () => Promise<void> {
pluginMarketplace,
pluginDownloads,
info,
pluginLists,
].filter(notNull);
const globalCleanup = dispatchers
.map((dispatcher) => dispatcher(store, logger))

View File

@@ -0,0 +1,120 @@
/**
* 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 Client from '../Client';
import {Logger} from '../fb-interfaces/Logger';
import {Store} from '../reducers';
import {pluginListsChanged} from '../reducers/pluginLists';
import {computePluginLists} from '../utils/pluginUtils';
import {sideEffect} from '../utils/sideEffect';
export default (store: Store, _logger: Logger) => {
const recomputePluginList = () => {
store.dispatch(
pluginListsChanged(
computePluginLists(
store.getState().connections,
store.getState().plugins,
),
),
);
};
let prevClient: null | Client = null;
sideEffect(
store,
{name: 'computePluginLists', throttleMs: 100, fireImmediately: true},
(state) => {
const {
activeClient,
activeDevice,
metroDevice,
enabledDevicePlugins,
enabledPlugins,
} = state.connections;
const {
bundledPlugins,
marketplacePlugins,
loadedPlugins,
devicePlugins,
disabledPlugins,
gatekeepedPlugins,
failedPlugins,
clientPlugins,
} = state.plugins;
return {
activeClient,
activeDevice,
metroDevice,
enabledDevicePlugins,
enabledPlugins,
bundledPlugins,
marketplacePlugins,
loadedPlugins,
devicePlugins,
disabledPlugins,
gatekeepedPlugins,
failedPlugins,
clientPlugins,
};
},
(
{
activeClient,
activeDevice,
metroDevice,
enabledDevicePlugins,
enabledPlugins,
bundledPlugins,
marketplacePlugins,
loadedPlugins,
devicePlugins,
disabledPlugins,
gatekeepedPlugins,
failedPlugins,
clientPlugins,
},
store,
) => {
store.dispatch(
pluginListsChanged(
computePluginLists(
{
activeClient,
activeDevice,
metroDevice,
enabledDevicePlugins,
enabledPlugins,
},
{
bundledPlugins,
marketplacePlugins,
loadedPlugins,
devicePlugins,
disabledPlugins,
gatekeepedPlugins,
failedPlugins,
clientPlugins,
},
),
),
);
if (activeClient !== prevClient) {
if (prevClient) {
prevClient.off('plugins-change', recomputePluginList);
}
prevClient = activeClient;
if (prevClient) {
prevClient.on('plugins-change', recomputePluginList);
}
}
},
);
};

View File

@@ -78,6 +78,9 @@ type StateV2 = {
}>;
deepLinkPayload: unknown;
staticView: StaticView;
activeClient: Client | null;
activeDevice: BaseDevice | null;
metroDevice: MetroDevice | null;
};
type StateV1 = Omit<StateV2, 'enabledPlugins' | 'enabledDevicePlugins'> & {
@@ -201,6 +204,9 @@ const INITAL_STATE: State = {
uninitializedClients: [],
deepLinkPayload: null,
staticView: WelcomeScreenStaticView,
activeClient: null,
activeDevice: null,
metroDevice: null,
};
export default (state: State = INITAL_STATE, action: Actions): State => {
@@ -619,16 +625,33 @@ function updateSelection(state: Readonly<State>): State {
}
// Select client based on device
const client = getBestAvailableClient(
updates.activeClient = getBestAvailableClient(
device,
state.clients,
state.selectedApp || state.userPreferredApp,
);
updates.selectedApp = client ? client.id : null;
updates.selectedApp = updates.activeClient ? updates.activeClient.id : null;
updates.metroDevice =
(state.devices?.find(
(device) => device.os === 'Metro' && !device.isArchived,
) as MetroDevice) ?? null;
updates.activeClient =
state.clients.find(
(c) => c.id === (updates.selectedApp || state.userPreferredApp),
) ?? null;
// if the selected device is Metro, we want to keep the owner of the selected App as active device if possible
updates.activeDevice = findBestDevice(
state,
updates.activeClient,
updates.metroDevice,
);
const availablePlugins: string[] = [
...(device?.devicePlugins || []),
...(client?.plugins || []),
...(updates.activeClient?.plugins || []),
];
if (
@@ -649,6 +672,31 @@ function updateSelection(state: Readonly<State>): State {
return {...state, ...updates};
}
export function findBestDevice(
state: State,
client: Client | null,
metroDevice: BaseDevice | null,
): BaseDevice | null {
// if not Metro device, use the selected device as metro device
const selected = state.selectedDevice ?? null;
if (selected !== metroDevice) {
return selected;
}
// if there is an active app, use device owning the app
if (client) {
return client.deviceSync;
}
// if no active app, use the preferred device
if (state.userPreferredDevice) {
return (
state.devices.find(
(device) => device.title === state.userPreferredDevice,
) ?? selected
);
}
return selected;
}
export function getSelectedPluginKey(state: State): string | undefined {
return state.selectedPlugin
? getPluginKey(

View File

@@ -64,6 +64,10 @@ import usageTracking, {
Action as TrackingAction,
State as TrackingState,
} from './usageTracking';
import pluginLists, {
State as PluginListsState,
Action as PluginListsAction,
} from './pluginLists';
import user, {State as UserState, Action as UserAction} from './user';
import JsonFileStorage from '../utils/jsonFileReduxPersistStorage';
import LauncherSettingsStorage from '../utils/launcherSettingsStorage';
@@ -93,6 +97,7 @@ export type Actions =
| HealthcheckAction
| TrackingAction
| PluginDownloadsAction
| PluginListsAction
| {type: 'INIT'};
export type State = {
@@ -110,6 +115,7 @@ export type State = {
healthchecks: HealthcheckState & PersistPartial;
usageTracking: TrackingState;
pluginDownloads: PluginDownloadsState;
pluginLists: PluginListsState;
};
export type Store = ReduxStore<State, Actions>;
@@ -211,4 +217,5 @@ export default combineReducers<State, Actions>({
),
usageTracking,
pluginDownloads,
pluginLists,
});

View File

@@ -0,0 +1,68 @@
/**
* 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 {
PluginDetails,
DownloadablePluginDetails,
BundledPluginDetails,
} from 'flipper-plugin-lib';
import {Actions} from '.';
import {
DevicePluginDefinition,
ClientPluginDefinition,
PluginDefinition,
} from '../plugin';
import produce from 'immer';
export type State = {
devicePlugins: DevicePluginDefinition[];
metroPlugins: DevicePluginDefinition[];
enabledPlugins: ClientPluginDefinition[];
disabledPlugins: PluginDefinition[];
unavailablePlugins: [plugin: PluginDetails, reason: string][];
downloadablePlugins: (DownloadablePluginDetails | BundledPluginDetails)[];
};
const INITIAL_STATE: State = {
devicePlugins: [],
metroPlugins: [],
enabledPlugins: [],
disabledPlugins: [],
unavailablePlugins: [],
downloadablePlugins: [],
};
export type Action = {
type: 'PLUGIN_LISTS_CHANGED';
payload: State;
};
export default function reducer(
state: State | undefined = INITIAL_STATE,
action: Actions,
): State {
if (action.type === 'PLUGIN_LISTS_CHANGED') {
const payload = action.payload;
return produce(state, (draft) => {
draft.devicePlugins = payload.devicePlugins;
draft.metroPlugins = payload.metroPlugins;
draft.enabledPlugins = payload.enabledPlugins;
draft.disabledPlugins = payload.disabledPlugins;
draft.unavailablePlugins = payload.unavailablePlugins;
draft.downloadablePlugins = payload.downloadablePlugins;
});
} else {
return state;
}
}
export const pluginListsChanged = (payload: State): Action => ({
type: 'PLUGIN_LISTS_CHANGED',
payload,
});

View File

@@ -11,7 +11,7 @@ import React from 'react';
import {Typography} from 'antd';
import {LeftSidebar, SidebarTitle, InfoIcon} from '../LeftSidebar';
import {Layout, Link, styled} from '../../ui';
import {theme, useValue, useMemoize} from 'flipper-plugin';
import {theme, useValue} from 'flipper-plugin';
import {AppSelector} from './AppSelector';
import {useStore} from '../../utils/useStore';
import {PluginList} from './PluginList';
@@ -19,9 +19,7 @@ import ScreenCaptureButtons from '../../chrome/ScreenCaptureButtons';
import MetroButton from '../../chrome/MetroButton';
import {BookmarkSection} from './BookmarkSection';
import Client from '../../Client';
import {State} from '../../reducers';
import BaseDevice from '../../devices/BaseDevice';
import MetroDevice from '../../devices/MetroDevice';
import {ExclamationCircleOutlined, FieldTimeOutlined} from '@ant-design/icons';
const {Text} = Typography;
@@ -40,20 +38,9 @@ const appTooltip = (
export function AppInspect() {
const connections = useStore((state) => state.connections);
const metroDevice = useMemoize(findMetroDevice, [connections.devices]);
const client = useMemoize(findBestClient, [
connections.clients,
connections.selectedApp,
connections.userPreferredApp,
]);
// // if the selected device is Metro, we want to keep the owner of the selected App as active device if possible
const activeDevice = useMemoize(findBestDevice, [
client,
connections.devices,
connections.selectedDevice,
metroDevice,
connections.userPreferredDevice,
]);
const metroDevice = connections.metroDevice;
const client = connections.activeClient;
const activeDevice = connections.activeDevice;
const isDeviceConnected = useValue(activeDevice?.connected, false);
const isAppConnected = useValue(client?.connected, false);
@@ -101,51 +88,10 @@ const Toolbar = styled(Layout.Horizontal)({
},
});
export function findBestClient(
clients: Client[],
selectedApp: string | null,
userPreferredApp: string | null,
): Client | undefined {
return clients.find((c) => c.id === (selectedApp || userPreferredApp));
}
export function findMetroDevice(
devices: State['connections']['devices'],
): MetroDevice | undefined {
return devices?.find(
(device) => device.os === 'Metro' && !device.isArchived,
) as MetroDevice;
}
export function findBestDevice(
client: Client | undefined,
devices: State['connections']['devices'],
selectedDevice: BaseDevice | null,
metroDevice: BaseDevice | undefined,
userPreferredDevice: string | null,
): BaseDevice | undefined {
// if not Metro device, use the selected device as metro device
const selected = selectedDevice ?? undefined;
if (selected !== metroDevice) {
return selected;
}
// if there is an active app, use device owning the app
if (client) {
return client.deviceSync;
}
// if no active app, use the preferred device
if (userPreferredDevice) {
return (
devices.find((device) => device.title === userPreferredDevice) ?? selected
);
}
return selected;
}
function renderStatusMessage(
isDeviceConnected: boolean,
activeDevice: BaseDevice | undefined,
client: Client | undefined,
activeDevice: BaseDevice | null,
client: Client | null,
isAppConnected: boolean,
): React.ReactNode {
if (!activeDevice) {

View File

@@ -20,11 +20,7 @@ import {
import {Glyph, Layout, styled} from '../../ui';
import {theme, NUX, Tracked, useValue, useMemoize} from 'flipper-plugin';
import {useDispatch, useStore} from '../../utils/useStore';
import {
computePluginLists,
getPluginTitle,
getPluginTooltip,
} from '../../utils/pluginUtils';
import {getPluginTitle, getPluginTooltip} from '../../utils/pluginUtils';
import {selectPlugin} from '../../reducers/connections';
import Client from '../../Client';
import BaseDevice from '../../devices/BaseDevice';
@@ -52,27 +48,17 @@ export const PluginList = memo(function PluginList({
activeDevice,
metroDevice,
}: {
client: Client | undefined;
activeDevice: BaseDevice | undefined;
metroDevice: MetroDevice | undefined;
client: Client | null;
activeDevice: BaseDevice | null;
metroDevice: MetroDevice | null;
}) {
const dispatch = useDispatch();
const connections = useStore((state) => state.connections);
const plugins = useStore((state) => state.plugins);
const pluginLists = useStore((state) => state.pluginLists);
const downloads = useStore((state) => state.pluginDownloads);
// client is a mutable structure, so we need the event emitter to detect the addition of plugins....
const [pluginsChanged, setPluginsChanged] = useState(0);
useEffect(() => {
if (!client) {
return;
}
const listener = () => setPluginsChanged((v) => v + 1);
client.on('plugins-change', listener);
return () => {
client.off('plugins-change', listener);
};
}, [client]);
const isConnected = useValue(activeDevice?.connected, false);
const metroConnected = useValue(metroDevice?.connected, false);
const {
devicePlugins,
@@ -81,17 +67,8 @@ export const PluginList = memo(function PluginList({
disabledPlugins,
unavailablePlugins,
downloadablePlugins,
} = useMemoize(computePluginLists, [
activeDevice,
metroDevice,
client,
plugins,
connections.enabledPlugins,
connections.enabledDevicePlugins,
pluginsChanged,
]);
const isConnected = useValue(activeDevice?.connected, false);
const metroConnected = useValue(metroDevice?.connected, false);
} = pluginLists;
const isArchived = activeDevice?.isArchived;
const annotatedDownloadablePlugins = useMemoize<

View File

@@ -11,7 +11,6 @@ import {
createMockFlipperWithPlugin,
MockFlipperResult,
} from '../../../test-utils/createMockFlipperWithPlugin';
import {findBestClient, findBestDevice, findMetroDevice} from '../AppInspect';
import {FlipperPlugin} from '../../../plugin';
import MetroDevice from '../../../devices/MetroDevice';
import BaseDevice from '../../../devices/BaseDevice';
@@ -47,39 +46,21 @@ describe('basic findBestDevice', () => {
});
test('findBestDevice prefers selected device', () => {
const {client, device} = flipper;
const {device} = flipper;
const {connections} = flipper.store.getState();
expect(
findBestDevice(
client,
connections.devices,
device,
undefined,
device.title,
),
).toBe(device);
expect(connections.activeDevice).toBe(device);
});
test('findBestDevice picks device of current client', () => {
const {client, device} = flipper;
const {device} = flipper;
const {connections} = flipper.store.getState();
expect(
findBestDevice(client, connections.devices, null, undefined, null),
).toBe(device);
expect(connections.activeDevice).toBe(device);
});
test('findBestDevice picks preferred device if no client and device', () => {
const {device} = flipper;
const {connections} = flipper.store.getState();
expect(
findBestDevice(
undefined,
connections.devices,
null,
undefined,
device.title,
),
).toBe(device);
expect(connections.activeDevice).toBe(device);
});
});
@@ -96,9 +77,7 @@ describe('basic findBestDevice with metro present', () => {
testDevice = flipper.device;
// flipper.store.dispatch(registerPlugins([LogsPlugin]))
await registerMetroDevice(undefined, flipper.store, flipper.logger);
metro = findMetroDevice(
flipper.store.getState().connections.devices,
)! as MetroDevice;
metro = flipper.store.getState().connections.metroDevice!;
metro.supportsPlugin = (p) => {
return p.id !== 'unsupportedDevicePlugin';
};
@@ -118,13 +97,7 @@ describe('basic findBestDevice with metro present', () => {
userPreferredPlugin: 'DeviceLogs',
userPreferredApp: 'TestApp#Android#MockAndroidDevice#serial',
});
expect(
findBestClient(
connections.clients,
connections.selectedApp,
connections.userPreferredApp,
),
).toBe(flipper.client);
expect(connections.activeClient).toBe(flipper.client);
});
test('selecting Metro Logs works but keeps normal device preferred', () => {
@@ -147,37 +120,14 @@ describe('basic findBestDevice with metro present', () => {
});
const {connections} = flipper.store.getState();
// find best device is still metro
expect(
findBestDevice(
undefined,
connections.devices,
connections.selectedDevice,
metro,
connections.userPreferredDevice,
),
).toBe(testDevice);
expect(connections.activeDevice).toBe(testDevice);
// find best client still returns app
expect(
findBestClient(
connections.clients,
connections.selectedApp,
connections.userPreferredApp,
),
).toBe(flipper.client);
expect(connections.activeClient).toBe(flipper.client);
});
test('computePluginLists', () => {
const state = flipper.store.getState();
expect(
computePluginLists(
testDevice,
metro,
flipper.client,
state.plugins,
state.connections.enabledPlugins,
state.connections.enabledDevicePlugins,
),
).toEqual({
expect(computePluginLists(state.connections, state.plugins)).toEqual({
downloadablePlugins: [],
devicePlugins: [logsPlugin],
metroPlugins: [logsPlugin],
@@ -281,14 +231,7 @@ describe('basic findBestDevice with metro present', () => {
];
let state = flipper.store.getState();
const pluginLists = computePluginLists(
testDevice,
metro,
flipper.client,
state.plugins,
state.connections.enabledPlugins,
state.connections.enabledDevicePlugins,
);
const pluginLists = computePluginLists(state.connections, state.plugins);
expect(pluginLists).toEqual({
devicePlugins: [logsPlugin],
metroPlugins: [logsPlugin],
@@ -322,16 +265,7 @@ describe('basic findBestDevice with metro present', () => {
}),
);
state = flipper.store.getState();
expect(
computePluginLists(
testDevice,
metro,
flipper.client,
state.plugins,
state.connections.enabledPlugins,
state.connections.enabledDevicePlugins,
),
).toMatchObject({
expect(computePluginLists(state.connections, state.plugins)).toMatchObject({
enabledPlugins: [plugin2],
disabledPlugins: [plugin1],
});

View File

@@ -83,15 +83,7 @@ export function getExportablePlugins(
device: BaseDevice | undefined | null,
client?: Client,
): {id: string; label: string}[] {
const availablePlugins = computePluginLists(
device ?? undefined,
undefined,
client,
state.plugins,
state.connections.enabledPlugins,
state.connections.enabledDevicePlugins,
);
const availablePlugins = computePluginLists(state.connections, state.plugins);
return [
...availablePlugins.devicePlugins.filter((plugin) => {
return isExportablePlugin(state, device, client, plugin);
@@ -180,14 +172,38 @@ export function getPluginTooltip(details: PluginDetails): string {
}
export function computePluginLists(
device: BaseDevice | undefined,
metroDevice: BaseDevice | undefined,
client: Client | undefined,
plugins: State['plugins'],
enabledPluginsState: State['connections']['enabledPlugins'],
enabledDevicePluginsState: Set<string>,
_pluginsChanged?: number, // this argument is purely used to invalidate the memoization cache
) {
connections: Pick<
State['connections'],
| 'activeDevice'
| 'activeClient'
| 'metroDevice'
| 'enabledDevicePlugins'
| 'enabledPlugins'
>,
plugins: Pick<
State['plugins'],
| 'bundledPlugins'
| 'marketplacePlugins'
| 'loadedPlugins'
| 'devicePlugins'
| 'disabledPlugins'
| 'gatekeepedPlugins'
| 'failedPlugins'
| 'clientPlugins'
>,
): {
devicePlugins: DevicePluginDefinition[];
metroPlugins: DevicePluginDefinition[];
enabledPlugins: ClientPluginDefinition[];
disabledPlugins: PluginDefinition[];
unavailablePlugins: [plugin: PluginDetails, reason: string][];
downloadablePlugins: (DownloadablePluginDetails | BundledPluginDetails)[];
} {
const device = connections.activeDevice;
const client = connections.activeClient;
const metroDevice = connections.metroDevice;
const enabledDevicePluginsState = connections.enabledDevicePlugins;
const enabledPluginsState = connections.enabledPlugins;
const uninstalledMarketplacePlugins = getLatestCompatibleVersionOfEachPlugin([
...plugins.bundledPlugins.values(),
...plugins.marketplacePlugins,