Make plugin loading async

Summary: This diff makes plugin loading async, which we'd need in a browser env (either because we'd use `import()` or we need to fetch the source and than eval it), and deals with all the fallout of that

Reviewed By: timur-valiev

Differential Revision: D32669995

fbshipit-source-id: 73babf38a6757c451b8200c3b320409f127b8b5b

desktop/app/src/electron/initializeElectron.tsx

@@ -177,6 +177,9 @@ export function initializeElectron(
       return flipperServerConfig.gatekeepers[gatekeeper] ?? false;
     },
     flipperServer,
+    async requirePlugin(path) {
+      return (window as any).electronRequire(path);
+    },
   } as RenderHost;
 
   setupMenuBar();

desktop/flipper-common/src/index.tsx

@@ -25,6 +25,7 @@ export {
   reportPlatformFailures,
   reportUsage,
   reportPluginFailures,
+  tryCatchReportPluginFailuresAsync,
   tryCatchReportPlatformFailures,
   tryCatchReportPluginFailures,
   UnsupportedError,

desktop/flipper-common/src/server-types.tsx

@@ -169,6 +169,7 @@ export type FlipperServerCommands = {
   'plugin-start-download': (
     plugin: DownloadablePluginDetails,
   ) => Promise<InstalledPluginDetails>;
+  'plugin-source': (path: string) => Promise<string>;
   'plugins-install-from-npm': (name: string) => Promise<InstalledPluginDetails>;
   'plugins-install-from-file': (
     path: string,

desktop/flipper-common/src/utils/metrics.tsx

@@ -134,6 +134,29 @@ export function tryCatchReportPluginFailures<T>(
   }
 }
 
+/*
+ * Wraps a closure, preserving it's functionality but logging the success or
+ failure state of it.
+ */
+export async function tryCatchReportPluginFailuresAsync<T>(
+  closure: () => Promise<T>,
+  name: string,
+  plugin: string,
+): Promise<T> {
+  try {
+    const result = await closure();
+    logPluginSuccessRate(name, plugin, {kind: 'success'});
+    return result;
+  } catch (e) {
+    logPluginSuccessRate(name, plugin, {
+      kind: 'failure',
+      supportedOperation: !(e instanceof UnsupportedError),
+      error: e,
+    });
+    throw e;
+  }
+}
+
 /**
  * Track usage of a feature.
  * @param action Unique name for the action performed. E.g. captureScreenshot

desktop/flipper-server-core/src/FlipperServerImpl.tsx

@@ -268,6 +268,7 @@ export class FlipperServerImpl implements FlipperServer {
       this.pluginManager.installPluginFromFile(path),
     'plugins-install-from-npm': (name) =>
       this.pluginManager.installPluginFromNpm(name),
+    'plugin-source': (path) => this.pluginManager.loadSource(path),
   };
 
   registerDevice(device: ServerDevice) {

desktop/flipper-server-core/src/plugins/PluginManager.tsx

@@ -61,6 +61,10 @@ export class PluginManager {
   installPluginFromFile = installPluginFromFile;
   installPluginFromNpm = installPluginFromNpm;
 
+  async loadSource(path: string) {
+    return await fs.readFile(path, 'utf8');
+  }
+
   async getBundledPlugins(): Promise<Array<BundledPluginDetails>> {
     if (process.env.NODE_ENV === 'test') {
       return [];

desktop/flipper-ui-browser/src/initializeRenderHost.tsx

@@ -8,6 +8,7 @@
  */
 
 import {FlipperServer, FlipperServerConfig} from 'flipper-common';
+import {getRenderHostInstance} from 'flipper-ui-core';
 
 export function initializeRenderHost(
   flipperServer: FlipperServer,
@@ -62,6 +63,15 @@ export function initializeRenderHost(
       return flipperServerConfig.gatekeepers[gatekeeper] ?? false;
     },
     flipperServer,
+    async requirePlugin(path) {
+      // TODO: use `await import(path)`?
+      const source = await getRenderHostInstance().flipperServer.exec(
+        'plugin-source',
+        path,
+      );
+      // eslint-disable-next-line no-eval
+      return eval(source);
+    },
   };
 }
 

desktop/flipper-ui-core/src/PluginContainer.tsx

@@ -242,6 +242,10 @@ class PluginContainer extends PureComponent<Props, State> {
   }
 
   renderPluginInfo() {
+    if (isTest()) {
+      // Plugin info uses Antd animations, generating a gazillion warnings
+      return 'Stubbed plugin info';
+    }
     return <PluginInfo />;
   }
 

desktop/flipper-ui-core/src/RenderHost.tsx

@@ -98,6 +98,7 @@ export interface RenderHost {
   GK(gatekeeper: string): boolean;
   flipperServer: FlipperServer;
   serverConfig: FlipperServerConfig;
+  requirePlugin(path: string): Promise<any>;
 }
 
 export function getRenderHostInstance(): RenderHost {

desktop/flipper-ui-core/src/__tests__/PluginContainer.node.tsx

@@ -7,7 +7,7 @@
  * @format
  */
 
-jest.useFakeTimers();
+// jest.useFakeTimers();
 
 import React from 'react';
 import produce from 'immer';
@@ -22,10 +22,12 @@ import {
   DevicePluginClient,
   DeviceLogEntry,
   useValue,
+  sleep,
 } from 'flipper-plugin';
 import {selectPlugin} from '../reducers/connections';
 import {updateSettings} from '../reducers/settings';
 import {switchPlugin} from '../reducers/pluginManager';
+import {awaitPluginCommandQueueEmpty} from '../dispatcher/pluginManager';
 
 interface PersistedState {
   count: 1;
@@ -57,7 +59,7 @@ class TestPlugin extends FlipperPlugin<any, any, any> {
   render() {
     return (
       <h1>
-        Hello:{' '}
+        <span>Hello</span>
         <span data-testid="counter">{this.props.persistedState.count}</span>
       </h1>
     );
@@ -82,8 +84,9 @@ test('Plugin container can render plugin and receive updates', async () => {
               class="css-1woty6b-Container"
             >
               <h1>
-                Hello:
+                <span>
-                 
+                  Hello
+                </span>
                 <span
                   data-testid="counter"
                 >
@@ -348,19 +351,21 @@ test('PluginContainer can render Sandy plugins', async () => {
       }),
     );
   });
+
   // note: this is the old pluginInstance, so that one is not reconnected!
   expect(pluginInstance.connectedStub).toBeCalledTimes(2);
   expect(pluginInstance.disconnectedStub).toBeCalledTimes(2);
   expect(pluginInstance.activatedStub).toBeCalledTimes(2);
   expect(pluginInstance.deactivatedStub).toBeCalledTimes(2);
 
-  expect(
+  await awaitPluginCommandQueueEmpty(store);
-    client.sandyPluginStates.get('TestPlugin')!.instanceApi.connectedStub,
+  await sleep(10);
-  ).toBeCalledTimes(1);
+  const newPluginInstance =
+    client.sandyPluginStates.get('TestPlugin')!.instanceApi;
+  expect(newPluginInstance).not.toBe(pluginInstance);
+  expect(newPluginInstance.connectedStub).toBeCalledTimes(1);
   expect(client.rawSend).toBeCalledWith('init', {plugin: 'TestPlugin'});
-  expect(
+  expect(newPluginInstance.count.get()).toBe(0);
-    client.sandyPluginStates.get('TestPlugin')!.instanceApi.count.get(),
-  ).toBe(0);
 });
 
 test('PluginContainer triggers correct lifecycles for background plugin', async () => {
@@ -478,6 +483,9 @@ test('PluginContainer triggers correct lifecycles for background plugin', async
       }),
     );
   });
