Merge pull request #1211 from MxFr/optimize-stringMap-calculation

Optimized stringMap calculation
FormidableLabs · Jan 7, 2019 · 214f72d · 214f72d
2 parents c03ae19 + a5b17f2
commit 214f72d
Show file tree

Hide file tree

Showing 6 changed files with 94 additions and 49 deletions.
diff --git a/packages/victory-chart/src/helper-methods.js b/packages/victory-chart/src/helper-methods.js
@@ -55,14 +55,10 @@ function getCalculatedProps(props, childComponents) {
   const style = getStyles(props);
   const horizontal = Helpers.isHorizontal(props);
   // TODO: check
-  const categories = {
-    x: Wrapper.getCategories(props, "x", childComponents),
-    y: Wrapper.getCategories(props, "y", childComponents)
-  };
-  const stringMap = {
-    x: createStringMap(props, "x", childComponents),
-    y: createStringMap(props, "y", childComponents)
-  };
+  const categories = Wrapper.getCategories(props, childComponents);
+
+  const stringMap = createStringMap(props, childComponents);
+
   const axisComponents = {
     x: Axis.getAxisComponent(childComponents, "x"),
     y: Axis.getAxisComponent(childComponents, "y")
@@ -246,14 +242,26 @@ const getAxisOffset = (props, calculatedProps) => {
   };
 };
 
-const createStringMap = (props, axis, childComponents) => {
-  const allStrings = Wrapper.getStringsFromChildren(props, axis, childComponents);
-  return allStrings.length === 0
-    ? null
-    : allStrings.reduce((memo, string, index) => {
-        memo[string] = index + 1;
-        return memo;
-      }, {});
+const createStringMap = (props, childComponents) => {
+  const allStrings = Wrapper.getStringsFromChildren(props, childComponents);
+
+  const x =
+    !allStrings.x || allStrings.x.length === 0
+      ? null
+      : allStrings.x.reduce((memo, string, index) => {
+          memo[string] = index + 1;
+          return memo;
+        }, {});
+
+  const y =
+    !allStrings.y || allStrings.y.length === 0
+      ? null
+      : allStrings.y.reduce((memo, string, index) => {
+          memo[string] = index + 1;
+          return memo;
+        }, {});
+
+  return { x, y };
 };
 
 export { getChildren, getCalculatedProps, getChildComponents };
diff --git a/packages/victory-core/src/victory-util/helpers.js b/packages/victory-core/src/victory-util/helpers.js
@@ -190,9 +190,19 @@ function getCurrentAxis(axis, horizontal) {
  * @param {Array} children: an array of child components
  * @param {Function} iteratee: a function with arguments "child", "childName", and "parent"
  * @param {Object} parentProps: props from the parent that are applied to children
+ * @param {any}  initialMemo: The object in which the iteration results are combined.
+ * @param {Function} combine: Combines the result of the iteratee with the current memo
+ *   to the memo for the next iteration step
  * @returns {Array} returns an array of results from calling the iteratee on all nested children
  */
-function reduceChildren(children, iteratee, parentProps = {}) {
+/* eslint-disable max-params */
+function reduceChildren(
+  children,
+  iteratee,
+  parentProps = {},
+  initialMemo = [],
+  combine = (memo, item) => memo.concat(item)
+) {
   const sharedProps = [
     "data",
     "domain",
@@ -218,13 +228,17 @@ function reduceChildren(children, iteratee, parentProps = {}) {
               });
         const childNames = nestedChildren.map((c, i) => `${childName}-${i}`);
         const nestedResults = traverseChildren(nestedChildren, childNames, child);
-        memo = memo.concat(nestedResults);
+        memo = combine(memo, nestedResults);
       } else {
         const result = iteratee(child, childName, parent);
-        memo = result ? memo.concat(result) : memo;
+        if (Array.isArray(result)) {
+          memo = result.reduce(combine, memo);
+        } else if (result) {
+          memo = combine(memo, result);
+        }
       }
       return memo;
-    }, []);
+    }, initialMemo);
   };
   const childNames = children.map((c, i) => i);
   return traverseChildren(children, childNames);

