Use set instead of array to keep list of supported plugins per client

Summary: Use set instead of array to keep list of supported plugins per client. It is not used as array anyway, in most places it is used to determine whether a plugin is supported or not and it's much faster to use set for that.

Reviewed By: priteshrnandgaonkar

Differential Revision: D29200673

fbshipit-source-id: 5f3c404a1a668c153867d7c1b6c223941f0b3b36
This commit is contained in:
Anton Nikolaev
2021-06-29 13:00:18 -07:00
committed by Facebook GitHub Bot
parent 280c612157
commit fd9b5cc94d
14 changed files with 51 additions and 55 deletions

View File

@@ -15,9 +15,7 @@ import {Payload, ConnectionStatus} from 'rsocket-types';
import {Flowable, Single} from 'rsocket-flowable';
import {performance} from 'perf_hooks';
import {reportPluginFailures} from './utils/metrics';
import {notNull} from './utils/typeUtils';
import {default as isProduction} from './utils/isProduction';
import {registerPlugins} from './reducers/plugins';
import {EventEmitter} from 'events';
import invariant from 'invariant';
import {
@@ -39,7 +37,8 @@ import {freeze} from 'immer';
import GK from './fb-stubs/GK';
import {message} from 'antd';
type Plugins = Array<string>;
type Plugins = Set<string>;
type PluginsArr = Array<string>;
export type ClientQuery = {
app: string;
@@ -149,8 +148,8 @@ export default class Client extends EventEmitter {
) {
super();
this.connected.set(!!conn);
this.plugins = plugins ? plugins : [];
this.backgroundPlugins = [];
this.plugins = plugins ? plugins : new Set();
this.backgroundPlugins = new Set();
this.connection = conn;
this.id = id;
this.query = query;
@@ -184,11 +183,11 @@ export default class Client extends EventEmitter {
}
supportsPlugin(pluginId: string): boolean {
return this.plugins.includes(pluginId);
return this.plugins.has(pluginId);
}
isBackgroundPlugin(pluginId: string) {
return this.backgroundPlugins.includes(pluginId);
return this.backgroundPlugins.has(pluginId);
}
isEnabledPlugin(pluginId: string) {
@@ -210,7 +209,7 @@ export default class Client extends EventEmitter {
this.plugins.forEach((pluginId) =>
this.startPluginIfNeeded(this.getPlugin(pluginId)),
);
this.backgroundPlugins = await this.getBackgroundPlugins();
this.backgroundPlugins = new Set(await this.getBackgroundPlugins());
this.backgroundPlugins.forEach((plugin) => {
if (this.shouldConnectAsBackgroundPlugin(plugin)) {
this.initPlugin(plugin);
@@ -244,7 +243,7 @@ export default class Client extends EventEmitter {
'getPlugins',
false,
);
this.plugins = plugins;
this.plugins = new Set(plugins);
return plugins;
}
@@ -313,13 +312,14 @@ export default class Client extends EventEmitter {
}
// get the supported background plugins
async getBackgroundPlugins(): Promise<Plugins> {
async getBackgroundPlugins(): Promise<PluginsArr> {
if (this.sdkVersion < 4) {
return [];
}
return this.rawCall<{plugins: Plugins}>('getBackgroundPlugins', false).then(
(data) => data.plugins,
);
return this.rawCall<{plugins: PluginsArr}>(
'getBackgroundPlugins',
false,
).then((data) => data.plugins);
}
// get the plugins, and update the UI
@@ -330,11 +330,11 @@ export default class Client extends EventEmitter {
this.startPluginIfNeeded(this.getPlugin(pluginId)),
);
const newBackgroundPlugins = await this.getBackgroundPlugins();
this.backgroundPlugins = newBackgroundPlugins;
this.backgroundPlugins = new Set(newBackgroundPlugins);
// diff the background plugin list, disconnect old, connect new ones
oldBackgroundPlugins.forEach((plugin) => {
if (
!newBackgroundPlugins.includes(plugin) &&
!this.backgroundPlugins.has(plugin) &&
this.store
.getState()
.connections.enabledPlugins[this.query.app]?.includes(plugin)
@@ -344,7 +344,7 @@ export default class Client extends EventEmitter {
});
newBackgroundPlugins.forEach((plugin) => {
if (
!oldBackgroundPlugins.includes(plugin) &&
!oldBackgroundPlugins.has(plugin) &&
this.shouldConnectAsBackgroundPlugin(plugin)
) {
this.initPlugin(plugin);

View File

@@ -417,10 +417,7 @@ class PluginContainer extends PureComponent<Props, State> {
selectPlugin: (pluginID: string, deepLinkPayload: unknown) => {
const {target} = this.props;
// check if plugin will be available
if (
target instanceof Client &&
target.plugins.some((p) => p === pluginID)
) {
if (target instanceof Client && target.plugins.has(pluginID)) {
this.props.selectPlugin({
selectedPlugin: pluginID,
deepLinkPayload,

View File

@@ -47,7 +47,7 @@ test('can create a Fake flipper', async () => {
expect(device).toBeTruthy();
expect(store).toBeTruthy();
expect(sendMessage).toBeTruthy();
expect(client.plugins.includes(TestPlugin.id)).toBe(true);
expect(client.plugins.has(TestPlugin.id)).toBe(true);
expect(store.getState().connections).toMatchSnapshot();
expect(store.getState().plugins).toMatchSnapshot();
sendMessage('inc', {});
@@ -73,7 +73,7 @@ test('can create a Fake flipper with legacy wrapper', async () => {
expect(device).toBeTruthy();
expect(store).toBeTruthy();
expect(sendMessage).toBeTruthy();
expect(client.plugins.includes(TestPlugin.id)).toBe(true);
expect(client.plugins.has(TestPlugin.id)).toBe(true);
expect(client.sandyPluginStates.has(TestPlugin.id)).toBe(true);
const state = store.getState();
expect(state.connections).toMatchSnapshot();

View File

@@ -128,7 +128,7 @@ class PluginDebugger extends Component<Props> {
getSupportedClients(id: string): string {
return this.props.clients
.reduce((acc: Array<string>, cv: Client) => {
if (cv.plugins.includes(id)) {
if (cv.plugins.has(id)) {
acc.push(cv.query.app);
}
return acc;

View File

@@ -132,7 +132,7 @@ export class Group {
enabledPlugins != null && enabledPlugins.includes(requiredPlugin);
if (
selectedClient &&
selectedClient.plugins.includes(requiredPlugin) &&
selectedClient.plugins.has(requiredPlugin) &&
!requiredPluginEnabled
) {
const plugin =
@@ -146,7 +146,7 @@ export class Group {
);
} else if (
!selectedClient ||
!selectedClient.plugins.includes(requiredPlugin)
!selectedClient.plugins.has(requiredPlugin)
) {
unsupportedPlugins.push(requiredPlugin);
}

View File

@@ -224,11 +224,11 @@ describe('basic findBestDevice with metro present', () => {
);
// ok, this is a little hackish
flipper.client.plugins = [
flipper.client.plugins = new Set([
'plugin1',
'plugin2',
'supportedUninstalledPlugin',
];
]);
let state = flipper.store.getState();
const pluginLists = computePluginLists(state.connections, state.plugins);

View File

@@ -555,9 +555,9 @@ class Server extends EventEmitter {
client.init().then(() => {
console.debug(
`Device client initialised: ${id}. Supported plugins: ${client.plugins.join(
', ',
)}`,
`Device client initialised: ${id}. Supported plugins: ${Array.from(
client.plugins,
).join(', ')}`,
'server',
);

View File

@@ -181,7 +181,7 @@ export default class MockFlipper {
device.isArchived ? null : createStubConnection(),
this._logger,
this._store,
supportedPlugins,
new Set(supportedPlugins),
device,
);
// yikes

View File

@@ -718,7 +718,7 @@ test('test determinePluginsToProcess for mutilple clients having plugins present
null,
logger,
mockStore,
['TestPlugin', 'TestDevicePlugin'],
new Set(['TestPlugin', 'TestDevicePlugin']),
device1,
);
const client2 = new Client(
@@ -732,7 +732,7 @@ test('test determinePluginsToProcess for mutilple clients having plugins present
null,
logger,
mockStore,
['TestDevicePlugin'],
new Set(['TestDevicePlugin']),
device1,
);
const client3 = new Client(
@@ -746,7 +746,7 @@ test('test determinePluginsToProcess for mutilple clients having plugins present
null,
logger,
mockStore,
['TestPlugin', 'TestDevicePlugin'],
new Set(['TestPlugin', 'TestDevicePlugin']),
device1,
);
const plugins: PluginsState = {
@@ -807,7 +807,7 @@ test('test determinePluginsToProcess for no selected plugin present in any clien
null,
logger,
mockStore,
['TestPlugin', 'TestDevicePlugin'],
new Set(['TestPlugin', 'TestDevicePlugin']),
device1,
);
const client2 = new Client(
@@ -821,7 +821,7 @@ test('test determinePluginsToProcess for no selected plugin present in any clien
null,
logger,
mockStore,
['TestDevicePlugin'],
new Set(['TestDevicePlugin']),
device1,
);
const plugins: PluginsState = {
@@ -863,7 +863,7 @@ test('test determinePluginsToProcess for multiple clients on same device', async
null,
logger,
mockStore,
['TestPlugin', 'TestDevicePlugin'],
new Set(['TestPlugin', 'TestDevicePlugin']),
device1,
);
const client2 = new Client(
@@ -877,7 +877,7 @@ test('test determinePluginsToProcess for multiple clients on same device', async
null,
logger,
mockStore,
['TestDevicePlugin'],
new Set(['TestDevicePlugin']),
device1,
);
const plugins: PluginsState = {
@@ -925,7 +925,7 @@ test('test determinePluginsToProcess for multiple clients on different device',
null,
logger,
mockStore,
['TestPlugin', 'TestDevicePlugin'],
new Set(['TestPlugin', 'TestDevicePlugin']),
device1,
);
const client2Device1 = new Client(
@@ -939,7 +939,7 @@ test('test determinePluginsToProcess for multiple clients on different device',
null,
logger,
mockStore,
['TestDevicePlugin'],
new Set(['TestDevicePlugin']),
device1,
);
const client1Device2 = new Client(
@@ -953,7 +953,7 @@ test('test determinePluginsToProcess for multiple clients on different device',
null,
logger,
mockStore,
['TestPlugin', 'TestDevicePlugin'],
new Set(['TestPlugin', 'TestDevicePlugin']),
device1,
);
const client2Device2 = new Client(
@@ -967,7 +967,7 @@ test('test determinePluginsToProcess for multiple clients on different device',
null,
logger,
mockStore,
['TestDevicePlugin'],
new Set(['TestDevicePlugin']),
device1,
);
const plugins: PluginsState = {
@@ -1039,7 +1039,7 @@ test('test determinePluginsToProcess to ignore archived clients', async () => {
null,
logger,
mockStore,
['TestPlugin', 'TestDevicePlugin'],
new Set(['TestPlugin', 'TestDevicePlugin']),
archivedDevice,
);
const archivedClient = new Client(
@@ -1053,7 +1053,7 @@ test('test determinePluginsToProcess to ignore archived clients', async () => {
null,
logger,
mockStore,
['TestPlugin', 'TestDevicePlugin'],
new Set(['TestPlugin', 'TestDevicePlugin']),
archivedDevice,
);
const plugins: PluginsState = {
@@ -1303,7 +1303,7 @@ test('Sandy plugins are imported properly', async () => {
const client2 = store.getState().connections.clients[1];
expect(client2).not.toBeFalsy();
expect(client2).not.toBe(client);
expect(client2.plugins).toEqual([TestPlugin.id]);
expect(Array.from(client2.plugins)).toEqual([TestPlugin.id]);
expect(client.sandyPluginStates.get(TestPlugin.id)!.exportStateSync())
.toMatchInlineSnapshot(`

View File

@@ -619,11 +619,13 @@ export function determinePluginsToProcess(
}
const selectedFilteredPlugins = client
? selectedPlugins.length > 0
? client.plugins.filter((plugin) => selectedPlugins.includes(plugin))
? Array.from(client.plugins).filter((plugin) =>
selectedPlugins.includes(plugin),
)
: client.plugins
: [];
for (const plugin of selectedFilteredPlugins) {
if (!client.plugins.includes(plugin)) {
if (!client.plugins.has(plugin)) {
// Ignore clients which doesn't support the selected plugins.
continue;
}
@@ -827,7 +829,7 @@ export function importDataToStore(source: string, data: string, store: Store) {
clients.forEach((client: {id: string; query: ClientQuery}) => {
const sandyPluginStates = json.pluginStates2[client.id] || {};
const clientPlugins: Array<string> = [
const clientPlugins: Set<string> = new Set([
...keys
.filter((key) => {
const plugin = deconstructPluginKey(key);
@@ -835,7 +837,7 @@ export function importDataToStore(source: string, data: string, store: Store) {
})
.map((pluginKey) => deconstructPluginKey(pluginKey).pluginName),
...Object.keys(sandyPluginStates),
];
]);
store.dispatch({
type: 'NEW_CLIENT',
payload: new Client(

View File

@@ -352,9 +352,7 @@ function getFavoritePlugins(
return [];
}
// for *imported* devices, all stored plugins are enabled
return allPlugins.filter(
(plugin) => client.plugins.indexOf(plugin.id) !== -1,
);
return allPlugins.filter((plugin) => client.plugins.has(plugin.id));
}
if (!enabledPlugins || !enabledPlugins.length) {
return returnFavoredPlugins ? [] : allPlugins;

View File

@@ -27,6 +27,5 @@
"**/node_modules/",
"**/__tests__/",
"**/lib/",
"**/fb/"
]
}

View File

@@ -108,7 +108,7 @@ export interface RealFlipperClient {
device_id: string;
};
deviceSync: RealFlipperDevice;
plugins: string[];
plugins: Set<string>;
isBackgroundPlugin(pluginId: string): boolean;
initPlugin(pluginId: string): void;
deinitPlugin(pluginId: string): void;

View File

@@ -196,7 +196,7 @@ export function startPlugin<Module extends FlipperPluginModule<any>>(
const deviceName = 'TestDevice';
const fakeFlipperClient: RealFlipperClient = {
id: `${appName}#${testDevice.os}#${deviceName}#${testDevice.serial}`,
plugins: [definition.id],
plugins: new Set([definition.id]),
query: {
app: appName,
device: deviceName,