Command processing (1/n)

Summary:
*Stack summary*: this stack refactors plugin management actions to perform them in a dispatcher rather than in the root reducer (store.tsx) as all of these actions has side effects. To do that, we store requested plugin management actions (install/update/uninstall, star/unstar) in a queue which is then handled by pluginManager dispatcher. This dispatcher then dispatches all required state updates.

*Diff summary*: implemented basic plugin action queue processing.

Reviewed By: mweststrate

Differential Revision: D26164945

fbshipit-source-id: 5d8ad9b4d7b1300e92883d24a71da9ca1f85b183

desktop/app/src/PluginContainer.tsx

@@ -53,7 +53,7 @@ import {ContentContainer} from './sandy-chrome/ContentContainer';
 import {Alert, Typography} from 'antd';
 import {InstalledPluginDetails} from 'plugin-lib';
 import semver from 'semver';
-import {activatePlugin} from './reducers/pluginManager';
+import {loadPlugin} from './reducers/pluginManager';
 import {produce} from 'immer';
 import {reportUsage} from './utils/metrics';
 
@@ -129,7 +129,7 @@ type DispatchFromProps = {
   setPluginState: (payload: {pluginKey: string; state: any}) => void;
   setStaticView: (payload: StaticView) => void;
   starPlugin: typeof starPlugin;
-  activatePlugin: typeof activatePlugin;
+  loadPlugin: typeof loadPlugin;
 };
 
 type Props = StateFromProps & DispatchFromProps & OwnProps;
