[DataGrid] Avoid undefined value for pagination rowCount
#16488
Conversation
|
Deploy preview: https://deploy-preview-16488--material-ui-x.netlify.app/ |
| @@ -41,7 +41,7 @@ export const gridPaginationModelSelector = createSelector( | |||
| */ | |||
| export const gridPaginationRowCountSelector = createSelector( | |||
| gridPaginationSelector, | |||
| (pagination) => pagination.rowCount, | |||
| (pagination) => pagination.rowCount ?? 0, | |||
There was a problem hiding this comment.
rowCount is actually undefined on first render, and the TablePagination component complains about it, because such a value is not allowed.
WDYT about fallbacking to 0 in such cases?
Or do you have a better suggestion/location of where to solve it? 🤔
There was a problem hiding this comment.
can the default initial value be 0 instead of the selector fallback?
There was a problem hiding this comment.
I think it should be the same.
If you know the most proper location to move the initial value fallback to, feel free to make the change. 🙏
LukasTy
changed the title
undefined value for pagination rowCount
LukasTy
added
the
component: data grid
LukasTy
added
needs cherry-pick
|
It's a problem on the v7.x branch as well. |
|
Is this still happening after #15648? I remember fixing it somehwere – the pagination rowCount wasn't properly initialised, but set in a |
Sadly, yes, big time. 🙈 |
There was a problem hiding this comment.
Were you able to reproduce failing tests locally?
There was a problem hiding this comment.
Yes, it was "failing" (throwing the warning) 100% of runs.
There was a problem hiding this comment.
Karma or JSDOM? I only tried Karma, but the test was passing.
I'll try again.
There was a problem hiding this comment.
I think both are showing the same error.
Did you remember to run pnpm use-react-version ^18.3.1?
packages/x-data-grid-pro/src/tests/dataSourceTreeData.DataGridPro.test.tsx
Outdated
Show resolved
Hide resolved
| @@ -41,7 +41,7 @@ export const gridPaginationModelSelector = createSelector( | |||
| */ | |||
| export const gridPaginationRowCountSelector = createSelector( | |||
| gridPaginationSelector, | |||
| (pagination) => pagination.rowCount, | |||
| (pagination) => pagination.rowCount ?? 0, | |||
There was a problem hiding this comment.
can the default initial value be 0 instead of the selector fallback?
github-actions
bot
added
the
PR: out-of-date
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
github-actions
bot
removed
the
PR: out-of-date
There was a problem hiding this comment.
Looks like this regression was introduced in https://github.com/mui/mui-x/pull/15666/files#diff-a9fa7340e9088ad1aa390713a020c70597fb321d61f271309c56854d6ea53381
There was a problem hiding this comment.
The pagination state initializer depends on rows state, so rows state should be initialized before the pagination state:
There was a problem hiding this comment.
Well, I broke 40 tests 😅
Something super weird is going on there, I'll dig deeper ⛏️
|
There's only one test left that is failing with React 18: https://app.circleci.com/pipelines/github/mui/mui-x/81194/workflows/5a59782e-9bb5-45d7-bd79-0942b87c4308/jobs/462244?invite=true#step-107-6123_136 I think this PR is ready to be reviewed and merged, and the remaining failing test can be fixed in a follow-up. |
Yet another similar fix, a follow-up to #16486.
Avoid another warning in tests on React 18.
Avoid these failures:
After fixing the above tests, new ones pop up...
I think we need to fix the root problem. 🙈
@cherniavskii UPDATE: I think I found the root problem. Due to a more effective first mount in SPA mode (i.e. no SSR), some event handlers were not added to the elements because there were no rerenders after the grid was mounted. This PR fixes these issues.
Fixes #16521