diff --git a/packages/victory-core/src/victory-util/wrapper.js b/packages/victory-core/src/victory-util/wrapper.js
@@ -275,15 +275,15 @@ export default {
         const categories =
           childProps.categories && !Array.isArray(childProps.categories)
             ? childProps.categories[axis]
-            : childProps.rops.categories;
+            : childProps.props.categories;
         const categoryStrings = categories && categories.filter((val) => typeof val === "string");
         return categoryStrings ? Collection.removeUndefined(categoryStrings) : [];
       }
     };
     return Helpers.reduceChildren(childComponents.slice(0), iteratee);
   },
 
-  getStringsFromData(childComponents, axis) {
+  getStringsFromData(childComponents) {
     const iteratee = (child) => {
       const childProps = child.props || {};
       let data;
@@ -294,29 +294,58 @@ export default {
       } else {
         data = Data.getData(childProps);
       }
-      const attr = axis === "x" ? "xName" : "yName";
-      return data.map((d) => d[attr]).filter(Boolean);
+      return data.map((d) => ({ x: d.xName, y: d.yName }));
     };
-    return Helpers.reduceChildren(childComponents.slice(0), iteratee);
+
+    const initialMemo = { x: [], y: [] };
+    const combine = (memo, datum) => ({
+      x: datum.x !== undefined ? memo.x.concat(datum.x) : memo.x,
+      y: datum.y !== undefined ? memo.y.concat(datum.y) : memo.y
+    });
+
+    return Helpers.reduceChildren(childComponents.slice(0), iteratee, {}, initialMemo, combine);
   },
 
-  getStringsFromChildren(props, axis, childComponents) {
+  getCategoryAndAxisStringsFromChildren(props, axis, childComponents) {
     const currentAxis = Helpers.getCurrentAxis(axis, props.horizontal);
-    childComponents = childComponents || React.Children.toArray(props.children);
     const categories = isPlainObject(props.categories) ? props.categories[axis] : props.categories;
     const axisComponent = Axis.getAxisComponent(childComponents, currentAxis);
     const axisStrings = axisComponent ? Data.getStringsFromAxes(axisComponent.props, axis) : [];
     const categoryStrings = categories || this.getStringsFromCategories(childComponents, axis);
-    const dataStrings = this.getStringsFromData(childComponents, axis);
-    return uniq(flatten([...categoryStrings, ...dataStrings, ...axisStrings]));
+    return uniq(flatten([...categoryStrings, ...axisStrings]));
+  },
+
+  getStringsFromChildren(props, childComponents) {
+    childComponents = childComponents || React.Children.toArray(props.children);
+
+    const xStrings = this.getCategoryAndAxisStringsFromChildren(props, "x", childComponents);
+    const yStrings = this.getCategoryAndAxisStringsFromChildren(props, "y", childComponents);
+
+    const dataStrings = this.getStringsFromData(childComponents);
+
+    return {
+      x: uniq(flatten([...xStrings, ...dataStrings.x])),
+      y: uniq(flatten([...yStrings, ...dataStrings.y]))
+    };
   },
 
-  getCategories(props, axis) {
-    const propCategories =
-      props.categories && !Array.isArray(props.categories)
-        ? props.categories[axis]
-        : props.categories;
-    const categories = propCategories || this.getStringsFromChildren(props, axis);
-    return categories.length > 0 ? categories : undefined;
+  getCategories(props) {
+    const xPropCategories =
+      props.categories && !Array.isArray(props.categories) ? props.categories.x : props.categories;
+
+    const yPropCategories =
+      props.categories && !Array.isArray(props.categories) ? props.categories.y : props.categories;
+
+    const fallbackRequired = !xPropCategories || !yPropCategories;
+
+    const fallbackProps = fallbackRequired ? this.getStringsFromChildren(props) : {};
+
+    const xCategories = xPropCategories || fallbackProps.x;
+    const yCategories = yPropCategories || fallbackProps.y;
+
+    return {
+      x: xCategories.length > 0 ? xCategories : undefined,
+      y: yCategories.length > 0 ? yCategories : undefined
+    };
   }
 };
diff --git a/packages/victory-group/src/helper-methods.js b/packages/victory-group/src/helper-methods.js
@@ -20,10 +20,7 @@ function getCalculatedProps(props, childComponents) {
   const horizontal =
     modifiedProps.horizontal ||
     childComponents.every((component) => component.props && component.props.horizontal);
-  const categories = {
-    x: Wrapper.getCategories(modifiedProps, "x"),
-    y: Wrapper.getCategories(modifiedProps, "y")
-  };
+  const categories = Wrapper.getCategories(modifiedProps, childComponents);
   const datasets = Wrapper.getDataFromChildren(modifiedProps);
   const domain = {
     x: Wrapper.getDomain(assign({}, modifiedProps, { categories }), "x", childComponents),

diff --git a/packages/victory-stack/src/helper-methods.js b/packages/victory-stack/src/helper-methods.js
@@ -112,10 +112,7 @@ function getCalculatedProps(props, childComponents) {
   const style = Wrapper.getStyle(props.theme, props.style, role);
   const horizontal =
     props.horizontal || childComponents.every((component) => component.props.horizontal);
-  const categories = {
-    x: Wrapper.getCategories(props, "x"),
-    y: Wrapper.getCategories(props, "y")
-  };
+  const categories = Wrapper.getCategories(props, childComponents);
   const datasets = stackData(props);
   const children = childComponents.map((c, i) => {
     return React.cloneElement(c, { data: datasets[i] });

diff --git a/test/client/spec/victory-core/victory-util/wrapper.spec.js b/test/client/spec/victory-core/victory-util/wrapper.spec.js
@@ -50,31 +50,31 @@ describe("helpers/wrapper", () => {
     it("returns an array of strings from a data prop", () => {
       const props = { data: [{ x: "one", y: 1 }, { x: "red", y: 2 }, { x: "cat", y: 3 }] };
       const childComponents = [getVictoryLine(props)];
-      const dataStrings = Wrapper.getStringsFromData(childComponents, "x");
+      const dataStrings = Wrapper.getStringsFromData(childComponents).x;
       expect(dataStrings).to.eql(["one", "red", "cat"]);
     });
 
     it("returns an array of strings from array-type data", () => {
       const props = { data: [["one", 1], ["red", 2], ["cat", 3]], x: 0, y: 1 };
       const childComponents = [getVictoryLine(props)];
-      const dataStrings = Wrapper.getStringsFromData(childComponents, "x");
+      const dataStrings = Wrapper.getStringsFromData(childComponents).x;
       expect(dataStrings).to.eql(["one", "red", "cat"]);
     });
 
     it("only returns strings, if data is mixed", () => {
       const props = { data: [{ x: 1, y: 1 }, { x: "three", y: 3 }] };
       const childComponents = [getVictoryLine(props)];
-      expect(Wrapper.getStringsFromData(childComponents, "x")).to.eql(["three"]);
+      expect(Wrapper.getStringsFromData(childComponents).x).to.eql(["three"]);
     });
 
     it("returns an empty array when no strings are present", () => {
       const props = { data: [{ x: 1, y: 1 }, { x: 3, y: 3 }] };
       const childComponents = [getVictoryLine(props)];
-      expect(Wrapper.getStringsFromData(childComponents, "x")).to.eql([]);
+      expect(Wrapper.getStringsFromData(childComponents).x).to.eql([]);
     });
 
     it("returns an empty array when no children are given", () => {
-      expect(Wrapper.getStringsFromData([], "x")).to.eql([]);
+      expect(Wrapper.getStringsFromData([])).to.eql({ x: [], y: [] });
     });
   });
 });