Allow "uninstallation" of bundled plugins

Summary: Allow "uninstallation" of bundled plugins which means we mark them as uninstalled and not auto-updating anymore. Uninstalled bundled plugins are shown in "Detected in App" section together with Marketplace plugins where user can install them back. Changelog: Plugins can be uninstalled from sidebar in new Sandy UI. Reviewed By: passy Differential Revision: D25557789 fbshipit-source-id: 751cad68456313c069af639584541086efc7102b
2020-12-15 09:28:58 -08:00
parent 756edf9860
commit 3d6afdb529
7 changed files with 74 additions and 39 deletions
--- a/desktop/app/src/tests/snapshots/createMockFlipperWithPlugin.node.tsx.snap
+++ b/desktop/app/src/tests/snapshots/createMockFlipperWithPlugin.node.tsx.snap
@@ -49,6 +49,7 @@ Object {

 exports[`can create a Fake flipper 2`] = `
 Object {
+  "bundledPlugins": Map {},
  "clientPlugins": Map {
    "TestPlugin" => [Function],
  },
--- a/desktop/app/src/dispatcher/plugins.tsx
+++ b/desktop/app/src/dispatcher/plugins.tsx
@@ -20,6 +20,7 @@ import {
  addDisabledPlugins,
  addFailedPlugins,
  registerLoadedPlugins,
+  registerBundledPlugins,
 } from '../reducers/plugins';
 import GK from '../fb-stubs/GK';
 import {FlipperBasePlugin} from '../plugin';
@@ -33,7 +34,7 @@ import semver from 'semver';
 import {
  ActivatablePluginDetails,
  BundledPluginDetails,
-  InstalledPluginDetails,
+  PluginDetails,
 } from 'flipper-plugin-lib';
 import {tryCatchReportPluginFailures, reportUsage} from '../utils/metrics';
 import * as FlipperPluginSDK from 'flipper-plugin';
@@ -64,19 +65,21 @@ export default async (store: Store, logger: Logger) => {

  const uninstalledPlugins = store.getState().pluginManager.uninstalledPlugins;

+  const bundledPlugins = getBundledPlugins();
+
  const loadedPlugins = filterNewestVersionOfEachPlugin(
-    getBundledPlugins(),
+    bundledPlugins,
    await getDynamicPlugins(),
-  );
+  ).filter((p) => !uninstalledPlugins.has(p.name));

  const initialPlugins: PluginDefinition[] = loadedPlugins
-    .filter((p) => !uninstalledPlugins.has(p.name))
    .map(reportVersion)
    .filter(checkDisabled(disabledPlugins))
    .filter(checkGK(gatekeepedPlugins))
    .map(createRequirePluginFunction(failedPlugins))
    .filter(notNull);

+  store.dispatch(registerBundledPlugins(bundledPlugins));
  store.dispatch(registerLoadedPlugins(loadedPlugins));
  store.dispatch(addGatekeepedPlugins(gatekeepedPlugins));
  store.dispatch(addDisabledPlugins(disabledPlugins));
@@ -108,11 +111,11 @@ function reportVersion(pluginDetails: ActivatablePluginDetails) {
  return pluginDetails;
 }

-export function filterNewestVersionOfEachPlugin(
-  bundledPlugins: BundledPluginDetails[],
-  dynamicPlugins: InstalledPluginDetails[],
-): ActivatablePluginDetails[] {
-  const pluginByName: {[key: string]: ActivatablePluginDetails} = {};
+export function filterNewestVersionOfEachPlugin<
+  T1 extends PluginDetails,
+  T2 extends PluginDetails
+>(bundledPlugins: T1[], dynamicPlugins: T2[]): (T1 | T2)[] {
+  const pluginByName: {[key: string]: T1 | T2} = {};
  for (const plugin of bundledPlugins) {
    pluginByName[plugin.name] = plugin;
  }
--- a/desktop/app/src/reducers/tests/plugins.node.tsx
+++ b/desktop/app/src/reducers/tests/plugins.node.tsx
@@ -33,6 +33,7 @@ test('add clientPlugin', () => {
      devicePlugins: new Map(),
      clientPlugins: new Map(),
      loadedPlugins: new Map(),
+      bundledPlugins: new Map(),
      gatekeepedPlugins: [],
      failedPlugins: [],
      disabledPlugins: [],
@@ -50,6 +51,7 @@ test('add devicePlugin', () => {
      devicePlugins: new Map(),
      clientPlugins: new Map(),
      loadedPlugins: new Map(),
+      bundledPlugins: new Map(),
      gatekeepedPlugins: [],
      failedPlugins: [],
      disabledPlugins: [],
@@ -67,6 +69,7 @@ test('do not add plugin twice', () => {
      devicePlugins: new Map(),
      clientPlugins: new Map(),
      loadedPlugins: new Map(),
+      bundledPlugins: new Map(),
      gatekeepedPlugins: [],
      failedPlugins: [],
      disabledPlugins: [],
@@ -99,6 +102,7 @@ test('add gatekeeped plugin', () => {
      devicePlugins: new Map(),
      clientPlugins: new Map(),
      loadedPlugins: new Map(),
+      bundledPlugins: new Map(),
      gatekeepedPlugins: [],
      failedPlugins: [],
      disabledPlugins: [],
--- a/desktop/app/src/reducers/plugins.tsx
+++ b/desktop/app/src/reducers/plugins.tsx
@@ -11,6 +11,7 @@ import {DevicePluginMap, ClientPluginMap, PluginDefinition} from '../plugin';
 import {
  DownloadablePluginDetails,
  ActivatablePluginDetails,
+  BundledPluginDetails,
 } from 'flipper-plugin-lib';
 import {Actions} from '.';
 import produce from 'immer';
@@ -20,6 +21,7 @@ export type State = {
  devicePlugins: DevicePluginMap;
  clientPlugins: ClientPluginMap;
  loadedPlugins: Map<string, ActivatablePluginDetails>;
+  bundledPlugins: Map<string, BundledPluginDetails>;
  gatekeepedPlugins: Array<ActivatablePluginDetails>;
  disabledPlugins: Array<ActivatablePluginDetails>;
  failedPlugins: Array<[ActivatablePluginDetails, string]>;
@@ -57,12 +59,17 @@ export type Action =
  | {
      type: 'REGISTER_LOADED_PLUGINS';
      payload: Array<ActivatablePluginDetails>;
+    }
+  | {
+      type: 'REGISTER_BUNDLED_PLUGINS';
+      payload: Array<BundledPluginDetails>;
    };

 const INITIAL_STATE: State = {
  devicePlugins: new Map(),
  clientPlugins: new Map(),
  loadedPlugins: new Map(),
+  bundledPlugins: new Map(),
  gatekeepedPlugins: [],
  disabledPlugins: [],
  failedPlugins: [],
@@ -119,6 +126,11 @@ export default function reducer(
      ...state,
      loadedPlugins: new Map(action.payload.map((p) => [p.id, p])),
    };
+  } else if (action.type === 'REGISTER_BUNDLED_PLUGINS') {
+    return {
+      ...state,
+      bundledPlugins: new Map(action.payload.map((p) => [p.id, p])),
+    };
  } else {
    return state;
  }
@@ -168,3 +180,10 @@ export const registerLoadedPlugins = (
  type: 'REGISTER_LOADED_PLUGINS',
  payload,
 });
+
+export const registerBundledPlugins = (
+  payload: Array<BundledPluginDetails>,
+): Action => ({
+  type: 'REGISTER_BUNDLED_PLUGINS',
+  payload,
+});
--- a/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx
+++ b/desktop/app/src/sandy-chrome/appinspect/PluginList.tsx
@@ -35,7 +35,9 @@ import {
  PluginDownloadStatus,
  startPluginDownload,
 } from '../../reducers/pluginDownloads';
-import {uninstallPlugin} from '../../reducers/pluginManager';
+import {activatePlugin, uninstallPlugin} from '../../reducers/pluginManager';
+import {BundledPluginDetails} from 'plugin-lib/lib';
+import {filterNewestVersionOfEachPlugin} from '../../dispatcher/plugins';

 const {SubMenu} = Menu;
 const {Text} = Typography;
@@ -71,8 +73,14 @@ export const PluginList = memo(function PluginList({
  const isArchived = !!activeDevice?.isArchived;

  const annotatedDownloadablePlugins = useMemoize<
-    [Record<string, DownloadablePluginState>, DownloadablePluginDetails[]],
-    [plugin: DownloadablePluginDetails, downloadStatus?: PluginDownloadStatus][]
+    [
+      Record<string, DownloadablePluginState>,
+      (DownloadablePluginDetails | BundledPluginDetails)[],
+    ],
+    [
+      plugin: DownloadablePluginDetails | BundledPluginDetails,
+      downloadStatus?: PluginDownloadStatus,
+    ][]
  >(
    (downloads, downloadablePlugins) => {
      const downloadMap = new Map(
@@ -126,12 +134,11 @@ export const PluginList = memo(function PluginList({
  const handleInstallPlugin = useCallback(
    (id: string) => {
      const plugin = downloadablePlugins.find((p) => p.id === id)!;
-      dispatch(
-        startPluginDownload({
-          plugin,
-          startedByUser: true,
-        }),
-      );
+      if (plugin.isBundled) {
+        dispatch(activatePlugin({plugin, enable: true, notifyIfFailed: true}));
+      } else {
+        dispatch(startPluginDownload({plugin, startedByUser: true}));
+      }
    },
    [downloadablePlugins, dispatch],
  );
@@ -229,19 +236,14 @@ export const PluginList = memo(function PluginList({
                  tooltip={getPluginTooltip(plugin.details)}
                  actions={
                    <>
-                      {!plugin.details.isBundled && (
                      <ActionButton
                        id={plugin.id}
                        title="Uninstall plugin"
                        onClick={handleUninstallPlugin}
                        icon={
-                            <DeleteOutlined
-                              size={16}
-                              style={{marginRight: 0}}
-                            />
+                          <DeleteOutlined size={16} style={{marginRight: 0}} />
                        }
                      />
-                      )}
                      <ActionButton
                        id={plugin.id}
                        title="Enable plugin"
@@ -465,7 +467,10 @@ export function computePluginLists(
  const enabledPlugins: ClientPluginDefinition[] = [];
  const disabledPlugins: ClientPluginDefinition[] = [];
  const unavailablePlugins: [plugin: PluginDetails, reason: string][] = [];
-  const downloadablePlugins: DownloadablePluginDetails[] = [];
+  const downloadablePlugins: (
+    | DownloadablePluginDetails
+    | BundledPluginDetails
+  )[] = [];

  if (device) {
    // find all device plugins that aren't part of the current device / metro
@@ -528,13 +533,10 @@ export function computePluginLists(
        disabledPlugins.push(plugin);
      }
    });
-    const installedPluginIds = new Set<string>([
-      ...clientPlugins.map((p) => p.id),
-      ...unavailablePlugins.map(([p]) => p.id),
-    ]);
-    const uninstalledMarketplacePlugins = plugins.marketplacePlugins.filter(
-      (p) => !installedPluginIds.has(p.id),
-    );
+    const uninstalledMarketplacePlugins = filterNewestVersionOfEachPlugin(
+      [...plugins.bundledPlugins.values()],
+      plugins.marketplacePlugins,
+    ).filter((p) => !plugins.loadedPlugins.has(p.id));
    uninstalledMarketplacePlugins.forEach((plugin) => {
      if (client.supportsPlugin(plugin.id)) {
        downloadablePlugins.push(plugin);
--- a/desktop/app/src/store.tsx
+++ b/desktop/app/src/store.tsx
@@ -200,6 +200,7 @@ function uninstallPlugin(state: StoreState, plugin: PluginDefinition) {
    unloadPluginModule(plugin.details);
    draft.plugins.clientPlugins.delete(plugin.id);
    draft.plugins.devicePlugins.delete(plugin.id);
+    draft.plugins.loadedPlugins.delete(plugin.id);
    draft.pluginManager.uninstalledPlugins.add(plugin.details.name);
  });
 }
--- a/desktop/app/src/utils/tests/exportData.node.tsx
+++ b/desktop/app/src/utils/tests/exportData.node.tsx
@@ -767,6 +767,7 @@ test('test determinePluginsToProcess for mutilple clients having plugins present
      ['RandomPlugin', TestPlugin.details],
      ['TestDevicePlugin', TestDevicePlugin.details],
    ]),
+    bundledPlugins: new Map(),
    gatekeepedPlugins: [],
    disabledPlugins: [],
    failedPlugins: [],
@@ -838,6 +839,7 @@ test('test determinePluginsToProcess for no selected plugin present in any clien
      ['RandomPlugin', TestPlugin.details],
      ['TestDevicePlugin', TestDevicePlugin.details],
    ]),
+    bundledPlugins: new Map(),
    gatekeepedPlugins: [],
    disabledPlugins: [],
    failedPlugins: [],
@@ -886,6 +888,7 @@ test('test determinePluginsToProcess for multiple clients on same device', async
      ['TestPlugin', TestPlugin.details],
      ['TestDevicePlugin', TestDevicePlugin.details],
    ]),
+    bundledPlugins: new Map(),
    gatekeepedPlugins: [],
    disabledPlugins: [],
    failedPlugins: [],
@@ -972,6 +975,7 @@ test('test determinePluginsToProcess for multiple clients on different device',
      ['TestPlugin', TestPlugin.details],
      ['TestDevicePlugin', TestDevicePlugin.details],
    ]),
+    bundledPlugins: new Map(),
    gatekeepedPlugins: [],
    disabledPlugins: [],
    failedPlugins: [],
@@ -1055,6 +1059,7 @@ test('test determinePluginsToProcess to ignore archived clients', async () => {
      ['TestPlugin', TestPlugin.details],
      ['TestDevicePlugin', TestDevicePlugin.details],
    ]),
+    bundledPlugins: new Map(),
    gatekeepedPlugins: [],
    disabledPlugins: [],
    failedPlugins: [],