fixing settings hook and switching to memoize-one

elastic · Aug 30, 2019 · e7683aa · e7683aa
1 parent 9bfb1f8
commit e7683aa
Show file tree

Hide file tree

Showing 8 changed files with 42 additions and 34 deletions.
diff --git a/x-pack/legacy/plugins/ml/public/jobs/new_job_new/common/chart_loader/chart_loader.ts b/x-pack/legacy/plugins/ml/public/jobs/new_job_new/common/chart_loader/chart_loader.ts
@@ -4,7 +4,8 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { memoize as memoizeOrig } from 'lodash';
+import memoizeOne from 'memoize-one';
+import { isEqual } from 'lodash';
 import { IndexPattern } from 'ui/index_patterns';
 import { IndexPatternTitle } from '../../../../../common/types/kibana';
 import { Field, SplitField, AggFieldPair } from '../../../../../common/types/fields';
@@ -20,12 +21,12 @@ export interface LineChartPoint {
 type SplitFieldValue = string | null;
 export type LineChartData = Record<DetectorIndex, LineChartPoint[]>;
 
-const memoize = (f: Function) => memoizeOrig(f, (...args: any[]) => JSON.stringify(args));
+const eq = (newArgs: any[], lastArgs: any[]) => isEqual(newArgs, lastArgs);
 
-const newJobLineChart = memoize(ml.jobs.newJobLineChart);
-const newJobPopulationsChart = memoize(ml.jobs.newJobPopulationsChart);
-const getEventRateData = memoize(mlResultsService.getEventRateData);
-const getCategoryFields = memoize(getCategoryFieldsOrig);
+const newJobLineChart = memoizeOne(ml.jobs.newJobLineChart, eq);
+const newJobPopulationsChart = memoizeOne(ml.jobs.newJobPopulationsChart, eq);
+const getEventRateData = memoizeOne(mlResultsService.getEventRateData, eq);
+const getCategoryFields = memoizeOne(getCategoryFieldsOrig, eq);
 
 export class ChartLoader {
   private _indexPatternTitle: IndexPatternTitle = '';

diff --git a/x-pack/legacy/plugins/ml/public/jobs/new_job_new/pages/components/charts/common/settings.ts b/x-pack/legacy/plugins/ml/public/jobs/new_job_new/pages/components/charts/common/settings.ts
@@ -4,7 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { useContext, useEffect, useState } from 'react';
+import { useContext, useState } from 'react';
 import chrome from 'ui/chrome';
 import darkTheme from '@elastic/eui/dist/eui_theme_dark.json';
 import lightTheme from '@elastic/eui/dist/eui_theme_light.json';
@@ -56,27 +56,27 @@ export const seriesStyle = {
 };
 
 export function useChartSettings() {
-  const { jobCreator, jobCreatorUpdated, chartInterval } = useContext(JobCreatorContext);
+  const { jobCreator, chartInterval } = useContext(JobCreatorContext);
 
   const cs = {
     ...defaultChartSettings,
     intervalMs: chartInterval.getInterval().asMilliseconds(),
   };
 
-  if (isPopulationJobCreator(jobCreator)) {
-    // for population charts, use a larger interval based on
-    // the calculation from MlTimeBuckets, but without the
-    // bar target and max bars which have been set for the
-    // general chartInterval
-    const interval = new MlTimeBuckets();
-    interval.setInterval('auto');
-    interval.setBounds(chartInterval.getBounds());
-    cs.intervalMs = interval.getInterval().asMilliseconds();
-  }
-
   const [chartSettings, setChartSettings] = useState(cs);
 
-  useEffect(() => {
+  function getChartSettings(): ChartSettings {
+    if (isPopulationJobCreator(jobCreator)) {
+      // for population charts, use a larger interval based on
+      // the calculation from MlTimeBuckets, but without the
+      // bar target and max bars which have been set for the
+      // general chartInterval
+      const interval = new MlTimeBuckets();
+      interval.setInterval('auto');
+      interval.setBounds(chartInterval.getBounds());
+      cs.intervalMs = interval.getInterval().asMilliseconds();
+    }
+
     if (isMultiMetricJobCreator(jobCreator) || isPopulationJobCreator(jobCreator)) {
       if (jobCreator.aggFieldPairs.length > 2 && isMultiMetricJobCreator(jobCreator)) {
         cs.cols = 3;
@@ -89,7 +89,8 @@ export function useChartSettings() {
       }
     }
     setChartSettings(cs);
-  }, [jobCreatorUpdated]);
+    return cs;
+  }
 
-  return [chartSettings];
+  return { chartSettings, getChartSettings };
 }
diff --git a/...b_new/pages/components/pick_fields_step/components/multi_metric_view/metric_selection.tsx b/...b_new/pages/components/pick_fields_step/components/multi_metric_view/metric_selection.tsx
@@ -40,7 +40,7 @@ export const MultiMetricDetectors: FC<Props> = ({ setIsValid }) => {
   const [loadingData, setLoadingData] = useState(false);
   const [start, setStart] = useState(jobCreator.start);
   const [end, setEnd] = useState(jobCreator.end);
-  const [chartSettings] = useChartSettings();
+  const { chartSettings, getChartSettings } = useChartSettings();
   const [splitField, setSplitField] = useState(jobCreator.splitField);
   const [fieldValues, setFieldValues] = useState<string[]>([]);
   const [pageReady, setPageReady] = useState(false);
@@ -120,13 +120,14 @@ export const MultiMetricDetectors: FC<Props> = ({ setIsValid }) => {
     if (allDataReady()) {
       setLoadingData(true);
       try {
+        const cs = getChartSettings();
         const resp: LineChartData = await chartLoader.loadLineCharts(
           jobCreator.start,
           jobCreator.end,
           aggFieldPairList,
           jobCreator.splitField,
           fieldValues.length > 0 ? fieldValues[0] : null,
-          chartSettings.intervalMs
+          cs.intervalMs
         );
         setLineChartsData(resp);
       } catch (error) {

diff --git a/...ges/components/pick_fields_step/components/multi_metric_view/metric_selection_summary.tsx b/...ges/components/pick_fields_step/components/multi_metric_view/metric_selection_summary.tsx
@@ -26,7 +26,7 @@ export const MultiMetricDetectorsSummary: FC = () => {
   const [loadingData, setLoadingData] = useState(false);
   const [modelData, setModelData] = useState<Record<number, ModelItem[]>>([]);
   const [anomalyData, setAnomalyData] = useState<Record<number, Anomaly[]>>([]);
-  const [chartSettings] = useChartSettings();
+  const { chartSettings, getChartSettings } = useChartSettings();
   const [fieldValues, setFieldValues] = useState<string[]>([]);
 
   function setResultsWrapper(results: Results) {
@@ -64,13 +64,14 @@ export const MultiMetricDetectorsSummary: FC = () => {
     if (allDataReady()) {
       setLoadingData(true);
       try {
+        const cs = getChartSettings();
         const resp: LineChartData = await chartLoader.loadLineCharts(
           jobCreator.start,
           jobCreator.end,
           jobCreator.aggFieldPairs,
           jobCreator.splitField,
           fieldValues.length > 0 ? fieldValues[0] : null,
-          chartSettings.intervalMs
+          cs.intervalMs
         );
         setLineChartsData(resp);
       } catch (error) {

diff --git a/...job_new/pages/components/pick_fields_step/components/population_view/metric_selection.tsx b/...job_new/pages/components/pick_fields_step/components/population_view/metric_selection.tsx
@@ -44,7 +44,7 @@ export const PopulationDetectors: FC<Props> = ({ setIsValid }) => {
   const [loadingData, setLoadingData] = useState(false);
   const [start, setStart] = useState(jobCreator.start);
   const [end, setEnd] = useState(jobCreator.end);
-  const [chartSettings] = useChartSettings();
+  const { chartSettings, getChartSettings } = useChartSettings();
   const [splitField, setSplitField] = useState(jobCreator.splitField);
   const [fieldValuesPerDetector, setFieldValuesPerDetector] = useState<DetectorFieldValues>({});
   const [byFieldsUpdated, setByFieldsUpdated] = useReducer<(s: number) => number>(s => s + 1, 0);
@@ -137,12 +137,13 @@ export const PopulationDetectors: FC<Props> = ({ setIsValid }) => {
     if (allDataReady()) {
       setLoadingData(true);
       try {
+        const cs = getChartSettings();
         const resp: LineChartData = await chartLoader.loadPopulationCharts(
           jobCreator.start,
           jobCreator.end,
           aggFieldPairList,
           jobCreator.splitField,
-          chartSettings.intervalMs
+          cs.intervalMs
         );
 
         setLineChartsData(resp);

diff --git a/...pages/components/pick_fields_step/components/population_view/metric_selection_summary.tsx b/...pages/components/pick_fields_step/components/population_view/metric_selection_summary.tsx
@@ -34,7 +34,7 @@ export const PopulationDetectorsSummary: FC = () => {
   const [loadingData, setLoadingData] = useState(false);
   const [modelData, setModelData] = useState<Record<number, ModelItem[]>>([]);
   const [anomalyData, setAnomalyData] = useState<Record<number, Anomaly[]>>([]);
-  const [chartSettings] = useChartSettings();
+  const { chartSettings, getChartSettings } = useChartSettings();
   const [fieldValuesPerDetector, setFieldValuesPerDetector] = useState<DetectorFieldValues>({});
 
   function setResultsWrapper(results: Results) {
@@ -71,12 +71,13 @@ export const PopulationDetectorsSummary: FC = () => {
     if (allDataReady()) {
       setLoadingData(true);
       try {
+        const cs = getChartSettings();
         const resp: LineChartData = await chartLoader.loadPopulationCharts(
           jobCreator.start,
           jobCreator.end,
           aggFieldPairList,
           jobCreator.splitField,
-          chartSettings.intervalMs
+          cs.intervalMs
         );
 
         setLineChartsData(resp);

diff --git a/..._new/pages/components/pick_fields_step/components/single_metric_view/metric_selection.tsx b/..._new/pages/components/pick_fields_step/components/single_metric_view/metric_selection.tsx
@@ -43,7 +43,7 @@ export const SingleMetricDetectors: FC<Props> = ({ setIsValid }) => {
   const [loadingData, setLoadingData] = useState(false);
   const [start, setStart] = useState(jobCreator.start);
   const [end, setEnd] = useState(jobCreator.end);
-  const [chartSettings] = useChartSettings();
+  const { getChartSettings } = useChartSettings();
 
   function detectorChangeHandler(selectedOptionsIn: DropDownLabel[]) {
     setSelectedOptions(selectedOptionsIn);
@@ -78,13 +78,14 @@ export const SingleMetricDetectors: FC<Props> = ({ setIsValid }) => {
     if (aggFieldPair !== null) {
       setLoadingData(true);
       try {
+        const cs = getChartSettings();
         const resp: LineChartData = await chartLoader.loadLineCharts(
           jobCreator.start,
           jobCreator.end,
           [aggFieldPair],
           null,
           null,
-          chartSettings.intervalMs
+          cs.intervalMs
         );
         if (resp[DTR_IDX] !== undefined) {
           setLineChartData(resp);

diff --git a/...es/components/pick_fields_step/components/single_metric_view/metric_selection_summary.tsx b/...es/components/pick_fields_step/components/single_metric_view/metric_selection_summary.tsx
@@ -30,7 +30,7 @@ export const SingleMetricDetectorsSummary: FC = () => {
   const [loadingData, setLoadingData] = useState(false);
   const [modelData, setModelData] = useState<ModelItem[]>([]);
   const [anomalyData, setAnomalyData] = useState<Anomaly[]>([]);
-  const [chartSettings] = useChartSettings();
+  const { getChartSettings } = useChartSettings();
 
   function setResultsWrapper(results: Results) {
     const model = results.model[DTR_IDX];
@@ -56,13 +56,14 @@ export const SingleMetricDetectorsSummary: FC = () => {
     if (jobCreator.aggFieldPair !== null) {
       setLoadingData(true);
       try {
+        const cs = getChartSettings();
         const resp: LineChartData = await chartLoader.loadLineCharts(
           jobCreator.start,
           jobCreator.end,
           [jobCreator.aggFieldPair],
           null,
           null,
-          chartSettings.intervalMs
+          cs.intervalMs
         );
         if (resp[DTR_IDX] !== undefined) {
           setLineChartData(resp);