Efficient prop updates for resizable flex higher-order component

Ensures we don’t propagate prop updates from the resizable flex HOC unless something relevant has actually changed. The main functions in the `resizableFlex` module are independent of the current state, so we should just define them once and then merge them into the final props returned by the component. The props-returning function should return the same instance as previously if the props themselves are unchanged. To do this, we want to memoize the function so that new props are only returned if: * The flex list (received from Redux) has changed * The `ownProps` (passed directly to the component) have changed In the latter case, we can’t rely on `ownProps` to be strictly equal, however; instead, we need to create a custom memoized function that does a shallow value check (the same kind of check used for pure `react-redux` containers)
popcodeorg · Jun 5, 2018 · f61b9fa · f61b9fa
1 parent ca74b53
commit f61b9fa
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 59 deletions.
diff --git a/package.json b/package.json
@@ -215,6 +215,7 @@
     "remark-react-lowlight": "^0.7.0",
     "reselect": "^3.0.1",
     "scriptjs": "^2.5.8",
+    "shallowequal": "^1.0.2",
     "slowparse": "^1.1.4",
     "strip-markdown": "^3.0.0",
     "stylelint": "^9.2.0",

diff --git a/src/util/resizableFlex.js b/src/util/resizableFlex.js
@@ -4,7 +4,9 @@ import findIndex from 'lodash-es/findIndex';
 import isNull from 'lodash-es/isNull';
 import map from 'lodash-es/map';
 import merge from 'lodash-es/merge';
+import shallowequal from 'shallowequal';
 import times from 'lodash-es/times';
+import {createSelector, defaultMemoize} from 'reselect';
 
 import {makeGetResizableFlexGrow} from '../selectors';
 import {updateResizableFlex} from '../actions';
@@ -80,67 +82,76 @@ export default function resizableFlex(size) {
       const regions = times(size, () => ({current: null}));
       const initialMainSizes = times(size, () => null);
 
-      return (state, ownProps) => merge(
-        {
-          onResizableFlexDividerDrag(beforeIndex, event, payload) {
-            const afterIndex = findIndex(regions, 'current', beforeIndex + 1);
-            const [{current: before}, {current: after}] =
-              at(regions, [beforeIndex, afterIndex]);
-
-            const {getCurrentSize, getDesiredSize} =
-              directionAdapterFor(before);
-
-            const [desiredBeforeFlexGrow, desiredAfterFlexGrow] =
-              calculateFlexGrowAfterDrag(
-                {
-                  currentFlexGrow: Number(
-                    getComputedStyle(before)['flex-grow'],
-                  ),
-                  currentSize: getCurrentSize(before),
-                  desiredSize: getDesiredSize(before, payload),
-                  initialMainSize: initialMainSizes[beforeIndex],
-                },
-                {
-                  currentFlexGrow: Number(
-                    getComputedStyle(after)['flex-grow'],
-                  ),
-                  currentSize: getCurrentSize(after),
-                  initialMainSize: initialMainSizes[afterIndex],
-                },
-              );
-
-            dispatch(updateResizableFlex(
-              instanceId,
-              [
-                {index: beforeIndex, flexGrow: desiredBeforeFlexGrow},
-                {index: afterIndex, flexGrow: desiredAfterFlexGrow},
-              ],
-            ));
-          },
-
-          resizableFlexRefs: map(
-            regions,
-            (region, index) => (element) => {
-              region.current = element;
-              if (isNull(element)) {
-                initialMainSizes[index] = null;
-                return;
-              }
-
-              const flexGrowWas = element.style.flexGrow;
-              const flexShrinkWas = element.style.flexShrink;
-              element.style.flexGrow = element.style.flexShrink = '0';
-              initialMainSizes[index] = directionAdapterFor(element).
-                getCurrentSize(element);
-              element.style.flexGrow = flexGrowWas;
-              element.style.flexShrink = flexShrinkWas;
-            },
-          ),
-
-          resizableFlexGrow: getResizableFlexGrow(state),
+      const stateIndependentFunctions = {
+        onResizableFlexDividerDrag(beforeIndex, event, payload) {
+          const afterIndex = findIndex(regions, 'current', beforeIndex + 1);
+          const [{current: before}, {current: after}] =
+            at(regions, [beforeIndex, afterIndex]);
+
+          const {getCurrentSize, getDesiredSize} =
+            directionAdapterFor(before);
+
+          const [desiredBeforeFlexGrow, desiredAfterFlexGrow] =
+            calculateFlexGrowAfterDrag(
+              {
+                currentFlexGrow: Number(
+                  getComputedStyle(before)['flex-grow'],
+                ),
+                currentSize: getCurrentSize(before),
+                desiredSize: getDesiredSize(before, payload),
+                initialMainSize: initialMainSizes[beforeIndex],
+              },
+              {
+                currentFlexGrow: Number(
+                  getComputedStyle(after)['flex-grow'],
+                ),
+                currentSize: getCurrentSize(after),
+                initialMainSize: initialMainSizes[afterIndex],
+              },
+            );
+
+          dispatch(updateResizableFlex(
+            instanceId,
+            [
+              {index: beforeIndex, flexGrow: desiredBeforeFlexGrow},
+              {index: afterIndex, flexGrow: desiredAfterFlexGrow},
+            ],
+          ));
         },
 
-        ownProps,
+        resizableFlexRefs: map(
+          regions,
+          (region, index) => (element) => {
+            region.current = element;
+            if (isNull(element)) {
+              initialMainSizes[index] = null;
+              return;
+            }
+
+            const flexGrowWas = element.style.flexGrow;
+            const flexShrinkWas = element.style.flexShrink;
+            element.style.flexGrow = element.style.flexShrink = '0';
+            initialMainSizes[index] = directionAdapterFor(element).
+              getCurrentSize(element);
+            element.style.flexGrow = flexGrowWas;
+            element.style.flexShrink = flexShrinkWas;
+          },
+        ),
+      };
+
+      return createSelector(
+        [
+          getResizableFlexGrow,
+          defaultMemoize(
+            (_state, ownProps) => ownProps,
+            shallowequal,
+          ),
+        ],
+        (resizableFlexGrow, ownProps) => merge(
+          {resizableFlexGrow},
+          ownProps,
+          stateIndependentFunctions,
+        ),
       );
     },
     {

diff --git a/yarn.lock b/yarn.lock
@@ -9317,6 +9317,10 @@ shallow-copy@~0.0.1:
   version "0.0.1"
   resolved "https://registry.yarnpkg.com/shallow-copy/-/shallow-copy-0.0.1.tgz#415f42702d73d810330292cc5ee86eae1a11a170"
 
+shallowequal@^1.0.2:
+  version "1.0.2"
+  resolved "https://registry.yarnpkg.com/shallowequal/-/shallowequal-1.0.2.tgz#1561dbdefb8c01408100319085764da3fcf83f8f"
+
 shebang-command@^1.2.0:
   version "1.2.0"
   resolved "https://registry.yarnpkg.com/shebang-command/-/shebang-command-1.2.0.tgz#44aac65b695b03398968c39f363fee5deafdf1ea"