+
+  await awaitPluginCommandQueueEmpty(store);
+
   // note: this is the old pluginInstance, so that one is not reconnected!
   expect(pluginInstance.connectedStub).toBeCalledTimes(1);
   expect(pluginInstance.disconnectedStub).toBeCalledTimes(1);
@@ -533,7 +541,12 @@ test('PluginContainer + Sandy plugin supports deeplink', async () => {
       Component() {
         const instance = usePlugin(plugin);
         const linkState = useValue(instance.linkState);
-        return <h1>hello {linkState || 'world'}</h1>;
+        return (
+          <h1>
+            <span>hello</span>
+            <span>{linkState || 'world'}</span>
+          </h1>
+        );
       },
     },
   );
@@ -558,8 +571,12 @@ test('PluginContainer + Sandy plugin supports deeplink', async () => {
               class="css-1woty6b-Container"
             >
               <h1>
-                hello 
+                <span>
-                world
+                  hello
+                </span>
+                <span>
+                  world
+                </span>
               </h1>
             </div>
             <div
@@ -582,7 +599,7 @@ test('PluginContainer + Sandy plugin supports deeplink', async () => {
     );
   });
 
-  jest.runAllTimers();
+  await sleep(100);
   expect(linksSeen).toEqual(['universe!']);
   expect(renderer.baseElement).toMatchInlineSnapshot(`
     <body>
@@ -598,8 +615,12 @@ test('PluginContainer + Sandy plugin supports deeplink', async () => {
               class="css-1woty6b-Container"
             >
               <h1>
-                hello 
+                <span>
-                universe!
+                  hello
+                </span>
+                <span>
+                  universe!
+                </span>
               </h1>
             </div>
             <div
@@ -622,7 +643,7 @@ test('PluginContainer + Sandy plugin supports deeplink', async () => {
       }),
     );
   });
-  jest.runAllTimers();
+  await awaitPluginCommandQueueEmpty(store);
   expect(linksSeen).toEqual(['universe!']);
 
   // ...nor does a random other store update that does trigger a plugin container render
@@ -645,7 +666,8 @@ test('PluginContainer + Sandy plugin supports deeplink', async () => {
       }),
     );
   });
-  jest.runAllTimers();
+  await awaitPluginCommandQueueEmpty(store);
+  await sleep(10);
   expect(linksSeen).toEqual(['universe!', 'london!']);
 
   // and same link does trigger if something else was selected in the mean time
@@ -667,7 +689,8 @@ test('PluginContainer + Sandy plugin supports deeplink', async () => {
       }),
     );
   });
-  jest.runAllTimers();
+  await awaitPluginCommandQueueEmpty(store);
+  await sleep(10);
   expect(linksSeen).toEqual(['universe!', 'london!', 'london!']);
 });
 
@@ -689,7 +712,12 @@ test('PluginContainer can render Sandy device plugins', async () => {
       });
     }).toThrowError(/didn't match the type of the requested plugin/);
     const lastLogMessage = useValue(sandyApi.lastLogMessage);
-    return <div>Hello from Sandy: {lastLogMessage?.message}</div>;
+    return (
+      <div>
+        <span>Hello from Sandy:</span>
+        <span>{lastLogMessage?.message}</span>
+      </div>
+    );
   }
 
   const devicePlugin = (client: DevicePluginClient) => {
@@ -730,7 +758,10 @@ test('PluginContainer can render Sandy device plugins', async () => {
               class="css-1woty6b-Container"
             >
               <div>
-                Hello from Sandy: 
+                <span>
+                  Hello from Sandy:
+                </span>
+                <span />
               </div>
             </div>
             <div
@@ -754,6 +785,8 @@ test('PluginContainer can render Sandy device plugins', async () => {
       tag: 'test',
     });
   });
+  await sleep(10); // links are handled async
+
   expect(renders).toBe(2);
 
   expect(renderer.baseElement).toMatchInlineSnapshot(`
@@ -770,8 +803,12 @@ test('PluginContainer can render Sandy device plugins', async () => {
               class="css-1woty6b-Container"
             >
               <div>
-                Hello from Sandy: 
+                <span>
-                helleuh
+                  Hello from Sandy:
+                </span>
+                <span>
+                  helleuh
+                </span>
               </div>
             </div>
             <div
@@ -847,7 +884,12 @@ test('PluginContainer + Sandy device plugin supports deeplink', async () => {
       Component() {
         const instance = usePlugin(devicePlugin);
         const linkState = useValue(instance.linkState);
-        return <h1>hello {linkState || 'world'}</h1>;
+        return (
+          <h1>
+            <span>hello</span>
+            <span>{linkState || 'world'}</span>
+          </h1>
+        );
       },
     },
   );
@@ -877,8 +919,12 @@ test('PluginContainer + Sandy device plugin supports deeplink', async () => {
               class="css-1woty6b-Container"
             >
               <h1>
-                hello 
+                <span>
-                world
+                  hello
+                </span>
+                <span>
+                  world
+                </span>
               </h1>
             </div>
             <div
@@ -902,7 +948,8 @@ test('PluginContainer + Sandy device plugin supports deeplink', async () => {
     );
   });
 
-  jest.runAllTimers();
+  await awaitPluginCommandQueueEmpty(store);
+  await sleep(10); // links are handled async
   expect(linksSeen).toEqual([theUniverse]);
   expect(renderer.baseElement).toMatchInlineSnapshot(`
     <body>
@@ -918,8 +965,12 @@ test('PluginContainer + Sandy device plugin supports deeplink', async () => {
               class="css-1woty6b-Container"
             >
               <h1>
-                hello 
+                <span>
-                {"thisIs":"theUniverse"}
+                  hello
+                </span>
+                <span>
+                  {"thisIs":"theUniverse"}
+                </span>
               </h1>
             </div>
             <div
@@ -943,7 +994,7 @@ test('PluginContainer + Sandy device plugin supports deeplink', async () => {
       }),
     );
   });
-  jest.runAllTimers();
+  await awaitPluginCommandQueueEmpty(store);
   expect(linksSeen).toEqual([theUniverse]);
 
   // ...nor does a random other store update that does trigger a plugin container render
@@ -967,7 +1018,8 @@ test('PluginContainer + Sandy device plugin supports deeplink', async () => {
       }),
     );
   });
-  jest.runAllTimers();
+  await awaitPluginCommandQueueEmpty(store);
+  await sleep(10);
   expect(linksSeen).toEqual([theUniverse, 'london!']);
 
   // and same link does trigger if something else was selected in the mean time
@@ -991,7 +1043,8 @@ test('PluginContainer + Sandy device plugin supports deeplink', async () => {
       }),
     );
   });
-  jest.runAllTimers();
+  await awaitPluginCommandQueueEmpty(store);
+  await sleep(10);
   expect(linksSeen).toEqual([theUniverse, 'london!', 'london!']);
 });
 
@@ -1087,12 +1140,13 @@ test('Sandy plugins support isPluginSupported + selectPlugin', async () => {
   pluginInstance.selectPlugin(definition.id, 'data');
   expect(store.getState().connections.selectedPlugin).toBe(definition.id);
   expect(pluginInstance.activatedStub).toBeCalledTimes(2);
-  jest.runAllTimers();
+  await awaitPluginCommandQueueEmpty(store);
   expect(renderer.baseElement.querySelector('h1')).toMatchInlineSnapshot(`
     <h1>
       Plugin1
     </h1>
   `);
+  await sleep(10); // links are handled async
   expect(linksSeen).toEqual(['data']);
 
   // try to plugin 2 - it should be possible to select it even if it is not enabled
@@ -1137,7 +1191,12 @@ test('PluginContainer can render Sandy plugins for archived devices', async () =
         return {};
       });
     }).toThrowError(/didn't match the type of the requested plugin/);
-    return <div>Hello from Sandy{count}</div>;
+    return (
+      <div>
+        <span>Hello from Sandy</span>
+        <span>{count}</span>
+      </div>
+    );
   }
 
   type Events = {
@@ -1198,8 +1257,12 @@ test('PluginContainer can render Sandy plugins for archived devices', async () =
               class="css-1woty6b-Container"
             >
               <div>
-                Hello from Sandy
+                <span>
-                0
+                  Hello from Sandy
+                </span>
+                <span>
+                  0
+                </span>
               </div>
             </div>
             <div
@@ -1272,8 +1335,12 @@ test('PluginContainer can render Sandy plugins for archived devices', async () =
               class="css-1woty6b-Container"
             >
               <div>
-                Hello from Sandy
+                <span>
-                0
+                  Hello from Sandy
+                </span>
+                <span>
+                  0
+                </span>
               </div>
             </div>
             <div

desktop/flipper-ui-core/src/devices/__tests__/BaseDevice.node.tsx

@@ -292,20 +292,20 @@ test('log listeners are resumed and suspended automatically - 2', async () => {
   expect(entries.length).toBe(2);
 
   // disable one plugin
-  flipper.togglePlugin(Plugin.id);
+  await flipper.togglePlugin(Plugin.id);
   expect(device.stopLogging).toBeCalledTimes(0);
   device.addLogEntry(message);
   expect(entries.length).toBe(3);
 
   // disable the other plugin
-  flipper.togglePlugin(DevicePlugin.id);
+  await flipper.togglePlugin(DevicePlugin.id);
 
   expect(device.stopLogging).toBeCalledTimes(1);
   device.addLogEntry(message);
   expect(entries.length).toBe(3);
 
   // re-enable plugn
-  flipper.togglePlugin(Plugin.id);
+  await flipper.togglePlugin(Plugin.id);
   expect(device.startLogging).toBeCalledTimes(2);
   device.addLogEntry(message);
   expect(entries.length).toBe(4);

desktop/flipper-ui-core/src/dispatcher/__tests__/pluginManager.node.tsx

@@ -23,6 +23,7 @@ import MockFlipper from '../../test-utils/MockFlipper';
 import Client from '../../Client';
 import React from 'react';
 import BaseDevice from '../../devices/BaseDevice';
+import {awaitPluginCommandQueueEmpty} from '../pluginManager';
 
 const pluginDetails1 = TestUtils.createMockPluginDetails({
   id: 'plugin1',
@@ -71,7 +72,7 @@ let mockDevice: BaseDevice;
 
 beforeEach(async () => {
   mockedRequirePlugin.mockImplementation(
-    (details) =>
+    async (details) =>
       (details === pluginDetails1
         ? pluginDefinition1
         : details === pluginDetails2
@@ -99,6 +100,8 @@ test('load plugin when no other version loaded', async () => {
   mockFlipper.dispatch(
     loadPlugin({plugin: pluginDetails1, enable: false, notifyIfFailed: false}),
   );
+
+  await awaitPluginCommandQueueEmpty(mockFlipper.store);
   expect(mockFlipper.getState().plugins.clientPlugins.get('plugin1')).toBe(
     pluginDefinition1,
   );
@@ -119,6 +122,8 @@ test('load plugin when other version loaded', async () => {
       notifyIfFailed: false,
     }),
   );
+
+  await awaitPluginCommandQueueEmpty(mockFlipper.store);
   expect(mockFlipper.getState().plugins.clientPlugins.get('plugin1')).toBe(
     pluginDefinition1V2,
   );
@@ -132,6 +137,8 @@ test('load and enable Sandy plugin', async () => {
   mockFlipper.dispatch(
     loadPlugin({plugin: pluginDetails1, enable: true, notifyIfFailed: false}),
   );
+
+  await awaitPluginCommandQueueEmpty(mockFlipper.store);
   expect(mockFlipper.getState().plugins.clientPlugins.get('plugin1')).toBe(
     pluginDefinition1,
   );
@@ -146,6 +153,8 @@ test('uninstall plugin', async () => {
     loadPlugin({plugin: pluginDetails1, enable: true, notifyIfFailed: false}),
   );
   mockFlipper.dispatch(uninstallPlugin({plugin: pluginDefinition1}));
+
+  await awaitPluginCommandQueueEmpty(mockFlipper.store);
   expect(
     mockFlipper.getState().plugins.clientPlugins.has('plugin1'),
   ).toBeFalsy();
@@ -167,11 +176,13 @@ test('uninstall bundled plugin', async () => {
     version: '0.43.0',
   });
   const pluginDefinition = new SandyPluginDefinition(pluginDetails, TestPlugin);
-  mockedRequirePlugin.mockReturnValue(pluginDefinition);
+  mockedRequirePlugin.mockReturnValue(Promise.resolve(pluginDefinition));
   mockFlipper.dispatch(
     loadPlugin({plugin: pluginDetails, enable: true, notifyIfFailed: false}),
   );
   mockFlipper.dispatch(uninstallPlugin({plugin: pluginDefinition}));
+
+  await awaitPluginCommandQueueEmpty(mockFlipper.store);
   expect(
     mockFlipper.getState().plugins.clientPlugins.has('bundled-plugin'),
   ).toBeFalsy();
@@ -196,6 +207,8 @@ test('star plugin', async () => {
       selectedApp: mockClient.query.app,
     }),
   );
+
+  await awaitPluginCommandQueueEmpty(mockFlipper.store);
   expect(
     mockFlipper.getState().connections.enabledPlugins[mockClient.query.app],
   ).toContain('plugin1');
@@ -218,6 +231,8 @@ test('disable plugin', async () => {
       selectedApp: mockClient.query.app,
     }),
   );
+
+  await awaitPluginCommandQueueEmpty(mockFlipper.store);
   expect(
     mockFlipper.getState().connections.enabledPlugins[mockClient.query.app],
   ).not.toContain('plugin1');
@@ -237,6 +252,8 @@ test('star device plugin', async () => {
       plugin: devicePluginDefinition,
     }),
   );
+
+  await awaitPluginCommandQueueEmpty(mockFlipper.store);
   expect(
     mockFlipper.getState().connections.enabledDevicePlugins.has('device'),
   ).toBeTruthy();
@@ -261,6 +278,8 @@ test('disable device plugin', async () => {
       plugin: devicePluginDefinition,
     }),
   );
+
+  await awaitPluginCommandQueueEmpty(mockFlipper.store);
   expect(
     mockFlipper.getState().connections.enabledDevicePlugins.has('device'),
   ).toBeFalsy();

desktop/flipper-ui-core/src/dispatcher/__tests__/plugins.node.tsx

@@ -135,9 +135,9 @@ test('checkGK for failing plugin', () => {
   expect(gatekeepedPlugins[0].name).toEqual(name);
 });
 
-test('requirePlugin returns null for invalid requires', () => {
+test('requirePlugin returns null for invalid requires', async () => {
-  const requireFn = createRequirePluginFunction([], require);
+  const requireFn = createRequirePluginFunction([]);
-  const plugin = requireFn({
+  const plugin = await requireFn({
     ...sampleInstalledPluginDetails,
     name: 'pluginID',
     dir: '/Users/mock/.flipper/thirdparty/flipper-plugin-sample',
@@ -148,10 +148,10 @@ test('requirePlugin returns null for invalid requires', () => {
   expect(plugin).toBeNull();
 });
 
-test('requirePlugin loads plugin', () => {
+test('requirePlugin loads plugin', async () => {
   const name = 'pluginID';
-  const requireFn = createRequirePluginFunction([], require);
+  const requireFn = createRequirePluginFunction([]);
-  const plugin = requireFn({
+  const plugin = await requireFn({
     ...sampleInstalledPluginDetails,
     name,
     dir: '/Users/mock/.flipper/thirdparty/flipper-plugin-sample',
@@ -224,10 +224,10 @@ test('newest version of each plugin is used', () => {
   });
 });
 
-test('requirePlugin loads valid Sandy plugin', () => {
+test('requirePlugin loads valid Sandy plugin', async () => {
   const name = 'pluginID';
-  const requireFn = createRequirePluginFunction([], require);
+  const requireFn = createRequirePluginFunction([]);
-  const plugin = requireFn({
+  const plugin = (await requireFn({
     ...sampleInstalledPluginDetails,
     name,
     dir: path.join(
@@ -240,7 +240,7 @@ test('requirePlugin loads valid Sandy plugin', () => {
     ),
     version: '1.0.0',
     flipperSDKVersion: '0.0.0',
-  }) as _SandyPluginDefinition;
+  })) as _SandyPluginDefinition;
   expect(plugin).not.toBeNull();
   expect(plugin).toBeInstanceOf(_SandyPluginDefinition);
   expect(plugin.id).toBe('Sample');
@@ -261,11 +261,11 @@ test('requirePlugin loads valid Sandy plugin', () => {
   expect(typeof plugin.asPluginModule().plugin).toBe('function');
 });
 
-test('requirePlugin errors on invalid Sandy plugin', () => {
+test('requirePlugin errors on invalid Sandy plugin', async () => {
   const name = 'pluginID';
   const failedPlugins: any[] = [];
-  const requireFn = createRequirePluginFunction(failedPlugins, require);
+  const requireFn = createRequirePluginFunction(failedPlugins);
-  requireFn({
+  await requireFn({
     ...sampleInstalledPluginDetails,
     name,
     // Intentionally the wrong file:
@@ -279,10 +279,10 @@ test('requirePlugin errors on invalid Sandy plugin', () => {
   );
 });
 
-test('requirePlugin loads valid Sandy Device plugin', () => {
+test('requirePlugin loads valid Sandy Device plugin', async () => {
   const name = 'pluginID';
-  const requireFn = createRequirePluginFunction([], require);
+  const requireFn = createRequirePluginFunction([]);
-  const plugin = requireFn({
+  const plugin = (await requireFn({
     ...sampleInstalledPluginDetails,
     pluginType: 'device',
     name,
@@ -296,7 +296,7 @@ test('requirePlugin loads valid Sandy Device plugin', () => {
     ),
     version: '1.0.0',
     flipperSDKVersion: '0.0.0',
-  }) as _SandyPluginDefinition;
+  })) as _SandyPluginDefinition;
   expect(plugin).not.toBeNull();
   expect(plugin).toBeInstanceOf(_SandyPluginDefinition);
   expect(plugin.id).toBe('Sample');

desktop/flipper-ui-core/src/dispatcher/pluginManager.tsx

@@ -75,6 +75,7 @@ export default (
     });
   }
 
+  let running = false;
   const unsubscribeHandlePluginCommands = sideEffect(
     store,
     {
@@ -85,14 +86,49 @@ export default (
       noTimeBudgetWarns: true, // These side effects are critical, so we're doing them with zero throttling and want to avoid unnecessary warns
     },
     (state) => state.pluginManager.pluginCommandsQueue,
-    processPluginCommandsQueue,
+    async (_queue: PluginCommand[], store: Store) => {
+      // To make sure all commands are running in order, and not kicking off parallel command
+      // processing when new commands arrive (sideEffect doesn't await)
+      // we keep the 'running' flag, and keep running in a loop until the commandQueue is empty,
+      // to make sure any commands that have arrived during execution are executed
+      if (running) {
+        return; // will be picked up in while(true) loop
+      }
+      running = true;
+      try {
+        while (true) {
+          const remaining = store.getState().pluginManager.pluginCommandsQueue;
+          if (!remaining.length) {
+            return; // done
+          }
+          await processPluginCommandsQueue(remaining, store);
+          store.dispatch(pluginCommandsProcessed(remaining.length));
+        }
+      } finally {
+        running = false;
+      }
+    },
   );
   return async () => {
     unsubscribeHandlePluginCommands();
   };
 };
 
-export function processPluginCommandsQueue(
+export async function awaitPluginCommandQueueEmpty(store: Store) {
+  if (store.getState().pluginManager.pluginCommandsQueue.length === 0) {
+    return;
+  }
+  return new Promise<void>((resolve) => {
+    const unsubscribe = store.subscribe(() => {
+      if (store.getState().pluginManager.pluginCommandsQueue.length === 0) {
+        unsubscribe();
+        resolve();
+      }
+    });
+  });
+}
+
+async function processPluginCommandsQueue(
   queue: PluginCommand[],
   store: Store,
 ) {
@@ -100,7 +136,7 @@ export function processPluginCommandsQueue(
     try {
       switch (command.type) {
         case 'LOAD_PLUGIN':
-          loadPlugin(store, command.payload);
+          await loadPlugin(store, command.payload);
           break;
         case 'UNINSTALL_PLUGIN':
           uninstallPlugin(store, command.payload);
@@ -121,12 +157,11 @@ export function processPluginCommandsQueue(
       console.error('Failed to process command', command);
     }
   }
-  store.dispatch(pluginCommandsProcessed(queue.length));
 }
 
-function loadPlugin(store: Store, payload: LoadPluginActionPayload) {
+async function loadPlugin(store: Store, payload: LoadPluginActionPayload) {
   try {
-    const plugin = requirePlugin(payload.plugin);
+    const plugin = await requirePlugin(payload.plugin);
     const enablePlugin = payload.enable;
     updatePlugin(store, {plugin, enablePlugin});
   } catch (err) {

desktop/flipper-ui-core/src/dispatcher/plugins.tsx

@@ -8,7 +8,11 @@
  */
 
 import type {Store} from '../reducers/index';
-import type {InstalledPluginDetails, Logger} from 'flipper-common';
+import {
+  InstalledPluginDetails,
+  Logger,
+  tryCatchReportPluginFailuresAsync,
+} from 'flipper-common';
 import {PluginDefinition} from '../plugin';
 import React from 'react';
 import ReactDOM from 'react-dom';
@@ -31,7 +35,7 @@ import {
   BundledPluginDetails,
   ConcretePluginDetails,
 } from 'flipper-common';
-import {tryCatchReportPluginFailures, reportUsage} from 'flipper-common';
+import {reportUsage} from 'flipper-common';
 import * as FlipperPluginSDK from 'flipper-plugin';
 import {_SandyPluginDefinition} from 'flipper-plugin';
 import * as Immer from 'immer';
@@ -46,6 +50,8 @@ import isPluginCompatible from '../utils/isPluginCompatible';
 import isPluginVersionMoreRecent from '../utils/isPluginVersionMoreRecent';
 import {createSandyPluginWrapper} from '../utils/createSandyPluginWrapper';
 import {getRenderHostInstance} from '../RenderHost';
+import pMap from 'p-map';
+
 let defaultPluginsIndex: any = null;
 
 export default async (store: Store, _logger: Logger) => {
@@ -88,12 +94,15 @@ export default async (store: Store, _logger: Logger) => {
   const loadedPlugins =
     getLatestCompatibleVersionOfEachPlugin(allLocalVersions);
 
-  const initialPlugins: PluginDefinition[] = loadedPlugins
+  const pluginsToLoad = loadedPlugins
     .map(reportVersion)
     .filter(checkDisabled(disabledPlugins))
-    .filter(checkGK(gatekeepedPlugins))
+    .filter(checkGK(gatekeepedPlugins));
-    .map(createRequirePluginFunction(failedPlugins))
+  const loader = createRequirePluginFunction(failedPlugins);
-    .filter(notNull);
+
+  const initialPlugins: PluginDefinition[] = (
+    await pMap(pluginsToLoad, loader)
+  ).filter(notNull);
 
   const classicPlugins = initialPlugins.filter(
     (p) => !isSandyPlugin(p.details),
@@ -235,11 +244,12 @@ export const checkDisabled = (
 
 export const createRequirePluginFunction = (
   failedPlugins: Array<[ActivatablePluginDetails, string]>,
-  reqFn: Function = global.electronRequire,
 ) => {
-  return (pluginDetails: ActivatablePluginDetails): PluginDefinition | null => {
+  return async (
+    pluginDetails: ActivatablePluginDetails,
+  ): Promise<PluginDefinition | null> => {
     try {
-      const pluginDefinition = requirePlugin(pluginDetails, reqFn);
+      const pluginDefinition = await requirePlugin(pluginDetails);
       if (
         pluginDefinition &&
         isDevicePluginDefinition(pluginDefinition) &&
@@ -260,8 +270,7 @@ export const createRequirePluginFunction = (
 
 export const requirePlugin = (
   pluginDetails: ActivatablePluginDetails,
-  reqFn: Function = global.electronRequire,
+): Promise<PluginDefinition> => {
-): PluginDefinition => {
   reportUsage(
     'plugin:load',
     {
@@ -269,8 +278,8 @@ export const requirePlugin = (
     },
     pluginDetails.id,
   );
-  return tryCatchReportPluginFailures(
+  return tryCatchReportPluginFailuresAsync(
-    () => requirePluginInternal(pluginDetails, reqFn),
+    () => requirePluginInternal(pluginDetails),
     'plugin:load',
     pluginDetails.id,
   );
@@ -280,13 +289,12 @@ const isSandyPlugin = (pluginDetails: ActivatablePluginDetails) => {
   return !!pluginDetails.flipperSDKVersion;
 };
 
-const requirePluginInternal = (
+const requirePluginInternal = async (
   pluginDetails: ActivatablePluginDetails,
-  reqFn: Function = global.electronRequire,
+): Promise<PluginDefinition> => {
-): PluginDefinition => {
   let plugin = pluginDetails.isBundled
     ? defaultPluginsIndex[pluginDetails.name]
-    : reqFn(pluginDetails.entry);
+    : await getRenderHostInstance().requirePlugin(pluginDetails.entry);
   if (isSandyPlugin(pluginDetails)) {
     // Sandy plugin
     return new _SandyPluginDefinition(pluginDetails, plugin);

desktop/flipper-ui-core/src/reducers/__tests__/sandyplugins.node.tsx

@@ -18,6 +18,7 @@ import {
   TestUtils,
 } from 'flipper-plugin';
 import {switchPlugin} from '../pluginManager';
+import {awaitPluginCommandQueueEmpty} from '../../dispatcher/pluginManager';
 
 const pluginDetails = TestUtils.createMockPluginDetails();
 
@@ -184,6 +185,8 @@ test('it should not initialize a sandy plugin if not enabled', async () => {
     }),
   );
 
+  await awaitPluginCommandQueueEmpty(store);
+
   expect(client.sandyPluginStates.get(Plugin2.id)).toBeUndefined();
   expect(instance.connectStub).toHaveBeenCalledTimes(0);
   // disconnect wasn't called because connect was never called

desktop/flipper-ui-core/src/test-utils/TestDevice.tsx

@@ -28,4 +28,8 @@ export class TestDevice extends BaseDevice {
       specs,
     });
   }
+
+  async startLogging() {
+    // noop
+  }
 }

desktop/flipper-ui-core/src/test-utils/createMockFlipperWithPlugin.tsx

@@ -37,6 +37,7 @@ import {switchPlugin} from '../reducers/pluginManager';
 import {createSandyPluginFromClassicPlugin} from '../dispatcher/plugins';
 import {createMockActivatablePluginDetails} from '../utils/testUtils';
 import {_SandyPluginDefinition} from 'flipper-plugin';
+import {awaitPluginCommandQueueEmpty} from '../dispatcher/pluginManager';
 
 export type MockFlipperResult = {
   client: Client;
@@ -54,7 +55,7 @@ export type MockFlipperResult = {
     skipRegister?: boolean,
   ): Promise<Client>;
   logger: Logger;
-  togglePlugin(plugin?: string): void;
+  togglePlugin(plugin?: string): Promise<void>;
   selectPlugin(
     id?: string,
     client?: Client,
@@ -168,6 +169,7 @@ export async function createMockFlipperWithPlugin(
         }
       });
     }
+    await awaitPluginCommandQueueEmpty(store);
     return client;
   };
 
@@ -233,7 +235,7 @@ export async function createMockFlipperWithPlugin(
     createClient,
     logger,
     pluginKey: getPluginKey(client.id, device, pluginClazz.id),
-    togglePlugin(id?: string) {
+    async togglePlugin(id?: string) {
       const plugin = id
         ? store.getState().plugins.clientPlugins.get(id) ??
           store.getState().plugins.devicePlugins.get(id)
@@ -247,6 +249,7 @@ export async function createMockFlipperWithPlugin(
           selectedApp: client.query.app,
         }),
       );
+      await awaitPluginCommandQueueEmpty(store);
     },
   };
 }

desktop/flipper-ui-core/src/utils/__tests__/messageQueueSandy.node.tsx

@@ -36,6 +36,7 @@ import pluginMessageQueue, {
   State,
   queueMessages,
 } from '../../reducers/pluginMessageQueue';
+import {awaitPluginCommandQueueEmpty} from '../../dispatcher/pluginManager';
 
 type Events = {
   inc: {
@@ -67,13 +68,14 @@ const TestPlugin = new _SandyPluginDefinition(
   },
 );
 
-function switchTestPlugin(store: Store, client: Client) {
+async function switchTestPlugin(store: Store, client: Client) {
   store.dispatch(
     switchPlugin({
       plugin: TestPlugin,
       selectedApp: client.query.app,
     }),
   );
+  await awaitPluginCommandQueueEmpty(store);
 }
 
 function selectDeviceLogs(store: Store) {
@@ -190,7 +192,7 @@ test('queue - events are NOT processed immediately if plugin is NOT selected (bu
   });
 
   // disable. Messages don't arrive anymore
-  switchTestPlugin(store, client);
+  await switchTestPlugin(store, client);
   // weird state...
   selectTestPlugin(store, client);
   sendMessage('inc', {delta: 3});
@@ -206,7 +208,7 @@ test('queue - events are NOT processed immediately if plugin is NOT selected (bu
   expect(store.getState().pluginMessageQueue).toEqual({});
 
   // star again, plugin still not selected, message is queued
-  switchTestPlugin(store, client);
+  await switchTestPlugin(store, client);
   sendMessage('inc', {delta: 5});
   client.flushMessageBuffer();
 
@@ -699,14 +701,14 @@ test('queue - messages that have not yet flushed be lost when disabling the plug
   `);
 
   // disable
-  switchTestPlugin(store, client);
+  await switchTestPlugin(store, client);
   expect(client.messageBuffer).toMatchInlineSnapshot(`Object {}`);
   expect(store.getState().pluginMessageQueue).toMatchInlineSnapshot(
     `Object {}`,
   );
 
   // re-enable, no messages arrive
-  switchTestPlugin(store, client);
+  await switchTestPlugin(store, client);
   client.flushMessageBuffer();
   processMessageQueue(
     client.sandyPluginStates.get(TestPlugin.id)!,

desktop/scripts/jest-setup-after.ts

@@ -168,5 +168,8 @@ function createStubRenderHost(): RenderHost {
       return stubConfig.gatekeepers[gk] ?? false;
     },
     flipperServer: TestUtils.createFlipperServerMock(),
+    async requirePlugin(path: string) {
+      return require(path);
+    },
   };
 }