[Table] Eliminate unnecessary re-renders everywhere (#998)

* Add Utils.shallowCompare plus unit tests for shallowCompare and shallowCompareKeys

* PureRender everywhere

* Add shouldComponentUpdate to GuideLayer

* Modify shallowCompareKeys to optionally check all keys

* Add Utils.arraysEqual + tests

* Refactor RegionLayer to receive pre-computed regionStyles. avoids unnecessary re-renders!

* Minor cleanups throughout

* Eek, was testing arraysEqual improperly. Fixed a bug.

* Fix bug with className comparison in GuideLayer.shouldComponentUpdate

* Use bothUndefined and bothNull helpers in arraysEqual

* Make shallowCompareKeys return false for two arrays

* Respond to Gilad CR feedback

* Delete unnecessary tests

* Oops, undo describe.only

* Reduce duplicate logic in shallowCompareKeys

packages/table/src/cell/formats/jsonFormat.tsx

@@ -6,6 +6,7 @@
  */
 
 import * as classNames from "classnames";
+import * as PureRender from "pure-render-decorator";
 import * as React from "react";
 import * as Classes from "../../common/classes";
 import { ITruncatedFormatProps, TruncatedFormat, TruncatedPopoverMode } from "./truncatedFormat";
@@ -29,6 +30,7 @@ export interface IJSONFormatProps extends ITruncatedFormatProps {
     stringify?: (obj: any) => string;
 }
 
+@PureRender
 export class JSONFormat extends React.Component<IJSONFormatProps, {}> {
     public static defaultProps: IJSONFormatProps = {
         detectTruncation: true,

packages/table/src/cell/formats/truncatedFormat.tsx

@@ -8,6 +8,7 @@
 import { Classes as CoreClasses, IProps, Popover, Position } from "@blueprintjs/core";
 
 import * as classNames from "classnames";
+import * as PureRender from "pure-render-decorator";
 import * as React from "react";
 
 import * as Classes from "../../common/classes";
@@ -66,6 +67,7 @@ export interface ITruncatedFormatState {
     isTruncated: boolean;
 }
 
+@PureRender
 export class TruncatedFormat extends React.Component<ITruncatedFormatProps, ITruncatedFormatState> {
     public static defaultProps: ITruncatedFormatProps = {
         detectTruncation: true,

packages/table/src/column.tsx

@@ -5,6 +5,7 @@
  * and https://github.com/palantir/blueprint/blob/master/PATENTS
  */
 
+import * as PureRender from "pure-render-decorator";
 import * as React from "react";
 
 import { IProps } from "@blueprintjs/core";
@@ -46,6 +47,7 @@ export interface IColumnProps extends IColumnNameProps, IProps {
     renderColumnHeader?: IColumnHeaderRenderer;
 }
 
+@PureRender
 export class Column extends React.Component<IColumnProps, {}> {
     public static defaultProps: IColumnProps = {
         renderCell: emptyCellRenderer,

packages/table/src/common/contextMenuTargetWrapper.tsx

@@ -6,6 +6,7 @@
  */
 
 import { ContextMenuTarget, IProps } from "@blueprintjs/core";
+import * as PureRender from "pure-render-decorator";
 import * as React from "react";
 
 export interface IContextMenuTargetWrapper extends IProps {
@@ -20,6 +21,7 @@ export interface IContextMenuTargetWrapper extends IProps {
  * chains.
  */
 @ContextMenuTarget
+@PureRender
 export class ContextMenuTargetWrapper extends React.Component<IContextMenuTargetWrapper, {}> {
     public render() {
         const { className, children, style } = this.props;

packages/table/src/common/loadableContent.tsx

@@ -5,6 +5,7 @@
  * and https://github.com/palantir/blueprint/blob/master/PATENTS
  */
 
+import * as PureRender from "pure-render-decorator";
 import * as React from "react";
 
 import { Classes } from "@blueprintjs/core";
@@ -24,6 +25,7 @@ export interface ILoadableContentProps {
 }
 
 // This class expects a single, non-string child.
+@PureRender
 export class LoadableContent extends React.Component<ILoadableContentProps, {}> {
     private style: React.CSSProperties;
 

packages/table/src/common/utils.ts

@@ -200,15 +200,25 @@ export const Utils = {
     },
 
     /**
-     * Partial shallow comparison between objects using the given list of keys.
+     * Shallow comparison between objects. If `keys` is provided, just that subset of keys will be
+     * compared; otherwise, all keys will be compared.
      */
-    shallowCompareKeys(objA: any, objB: any, keys: string[]) {
-        for (const key of keys) {
-            if (objA[key] !== objB[key]) {
-                return false;
-            }
+    shallowCompareKeys(objA: any, objB: any, keys?: string[]) {
+        // treat `null` and `undefined` as the same
+        if (objA == null && objB == null) {
+            return true;
+        } else if (objA == null || objB == null) {
+            return false;
+        } else if (Array.isArray(objA) || Array.isArray(objB)) {
+            return false;
+        } else if (keys != null) {
+            return _shallowCompareKeys(objA, objB, keys);
+        } else {
+            const keysA = Object.keys(objA);
+            const keysB = Object.keys(objB);
+            return _shallowCompareKeys(objA, objB, keysA)
+                && _shallowCompareKeys(objA, objB, keysB);
         }
-        return true;
     },
 
     /**
@@ -333,4 +343,29 @@ export const Utils = {
     isLeftClick(event: MouseEvent) {
         return event.button === 0;
     },
+
+    /**
+     * Returns true if the arrays are equal. Elements will be shallowly compared by default, or they
+     * will be compared using the custom `compare` function if one is provided.
+     */
+    arraysEqual(arrA: any[], arrB: any[], compare = (a: any, b: any) => a === b) {
+        // treat `null` and `undefined` as the same
+        if (arrA == null && arrB == null) {
+            return true;
+        } else if (arrA == null || arrB == null || arrA.length !== arrB.length) {
+            return false;
+        } else {
+            return arrA.every((a, i) => compare(a, arrB[i]));
+        }
+    },
 };
+
+/**
+ * Partial shallow comparison between objects using the given list of keys.
+ */
+function _shallowCompareKeys(objA: any, objB: any, keys: string[]) {
+    return keys.every((key) => {
+        return objA.hasOwnProperty(key) === objB.hasOwnProperty(key)
+            && objA[key] === objB[key];
+    });
+}

packages/table/src/headers/editableName.tsx

@@ -7,6 +7,7 @@
 
 import { EditableText, IIntentProps, IProps } from "@blueprintjs/core";
 import * as classNames from "classnames";
+import * as PureRender from "pure-render-decorator";
 import * as React from "react";
 import * as Classes from "../common/classes";
 
@@ -37,6 +38,7 @@ export interface IEditableNameProps extends IIntentProps, IProps {
     onConfirm?: (value: string) => void;
 }
 
+@PureRender
 export class EditableName extends React.Component<IEditableNameProps, {}> {
     public render() {
         const { className, intent, name, onCancel, onChange, onConfirm } = this.props;

packages/table/src/headers/rowHeaderCell.tsx

@@ -6,6 +6,7 @@
  */
 
 import * as classNames from "classnames";
+import * as PureRender from "pure-render-decorator";
 import * as React from "react";
 
 import { Classes as CoreClasses, ContextMenuTarget, IProps } from "@blueprintjs/core";
@@ -67,6 +68,7 @@ export interface IRowHeaderState {
 }
 
 @ContextMenuTarget
+@PureRender
 export class RowHeaderCell extends React.Component<IRowHeaderCellProps, IRowHeaderState> {
     public state = {
         isActive: false,

packages/table/src/interactions/menus/copyCellsMenuItem.tsx

@@ -6,6 +6,7 @@
  */
 
 import { IMenuItemProps, MenuItem } from "@blueprintjs/core";
+import * as PureRender from "pure-render-decorator";
 import * as React from "react";
 
 import { Clipboard } from "../../common/clipboard";
@@ -37,6 +38,7 @@ export interface ICopyCellsMenuItemProps extends IMenuItemProps {
     onCopy?: (success: boolean) => void;
 }
 
+@PureRender
 export class CopyCellsMenuItem extends React.Component<ICopyCellsMenuItemProps, {}> {
     public render() {
         return <MenuItem {...this.props} onClick={this.handleClick} />;

packages/table/src/interactions/resizeHandle.tsx

@@ -7,6 +7,7 @@
 
 import { IProps } from "@blueprintjs/core";
 import * as classNames from "classnames";
+import * as PureRender from "pure-render-decorator";
 import * as React from "react";
 
 import * as Classes from "../common/classes";
@@ -57,6 +58,7 @@ export interface IResizeHandleState {
     isDragging: boolean;
 }
 
+@PureRender
 export class ResizeHandle extends React.Component<IResizeHandleProps, IResizeHandleState> {
     public state: IResizeHandleState = {
         isDragging: false,

packages/table/src/layers/guides.tsx

@@ -10,6 +10,7 @@ import * as classNames from "classnames";
 import * as React from "react";
 
 import * as Classes from "../common/classes";
+import { Utils } from "../common/utils";
 
 export interface IGuideLayerProps extends IProps {
     /**
@@ -24,6 +25,16 @@ export interface IGuideLayerProps extends IProps {
 }
 
 export class GuideLayer extends React.Component<IGuideLayerProps, {}> {
+    public shouldComponentUpdate(nextProps: IGuideLayerProps) {
+        if (this.props.className !== nextProps.className) {
+            return true;
+        }
+        // shallow-comparing guide arrays leads to tons of unnecessary re-renders, so we check the
+        // array contents explicitly.
+        return !Utils.arraysEqual(this.props.verticalGuides, nextProps.verticalGuides)
+            || !Utils.arraysEqual(this.props.horizontalGuides, nextProps.horizontalGuides);
+    }
+
     public render() {
         const { verticalGuides, horizontalGuides, className } = this.props;
         const verticals = (verticalGuides == null) ? undefined : verticalGuides.map(this.renderVerticalGuide);

packages/table/src/layers/regions.tsx

@@ -7,10 +7,10 @@
 
 import { IProps } from "@blueprintjs/core";
 import * as classNames from "classnames";
-import * as PureRender from "pure-render-decorator";
 import * as React from "react";
 import * as Classes from "../common/classes";
-import { IRegion } from "../regions";
+import { Utils } from "../common/utils";
+import { IRegion, Regions } from "../regions";
 
 export type IRegionStyler = (region: IRegion) => React.CSSProperties;
 
@@ -21,13 +21,27 @@ export interface IRegionLayerProps extends IProps {
     regions?: IRegion[];
 
     /**
-     * A callback interface for applying CSS styles to the regions.
+     * The array of CSS styles to apply to each region. The ith style object in this array will be
+     * applied to the ith region in `regions`.
      */
-    getRegionStyle: IRegionStyler;
+    regionStyles?: React.CSSProperties[];
 }
 
-@PureRender
+// don't include "regions" or "regionStyles" in here, because they can't be shallowly compared
+const UPDATE_PROPS_KEYS = [
+    "className",
+];
+
 export class RegionLayer extends React.Component<IRegionLayerProps, {}> {
+    public shouldComponentUpdate(nextProps: IRegionLayerProps) {
+        // shallowly comparable props like "className" tend not to change in the default table
+        // implementation, so do that check last with hope that we return earlier and avoid it
+        // altogether.
+        return !Utils.arraysEqual(this.props.regions, nextProps.regions, Regions.regionsEqual)
+            || !Utils.arraysEqual(this.props.regionStyles, nextProps.regionStyles, Utils.shallowCompareKeys)
+            || !Utils.shallowCompareKeys(this.props, nextProps, UPDATE_PROPS_KEYS);
+    }
+
     public render() {
         return <div className={Classes.TABLE_OVERLAY_LAYER}>{this.renderRegionChildren()}</div>;
     }
@@ -40,13 +54,13 @@ export class RegionLayer extends React.Component<IRegionLayerProps, {}> {
         return regions.map(this.renderRegion);
     }
 
-    private renderRegion = (region: IRegion, index: number) => {
-        const { className, getRegionStyle } = this.props;
+    private renderRegion = (_region: IRegion, index: number) => {
+        const { className, regionStyles } = this.props;
         return (
             <div
                 className={classNames(Classes.TABLE_OVERLAY, Classes.TABLE_REGION, className)}
                 key={index}
-                style={getRegionStyle(region)}
+                style={regionStyles[index]}
             />
         );
     }

packages/table/src/regions.ts

@@ -528,6 +528,11 @@ export class Regions {
         return regionGroups;
     }
 
+    public static regionsEqual(regionA: IRegion, regionB: IRegion) {
+        return Regions.intervalsEqual(regionA.rows, regionB.rows)
+            && Regions.intervalsEqual(regionA.cols, regionB.cols);
+    }
+
     /**
      * Iterates over the cells within an `IRegion`, invoking the callback with
      * each cell's coordinates.
@@ -573,11 +578,6 @@ export class Regions {
         }
     }
 
-    private static regionsEqual(regionA: IRegion, regionB: IRegion) {
-        return Regions.intervalsEqual(regionA.rows, regionB.rows)
-            && Regions.intervalsEqual(regionA.cols, regionB.cols);
-    }
-
     private static regionContains(regionA: IRegion, regionB: IRegion) {
         // containsRegion expects an array of regions as the first param
         return Regions.overlapsRegion([regionA], regionB, false);