fix: 修复 SheetComponent 未使用 getSpreadSheet 也会报 warning 的问题 (#1843)

antvis · Oct 21, 2022 · 47765b8 · 47765b8 · vercel · Oct 21, 2022
1 parent 7bdceb2
commit 47765b8
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 45 deletions.
diff --git a/packages/s2-react/__tests__/unit/components/sheets/index-spec.tsx b/packages/s2-react/__tests__/unit/components/sheets/index-spec.tsx
@@ -29,15 +29,15 @@ describe('<SheetComponent/> Tests', () => {
   });
 
   describe('Render Tests', () => {
-    const sheets: SheetType[] = [
+    const sheetTypes: SheetType[] = [
       'pivot',
       'table',
       'strategy',
       'gridAnalysis',
       'editable',
     ];
 
-    test.each(sheets)(
+    test.each(sheetTypes)(
       'should render successfully for %s sheet',
       (sheetType) => {
         function render() {
@@ -55,49 +55,76 @@ describe('<SheetComponent/> Tests', () => {
       },
     );
 
-    test.each(sheets)('should render and destroy for %s sheet', (sheetType) => {
-      let getSpreadSheetRef: SpreadSheet;
-      let onMountedRef: SpreadSheet;
+    test.each(sheetTypes)(
+      'should not throw getSpreadSheet deprecated warning for %s sheet',
+      (sheetType) => {
+        const warnSpy = jest
+          .spyOn(console, 'warn')
+          .mockImplementationOnce(() => {});
 
-      const getSpreadSheet = jest.fn((instance) => {
-        getSpreadSheetRef = instance;
-      });
-      const onMounted = jest.fn((instance) => {
-        onMountedRef = instance;
-      });
-      const onDestroy = jest.fn();
-      const warnSpy = jest
-        .spyOn(console, 'warn')
-        .mockImplementationOnce(() => {});
+        act(() => {
+          ReactDOM.render(
+            <SheetComponent
+              sheetType={sheetType}
+              options={{ width: 200, height: 200 }}
+              dataCfg={null as unknown as S2DataConfig}
+            />,
+            container,
+          );
+        });
 
-      act(() => {
-        ReactDOM.render(
-          <SheetComponent
-            sheetType={sheetType}
-            options={{ width: 200, height: 200 }}
-            dataCfg={null as unknown as S2DataConfig}
-            getSpreadSheet={getSpreadSheet}
-            onMounted={onMounted}
-            onDestroy={onDestroy}
-          />,
-          container,
+        expect(warnSpy).not.toHaveBeenCalledWith(
+          '[SheetComponent] `getSpreadSheet` is deprecated. Please use `onMounted` instead.',
         );
-      });
+      },
+    );
 
-      expect(getSpreadSheet).toHaveBeenCalledWith(getSpreadSheetRef);
-      expect(onMounted).toHaveBeenCalledWith(onMountedRef);
-      expect(onMountedRef).toEqual(getSpreadSheetRef);
-      expect(onDestroy).not.toHaveBeenCalled();
-      expect(warnSpy).toHaveBeenCalledWith(
-        '[SheetComponent] `getSpreadSheet` is deprecated. Please use `onMounted` instead.',
-      );
+    test.each(sheetTypes)(
+      'should render and destroy for %s sheet',
+      (sheetType) => {
+        let getSpreadSheetRef: SpreadSheet;
+        let onMountedRef: SpreadSheet;
 
-      act(() => {
-        ReactDOM.unmountComponentAtNode(container);
-      });
+        const getSpreadSheet = jest.fn((instance) => {
+          getSpreadSheetRef = instance;
+        });
+        const onMounted = jest.fn((instance) => {
+          onMountedRef = instance;
+        });
+        const onDestroy = jest.fn();
+        const warnSpy = jest
+          .spyOn(console, 'warn')
+          .mockImplementationOnce(() => {});
 
-      expect(onDestroy).toHaveBeenCalledTimes(1);
-    });
+        act(() => {
+          ReactDOM.render(
+            <SheetComponent
+              sheetType={sheetType}
+              options={{ width: 200, height: 200 }}
+              dataCfg={null as unknown as S2DataConfig}
+              getSpreadSheet={getSpreadSheet}
+              onMounted={onMounted}
+              onDestroy={onDestroy}
+            />,
+            container,
+          );
+        });
+
+        expect(getSpreadSheet).toHaveBeenCalledWith(getSpreadSheetRef);
+        expect(onMounted).toHaveBeenCalledWith(onMountedRef);
+        expect(onMountedRef).toEqual(getSpreadSheetRef);
+        expect(onDestroy).not.toHaveBeenCalled();
+        expect(warnSpy).toHaveBeenCalledWith(
+          '[SheetComponent] `getSpreadSheet` is deprecated. Please use `onMounted` instead.',
+        );
+
+        act(() => {
+          ReactDOM.unmountComponentAtNode(container);
+        });
+
+        expect(onDestroy).toHaveBeenCalledTimes(1);
+      },
+    );
 
     test('should get latest instance after sheet type changed', () => {
       let s2: SpreadSheet;

diff --git a/packages/s2-react/playground/index.tsx b/packages/s2-react/playground/index.tsx
@@ -135,6 +135,7 @@ const partDrillDown: PartDrillDown = {
 };
 
 const onSheetMounted = (s2: SpreadSheet) => {
+  console.log('onSheetMounted: ', s2);
   // @ts-ignore
   window.s2 = s2;
   // @ts-ignore
@@ -970,11 +971,10 @@ function MainLayout() {
                 exportCfg: { open: true },
                 advancedSortCfg: { open: true },
               }}
-              onMounted={onSheetMounted}
-              getSpreadSheet={logHandler('getSpreadSheet')}
               onDataCellTrendIconClick={logHandler('onDataCellTrendIconClick')}
               onAfterRender={logHandler('onAfterRender')}
               onRangeSort={logHandler('onRangeSort')}
+              onMounted={onSheetMounted}
               onDestroy={logHandler('onDestroy', () => {
                 clearInterval(scrollTimer.current);
               })}

diff --git a/packages/s2-react/src/components/sheets/base-sheet/index.tsx b/packages/s2-react/src/components/sheets/base-sheet/index.tsx
@@ -6,7 +6,10 @@ import { SpreadSheetContext } from '../../../utils/SpreadSheetContext';
 import { getSheetComponentOptions } from '../../../utils';
 import { Header } from '../../header';
 import { S2Pagination } from '../../pagination';
-import type { SheetComponentsProps } from '../../sheets/interface';
+import type {
+  SheetComponentOptions,
+  SheetComponentsProps,
+} from '../../sheets/interface';
 
 import './index.less';
 
@@ -63,7 +66,7 @@ export const BaseSheet = React.forwardRef<
 
 BaseSheet.displayName = 'BaseSheet';
 BaseSheet.defaultProps = {
-  options: {} as SheetComponentsProps['options'],
+  options: {} as SheetComponentOptions,
   adaptive: false,
   showPagination: false,
 };
diff --git a/packages/s2-react/src/components/sheets/index.tsx b/packages/s2-react/src/components/sheets/index.tsx
@@ -18,11 +18,11 @@ const Sheet = React.forwardRef(
     const sheetProps = React.useMemo<SheetComponentsProps>(() => {
       return {
         ...props,
-        getSpreadSheet: (instance) => {
+        onMounted: (instance) => {
           if (ref) {
             ref.current = instance;
           }
-          props.getSpreadSheet?.(instance);
+          props.onMounted?.(instance);
         },
       };
     }, [props, ref]);