[DataGrid] Refactor rendering, remove rafUpdate (#532)

* refactored rendering, removed rafUpdate * fix lint * Update packages/grid/_modules_/grid/models/api/stateApi.ts Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com> * cleanup * cleanup * refactor window to ownerdoc... * Update packages/grid/_modules_/grid/components/AutoSizer.tsx Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com> * Update packages/grid/_modules_/grid/hooks/features/virtualization/useVirtualRows.ts Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com> * small refact * fix bad commit review * better story * revert refactoring as breaking the tests * prettier * slow down resize debounce * make sure the tests don't rely on the resize event * no any * fine grain timer in test * hedge against weak timing guarentees of setTimeout Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
mui · Nov 4, 2020 · 453e2f1 · 453e2f1
1 parent 81b4742
commit 453e2f1
Show file tree

Hide file tree

Showing 15 changed files with 86 additions and 154 deletions.
diff --git a/packages/grid/_modules_/grid/GridComponent.tsx b/packages/grid/_modules_/grid/GridComponent.tsx
@@ -81,17 +81,17 @@ export const GridComponent = React.forwardRef<HTMLDivElement, GridComponentProps
 
     const customComponents = useComponents(props.components, apiRef, rootContainerRef);
 
-    logger.info(
-      `Rendering, page: ${renderCtx?.page}, col: ${renderCtx?.firstColIdx}-${renderCtx?.lastColIdx}, row: ${renderCtx?.firstRowIdx}-${renderCtx?.lastRowIdx}`,
-      renderCtx,
-    );
-
     // TODO move that to renderCtx
     const getTotalHeight = React.useCallback(
       (size) => getCurryTotalHeight(gridState.options, gridState.containerSizes, footerRef)(size),
       [gridState.options, gridState.containerSizes],
     );
 
+    logger.info(
+      `Rendering, page: ${renderCtx?.page}, col: ${renderCtx?.firstColIdx}-${renderCtx?.lastColIdx}, row: ${renderCtx?.firstRowIdx}-${renderCtx?.lastRowIdx}`,
+      apiRef.current.state,
+    );
+
     return (
       <AutoSizer onResize={onResize}>
         {(size: any) => (

diff --git a/packages/grid/_modules_/grid/components/AutoSizer.tsx b/packages/grid/_modules_/grid/components/AutoSizer.tsx
@@ -97,12 +97,12 @@ export const AutoSizer = React.forwardRef<HTMLDivElement, AutoSizerProps>(functi
         (!disableWidth && state.width !== newWidth)
       ) {
         setState({
-          height: height - paddingTop - paddingBottom,
-          width: width - paddingLeft - paddingRight,
+          height: newHeight,
+          width: newWidth,
         });
 
         if (onResize) {
-          onResize({ height, width });
+          onResize({ height: newHeight, width: newWidth });
         }
       }
     }
@@ -143,7 +143,6 @@ export const AutoSizer = React.forwardRef<HTMLDivElement, AutoSizerProps>(functi
   }
 
   const handleRef = useForkRef(rootRef, ref);
-
   return (
     <div
       ref={handleRef}

diff --git a/packages/grid/_modules_/grid/hooks/features/core/useGridApi.ts b/packages/grid/_modules_/grid/hooks/features/core/useGridApi.ts
@@ -1,13 +1,16 @@
+import * as React from 'react';
 import { ApiRef } from '../../../models/api/apiRef';
 import { GridApi } from '../../../models/api/gridApi';
 import { useLogger } from '../../utils/useLogger';
 import { getInitialState } from './gridState';
 
 export const useGridApi = (apiRef: ApiRef): GridApi => {
   const logger = useLogger('useGridApi');
+  const [, forceUpdate] = React.useState();
   if (!apiRef.current.isInitialised && !apiRef.current.state) {
     logger.info('Initialising state.');
     apiRef.current.state = getInitialState();
+    apiRef.current.forceUpdate = forceUpdate;
     apiRef.current.getState = <State>(stateId?: string) =>
       (stateId ? apiRef.current.state[stateId] : apiRef.current.state) as State;
   }

diff --git a/packages/grid/_modules_/grid/hooks/features/core/useGridState.ts b/packages/grid/_modules_/grid/hooks/features/core/useGridState.ts
@@ -1,22 +1,21 @@
 import * as React from 'react';
 import { ApiRef } from '../../../models/api/apiRef';
-import { useRafUpdate } from '../../utils/useRafUpdate';
 import { GridState } from './gridState';
 import { useGridApi } from './useGridApi';
 
 export const useGridState = (
   apiRef: ApiRef,
 ): [GridState, (stateUpdaterFn: (oldState: GridState) => GridState) => void, () => void] => {
   const api = useGridApi(apiRef);
-  const [, forceUpdate] = React.useState();
-  const [rafUpdate] = useRafUpdate(apiRef, () => forceUpdate((p: any) => !p));
-
+  const forceUpdate = React.useCallback(
+    () => apiRef.current.forceUpdate(() => apiRef.current.state),
+    [apiRef],
+  );
   const setGridState = React.useCallback(
     (stateUpdaterFn: (oldState: GridState) => GridState) => {
       api.state = stateUpdaterFn(api.state);
     },
     [api],
   );
-
-  return [api.state, setGridState, rafUpdate];
+  return [api.state, setGridState, forceUpdate];
 };
diff --git a/packages/grid/_modules_/grid/hooks/features/rows/useRows.ts b/packages/grid/_modules_/grid/hooks/features/rows/useRows.ts
@@ -30,10 +30,25 @@ export function convertRowsPropToState({
 
 export const useRows = (rows: RowsProp, apiRef: ApiRef): void => {
   const logger = useLogger('useRows');
-  const [gridState, setGridState, forceUpdate] = useGridState(apiRef);
+  const [gridState, setGridState, updateComponent] = useGridState(apiRef);
+  const updateTimeout = React.useRef<any>();
+
+  const forceUpdate = React.useCallback(() => {
+    if (updateTimeout!.current == null) {
+      updateTimeout!.current = setTimeout(() => {
+        logger.debug(`Updating component`);
+        updateTimeout.current = null;
+        return updateComponent();
+      }, 100);
+    }
+  }, [logger, updateComponent]);
 
   const internalRowsState = React.useRef<InternalRowsState>(gridState.rows);
 
+  React.useEffect(() => {
+    return () => clearTimeout(updateTimeout!.current);
+  }, []);
+
   React.useEffect(() => {
     setGridState((state) => {
       internalRowsState.current = convertRowsPropToState({
@@ -85,7 +100,7 @@ export const useRows = (rows: RowsProp, apiRef: ApiRef): void => {
 
   const updateRowModels = React.useCallback(
     (updates: Partial<RowModel>[]) => {
-      logger.debug(`updating ${updates.length} row models`);
+      logger.debug(`updating row models`);
       const addedRows: RowModel[] = [];
 
       updates.forEach((partialRow) => {
@@ -121,8 +136,6 @@ export const useRows = (rows: RowsProp, apiRef: ApiRef): void => {
 
   const updateRowData = React.useCallback(
     (updates: RowData[]) => {
-      logger.debug(`updating rows data`);
-
       // we removes duplicate updates. A server can batch updates, and send several updates for the same row in one fn call.
       const uniqUpdates = updates.reduce((uniq, update) => {
         if (update.id == null) {
@@ -148,7 +161,7 @@ export const useRows = (rows: RowsProp, apiRef: ApiRef): void => {
       });
       return updateRowModels(rowModelUpdates);
     },
-    [updateRowModels, logger, getRowFromId],
+    [updateRowModels, getRowFromId],
   );
 
   const getRowModels = React.useCallback(

diff --git a/packages/grid/_modules_/grid/hooks/features/virtualization/useVirtualRows.ts b/packages/grid/_modules_/grid/hooks/features/virtualization/useVirtualRows.ts
@@ -34,8 +34,7 @@ export const useVirtualRows = (
   const logger = useLogger('useVirtualRows');
 
   const updateViewportRef = React.useRef<(...args: any[]) => void>();
-  const [, forceUpdate] = React.useState();
-  const [gridState, setGridState, rafUpdate] = useGridState(apiRef);
+  const [gridState, setGridState, forceUpdate] = useGridState(apiRef);
   const options = useGridSelector(apiRef, optionsSelector);
   const paginationState = useGridSelector<PaginationState>(apiRef, paginationSelector);
   const totalRowCount = useGridSelector<number>(apiRef, rowCountSelector);
@@ -49,8 +48,8 @@ export const useVirtualRows = (
       setGridState((oldState) => {
         const currentRenderingState = { ...oldState.rendering, ...state };
         if (!isEqual(oldState.rendering, currentRenderingState)) {
-          oldState.rendering = currentRenderingState;
           stateChanged = true;
+          return { ...oldState, rendering: currentRenderingState };
         }
         return oldState;
       });
@@ -104,16 +103,10 @@ export const useVirtualRows = (
       renderedSizes: apiRef.current.state.containerSizes,
     });
     if (hasChanged) {
-      if (apiRef.current.state.isScrolling) {
-        logger.debug('reRender: Raf rendering');
-        rafUpdate();
-      } else {
-        logger.debug('reRender: Force rendering');
-        // we force this update, the func makes react force run this state update and rerender
-        forceUpdate((p) => !p);
-      }
+      logger.debug('reRender: trigger rendering');
+      forceUpdate();
     }
-  }, [apiRef, getRenderingState, logger, rafUpdate, setRenderingState]);
+  }, [apiRef, getRenderingState, logger, forceUpdate, setRenderingState]);
 
   const updateViewport = React.useCallback(
     (forceReRender = false) => {
@@ -272,8 +265,7 @@ export const useVirtualRows = (
     scrollingTimeout.current = setTimeout(() => {
       scrollingTimeout.current = null;
       apiRef.current.state.isScrolling = false;
-      // We let react decide to run this update.
-      forceUpdate(true);
+      forceUpdate();
     }, 300);
 
     if (updateViewportRef.current) {

diff --git a/packages/grid/_modules_/grid/hooks/root/useContainerProps.ts b/packages/grid/_modules_/grid/hooks/root/useContainerProps.ts
@@ -138,13 +138,18 @@ export const useContainerProps = (windowRef: React.RefObject<HTMLDivElement>, ap
 
   const updateContainerState = React.useCallback(
     (containerState: ContainerProps | null) => {
+      let updated = false;
       setGridState((state) => {
         if (!isEqual(state.containerSizes, containerState)) {
           state.containerSizes = containerState;
-          forceUpdate();
+          updated = true;
+          return { ...state };
         }
         return state;
       });
+      if (updated) {
+        forceUpdate();
+      }
     },
     [forceUpdate, setGridState],
   );

diff --git a/packages/grid/_modules_/grid/hooks/utils/index.ts b/packages/grid/_modules_/grid/hooks/utils/index.ts
@@ -1,3 +1,2 @@
 export * from './useLogger';
-export * from './useRafUpdate';
 export * from './useScrollFn';
diff --git a/packages/grid/_modules_/grid/hooks/utils/useRafUpdate.ts b/packages/grid/_modules_/grid/hooks/utils/useRafUpdate.ts
diff --git a/packages/grid/_modules_/grid/hooks/utils/useResizeContainer.ts b/packages/grid/_modules_/grid/hooks/utils/useResizeContainer.ts
@@ -38,7 +38,7 @@ export function useResizeContainer(apiRef): (size: ElementSize) => void {
     },
     [gridLogger, apiRef],
   );
-  const debouncedOnResize = React.useMemo(() => debounce(onResize, 100), [onResize]) as any;
+  const debouncedOnResize = React.useMemo(() => debounce(onResize, 50), [onResize]);
 
   React.useEffect(() => {
     return () => {

diff --git a/packages/grid/_modules_/grid/hooks/utils/useScrollFn.ts b/packages/grid/_modules_/grid/hooks/utils/useScrollFn.ts
@@ -2,13 +2,12 @@ import { debounce } from '@material-ui/core/utils';
 import * as React from 'react';
 import { ScrollFn, ScrollParams } from '../../models/params/scrollParams';
 import { useLogger } from './useLogger';
-import { useRafUpdate } from './useRafUpdate';
 
 export function useScrollFn(
   apiRef: any,
   renderingZoneElementRef: React.RefObject<HTMLDivElement>,
   columnHeadersElementRef: React.RefObject<HTMLDivElement>,
-): [ScrollFn, ScrollFn] {
+): [ScrollFn] {
   const logger = useLogger('useScrollFn');
   const previousValue = React.useRef<ScrollParams>();
 
@@ -40,13 +39,11 @@ export function useScrollFn(
     [renderingZoneElementRef, logger, columnHeadersElementRef, debouncedResetPointerEvents],
   );
 
-  const [runScroll] = useRafUpdate(apiRef, scrollTo);
-
   React.useEffect(() => {
     return () => {
       debouncedResetPointerEvents.clear();
     };
   }, [renderingZoneElementRef, debouncedResetPointerEvents]);
 
-  return [runScroll, scrollTo];
+  return [scrollTo];
 }
diff --git a/packages/grid/_modules_/grid/hooks/utils/useStateRef.ts b/packages/grid/_modules_/grid/hooks/utils/useStateRef.ts
diff --git a/packages/grid/_modules_/grid/models/api/stateApi.ts b/packages/grid/_modules_/grid/models/api/stateApi.ts
@@ -1,3 +1,4 @@
+import * as React from 'react';
 import { GridState } from '../../hooks/features/core/gridState';
 
 export interface StateApi {
@@ -9,4 +10,8 @@ export interface StateApi {
    * allows to get the whole state of the grid if stateId is null or to get a part of the state if stateId has a value.
    */
   getState: <T>(stateId?: string) => T;
+  /**
+   * Allows forcing the grid to rerender after a state update.
+   */
+  forceUpdate: React.Dispatch<any>;
 }