@@ -381,7 +381,7 @@ class PluginContainer extends PureComponent<Props, State> {
   }
 
   reloadPlugin() {
-    const {activatePlugin, latestInstalledVersion} = this.props;
+    const {loadPlugin, latestInstalledVersion} = this.props;
     if (latestInstalledVersion) {
       reportUsage(
         'plugin-auto-update:alert:reloadClicked',
@@ -390,7 +390,7 @@ class PluginContainer extends PureComponent<Props, State> {
         },
         latestInstalledVersion.id,
       );
-      activatePlugin({
+      loadPlugin({
         plugin: latestInstalledVersion,
         enable: false,
         notifyIfFailed: true,
@@ -619,6 +619,6 @@ export default connect<StateFromProps, DispatchFromProps, OwnProps, Store>(
     selectPlugin,
     setStaticView,
     starPlugin,
-    activatePlugin,
+    loadPlugin: loadPlugin,
   },
 )(PluginContainer);

desktop/app/src/dispatcher/pluginDownloads.tsx

@@ -27,7 +27,7 @@ import path from 'path';
 import tmp from 'tmp';
 import {promisify} from 'util';
 import {reportPlatformFailures, reportUsage} from '../utils/metrics';
-import {activatePlugin, pluginInstalled} from '../reducers/pluginManager';
+import {loadPlugin, pluginInstalled} from '../reducers/pluginManager';
 import {showErrorNotification} from '../utils/notifications';
 
 // Adapter which forces node.js implementation for axios instead of browser implementation
@@ -130,7 +130,7 @@ async function handlePluginDownload(
     }
     if (pluginIsDisabledForAllConnectedClients(store.getState(), plugin)) {
       dispatch(
-        activatePlugin({
+        loadPlugin({
           plugin: installedPlugin,
           enable: startedByUser,
           notifyIfFailed: startedByUser,

desktop/app/src/dispatcher/pluginManager.tsx

@@ -10,7 +10,8 @@
 import {Store} from '../reducers/index';
 import {Logger} from '../fb-interfaces/Logger';
 import {
-  pluginActivationHandled,
+  LoadPluginActionPayload,
+  pluginCommandsProcessed,
   registerInstalledPlugins,
 } from '../reducers/pluginManager';
 import {
@@ -22,7 +23,6 @@ import {sideEffect} from '../utils/sideEffect';
 import {requirePlugin} from './plugins';
 import {registerPluginUpdate} from '../reducers/connections';
 import {showErrorNotification} from '../utils/notifications';
-import {reportUsage} from '../utils/metrics';
 
 const maxInstalledPluginVersionsToKeep = 2;
 
@@ -44,21 +44,27 @@ export default (store: Store, _logger: Logger) => {
   sideEffect(
     store,
     {name: 'handlePluginActivation', throttleMs: 1000, fireImmediately: true},
-    (state) => state.pluginManager.pluginActivationQueue,
+    (state) => state.pluginManager.pluginCommandsQueue,
     (queue, store) => {
-      for (const request of queue) {
+      for (const command of queue) {
-        try {
+        switch (command.type) {
-          reportUsage(
+          case 'LOAD_PLUGIN':
-            'plugin:activate',
+            loadPlugin(store, command.payload);
-            {
+            break;
-              version: request.plugin.version,
+          default:
-              enable: request.enable ? '1' : '0',
+            console.error('Unexpected plugin command', command);
-              notifyIfFailed: request.notifyIfFailed ? '1' : '0',
+            break;
+        }
+      }
+      store.dispatch(pluginCommandsProcessed(queue.length));
     },
-            request.plugin.id,
   );
-          const plugin = requirePlugin(request.plugin);
+};
-          const enablePlugin = request.enable;
+
+function loadPlugin(store: Store, payload: LoadPluginActionPayload) {
+  try {
+    const plugin = requirePlugin(payload.plugin);
+    const enablePlugin = payload.enable;
     store.dispatch(
       registerPluginUpdate({
         plugin,
@@ -67,17 +73,13 @@ export default (store: Store, _logger: Logger) => {
     );
   } catch (err) {
     console.error(
-            `Failed to activate plugin ${request.plugin.title} v${request.plugin.version}`,
+      `Failed to activate plugin ${payload.plugin.title} v${payload.plugin.version}`,
       err,
     );
-          if (request.notifyIfFailed) {
+    if (payload.notifyIfFailed) {
       showErrorNotification(
-              `Failed to load plugin "${request.plugin.title}" v${request.plugin.version}`,
+        `Failed to load plugin "${payload.plugin.title}" v${payload.plugin.version}`,
       );
     }
   }
 }
-      store.dispatch(pluginActivationHandled(queue.length));
-    },
-  );
-};

desktop/app/src/reducers/pluginManager.tsx

@@ -19,15 +19,22 @@ import semver from 'semver';
 export type State = {
   installedPlugins: Map<string, InstalledPluginDetails>;
   uninstalledPlugins: Set<string>;
-  pluginActivationQueue: PluginActivationRequest[];
+  pluginCommandsQueue: PluginCommand[];
 };
 
-export type PluginActivationRequest = {
+export type PluginCommand = LoadPluginAction;
+
+export type LoadPluginActionPayload = {
   plugin: ActivatablePluginDetails;
   enable: boolean;
   notifyIfFailed: boolean;
 };
 
+export type LoadPluginAction = {
+  type: 'LOAD_PLUGIN';
+  payload: LoadPluginActionPayload;
+};
+
 export type Action =
   | {
       type: 'REGISTER_INSTALLED_PLUGINS';
@@ -43,18 +50,15 @@ export type Action =
       payload: InstalledPluginDetails;
     }
   | {
-      type: 'ACTIVATE_PLUGINS';
+      type: 'PLUGIN_COMMANDS_PROCESSED';
-      payload: PluginActivationRequest[];
-    }
-  | {
-      type: 'PLUGIN_ACTIVATION_HANDLED';
       payload: number;
-    };
+    }
+  | LoadPluginAction;
 
 const INITIAL_STATE: State = {
   installedPlugins: new Map<string, InstalledPluginDetails>(),
   uninstalledPlugins: new Set<string>(),
-  pluginActivationQueue: [],
+  pluginCommandsQueue: [],
 };
 
 export default function reducer(
@@ -77,13 +81,16 @@ export default function reducer(
         draft.installedPlugins.set(plugin.name, plugin);
       }
     });
-  } else if (action.type === 'ACTIVATE_PLUGINS') {
+  } else if (action.type === 'LOAD_PLUGIN') {
     return produce(state, (draft) => {
-      draft.pluginActivationQueue.push(...action.payload);
+      draft.pluginCommandsQueue.push({
+        type: 'LOAD_PLUGIN',
+        payload: action.payload,
       });
-  } else if (action.type === 'PLUGIN_ACTIVATION_HANDLED') {
+    });
+  } else if (action.type === 'PLUGIN_COMMANDS_PROCESSED') {
     return produce(state, (draft) => {
-      draft.pluginActivationQueue.splice(0, action.payload);
+      draft.pluginCommandsQueue.splice(0, action.payload);
     });
   } else {
     return {...state};
@@ -107,12 +114,12 @@ export const pluginInstalled = (payload: InstalledPluginDetails): Action => ({
   payload,
 });
 
-export const activatePlugin = (payload: PluginActivationRequest): Action => ({
+export const loadPlugin = (payload: LoadPluginActionPayload): Action => ({
-  type: 'ACTIVATE_PLUGINS',
+  type: 'LOAD_PLUGIN',
-  payload: [payload],
+  payload,
-});
+});
-
+
-export const pluginActivationHandled = (payload: number): Action => ({
+export const pluginCommandsProcessed = (payload: number): Action => ({
-  type: 'PLUGIN_ACTIVATION_HANDLED',
+  type: 'PLUGIN_COMMANDS_PROCESSED',
   payload,
 });

desktop/app/src/sandy-chrome/appinspect/PluginList.tsx

@@ -36,7 +36,7 @@ import {
   PluginDownloadStatus,
   startPluginDownload,
 } from '../../reducers/pluginDownloads';
-import {activatePlugin, uninstallPlugin} from '../../reducers/pluginManager';
+import {loadPlugin, uninstallPlugin} from '../../reducers/pluginManager';
 import {BundledPluginDetails} from 'plugin-lib';
 import {reportUsage} from '../../utils/metrics';
 
@@ -153,7 +153,7 @@ export const PluginList = memo(function PluginList({
       const plugin = downloadablePlugins.find((p) => p.id === id)!;
       reportUsage('plugin:install', {version: plugin.version}, plugin.id);
       if (plugin.isBundled) {
-        dispatch(activatePlugin({plugin, enable: true, notifyIfFailed: true}));
+        dispatch(loadPlugin({plugin, enable: true, notifyIfFailed: true}));
       } else {
         dispatch(startPluginDownload({plugin, startedByUser: true}));
       }