[DataGrid] autoPageSize does not work anymore

[ddolcimascolo]

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

autoPageSize does not work anymore in v4.0.0-alpha.24. Only one row is displayed while there are room for more.

Expected Behavior 🤔

autoPageSize works and N rows are displayed. This works in v4.0.0-alpha.23

Steps to Reproduce 🕹

Steps:

1. Use autoPageSize with v4.0.0-alpha.23. Have enough height for at least 2 rows
2. Upgrade to v4.0.0-alpha.24
3. Only one row is displayed

Context 🔦

Our build started to fail because a test expects 3 rows to be displayed, but only one is show. I just opened a browser to confirm

Your Environment 🌎

`npx @material-ui/envinfo`

  System:
    OS: Linux 5.4 Linux Mint 20.1 (Ulyssa)
  Binaries:
    Node: 14.15.1 - ~/.nvm/versions/node/v14.15.1/bin/node
    Yarn: Not Found
    npm: 6.14.8 - ~/.nvm/versions/node/v14.15.1/bin/npm
  Browsers:
    Chrome: 89.0.4389.114
    Firefox: 87.0

Problem exists in Chrome. Firefox not tested

[oliviertassinari]

I can't reproduce: https://codesandbox.io/s/material-demo-forked-sifup?file=/demo.tsx

Please provide a minimal reproduction test case with the latest version. This would help a lot 👷 .
A live example would be perfect. This codesandbox.io template may be a good starting point. Thank you!

[ddolcimascolo]

Thx for checking @oliviertassinari This must be specific to our environment then, I'll try to give more context and a reproducible example.

David

[ddolcimascolo]

@oliviertassinari I managed to reproduce the issue, but a little bit differently in https://codesandbox.io/s/charming-brook-os2os?file=/src/Demo.js

The problem appears when you change the page, while in my app I get it on the first page directly. In the codesandbox example if you go up to page 3 and com back, you'll see that only one row is displayed. Change the dependency to alpha.23 in package.json and the problem is gone.

Cheers !

David

[oliviertassinari]

@ddolcimascolo Thanks for the reproduction. Yeap, it looks completely broken, indeed. I could isolate the origin of the reproduction down to #1319.

[oliviertassinari]

Ok, I spent 30 minutes looking into this. Merely to force me to better understand how the product works. It seems that #1319 is only the tip of the iceberg. The changes there look correct. However, I believe they surface something that is wrong.

We have useGridPagination that uses state.containerSizes.viewportPageSize to determine state.pagination.pageSize. This looks correct.

However, in useGridContainerProps, the logic sets state.containerSizes.viewportPageSize based on the number of visible rows ⚠️ not on the space available. This looks wrong.

This fix seems to do it:

diff --git a/packages/grid/_modules_/grid/hooks/root/useGridContainerProps.ts b/packages/grid/_modules_/grid/hooks/root/useGridContainerProps.ts
index 5fbebef9..0396e856 100644
--- a/packages/grid/_modules_/grid/hooks/root/useGridContainerProps.ts
+++ b/packages/grid/_modules_/grid/hooks/root/useGridContainerProps.ts
@@ -134,15 +134,19 @@ export const useGridContainerProps = (
       if (options.autoPageSize || options.autoHeight || !isVirtualized) {
         // we don't need vertical virtualization in these cases.
         const viewportPageSize =
-          options.autoHeight || !isVirtualized
+          options.autoHeight
             ? rowsCount
             : Math.floor(viewportSizes.height / rowHeight);
+        const virtualRowsCount =
+            options.autoHeight || !isVirtualized
+              ? rowsCount
+              : Math.floor(viewportSizes.height / rowHeight);
         const requiredHeight = viewportPageSize * rowHeight + scrollBarState.scrollBarSize.x;

         const indexes: GridContainerProps = {
           isVirtualized,
-          virtualRowsCount: viewportPageSize,
-          renderingZonePageSize: viewportPageSize,
+          virtualRowsCount,
+          renderingZonePageSize: virtualRowsCount,
           viewportPageSize,
           totalSizes: {
             width: columnsTotalWidth,

It restores viewportPageSize to what it should be ("The number of rows that fit in the viewport."). All tests are green.

viewportPageSize !== virtualRowsCount