From abe63f36a512bf0c1e12269f356a4e8142c0efe6 Mon Sep 17 00:00:00 2001 From: damien Date: Tue, 3 Nov 2020 15:10:39 +0100 Subject: [PATCH 01/18] refactored rendering, removed rafUpdate --- .../grid/_modules_/grid/GridComponent.tsx | 10 +-- .../_modules_/grid/components/AutoSizer.tsx | 13 ++-- .../grid/hooks/features/core/useGridApi.ts | 3 + .../grid/hooks/features/core/useGridState.ts | 11 ++- .../grid/hooks/features/rows/useRows.ts | 21 ++++-- .../features/virtualization/useVirtualRows.ts | 19 ++--- .../grid/hooks/root/useContainerProps.ts | 7 +- .../grid/_modules_/grid/hooks/utils/index.ts | 1 - .../grid/hooks/utils/useRafUpdate.ts | 43 ------------ .../grid/hooks/utils/useResizeContainer.ts | 2 +- .../_modules_/grid/hooks/utils/useScrollFn.ts | 7 +- .../_modules_/grid/hooks/utils/useStateRef.ts | 14 ---- .../_modules_/grid/models/api/stateApi.ts | 5 ++ packages/grid/x-grid/src/XGrid.test.tsx | 70 +++++-------------- 14 files changed, 74 insertions(+), 152 deletions(-) delete mode 100644 packages/grid/_modules_/grid/hooks/utils/useRafUpdate.ts delete mode 100644 packages/grid/_modules_/grid/hooks/utils/useStateRef.ts diff --git a/packages/grid/_modules_/grid/GridComponent.tsx b/packages/grid/_modules_/grid/GridComponent.tsx index b9bf0cf732d07..8fa0af94f8a57 100644 --- a/packages/grid/_modules_/grid/GridComponent.tsx +++ b/packages/grid/_modules_/grid/GridComponent.tsx @@ -81,17 +81,17 @@ export const GridComponent = React.forwardRef 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 ( {(size: any) => ( diff --git a/packages/grid/_modules_/grid/components/AutoSizer.tsx b/packages/grid/_modules_/grid/components/AutoSizer.tsx index 788925272d6f7..d5f887484ce21 100644 --- a/packages/grid/_modules_/grid/components/AutoSizer.tsx +++ b/packages/grid/_modules_/grid/components/AutoSizer.tsx @@ -96,13 +96,13 @@ export const AutoSizer = React.forwardRef(functi (!disableHeight && state.height !== newHeight) || (!disableWidth && state.width !== newWidth) ) { - setState({ - height: height - paddingTop - paddingBottom, - width: width - paddingLeft - paddingRight, - }); + setState(() => ({ + height: newHeight, + width: newWidth, + })); if (onResize) { - onResize({ height, width }); + onResize({ height: newHeight, width: newWidth }); } } } @@ -119,10 +119,12 @@ export const AutoSizer = React.forwardRef(functi const detectElementResize = createDetectElementResize(nonce, win); detectElementResize.addResizeListener(parentElement.current, handleResize); + window.addEventListener('resize', handleResize); handleResize(); return () => { detectElementResize.removeResizeListener(parentElement.current, handleResize); + window.removeEventListener('resize', handleResize); }; }, [nonce, handleResize]); @@ -143,7 +145,6 @@ export const AutoSizer = React.forwardRef(functi } const handleRef = useForkRef(rootRef, ref); - return (
{ 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 = (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 index b48076fbfb2f0..a75761591e236 100644 --- a/packages/grid/_modules_/grid/hooks/features/core/useGridState.ts +++ b/packages/grid/_modules_/grid/hooks/features/core/useGridState.ts @@ -1,6 +1,5 @@ import * as React from 'react'; import { ApiRef } from '../../../models/api/apiRef'; -import { useRafUpdate } from '../../utils/useRafUpdate'; import { GridState } from './gridState'; import { useGridApi } from './useGridApi'; @@ -8,15 +7,15 @@ 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 index 0524e1b8d32f6..f622ec4e94dae 100644 --- 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(); + + 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(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[]) => { - 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) { diff --git a/packages/grid/_modules_/grid/hooks/features/virtualization/useVirtualRows.ts b/packages/grid/_modules_/grid/hooks/features/virtualization/useVirtualRows.ts index ae21d1f2e76b5..808bd791e62a4 100644 --- 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(apiRef, paginationSelector); const totalRowCount = useGridSelector(apiRef, rowCountSelector); @@ -51,6 +50,7 @@ export const useVirtualRows = ( if (!isEqual(oldState.rendering, currentRenderingState)) { oldState.rendering = currentRenderingState; stateChanged = true; + return { ...oldState }; } return oldState; }); @@ -104,16 +104,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 +266,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 index eb38e51070eb0..88f75f47fd249 100644 --- 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, 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 index 92e941a3e2bc4..5b442a078204f 100644 --- 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 deleted file mode 100644 index 8863f0d4dfa12..0000000000000 --- a/packages/grid/_modules_/grid/hooks/utils/useRafUpdate.ts +++ /dev/null @@ -1,43 +0,0 @@ -import * as React from 'react'; -import { useLogger } from './useLogger'; - -type UseRafUpdateReturnType = [(...args: any[]) => void, (fn: (args: any) => void) => void]; - -// ⚠️ Usage STRICTLY reserved to `useGridState`. -export function useRafUpdate( - apiRef: any, - initialFn?: (...args: any) => void, -): UseRafUpdateReturnType { - const logger = useLogger('useRafUpdate'); - const fn = React.useRef(initialFn); - - const setUpdate = React.useCallback((updateFn: (...args: any[]) => void) => { - fn.current = updateFn; - }, []); - - const runUpdate = React.useCallback( - (...args: any[]) => { - if (!fn.current) { - return; - } - if (apiRef.current.rafTimer > 0) { - logger.debug('Skipping previous update'); - cancelAnimationFrame(apiRef.current.rafTimer); - } - logger.debug('Queuing new update'); - apiRef.current.rafTimer = requestAnimationFrame(() => { - fn.current!(...args); - apiRef.current.rafTimer = 0; - }); - }, - [apiRef, logger], - ); - - React.useEffect(() => { - return () => { - cancelAnimationFrame(apiRef!.current!.rafTimer); - }; - }, [apiRef]); - - return [runUpdate, setUpdate]; -} diff --git a/packages/grid/_modules_/grid/hooks/utils/useResizeContainer.ts b/packages/grid/_modules_/grid/hooks/utils/useResizeContainer.ts index 9522156a578fd..796e7a863b7d9 100644 --- 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, 10), [onResize]) as any; React.useEffect(() => { return () => { diff --git a/packages/grid/_modules_/grid/hooks/utils/useScrollFn.ts b/packages/grid/_modules_/grid/hooks/utils/useScrollFn.ts index 637ae52fd3597..523da0f3441bd 100644 --- 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, columnHeadersElementRef: React.RefObject, -): [ScrollFn, ScrollFn] { +): [ScrollFn] { const logger = useLogger('useScrollFn'); const previousValue = React.useRef(); @@ -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 deleted file mode 100644 index 78704ca89354c..0000000000000 --- a/packages/grid/_modules_/grid/hooks/utils/useStateRef.ts +++ /dev/null @@ -1,14 +0,0 @@ -import * as React from 'react'; - -/** - * Allow to put a forwarded ref in the dependency array of an effect and do something when current change. - */ -export function useStateRef(forwaredRef: React.RefObject) { - const [refState, setRefState] = React.useState(forwaredRef.current); - const prevColumnsRef = React.useRef(null); - if (prevColumnsRef && forwaredRef && prevColumnsRef.current !== forwaredRef.current) { - prevColumnsRef.current = forwaredRef.current; - setRefState(forwaredRef.current); - } - return refState; -} diff --git a/packages/grid/_modules_/grid/models/api/stateApi.ts b/packages/grid/_modules_/grid/models/api/stateApi.ts index 6b2a7105a405d..39d5014f5be3b 100644 --- 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: (stateId?: string) => T; + /** + * allows to force the grid to rerender after a state update + */ + forceUpdate: React.Dispatch; } diff --git a/packages/grid/x-grid/src/XGrid.test.tsx b/packages/grid/x-grid/src/XGrid.test.tsx index 284934af8cc3e..e9ebcdf91ac4a 100644 --- a/packages/grid/x-grid/src/XGrid.test.tsx +++ b/packages/grid/x-grid/src/XGrid.test.tsx @@ -1,6 +1,6 @@ import * as React from 'react'; // @ts-expect-error need to migrate helpers to TypeScript -import { screen, createClientRender, act, fireEvent } from 'test/utils'; +import { fireEvent, screen, createClientRender } from 'test/utils'; import { expect } from 'chai'; import { XGrid, useApiRef, Columns } from '@material-ui/x-grid'; import { useData } from 'packages/storybook/src/hooks/useData'; @@ -25,19 +25,6 @@ async function sleep(duration: number) { }); } -async function raf() { - return new Promise((resolve) => { - // Chrome and Safari have a bug where calling rAF once returns the current - // frame instead of the next frame, so we need to call a double rAF here. - // See crbug.com/675795 for more. - requestAnimationFrame(() => { - requestAnimationFrame(() => { - resolve(); - }); - }); - }); -} - function getColumnValues() { return Array.from(document.querySelectorAll('[role="cell"][aria-colindex="0"]')).map( (node) => node!.textContent, @@ -119,38 +106,30 @@ describe('', () => { ); }; - it('cell navigation with arrows ', async () => { + it('cell navigation with arrows ', () => { render(); - await sleep(100); // @ts-ignore document.querySelector('[role="cell"][data-rowindex="0"][aria-colindex="0"]').focus(); expect(getActiveCell()).to.equal('0-0'); - fireEvent.keyDown(document.activeElement, { code: 'ArrowRight' }); - await sleep(100); + fireEvent.keyDown(document.activeElement!, { code: 'ArrowRight' }); expect(getActiveCell()).to.equal('0-1'); - fireEvent.keyDown(document.activeElement, { code: 'ArrowDown' }); - await sleep(100); + fireEvent.keyDown(document.activeElement!, { code: 'ArrowDown' }); expect(getActiveCell()).to.equal('1-1'); - fireEvent.keyDown(document.activeElement, { code: 'ArrowLeft' }); - await sleep(100); + fireEvent.keyDown(document.activeElement!, { code: 'ArrowLeft' }); expect(getActiveCell()).to.equal('1-0'); - fireEvent.keyDown(document.activeElement, { code: 'ArrowUp' }); - await sleep(100); + fireEvent.keyDown(document.activeElement!, { code: 'ArrowUp' }); expect(getActiveCell()).to.equal('0-0'); }); it('Home / End navigation', async () => { render(); - await sleep(100); // @ts-ignore document.querySelector('[role="cell"][data-rowindex="1"][aria-colindex="1"]').focus(); expect(getActiveCell()).to.equal('1-1'); - fireEvent.keyDown(document.activeElement, { code: 'Home' }); - await sleep(100); + fireEvent.keyDown(document.activeElement!, { code: 'Home' }); expect(getActiveCell()).to.equal('1-0'); - await sleep(100); - fireEvent.keyDown(document.activeElement, { code: 'End' }); - await sleep(150); + fireEvent.keyDown(document.activeElement!, { code: 'End' }); + await sleep(50); expect(getActiveCell()).to.equal('1-19'); }); /* eslint-enable material-ui/disallow-active-element-as-key-event-target */ @@ -170,26 +149,21 @@ describe('', () => { ); }; - const { container, setProps } = render(); + const { container, rerender } = render(); let rect; - await raf(); - // @ts-ignore rect = container.querySelector('[role="row"][data-rowindex="0"]').getBoundingClientRect(); expect(rect.width).to.equal(300 - 2); - setProps({ width: 400 }); - act(() => { - window.dispatchEvent(new window.Event('resize', {})); - }); - await sleep(100); // resize debounce - await sleep(100); // Not sure why - // @ts-ignore + + rerender(); + fireEvent(window, new Event('resize')); + await sleep(100); rect = container.querySelector('[role="row"][data-rowindex="0"]').getBoundingClientRect(); expect(rect.width).to.equal(400 - 2); }); describe('prop: checkboxSelection', () => { - it('should check and uncheck when double clicking the row', async () => { + it('should check and uncheck when double clicking the row', () => { render(
', () => { />
, ); - await raf(); - await raf(); - const row = document.querySelector('[role="row"][aria-rowindex="2"]'); const checkbox = row!.querySelector('input'); expect(row).to.not.have.class('Mui-selected'); expect(checkbox).to.have.property('checked', false); fireEvent.click(screen.getByRole('cell', { name: 'Nike' })); - await sleep(100); - expect(row!.classList.contains('Mui-selected')).to.equal(true, 'class mui-selected 1'); expect(checkbox).to.have.property('checked', true); fireEvent.click(screen.getByRole('cell', { name: 'Nike' })); - await sleep(100); expect(row!.classList.contains('Mui-selected')).to.equal(false, 'class mui-selected 2'); expect(checkbox).to.have.property('checked', false); }); @@ -262,29 +230,25 @@ describe('', () => { }; render(); - await sleep(100); const cell = document.querySelector('[role="cell"][aria-colindex="0"]')!; expect(cell).to.have.text('Addidas'); }); }); describe('sorting', () => { - it('should sort when clicking the header cell', async () => { + it('should sort when clicking the header cell', async (done) => { render(
, ); - await raf(); const header = screen.getByRole('columnheader', { name: 'brand' }); - await sleep(100); expect(getColumnValues()).to.deep.equal(['Nike', 'Adidas', 'Puma']); fireEvent.click(header); - await sleep(100); expect(getColumnValues()).to.deep.equal(['Adidas', 'Nike', 'Puma']); fireEvent.click(header); - await sleep(100); expect(getColumnValues()).to.deep.equal(['Puma', 'Nike', 'Adidas']); + done(); }); }); }); From 7dd3cd54c6e0f0627022c8b0d01af810be89e62a Mon Sep 17 00:00:00 2001 From: damien Date: Tue, 3 Nov 2020 15:48:43 +0100 Subject: [PATCH 02/18] fix lint --- packages/grid/_modules_/grid/hooks/features/rows/useRows.ts | 2 +- packages/grid/x-grid/src/XGrid.test.tsx | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/grid/_modules_/grid/hooks/features/rows/useRows.ts b/packages/grid/_modules_/grid/hooks/features/rows/useRows.ts index f622ec4e94dae..0d00c0ccc1718 100644 --- a/packages/grid/_modules_/grid/hooks/features/rows/useRows.ts +++ b/packages/grid/_modules_/grid/hooks/features/rows/useRows.ts @@ -161,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/x-grid/src/XGrid.test.tsx b/packages/grid/x-grid/src/XGrid.test.tsx index e9ebcdf91ac4a..cca85d0bd8d67 100644 --- a/packages/grid/x-grid/src/XGrid.test.tsx +++ b/packages/grid/x-grid/src/XGrid.test.tsx @@ -93,7 +93,6 @@ describe('', () => { }); describe('keyboard', () => { - /* eslint-disable material-ui/disallow-active-element-as-key-event-target */ const KeyboardTest = () => { const data = useData(100, 20); const transformColSizes = (columns: Columns) => From 652020543b2ba476e107132d98b08b71949d4f7b Mon Sep 17 00:00:00 2001 From: damien tassone Date: Tue, 3 Nov 2020 16:24:16 +0100 Subject: [PATCH 03/18] Update packages/grid/_modules_/grid/models/api/stateApi.ts Co-authored-by: Olivier Tassinari --- packages/grid/_modules_/grid/models/api/stateApi.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grid/_modules_/grid/models/api/stateApi.ts b/packages/grid/_modules_/grid/models/api/stateApi.ts index 39d5014f5be3b..565ea79f06348 100644 --- a/packages/grid/_modules_/grid/models/api/stateApi.ts +++ b/packages/grid/_modules_/grid/models/api/stateApi.ts @@ -11,7 +11,7 @@ export interface StateApi { */ getState: (stateId?: string) => T; /** - * allows to force the grid to rerender after a state update + * Allows forcing the grid to rerender after a state update. */ forceUpdate: React.Dispatch; } From 9ed2d058a26140b49a7c325f080aff73d6a4110b Mon Sep 17 00:00:00 2001 From: damien Date: Tue, 3 Nov 2020 16:48:50 +0100 Subject: [PATCH 04/18] cleanup --- packages/grid/x-grid/src/XGrid.test.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/grid/x-grid/src/XGrid.test.tsx b/packages/grid/x-grid/src/XGrid.test.tsx index cca85d0bd8d67..37a38d7b1355c 100644 --- a/packages/grid/x-grid/src/XGrid.test.tsx +++ b/packages/grid/x-grid/src/XGrid.test.tsx @@ -235,7 +235,7 @@ describe('', () => { }); describe('sorting', () => { - it('should sort when clicking the header cell', async (done) => { + it('should sort when clicking the header cell', () => { render(
@@ -247,7 +247,6 @@ describe('', () => { expect(getColumnValues()).to.deep.equal(['Adidas', 'Nike', 'Puma']); fireEvent.click(header); expect(getColumnValues()).to.deep.equal(['Puma', 'Nike', 'Adidas']); - done(); }); }); }); From dc1fe695727d62b72663e25baa584c513ab4be59 Mon Sep 17 00:00:00 2001 From: damien Date: Tue, 3 Nov 2020 16:50:57 +0100 Subject: [PATCH 05/18] cleanup --- packages/grid/x-grid/src/XGrid.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grid/x-grid/src/XGrid.test.tsx b/packages/grid/x-grid/src/XGrid.test.tsx index 37a38d7b1355c..c49cb171fc85c 100644 --- a/packages/grid/x-grid/src/XGrid.test.tsx +++ b/packages/grid/x-grid/src/XGrid.test.tsx @@ -194,7 +194,7 @@ describe('', () => { }); describe('prop: apiRef', () => { - it('should apply setPage correctly', async () => { + it('should apply setPage correctly', () => { const rows = [ { id: 0, From 8e2e98957b507fbff714d5082418d87c159ffb45 Mon Sep 17 00:00:00 2001 From: damien Date: Tue, 3 Nov 2020 16:55:43 +0100 Subject: [PATCH 06/18] refactor window to ownerdoc... --- packages/grid/_modules_/grid/components/AutoSizer.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/grid/_modules_/grid/components/AutoSizer.tsx b/packages/grid/_modules_/grid/components/AutoSizer.tsx index d5f887484ce21..69d894179d8bf 100644 --- a/packages/grid/_modules_/grid/components/AutoSizer.tsx +++ b/packages/grid/_modules_/grid/components/AutoSizer.tsx @@ -119,12 +119,12 @@ export const AutoSizer = React.forwardRef(functi const detectElementResize = createDetectElementResize(nonce, win); detectElementResize.addResizeListener(parentElement.current, handleResize); - window.addEventListener('resize', handleResize); + win.addEventListener('resize', handleResize); handleResize(); return () => { detectElementResize.removeResizeListener(parentElement.current, handleResize); - window.removeEventListener('resize', handleResize); + win.removeEventListener('resize', handleResize); }; }, [nonce, handleResize]); From 3eb153733740b11f420f1b82c360e5bec3d6faef Mon Sep 17 00:00:00 2001 From: damien tassone Date: Tue, 3 Nov 2020 17:09:41 +0100 Subject: [PATCH 07/18] Update packages/grid/_modules_/grid/components/AutoSizer.tsx Co-authored-by: Olivier Tassinari --- packages/grid/_modules_/grid/components/AutoSizer.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grid/_modules_/grid/components/AutoSizer.tsx b/packages/grid/_modules_/grid/components/AutoSizer.tsx index 69d894179d8bf..ec77544881441 100644 --- a/packages/grid/_modules_/grid/components/AutoSizer.tsx +++ b/packages/grid/_modules_/grid/components/AutoSizer.tsx @@ -96,7 +96,7 @@ export const AutoSizer = React.forwardRef(functi (!disableHeight && state.height !== newHeight) || (!disableWidth && state.width !== newWidth) ) { - setState(() => ({ + setState({ height: newHeight, width: newWidth, })); From 7af936cf560715ded54a75d395cecc449bc84a9b Mon Sep 17 00:00:00 2001 From: damien tassone Date: Tue, 3 Nov 2020 17:11:39 +0100 Subject: [PATCH 08/18] Update packages/grid/_modules_/grid/hooks/features/virtualization/useVirtualRows.ts Co-authored-by: Olivier Tassinari --- .../grid/hooks/features/virtualization/useVirtualRows.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/grid/_modules_/grid/hooks/features/virtualization/useVirtualRows.ts b/packages/grid/_modules_/grid/hooks/features/virtualization/useVirtualRows.ts index 808bd791e62a4..8cbbf04a653df 100644 --- a/packages/grid/_modules_/grid/hooks/features/virtualization/useVirtualRows.ts +++ b/packages/grid/_modules_/grid/hooks/features/virtualization/useVirtualRows.ts @@ -48,9 +48,8 @@ export const useVirtualRows = ( setGridState((oldState) => { const currentRenderingState = { ...oldState.rendering, ...state }; if (!isEqual(oldState.rendering, currentRenderingState)) { - oldState.rendering = currentRenderingState; stateChanged = true; - return { ...oldState }; + return { ...oldState, rendering: currentRenderingState }; } return oldState; }); From 99a18339d7b7326e994c4de069617c4518136e53 Mon Sep 17 00:00:00 2001 From: damien Date: Tue, 3 Nov 2020 17:14:27 +0100 Subject: [PATCH 09/18] small refact --- .../grid/_modules_/grid/hooks/features/core/useGridState.ts | 2 +- packages/grid/_modules_/grid/hooks/root/useContainerProps.ts | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/grid/_modules_/grid/hooks/features/core/useGridState.ts b/packages/grid/_modules_/grid/hooks/features/core/useGridState.ts index a75761591e236..08a6cd3ecd987 100644 --- a/packages/grid/_modules_/grid/hooks/features/core/useGridState.ts +++ b/packages/grid/_modules_/grid/hooks/features/core/useGridState.ts @@ -8,7 +8,7 @@ export const useGridState = ( ): [GridState, (stateUpdaterFn: (oldState: GridState) => GridState) => void, () => void] => { const api = useGridApi(apiRef); const forceUpdate = React.useCallback( - () => apiRef.current.forceUpdate(() => apiRef.current.state), + () => apiRef.current.forceUpdate(apiRef.current.state), [apiRef], ); const setGridState = React.useCallback( diff --git a/packages/grid/_modules_/grid/hooks/root/useContainerProps.ts b/packages/grid/_modules_/grid/hooks/root/useContainerProps.ts index 88f75f47fd249..0a9efefdea2f8 100644 --- a/packages/grid/_modules_/grid/hooks/root/useContainerProps.ts +++ b/packages/grid/_modules_/grid/hooks/root/useContainerProps.ts @@ -141,9 +141,8 @@ export const useContainerProps = (windowRef: React.RefObject, ap let updated = false; setGridState((state) => { if (!isEqual(state.containerSizes, containerState)) { - state.containerSizes = containerState; updated = true; - return { ...state }; + return { ...state, containerSizes: containerState}; } return state; }); From 254d070170bc2e4c2a3e14e6f5c9430bf6975d1d Mon Sep 17 00:00:00 2001 From: damien Date: Tue, 3 Nov 2020 17:18:15 +0100 Subject: [PATCH 10/18] fix bad commit review --- packages/grid/_modules_/grid/components/AutoSizer.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grid/_modules_/grid/components/AutoSizer.tsx b/packages/grid/_modules_/grid/components/AutoSizer.tsx index ec77544881441..d1c12f9ecd0a2 100644 --- a/packages/grid/_modules_/grid/components/AutoSizer.tsx +++ b/packages/grid/_modules_/grid/components/AutoSizer.tsx @@ -99,7 +99,7 @@ export const AutoSizer = React.forwardRef(functi setState({ height: newHeight, width: newWidth, - })); + }); if (onResize) { onResize({ height: newHeight, width: newWidth }); From d85aa37c8d92c26a18650118468bf3d5aba11c58 Mon Sep 17 00:00:00 2001 From: damien Date: Tue, 3 Nov 2020 17:44:03 +0100 Subject: [PATCH 11/18] better story --- packages/storybook/src/stories/grid-pagination.stories.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/storybook/src/stories/grid-pagination.stories.tsx b/packages/storybook/src/stories/grid-pagination.stories.tsx index a5b428c980a1d..1d712a85b7f24 100644 --- a/packages/storybook/src/stories/grid-pagination.stories.tsx +++ b/packages/storybook/src/stories/grid-pagination.stories.tsx @@ -179,7 +179,7 @@ export function AutoPagination() { function loadServerRows(params: PageChangeParams): Promise { return new Promise((resolve) => { - const data = getData(params.pageSize, 10); + const data = getData(params.pageSize * 5, 10); setTimeout(() => { const minId = (params.page - 1) * params.pageSize; From 6d38055064c31dc8d633727b2a6070cfcc9ef37c Mon Sep 17 00:00:00 2001 From: damien Date: Tue, 3 Nov 2020 17:56:30 +0100 Subject: [PATCH 12/18] revert refactoring as breaking the tests --- .../grid/_modules_/grid/hooks/features/core/useGridState.ts | 2 +- packages/grid/_modules_/grid/hooks/root/useContainerProps.ts | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/grid/_modules_/grid/hooks/features/core/useGridState.ts b/packages/grid/_modules_/grid/hooks/features/core/useGridState.ts index 08a6cd3ecd987..055194a0b8a1e 100644 --- a/packages/grid/_modules_/grid/hooks/features/core/useGridState.ts +++ b/packages/grid/_modules_/grid/hooks/features/core/useGridState.ts @@ -8,7 +8,7 @@ export const useGridState = ( ): [GridState, (stateUpdaterFn: (oldState: GridState) => GridState) => void, () => void] => { const api = useGridApi(apiRef); const forceUpdate = React.useCallback( - () => apiRef.current.forceUpdate(apiRef.current.state), + () => apiRef.current.forceUpdate(()=> apiRef.current.state), [apiRef], ); const setGridState = React.useCallback( diff --git a/packages/grid/_modules_/grid/hooks/root/useContainerProps.ts b/packages/grid/_modules_/grid/hooks/root/useContainerProps.ts index 0a9efefdea2f8..88f75f47fd249 100644 --- a/packages/grid/_modules_/grid/hooks/root/useContainerProps.ts +++ b/packages/grid/_modules_/grid/hooks/root/useContainerProps.ts @@ -141,8 +141,9 @@ export const useContainerProps = (windowRef: React.RefObject, ap let updated = false; setGridState((state) => { if (!isEqual(state.containerSizes, containerState)) { + state.containerSizes = containerState; updated = true; - return { ...state, containerSizes: containerState}; + return { ...state }; } return state; }); From e0289cd00474fbb5fa3d01387eb147e72e3260a6 Mon Sep 17 00:00:00 2001 From: damien Date: Tue, 3 Nov 2020 18:42:57 +0100 Subject: [PATCH 13/18] prettier --- .../grid/_modules_/grid/hooks/features/core/useGridState.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grid/_modules_/grid/hooks/features/core/useGridState.ts b/packages/grid/_modules_/grid/hooks/features/core/useGridState.ts index 055194a0b8a1e..a75761591e236 100644 --- a/packages/grid/_modules_/grid/hooks/features/core/useGridState.ts +++ b/packages/grid/_modules_/grid/hooks/features/core/useGridState.ts @@ -8,7 +8,7 @@ export const useGridState = ( ): [GridState, (stateUpdaterFn: (oldState: GridState) => GridState) => void, () => void] => { const api = useGridApi(apiRef); const forceUpdate = React.useCallback( - () => apiRef.current.forceUpdate(()=> apiRef.current.state), + () => apiRef.current.forceUpdate(() => apiRef.current.state), [apiRef], ); const setGridState = React.useCallback( From fa9bc7f12f0bf8f70ba94c2bf409766b9cb6c6e6 Mon Sep 17 00:00:00 2001 From: damien Date: Tue, 3 Nov 2020 18:56:53 +0100 Subject: [PATCH 14/18] slow down resize debounce --- packages/grid/_modules_/grid/hooks/utils/useResizeContainer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grid/_modules_/grid/hooks/utils/useResizeContainer.ts b/packages/grid/_modules_/grid/hooks/utils/useResizeContainer.ts index 796e7a863b7d9..4202be5ea7190 100644 --- 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, 10), [onResize]) as any; + const debouncedOnResize = React.useMemo(() => debounce(onResize, 50), [onResize]) as any; React.useEffect(() => { return () => { From cf0aa5efa565d10ee02ef4f0b2992d9f251338c1 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Tue, 3 Nov 2020 19:54:20 +0100 Subject: [PATCH 15/18] make sure the tests don't rely on the resize event --- packages/grid/_modules_/grid/components/AutoSizer.tsx | 2 -- packages/grid/x-grid/src/XGrid.test.tsx | 1 - 2 files changed, 3 deletions(-) diff --git a/packages/grid/_modules_/grid/components/AutoSizer.tsx b/packages/grid/_modules_/grid/components/AutoSizer.tsx index d1c12f9ecd0a2..02763e673efca 100644 --- a/packages/grid/_modules_/grid/components/AutoSizer.tsx +++ b/packages/grid/_modules_/grid/components/AutoSizer.tsx @@ -119,12 +119,10 @@ export const AutoSizer = React.forwardRef(functi const detectElementResize = createDetectElementResize(nonce, win); detectElementResize.addResizeListener(parentElement.current, handleResize); - win.addEventListener('resize', handleResize); handleResize(); return () => { detectElementResize.removeResizeListener(parentElement.current, handleResize); - win.removeEventListener('resize', handleResize); }; }, [nonce, handleResize]); diff --git a/packages/grid/x-grid/src/XGrid.test.tsx b/packages/grid/x-grid/src/XGrid.test.tsx index c49cb171fc85c..75b765d4a76a4 100644 --- a/packages/grid/x-grid/src/XGrid.test.tsx +++ b/packages/grid/x-grid/src/XGrid.test.tsx @@ -155,7 +155,6 @@ describe('', () => { expect(rect.width).to.equal(300 - 2); rerender(); - fireEvent(window, new Event('resize')); await sleep(100); rect = container.querySelector('[role="row"][data-rowindex="0"]').getBoundingClientRect(); expect(rect.width).to.equal(400 - 2); From bd72f93c55d564ebc9ca5baee3201b850c9759a9 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Tue, 3 Nov 2020 19:55:16 +0100 Subject: [PATCH 16/18] no any --- packages/grid/_modules_/grid/hooks/utils/useResizeContainer.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grid/_modules_/grid/hooks/utils/useResizeContainer.ts b/packages/grid/_modules_/grid/hooks/utils/useResizeContainer.ts index 4202be5ea7190..e9750e8a6d503 100644 --- 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, 50), [onResize]) as any; + const debouncedOnResize = React.useMemo(() => debounce(onResize, 50), [onResize]); React.useEffect(() => { return () => { From 58ee78c7aa55a542dccd7714a6409788fddbfb60 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Tue, 3 Nov 2020 20:10:10 +0100 Subject: [PATCH 17/18] fine grain timer in test --- packages/grid/x-grid/src/XGrid.test.tsx | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/packages/grid/x-grid/src/XGrid.test.tsx b/packages/grid/x-grid/src/XGrid.test.tsx index 75b765d4a76a4..6aed196044337 100644 --- a/packages/grid/x-grid/src/XGrid.test.tsx +++ b/packages/grid/x-grid/src/XGrid.test.tsx @@ -5,6 +5,19 @@ import { expect } from 'chai'; import { XGrid, useApiRef, Columns } from '@material-ui/x-grid'; import { useData } from 'packages/storybook/src/hooks/useData'; +async function raf() { + return new Promise((resolve) => { + // Chrome and Safari have a bug where calling rAF once returns the current + // frame instead of the next frame, so we need to call a double rAF here. + // See crbug.com/675795 for more. + requestAnimationFrame(() => { + requestAnimationFrame(() => { + resolve(); + }); + }); + }); +} + function getActiveCell() { const activeElement = document.activeElement; @@ -148,14 +161,15 @@ describe('', () => { ); }; - const { container, rerender } = render(); + const { container, setProps } = render(); let rect; rect = container.querySelector('[role="row"][data-rowindex="0"]').getBoundingClientRect(); expect(rect.width).to.equal(300 - 2); - rerender(); - await sleep(100); + setProps({ width: 400 }); + await raf(); // wait for the AutoSize's dimension detection logic + await sleep(50); // wait for the resize event debounce (in useResizeContainer) rect = container.querySelector('[role="row"][data-rowindex="0"]').getBoundingClientRect(); expect(rect.width).to.equal(400 - 2); }); From 112b830af46a05862cd31ca669316d1d4ea0a0dd Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Tue, 3 Nov 2020 20:23:56 +0100 Subject: [PATCH 18/18] hedge against weak timing guarentees of setTimeout --- packages/grid/x-grid/src/XGrid.test.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/grid/x-grid/src/XGrid.test.tsx b/packages/grid/x-grid/src/XGrid.test.tsx index 6aed196044337..61d15d4579469 100644 --- a/packages/grid/x-grid/src/XGrid.test.tsx +++ b/packages/grid/x-grid/src/XGrid.test.tsx @@ -18,6 +18,8 @@ async function raf() { }); } +const CLOCK_SYNC_FACTOR = 10; + function getActiveCell() { const activeElement = document.activeElement; @@ -169,7 +171,7 @@ describe('', () => { setProps({ width: 400 }); await raf(); // wait for the AutoSize's dimension detection logic - await sleep(50); // wait for the resize event debounce (in useResizeContainer) + await sleep(50 + CLOCK_SYNC_FACTOR); // wait for the resize event debounce (in useResizeContainer) rect = container.querySelector('[role="row"][data-rowindex="0"]').getBoundingClientRect(); expect(rect.width).to.equal(400 - 2); });