refactor: use better typing and interface for Series

Author: nima-taheri-mongodb

charts/core/src/Echart/Echart.types.ts

@@ -8,16 +8,45 @@ import type {
   TooltipComponentOption,
 } from 'echarts/components';
 import type { ComposeOption, EChartsType } from 'echarts/core';
-import type { LineSeriesOption, SeriesOption } from 'echarts/types/dist/shared';
+import {
+  BarSeriesOption,
+  LineSeriesOption,
+  SeriesOption,
+} from 'echarts/types/dist/shared';
 
-import { Theme } from '@leafygreen-ui/lib';
+import { Theme, ValuesOf } from '@leafygreen-ui/lib';
 
 // Type not exported by echarts.
 // reference: https://github.com/apache/echarts/blob/master/src/coord/axisCommonTypes.ts#L193
 export type AxisLabelValueFormatter = (value: number, index?: number) => string;
 
-export type EChartLineSeriesOption = LineSeriesOption;
-export type EChartSeriesOption = { name: string } & SeriesOption;
+export interface StylingContext {
+  seriesColor?: string;
+}
+
+// to convert an SeriesOption type of echarts into a more structured form aligned with LeafyGreen design standards,
+// where the 'type', 'name', and 'data' fields are explicitly required and typed,
+// and all additional properties related to series styling are encapsulated within the 'stylingOptions' object
+interface DisciplinedSeriesOption<EChartType extends SeriesOption> {
+  type: NonNullable<EChartType['type']>;
+  name: string;
+  data: NonNullable<EChartType['data']>;
+  stylingOptions: Omit<EChartType, 'type' | 'name' | 'data'>;
+}
+
+// all supported series options types disciplined and grouped into a single interface
+export interface EChartSeriesOptions {
+  line: DisciplinedSeriesOption<LineSeriesOption>;
+  // TODO: to be leveraged in a follow-up PR that adds Bar chart support
+  bar: DisciplinedSeriesOption<BarSeriesOption>;
+}
+
+// a disciplined substitute for SeriesOption type of echarts limited to the ones supported here
+export type EChartSeriesOption = Omit<
+  ValuesOf<EChartSeriesOptions>,
+  'stylingOptions'
+> &
+  ValuesOf<EChartSeriesOptions>['stylingOptions'];
 
 /**
  * TODO: This might need to be improved. `ComposeOption` appears to make most base option
@@ -107,6 +136,7 @@ interface EChartsEventHandlerType {
     callback: (params: any) => void,
     options?: Partial<{ useCanvasAsTrigger: boolean }>,
   ): void;
+
   (
     event: 'zoomselect',
     callback: (params: EChartZoomSelectionEvent) => void,

charts/core/src/Echart/utils/updateUtils.spec.ts

@@ -1,33 +1,44 @@
-import { EChartOptions } from '../Echart.types';
+import { EChartOptions, EChartSeriesOption } from '../Echart.types';
 
 import { addSeries, removeSeries, updateOptions } from './updateUtils';
 
 describe('@lg-charts/core/Chart/hooks/updateUtils', () => {
   test('addSeries should add a series to the chart options', () => {
     const currentOptions: Partial<EChartOptions> = {
-      series: [{ name: 'series1' }],
+      series: [{ type: 'line', name: 'series1', data: [] }],
     };
     const newSeriesName = 'series2';
-    const data = { name: newSeriesName };
+    const data: EChartSeriesOption = {
+      type: 'line',
+      name: newSeriesName,
+      data: [],
+    };
     const updatedOptions = addSeries(currentOptions, data);
     expect(updatedOptions.series).toHaveLength(2);
     expect(updatedOptions.series?.[1].name).toBe(newSeriesName);
   });
 
   test('addSeries should not add a series if a chart with the same name exists', () => {
     const currentOptions: Partial<EChartOptions> = {
-      series: [{ name: 'series1' }],
+      series: [{ type: 'line', name: 'series1', data: [] }],
     };
     const newSeriesName = 'series1';
-    const data = { name: newSeriesName };
+    const data: EChartSeriesOption = {
+      type: 'line',
+      name: newSeriesName,
+      data: [],
+    };
     const updatedOptions = addSeries(currentOptions, data);
     expect(updatedOptions.series).toHaveLength(1);
     expect(updatedOptions.series?.[0].name).toBe(newSeriesName);
   });
 
   test('removeSeries should remove a series from the chart options', () => {
     const currentOptions: Partial<EChartOptions> = {
-      series: [{ name: 'series1' }, { name: 'series2' }],
+      series: [
+        { type: 'line', name: 'series1', data: [] },
+        { type: 'line', name: 'series2', data: [] },
+      ],
     };
     const seriesName1 = 'series1';
     const seriesName2 = 'series2';

charts/core/src/EventMarkers/BaseEventMarker/utils.ts

@@ -109,6 +109,7 @@ export function getMarkConfig({
         symbolSize: [16, 16],
         symbolRotate: 360, // Icon shows upside down without this
       },
+      data: [],
     } as SeriesOption;
   } else {
     return {
@@ -130,6 +131,7 @@ export function getMarkConfig({
         symbolSize: [16, 16],
         symbol: generateSymbolDataUri(level, theme),
       },
+      data: [],
     } as SeriesOption;
   }
 }

charts/core/src/Series/Line/Line.tsx

@@ -1,52 +1,44 @@
 import React from 'react';
-import { useSeriesContext } from '@lg-charts/series-provider';
-import _ from 'lodash';
 
-import { useDarkMode } from '@leafygreen-ui/leafygreen-provider';
-
-import { EChartSeriesOption } from '../../Echart';
-import { EChartLineSeriesOption } from '../../Echart/Echart.types';
+import { EChartSeriesOptions, StylingContext } from '../../Echart/Echart.types';
 import { Series } from '../Series';
 import { SeriesProps } from '../Series.types';
 
 export type LineProps = SeriesProps;
 
-const defaultLineOptions: EChartLineSeriesOption = {
-  type: 'line',
-  showSymbol: false,
-  symbol: 'circle',
-  clip: false,
-  symbolSize: 7,
-  emphasis: {
-    focus: 'series',
-  },
-  blur: {
+function getDefaultLineOptions(
+  stylingContext: StylingContext,
+): EChartSeriesOptions['line']['stylingOptions'] {
+  return {
+    showSymbol: false,
+    symbol: 'circle',
+    clip: false,
+    symbolSize: 7,
+    emphasis: {
+      focus: 'series',
+    },
+    blur: {
+      lineStyle: {
+        opacity: 0.5,
+      },
+    },
+    itemStyle: {
+      color: stylingContext.seriesColor,
+    },
     lineStyle: {
-      opacity: 0.5,
+      color: stylingContext.seriesColor,
+      width: 1,
     },
-  },
-  lineStyle: {
-    width: 1,
-  },
-};
-
-export function Line(props: LineProps) {
-  const { getColor } = useSeriesContext();
-  const { theme } = useDarkMode();
-  const color = getColor(props.name, theme);
-  const colorOverrides: EChartLineSeriesOption = {
-    lineStyle: { color: color || undefined },
-    itemStyle: { color: color || undefined },
   };
-
-  const options: EChartSeriesOption = _.merge(
-    {},
-    defaultLineOptions,
-    props,
-    colorOverrides,
-  );
-
-  return <Series {...options} />;
 }
 
+export const Line = (props: LineProps) => (
+  <Series
+    type={'line'}
+    name={props.name}
+    data={props.data}
+    stylingOptions={getDefaultLineOptions}
+  />
+);
+
 Line.displayName = 'Line';

charts/core/src/Series/Series.tsx

@@ -1,34 +1,64 @@
 import { useEffect } from 'react';
 import { useSeriesContext } from '@lg-charts/series-provider';
 
+import { useDarkMode } from '@leafygreen-ui/leafygreen-provider';
+import { ValuesOf } from '@leafygreen-ui/lib';
+
 import { useChartContext } from '../ChartContext';
-import { EChartSeriesOption } from '../Echart';
+import { EChartSeriesOptions, StylingContext } from '../Echart/Echart.types';
 
-export function Series(options: EChartSeriesOption) {
+export function Series<T extends ValuesOf<EChartSeriesOptions>>({
+  type,
+  name,
+  data,
+  stylingOptions,
+}: {
+  type: T['type'];
+  name: T['name'];
+  data: T['data'];
+  stylingOptions: (ctx: StylingContext) => T['stylingOptions'];
+}) {
   const {
     chart: { addSeries, ready, removeSeries },
   } = useChartContext();
-  const { isChecked } = useSeriesContext();
-
-  const isVisible = isChecked(options.name);
+  const { theme } = useDarkMode();
+  const { isChecked, getColor } = useSeriesContext();
+  const seriesColor = getColor(name, theme) || undefined;
+  const isVisible = isChecked(name);
 
   useEffect(() => {
     if (!ready) return;
 
     if (isVisible) {
-      addSeries(options);
+      const context = { seriesColor };
+      addSeries({
+        type,
+        name,
+        data,
+        ...stylingOptions(context),
+      });
     } else {
-      removeSeries(options.name);
+      removeSeries(name);
     }
 
     return () => {
       /**
        * Remove the series when the component unmounts to make sure the series
        * is removed when a `Series` is hidden.
        */
-      removeSeries(options.name);
+      removeSeries(name);
     };
-  }, [addSeries, isVisible, ready, removeSeries]);
+  }, [
+    addSeries,
+    isVisible,
+    seriesColor,
+    ready,
+    removeSeries,
+    type,
+    name,
+    data,
+    stylingOptions,
+  ]);
 
   return null;
 }

charts/core/src/ThresholdLine/ThresholdLine.tsx

@@ -88,6 +88,7 @@ function getThresholdLineConfig({
       symbolSize: [7, 10],
       symbolRotate: 360,
     },
+    data: [],
   };
 }