prevent unnecessary rerenders

Summary:
Updates of the redux store caused rerendering of UI components, even without the acutal UI changing. This diff ensures the UI is only updated, when

The PluginContainer received all plugin states and selected the pluginState of the active plugin to pass it down to the plugin. However, this caused a rerender everytime a pluginState changed (even if the plugin was in the background).

This diff moves the selection of the active plugin to the `connect`-function and only passes the state of the active plugin into the container. This makes sure the plugin rerenders only if it's own state changes.

The main sidebar displays the number of notifications. Therefore, it was passed the array of notifications. However, this array is regenerated, everytime a new notification **might** be triggered.

Now, the number of notifications is calculated in the `connect`-method and only the number itself is passed into the component. This makes sure the sidebar is only rerendered, when the actual number of notifications changes.

Reviewed By: passy

Differential Revision: D13276096

fbshipit-source-id: bf1e6c4a186f7a1cf7f7427cd3523b5b71eb003a

src/PluginContainer.js

@@ -12,7 +12,7 @@ import type {Props as PluginProps} from './plugin';
 import Client from './Client.js';
 import {
   ErrorBoundary,
-  Component,
+  PureComponent,
   FlexColumn,
   FlexRow,
   colors,
@@ -40,80 +40,32 @@ const SidebarContainer = styled(FlexRow)({
 
 type Props = {
   logger: LogManager,
-  selectedDevice: BaseDevice,
-  selectedPlugin: ?string,
-  selectedApp: ?string,
-  pluginStates: {
-    [pluginKey: string]: Object,
-  },
-  clients: Array<Client>,
-  setPluginState: (payload: {
-    pluginKey: string,
-    state: Object,
-  }) => void,
+  pluginState: Object,
+  activePlugin: ?Class<FlipperPlugin<> | FlipperDevicePlugin<>>,
+  target: Client | BaseDevice | null,
+  pluginKey: string,
   deepLinkPayload: ?string,
+
   selectPlugin: (payload: {|
     selectedPlugin: ?string,
     selectedApp?: ?string,
     deepLinkPayload: ?string,
   |}) => mixed,
-  devicePlugins: Map<string, Class<FlipperDevicePlugin<>>>,
-  clientPlugins: Map<string, Class<FlipperPlugin<>>>,
-};
-
-type State = {
-  activePlugin: ?Class<FlipperPlugin<> | FlipperDevicePlugin<>>,
-  target: Client | BaseDevice | null,
-  pluginKey: string,
+  setPluginState: (payload: {
+    pluginKey: string,
+    state: Object,
+  }) => void,
 };
 
-class PluginContainer extends Component<Props, State> {
-  static getDerivedStateFromProps(props: Props): State {
-    const {selectedPlugin} = props;
-    let pluginKey = 'unknown';
-    let target = null;
-    let activePlugin: ?Class<FlipperPlugin<> | FlipperDevicePlugin<>> = null;
-
-    if (selectedPlugin) {
-      if (selectedPlugin === NotificationsHub.id) {
-        activePlugin = NotificationsHub;
-      } else if (props.selectedPlugin) {
-        activePlugin = props.devicePlugins.get(props.selectedPlugin);
-      }
-      target = props.selectedDevice;
-      if (activePlugin) {
-        pluginKey = `${props.selectedDevice.serial}#${activePlugin.id}`;
-      } else {
-        target = props.clients.find(
-          (client: Client) => client.id === props.selectedApp,
-        );
-        activePlugin = props.clientPlugins.get(selectedPlugin);
-        if (!activePlugin || !target) {
-          throw new Error(
-            `Plugin "${props.selectedPlugin || ''}" could not be found.`,
-          );
-        }
-        pluginKey = `${target.id}#${activePlugin.id}`;
-      }
-    }
-
-    return {
-      activePlugin,
-      target,
-      pluginKey,
-    };
-  }
-
-  state: State = this.constructor.getDerivedStateFromProps(this.props);
+class PluginContainer extends PureComponent<Props> {
   plugin: ?FlipperPlugin<> | FlipperDevicePlugin<>;
 
   refChanged = (ref: ?FlipperPlugin<> | FlipperDevicePlugin<>) => {
     if (this.plugin) {
       this.plugin._teardown();
       this.plugin = null;
     }
-    const {target} = this.state;
-    if (ref && target) {
+    if (ref && this.props.target) {
       activateMenuItems(ref);
       ref._init();
       this.props.logger.trackTimeSince(`activePlugin-${ref.constructor.id}`);
@@ -122,8 +74,13 @@ class PluginContainer extends Component<Props, State> {
   };
 
   render() {
-    const {pluginStates, setPluginState} = this.props;
-    const {activePlugin, pluginKey, target} = this.state;
+    const {
+      pluginState,
+      setPluginState,
+      activePlugin,
+      pluginKey,
+      target,
+    } = this.props;
 
     if (!activePlugin || !target) {
       return null;
@@ -134,14 +91,14 @@ class PluginContainer extends Component<Props, State> {
       persistedState: activePlugin.defaultPersistedState
         ? {
             ...activePlugin.defaultPersistedState,
-            ...pluginStates[pluginKey],
+            ...pluginState,
           }
-        : pluginStates[pluginKey],
+        : pluginState,
       setPersistedState: state => setPluginState({pluginKey, state}),
       target,
       deepLinkPayload: this.props.deepLinkPayload,
       selectPlugin: (pluginID: string, deepLinkPayload: ?string) => {
-        const {target} = this.state;
+        const {target} = this.props;
         // check if plugin will be available
         if (
           target instanceof Client &&
@@ -192,16 +149,40 @@ export default connect(
     },
     pluginStates,
     plugins: {devicePlugins, clientPlugins},
-  }) => ({
-    selectedPlugin,
-    selectedDevice,
-    pluginStates,
-    selectedApp,
-    clients,
-    deepLinkPayload,
-    devicePlugins,
-    clientPlugins,
-  }),
+  }) => {
+    let pluginKey = 'unknown';
+    let target = null;
+    let activePlugin: ?Class<FlipperPlugin<> | FlipperDevicePlugin<>> = null;
+
+    if (selectedPlugin) {
+      if (selectedPlugin === NotificationsHub.id) {
+        activePlugin = NotificationsHub;
+      } else if (selectedPlugin) {
+        activePlugin = devicePlugins.get(selectedPlugin);
+      }
+      target = selectedDevice;
+      if (activePlugin) {
+        pluginKey = `${selectedDevice.serial}#${activePlugin.id}`;
+      } else {
+        target = clients.find((client: Client) => client.id === selectedApp);
+        activePlugin = clientPlugins.get(selectedPlugin);
+        if (!activePlugin || !target) {
+          throw new Error(
+            `Plugin "${selectedPlugin || ''}" could not be found.`,
+          );
+        }
+        pluginKey = `${target.id}#${activePlugin.id}`;
+      }
+    }
+
+    return {
+      pluginState: pluginStates[pluginKey],
+      activePlugin,
+      target,
+      deepLinkPayload,
+      pluginKey,
+    };
+  },
   {
     setPluginState,
     selectPlugin,

src/chrome/MainSidebar.js

@@ -12,6 +12,7 @@ import type {UninitializedClient} from '../UninitializedClient.js';
 import type {PluginNotification} from '../reducers/notifications';
 
 import {
+  PureComponent,
   Component,
   Sidebar,
   FlexBox,
@@ -177,21 +178,20 @@ type MainSidebarProps = {|
     client: UninitializedClient,
     deviceId?: string,
   }>,
-  activeNotifications: Array<PluginNotification>,
-  blacklistedPlugins: Array<string>,
+  numNotifications: number,
   devicePlugins: Map<string, Class<FlipperDevicePlugin<>>>,
   clientPlugins: Map<string, Class<FlipperPlugin<>>>,
 |};
 
-class MainSidebar extends Component<MainSidebarProps> {
+class MainSidebar extends PureComponent<MainSidebarProps> {
   render() {
     const {
       selectedDevice,
       selectedPlugin,
       selectedApp,
       selectPlugin,
       windowIsFocused,
-      activeNotifications,
+      numNotifications,
     } = this.props;
     let {clients, uninitializedClients} = this.props;
 
@@ -202,11 +202,6 @@ class MainSidebar extends Component<MainSidebarProps> {
       )
       .sort((a, b) => (a.query.app || '').localeCompare(b.query.app));
 
-    let blacklistedPlugins = new Set(this.props.blacklistedPlugins);
-    const notifications = activeNotifications.filter(
-      (n: PluginNotification) => !blacklistedPlugins.has(n.pluginId),
-    );
-
     return (
       <Sidebar
         position="left"
@@ -226,13 +221,11 @@ class MainSidebar extends Component<MainSidebarProps> {
             }>
             <PluginIcon
               color={colors.light50}
-              name={
-                notifications.length > 0 ? NotificationsHub.icon : 'bell-null'
-              }
+              name={numNotifications > 0 ? NotificationsHub.icon : 'bell-null'}
               isActive={selectedPlugin === NotificationsHub.id}
             />
             <PluginName
-              count={notifications.length}
+              count={numNotifications}
               isActive={selectedPlugin === NotificationsHub.id}>
               {NotificationsHub.title}
             </PluginName>
@@ -322,8 +315,12 @@ export default connect(
     notifications: {activeNotifications, blacklistedPlugins},
     plugins: {devicePlugins, clientPlugins},
   }) => ({
-    blacklistedPlugins,
-    activeNotifications,
+    numNotifications: (() => {
+      const blacklist = new Set(blacklistedPlugins);
+      return activeNotifications.filter(
+        (n: PluginNotification) => !blacklist.has(n.pluginId),
+      ).length;
+    })(),
     windowIsFocused,
     selectedDevice,
     selectedPlugin,