fix: Infinite loop with grid rendering (#1631)

- Caused by #1626
- The children on IrisGrid were getting re-rendered as a result of
IrisGrid.handleViewChanged getting called after updating the canvas in
Grid.componentDidUpdate
- Another fix would be to memoize metrics and not emit a view change if
they are exactly the same as previous metrics, but thought that was a
bigger change (need to deep equals a bunch of maps and arrays in the
metrics)
- Tested by opening up a table with React dev tools highlighting
re-renders. Table did not re-render when not being interacted with.

Author: mofojed

packages/grid/src/Grid.tsx

@@ -8,7 +8,7 @@ import React, {
 import classNames from 'classnames';
 import memoize from 'memoize-one';
 import clamp from 'lodash.clamp';
-import { assertNotNull, EMPTY_ARRAY } from '@deephaven/utils';
+import { assertNotNull, EMPTY_ARRAY, getChangedKeys } from '@deephaven/utils';
 import GridMetricCalculator, { GridMetricState } from './GridMetricCalculator';
 import GridModel from './GridModel';
 import GridMouseHandler, {
@@ -491,6 +491,17 @@ class Grid extends PureComponent<GridProps, GridState> {
   }
 
   componentDidUpdate(prevProps: GridProps, prevState: GridState): void {
+    const changedProps = getChangedKeys(prevProps, this.props);
+    const changedState = getChangedKeys(prevState, this.state);
+    // We don't need to bother re-checking any of the metrics if only the children have changed
+    if (
+      changedProps.length === 1 &&
+      changedProps[0] === 'children' &&
+      changedState.length === 0
+    ) {
+      return;
+    }
+
     const {
       isStickyBottom,
       isStickyRight,

packages/utils/src/ObjectUtils.tests.ts

@@ -0,0 +1,61 @@
+import { getChangedKeys } from './ObjectUtils';
+
+describe('getChangedKeys', () => {
+  it('should get changed keys', () => {
+    const oldObject = {
+      foo: 'bar',
+      baz: 'qux',
+      quux: 'corge',
+    };
+    const newObject = {
+      foo: 'bar',
+      baz: 'quux',
+      quux: 'corge',
+    };
+    expect(getChangedKeys(oldObject, newObject)).toEqual(['baz']);
+  });
+  it('should get changed keys when old object is empty', () => {
+    const oldObject = {};
+    const newObject = {
+      foo: 'bar',
+      baz: 'qux',
+      quux: 'corge',
+    };
+    expect(getChangedKeys(oldObject, newObject)).toEqual([
+      'foo',
+      'baz',
+      'quux',
+    ]);
+  });
+  it('should get changed keys when new object is empty', () => {
+    const oldObject = {
+      foo: 'bar',
+      baz: 'qux',
+      quux: 'corge',
+    };
+    const newObject = {};
+    expect(getChangedKeys(oldObject, newObject)).toEqual([
+      'foo',
+      'baz',
+      'quux',
+    ]);
+  });
+  it('should get no changed keys when both objects are empty', () => {
+    const oldObject = {};
+    const newObject = {};
+    expect(getChangedKeys(oldObject, newObject)).toEqual([]);
+  });
+  it('should get no changed keys when both objects are the same', () => {
+    const oldObject = {
+      foo: 'bar',
+      baz: 'qux',
+      quux: 'corge',
+    };
+    const newObject = {
+      foo: 'bar',
+      baz: 'qux',
+      quux: 'corge',
+    };
+    expect(getChangedKeys(oldObject, newObject)).toEqual([]);
+  });
+});

packages/utils/src/ObjectUtils.ts

@@ -0,0 +1,23 @@
+/**
+ * Get the keys that have changed between two objects.
+ * @param oldObject Old object to compare
+ * @param newObject New object to compare
+ * @returns Array of keys that have changed
+ */
+export function getChangedKeys(
+  oldObject: Record<string, unknown>,
+  newObject: Record<string, unknown>
+): string[] {
+  const keys = new Set([...Object.keys(oldObject), ...Object.keys(newObject)]);
+  const changedKeys: string[] = [];
+
+  keys.forEach(key => {
+    if (oldObject[key] !== newObject[key]) {
+      changedKeys.push(key);
+    }
+  });
+
+  return changedKeys;
+}
+
+export default { getChangedKeys };

packages/utils/src/index.ts

@@ -15,6 +15,7 @@ export { default as Pending } from './Pending';
 export * from './PromiseUtils';
 export * from './Asserts';
 export * from './ErrorUtils';
+export * from './ObjectUtils';
 export { default as RangeUtils, generateRange } from './RangeUtils';
 export type { Range } from './RangeUtils';
 export { default as TextUtils } from './TextUtils';