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
This commit is contained in:
Anton Nikolaev
2021-06-29 13:00:18 -07:00
committed by Facebook GitHub Bot
parent bf1b10c130
commit 7bc38c1735
14 changed files with 13 additions and 115 deletions

View File

@@ -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<typeof plugin> =
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(`
<h1>
@@ -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(

View File

@@ -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<string, _SandyDevicePluginInstance> = 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() {

View File

@@ -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 = [];
}
}

View File

@@ -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;
}
}

View File

@@ -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;

View File

@@ -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<any, any, any> {
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,

View File

@@ -649,20 +649,15 @@ function updateSelection(state: Readonly<State>): 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

View File

@@ -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={
<>
<ActionButton
@@ -285,6 +286,7 @@ export const PluginList = memo(function PluginList({
plugin={plugin}
scrollTo={plugin.id === connections.selectedPlugin}
tooltip={getPluginTooltip(plugin)}
onClick={handleAppPluginClick}
actions={
<ActionButton
id={plugin.id}
@@ -315,6 +317,7 @@ export const PluginList = memo(function PluginList({
tooltip={`${getPluginTitle(plugin)} (${plugin.id}@${
plugin.version
}): ${reason}`}
onClick={handleAppPluginClick}
disabled
actions={<InfoIcon>{reason}</InfoIcon>}
/>
@@ -396,7 +399,7 @@ const PluginEntry = function PluginEntry({
<Menu.Item
{...rest}
key={plugin.id}
disabled={disabled}
style={{cursor: 'pointer'}}
onClick={handleClick}>
<Layout.Horizontal
center

View File

@@ -1399,7 +1399,6 @@ test('Sandy device plugins are exported / imported properly', async () => {
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,

View File

@@ -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',

View File

@@ -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();

View File

@@ -23,11 +23,6 @@ export interface FlipperLib {
enableMenuEntries(menuEntries: NormalizedMenuEntry[]): void;
createPaste(input: string): Promise<string | undefined>;
GK(gatekeeper: string): boolean;
isPluginAvailable(
device: RealFlipperDevice,
client: RealFlipperClient | null,
pluginId: string,
): boolean;
selectPlugin(
device: RealFlipperDevice,
client: RealFlipperClient | null,

View File

@@ -88,11 +88,6 @@ export interface PluginClient<
*/
supportsMethod(method: keyof Methods): Promise<boolean>;
/**
* 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),

View File

@@ -314,7 +314,6 @@ export function startDevicePlugin<Module extends FlipperDevicePluginModule>(
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;