LG-4662: Fix bundle size + other charts improvements

.changeset/stupid-ways-collect.md

@@ -0,0 +1,6 @@
+---
+'@lg-charts/core': patch
+---
+
+- Adds treeshaking of echarts to minimize bundle size.
+- Improves echart initialization logic to better handle race conditions.

charts/core/src/Chart.stories.tsx

@@ -442,12 +442,22 @@ export const WithSameGroupIds: StoryObj<StorybookProps> = {
     renderHeader,
     headerTitle,
     headerShowDivider,
+    zoomSelectXAxis,
+    zoomSelectYAxis,
+    zoomSelectCallback,
   }) => {
     return (
       <div
         style={{ display: 'grid', gridTemplateColumns: '1fr', width: '100%' }}
       >
-        <Chart groupId="group1">
+        <Chart
+          groupId="group1"
+          zoomSelect={{
+            xAxis: zoomSelectXAxis,
+            yAxis: zoomSelectYAxis,
+          }}
+          onZoomSelect={zoomSelectCallback}
+        >
           {renderHeader && (
             <Header title={headerTitle} showDivider={headerShowDivider} />
           )}
@@ -482,7 +492,14 @@ export const WithSameGroupIds: StoryObj<StorybookProps> = {
             <Line name={name} data={data} key={name} />
           ))}
         </Chart>
-        <Chart groupId="group1">
+        <Chart
+          groupId="group1"
+          zoomSelect={{
+            xAxis: zoomSelectXAxis,
+            yAxis: zoomSelectYAxis,
+          }}
+          onZoomSelect={zoomSelectCallback}
+        >
           {renderHeader && (
             <Header title={headerTitle} showDivider={headerShowDivider} />
           )}

charts/core/src/Chart/Chart.spec.tsx

@@ -4,18 +4,21 @@ import { render, screen } from '@testing-library/react';
 import { ChartProvider } from '../ChartContext';
 
 import { Chart } from './Chart';
+import { useChart } from './hooks';
 
 jest.mock('../ChartContext', () => ({
   ChartProvider: jest.fn(({ children }) => <div>{children}</div>),
 }));
 
