Fix row expansion erroring out in v2.0-beta

src/reducers/columnStateHelper.js

@@ -23,7 +23,6 @@ const DRAG_SCROLL_BUFFER = 100;
 
 /**
  * Initialize scrollX state
- * TODO (jordan) Audit this method for cases where deep values are not properly cloned
  *
  * @param {!Object} state
  * @param {!Object} props
@@ -53,7 +52,7 @@ function initialize(state, props, oldProps) {
       : isColumnResizing;
   columnResizingData = isColumnResizing ? columnResizingData : {};
 
-  return Object.assign({}, state, {
+  Object.assign(state, {
     columnResizingData,
     isColumnResizing,
     maxScrollX,

src/reducers/computeRenderedRows.js

@@ -34,20 +34,19 @@ import updateRowHeight from './updateRowHeight';
  * @return {!Object} The updated state object
  */
 export default function computeRenderedRows(state, scrollAnchor) {
-  const newState = Object.assign({}, state);
-  let rowRange = calculateRenderedRowRange(newState, scrollAnchor);
+  let rowRange = calculateRenderedRowRange(state, scrollAnchor);
 
-  const { rowSettings, scrollContentHeight } = newState;
+  const { rowSettings, scrollContentHeight } = state;
   const { rowsCount } = rowSettings;
-  const { bodyHeight } = tableHeightsSelector(newState);
+  const { bodyHeight } = tableHeightsSelector(state);
   const maxScrollY = scrollContentHeight - bodyHeight;
   let firstRowOffset;
 
   // NOTE (jordan) This handles #115 where resizing the viewport may
   // leave only a subset of rows shown, but no scrollbar to scroll up to the first rows.
   if (maxScrollY === 0) {
     if (rowRange.firstViewportIdx > 0) {
-      rowRange = calculateRenderedRowRange(newState, {
+      rowRange = calculateRenderedRowRange(state, {
         firstOffset: 0,
         lastIndex: rowsCount - 1,
       });
@@ -61,15 +60,15 @@ export default function computeRenderedRows(state, scrollAnchor) {
   const firstRowIndex = rowRange.firstViewportIdx;
   const endRowIndex = rowRange.endViewportIdx;
 
-  computeRenderedRowOffsets(newState, rowRange, state.scrolling);
+  computeRenderedRowOffsets(state, rowRange, state.scrolling);
 
   let scrollY = 0;
   if (rowsCount > 0) {
-    scrollY = newState.rowOffsets[rowRange.firstViewportIdx] - firstRowOffset;
+    scrollY = state.rowOffsets[rowRange.firstViewportIdx] - firstRowOffset;
   }
   scrollY = clamp(scrollY, 0, maxScrollY);
 
-  return Object.assign(newState, {
+  Object.assign(state, {
     firstRowIndex,
     firstRowOffset,
     endRowIndex,
@@ -85,9 +84,6 @@ export default function computeRenderedRows(state, scrollAnchor) {
  * We use the scrollAnchor to determine what either the first or last row
  * will be, as well as the offset.
  *
- * NOTE (jordan) This alters state so it shouldn't be called
- * without state having been cloned first.
- *
  * @param {!Object} state
  * @param {{
  *   firstIndex?: number,
@@ -212,9 +208,6 @@ function calculateRenderedRowRange(state, scrollAnchor) {
  * Walk the rows to render and compute the height offsets and
  * positions in the row buffer.
  *
- * NOTE (jordan) This alters state so it shouldn't be called
- * without state having been cloned first.
- *
  * @param {!Object} state
  * @param {{
  *   endBufferIdx: number,

src/reducers/index.js

@@ -108,78 +108,70 @@ const slice = createSlice({
     initialize(state, action) {
       const props = action.payload;
 
-      let newState = setStateFromProps(state, props);
-      newState = initializeRowHeightsAndOffsets(newState);
-      const scrollAnchor = getScrollAnchor(newState, props);
-      newState = computeRenderedRows(newState, scrollAnchor);
-      return columnStateHelper.initialize(newState, props, {});
+      setStateFromProps(state, props);
+      initializeRowHeightsAndOffsets(state);
+      const scrollAnchor = getScrollAnchor(state, props);
+      computeRenderedRows(state, scrollAnchor);
+      columnStateHelper.initialize(state, props, {});
     },
     propChange(state, action) {
       const { newProps, oldProps } = action.payload;
-      let newState = setStateFromProps(state, newProps);
+      const oldState = _.clone(state);
+      setStateFromProps(state, newProps);
 
       if (
         oldProps.rowsCount !== newProps.rowsCount ||
         oldProps.rowHeight !== newProps.rowHeight ||
         oldProps.subRowHeight !== newProps.subRowHeight
       ) {
-        newState = initializeRowHeightsAndOffsets(newState);
+        initializeRowHeightsAndOffsets(state);
       }
 
       if (oldProps.rowsCount !== newProps.rowsCount) {
-        // NOTE (jordan) bad practice to modify state directly, but okay since
-        // we know setStateFromProps clones state internally
-        newState.rowBufferSet = new IntegerBufferSet();
+        state.rowBufferSet = new IntegerBufferSet();
       }
 
-      const scrollAnchor = getScrollAnchor(newState, newProps, oldProps);
+      const scrollAnchor = getScrollAnchor(state, newProps, oldProps);
 
       // If anything has changed in state, update our rendered rows
-      if (!shallowEqual(state, newState) || scrollAnchor.changed) {
-        newState = computeRenderedRows(newState, scrollAnchor);
+      if (!shallowEqual(state, oldState) || scrollAnchor.changed) {
+        computeRenderedRows(state, scrollAnchor);
       }
 
-      newState = columnStateHelper.initialize(newState, newProps, oldProps);
+      columnStateHelper.initialize(state, newProps, oldProps);
 
       // if scroll values have changed, then we're scrolling!
       if (
-        newState.scrollX !== state.scrollX ||
-        newState.scrollY !== state.scrollY
+        state.scrollX !== oldState.scrollX ||
+        state.scrollY !== oldState.scrollY
       ) {
-        newState.scrolling = newState.scrolling || true;
+        state.scrolling = state.scrolling || true;
       }
 
       // TODO REDUX_MIGRATION solve w/ evil-diff
       // TODO (jordan) check if relevant props unchanged and
       // children column widths and flex widths are unchanged
       // alternatively shallow diff and reconcile props
-      return newState;
     },
     scrollEnd(state) {
-      const newState = Object.assign({}, state, {
-        scrolling: false,
-      });
+      state.scrolling = false;
       const previousScrollAnchor = {
         firstIndex: state.firstRowIndex,
         firstOffset: state.firstRowOffset,
         lastIndex: state.lastIndex,
       };
-      return computeRenderedRows(newState, previousScrollAnchor);
+      computeRenderedRows(state, previousScrollAnchor);
     },
     scrollToY(state, action) {
-      let scrollY = action.payload;
-      const newState = Object.assign({}, state, {
-        scrolling: true,
-      });
-      const scrollAnchor = scrollTo(newState, scrollY);
-      return computeRenderedRows(newState, scrollAnchor);
+      const scrollY = action.payload;
+      state.scrolling = true;
+      const scrollAnchor = scrollTo(state, scrollY);
+      computeRenderedRows(state, scrollAnchor);
     },
     scrollToX(state, action) {
       const scrollX = action.payload;
-      return Object.assign({}, state, {
-        scrolling: true,
-        scrollX,
-      });
+      state.scrolling = true;
+      state.scrollX = scrollX;
     },
   },
 });
@@ -202,7 +194,7 @@ function initializeRowHeightsAndOffsets(state) {
   for (let idx = 0; idx < rowsCount; idx++) {
     storedHeights[idx] = defaultFullRowHeight;
   }
-  return Object.assign({}, state, {
+  Object.assign(state, {
     rowOffsetIntervalTree,
     scrollContentHeight,
     storedHeights,
@@ -223,15 +215,15 @@ function setStateFromProps(state, props) {
     useGroupHeader,
   } = convertColumnElementsToData(props.children);
 
-  const newState = Object.assign({}, state, {
+  Object.assign(state, {
     columnGroupProps,
     columnProps,
     elementTemplates,
   });
 
-  newState.elementHeights = Object.assign(
+  state.elementHeights = Object.assign(
     {},
-    newState.elementHeights,
+    state.elementHeights,
     pick(props, [
       'cellGroupWrapperHeight',
       'footerHeight',
@@ -240,37 +232,36 @@ function setStateFromProps(state, props) {
     ])
   );
   if (!useGroupHeader) {
-    newState.elementHeights.groupHeaderHeight = 0;
+    state.elementHeights.groupHeaderHeight = 0;
   }
 
-  newState.rowSettings = Object.assign(
+  state.rowSettings = Object.assign(
     {},
-    newState.rowSettings,
+    state.rowSettings,
     pick(props, ['bufferRowCount', 'rowHeight', 'rowsCount', 'subRowHeight'])
   );
-  const { rowHeight, subRowHeight } = newState.rowSettings;
-  newState.rowSettings.rowHeightGetter =
+  const { rowHeight, subRowHeight } = state.rowSettings;
+  state.rowSettings.rowHeightGetter =
     props.rowHeightGetter || (() => rowHeight);
-  newState.rowSettings.subRowHeightGetter =
+  state.rowSettings.subRowHeightGetter =
     props.subRowHeightGetter || (() => subRowHeight || 0);
-  newState.rowSettings.rowAttributesGetter = props.rowAttributesGetter;
+  state.rowSettings.rowAttributesGetter = props.rowAttributesGetter;
 
-  newState.scrollFlags = Object.assign(
+  state.scrollFlags = Object.assign(
     {},
-    newState.scrollFlags,
+    state.scrollFlags,
     pick(props, ['overflowX', 'overflowY', 'showScrollbarX', 'showScrollbarY'])
   );
 
-  newState.tableSize = Object.assign(
+  state.tableSize = Object.assign(
     {},
-    newState.tableSize,
+    state.tableSize,
     pick(props, ['height', 'maxHeight', 'ownerHeight', 'width'])
   );
-  newState.tableSize.useMaxHeight = newState.tableSize.height === undefined;
+  state.tableSize.useMaxHeight = state.tableSize.height === undefined;
 
-  newState.scrollbarXHeight = props.scrollbarXHeight;
-  newState.scrollbarYWidth = props.scrollbarYWidth;
-  return newState;
+  state.scrollbarXHeight = props.scrollbarXHeight;
+  state.scrollbarYWidth = props.scrollbarYWidth;
 }
 
 const { reducer, actions } = slice;

src/reducers/updateRowHeight.js

@@ -15,9 +15,6 @@
  * Update our cached row height for a specific index
  * based on the value from rowHeightGetter
  *
- * NOTE (jordan) This alters state so it shouldn't be called
- * without state having been cloned first.
- *
  * @param {!Object} state
  * @param {number} rowIdx
  * @return {number} The new row height