+const mockChartInstance = {
+  chartOptions: {},
+  updateChartOptions: jest.fn(),
+  addChartSeries: jest.fn(),
+  removeChartSeries: jest.fn(),
+};
+
 jest.mock('./hooks', () => ({
-  useChart: jest.fn(() => ({
-    chartOptions: {},
-    updateChartOptions: jest.fn(),
-    addChartSeries: jest.fn(),
-    removeChartSeries: jest.fn(),
-  })),
+  useChart: jest.fn(() => mockChartInstance),
 }));
 
 /**
@@ -24,23 +27,29 @@ jest.mock('./hooks', () => ({
  * Chromatic tests for rendering logic.
  */
 describe('lg-charts/core/Chart', () => {
-  it('renders the echart container', () => {
+  beforeEach(() => {
+    jest.clearAllMocks();
+  });
+
+  test('renders the echart container', () => {
     render(<Chart />);
     expect(
       screen.getByTestId('lg-charts-core-chart-echart'),
     ).toBeInTheDocument();
   });
 
-  it('passes the correct props to ChartProvider', () => {
+  test('passes the chart instance to ChartProvider', () => {
     render(<Chart />);
+
+    // Verify useChart was called
+    expect(useChart).toHaveBeenCalled();
+
+    // Verify ChartProvider was called with the correct chart instance
     expect(ChartProvider).toHaveBeenCalledWith(
       expect.objectContaining({
-        chartOptions: {},
-        updateChartOptions: expect.any(Function),
-        addChartSeries: expect.any(Function),
-        removeChartSeries: expect.any(Function),
+        chart: mockChartInstance,
       }),
-      expect.anything(),
+      expect.any(Object),
     );
   });
 });

charts/core/src/Chart/Chart.tsx

@@ -29,13 +29,7 @@ export function Chart({
   ...rest
 }: ChartProps) {
   const { theme } = useDarkMode(darkModeProp);
-  const {
-    chartOptions,
-    updateChartOptions,
-    addChartSeries,
-    removeChartSeries,
-    chartRef,
-  } = useChart({
+  const chart = useChart({
     theme,
     onChartReady,
     zoomSelect,
@@ -45,12 +39,7 @@ export function Chart({
 
   return (
     <LeafyGreenProvider darkMode={darkModeProp}>
-      <ChartProvider
-        chartOptions={chartOptions}
-        updateChartOptions={updateChartOptions}
-        addChartSeries={addChartSeries}
-        removeChartSeries={removeChartSeries}
-      >
+      <ChartProvider chart={chart}>
         <div className={cx(chartContainerStyles, className)}>
           <div>
             {/**
@@ -62,7 +51,7 @@ export function Chart({
             {children}
           </div>
           <div
-            ref={chartRef}
+            ref={chart.ref}
             className={chartStyles}
             data-testid="lg-charts-core-chart-echart"
             {...rest}

charts/core/src/Chart/Chart.types.ts

@@ -1,40 +1,18 @@
 import { PropsWithChildren } from 'react';
-import type { XAXisComponentOption, YAXisComponentOption } from 'echarts';
-import type { LineSeriesOption } from 'echarts/charts';
-import type {
-  DatasetComponentOption,
-  GridComponentOption,
-  LegendComponentOption,
-  TitleComponentOption,
-  ToolboxComponentOption,
-  TooltipComponentOption,
-} from 'echarts/components';
-import type { ComposeOption } from 'echarts/core';
 
 import { DarkModeProps, type HTMLElementProps } from '@leafygreen-ui/lib';
 
-import { ZoomSelect, ZoomSelectionEvent } from './hooks/useChart.types';
+import {
+  EChartOptions,
+  EChartSeriesOption,
+  EChartZoomSelectionEvent,
+} from '../Echart';
 
-type RequiredSeriesProps = 'type' | 'name' | 'data';
-export type SeriesOption = Pick<LineSeriesOption, RequiredSeriesProps> &
-  Partial<Omit<LineSeriesOption, RequiredSeriesProps>>;
+import { ZoomSelect } from './hooks/useChart.types';
 
-/**
- * TODO: This might need to be improved. `ComposeOption` appears to make most base option
- * keys "Arrayable". This is making it difficult to properly test partial options on
- * methods like updateUtils > updateOptions(), since something like `options.grid` could be
- * an array even if an object.
- */
-export type ChartOptions = ComposeOption<
-  | TooltipComponentOption
-  | GridComponentOption
-  | DatasetComponentOption
-  | TitleComponentOption
-  | LegendComponentOption
-  | ToolboxComponentOption
-  | XAXisComponentOption
-  | YAXisComponentOption
-> & { series?: Array<SeriesOption> };
+export type SeriesOption = EChartSeriesOption;
+export type ChartOptions = EChartOptions;
+type ZoomSelectionEvent = EChartZoomSelectionEvent;
 
 export type ChartProps = HTMLElementProps<'div'> &
   DarkModeProps &
@@ -59,9 +37,3 @@ export type ChartProps = HTMLElementProps<'div'> &
      */
     groupId?: string;
   }>;
-
-export const ChartActionType = {
-  addChartSeries: 'addChartSeries',
-  removeChartSeries: 'removeChartSeries',
-  updateOptions: 'updateOptions',
-} as const;

charts/core/src/Chart/hooks/useChart.spec.ts

@@ -1,81 +1,129 @@
 import { act } from 'react-dom/test-utils';
 
-import { renderHook, waitForState } from '@leafygreen-ui/testing-lib';
-
-import { SeriesOption } from '../Chart.types';
+import { renderHook } from '@leafygreen-ui/testing-lib';
 
 import { useChart } from './useChart';
 
-jest.mock('echarts', () => ({
-  init: jest.fn(() => ({
-    setOption: jest.fn(),
-    dispose: jest.fn(),
-    resize: jest.fn(),
+const EChartEventsMock = {
+  ZoomSelect: 'zoomselect',
+};
+
+// Mock the useEchart hook
+jest.mock('../../Echart', () => ({
+  useEchart: jest.fn(() => ({
+    ready: false,
+    addToGroup: jest.fn(),
+    setupZoomSelect: jest.fn(),
     on: jest.fn(),
   })),
-  use: jest.fn(),
+  EChartEvents: EChartEventsMock,
 }));
 
 describe('@lg-echarts/core/hooks/useChart', () => {
-  test('should return an object with the correct properties', () => {
-    const { result } = renderHook(() => useChart({ theme: 'dark' }));
-    expect(result.current).toEqual(
-      expect.objectContaining({
-        chartRef: expect.any(Object),
-        chartInstanceRef: expect.any(Object),
-        chartOptions: expect.any(Object),
-        addChartSeries: expect.any(Function),
-        removeChartSeries: expect.any(Function),
-        updateChartOptions: expect.any(Function),
-      }),
-    );
+  beforeEach(() => {
+    jest.clearAllMocks();
   });
 
-  test('should properly update state on addChartSeries call', async () => {
-    const { result } = renderHook(() => useChart({ theme: 'dark' }));
-    const series: SeriesOption = {
-      name: 'test',
-      data: [1, 2, 3],
-    };
-    act(() => {
-      result.current.addChartSeries(series);
+  test('call `onChartReady` when EchartInstance is ready', async () => {
+    const onChartReady = jest.fn();
+    const { useEchart } = require('../../Echart');
+
+    // Initially not ready
+    (useEchart as jest.Mock).mockReturnValue({
+      ready: false,
+      addToGroup: jest.fn(),
+      setupZoomSelect: jest.fn(),
+      on: jest.fn(),
     });
-    await waitForState(() => {
-      expect(result.current.chartOptions.series).toEqual([series]);
+
+    const { rerender } = renderHook(() =>
+      useChart({ theme: 'dark', onChartReady }),
+    );
+
+    expect(onChartReady).not.toHaveBeenCalled();
+
+    // Update to ready state
+    (useEchart as jest.Mock).mockReturnValue({
+      ready: true,
+      addToGroup: jest.fn(),
+      setupZoomSelect: jest.fn(),
+      on: jest.fn(),
     });
+
+    rerender();
+
+    expect(onChartReady).toHaveBeenCalledTimes(1);
   });
 
-  test('should properly update state on removeChartSeries call', async () => {
-    const { result } = renderHook(() => useChart({ theme: 'dark' }));
-    const series: SeriesOption = {
-      name: 'test',
-      data: [1, 2, 3],
+  test('should correctly configure zoom select', async () => {
+    const { useEchart } = require('../../Echart');
+    const setupZoomSelect = jest.fn();
+
+    (useEchart as jest.Mock).mockReturnValue({
+      ready: true,
+      addToGroup: jest.fn(),
+      setupZoomSelect,
+      on: jest.fn(),
+    });
+
+    const zoomSelect = {
+      xAxis: true,
+      yAxis: false,
     };
-    act(() => {
-      result.current.addChartSeries(series);
+
+    renderHook(() => useChart({ theme: 'dark', zoomSelect }));
+
+    expect(setupZoomSelect).toHaveBeenCalledWith({
+      xAxis: true,
+      yAxis: false,
     });
-    act(() => {
-      result.current.removeChartSeries('test');
+  });
+
+  test('should call `onZoomSelect` on the zoomselect event', async () => {
+    const onZoomSelect = jest.fn();
+    const { useEchart } = require('../../Echart');
+    const on = jest.fn();
+
+    (useEchart as jest.Mock).mockReturnValue({
+      ready: true,
+      addToGroup: jest.fn(),
+      setupZoomSelect: jest.fn(),
+      on,
     });
-    await waitForState(() => {
-      expect(result.current.chartOptions.series).toEqual([]);
+
+    renderHook(() => useChart({ theme: 'dark', onZoomSelect }));
+
+    expect(on).toHaveBeenCalledWith(
+      EChartEventsMock.ZoomSelect,
+      expect.any(Function),
+    );
+
+    // Simulate zoom select event
+    const zoomEventResponse = { start: 0, end: 100 };
+    const zoomSelectHandler = on.mock.calls[0][1];
+    act(() => {
+      zoomSelectHandler(zoomEventResponse);
     });
+
+    expect(onZoomSelect).toHaveBeenCalledWith(zoomEventResponse);
   });
 
-  test('should properly update state on updateChartOptions call', async () => {
-    const { result } = renderHook(() => useChart({ theme: 'dark' }));
-    const options = {
-      title: {
-        show: true,
-        text: 'test',
-      },
+  test('should return the chart instance', () => {
+    const { useEchart } = require('../../Echart');
+    const mockEchartInstance = {
+      ready: true,
+      addToGroup: jest.fn(),
+      setupZoomSelect: jest.fn(),
+      on: jest.fn(),
     };
-    act(() => {
-      result.current.updateChartOptions(options);
-    });
 
-    await waitForState(() => {
-      expect(result.current.chartOptions.title).toEqual(options.title);
+    (useEchart as jest.Mock).mockReturnValue(mockEchartInstance);
+
+    const { result } = renderHook(() => useChart({ theme: 'dark' }));
+
+    expect(result.current).toEqual({
+      ...mockEchartInstance,
+      ref: expect.any(Function),
     });
   });
 });