-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[DataGrid] Invert generic parameters order #6874
[DataGrid] Invert generic parameters order #6874
Conversation
These are the results for the performance tests:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you update the migration guide with the breaking changes and the PR description for the release? This is a very impactful change.
I noticed that valueGetter
should also be updated.
diff --git a/packages/grid/x-data-grid/src/models/colDef/gridColDef.ts b/packages/grid/x-data-grid/src/models/colDef/gridColDef.ts
index 9d37447ae..e39515cc1 100644
--- a/packages/grid/x-data-grid/src/models/colDef/gridColDef.ts
+++ b/packages/grid/x-data-grid/src/models/colDef/gridColDef.ts
@@ -128,7 +128,7 @@ export interface GridColDef<R extends GridValidRowModel = any, V = any, F = V> {
* @param {GridValueGetterParams<any, R>} params Object containing parameters for the getter.
* @returns {V} The cell value.
*/
- valueGetter?: (params: GridValueGetterParams<any, R>) => V;
+ valueGetter?: (params: GridValueGetterParams<R, any>) => V;
/**
* Function that allows to customize how the entered value is stored in the row.
* It only works with cell/row editing.
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
@m4theushw what would be the best way to describe the change in the migration guide? |
The main part is to tell that they need to provide values for 2 generic arguments before the argument that they already have. I checked the TypeScript tests and we could borrow some example from there, like -renderCell: (params: GridRenderCellParams<number>) => {
+renderCell: (params: GridRenderCellParams<any, any, number>) => { |
@m4theushw I've updated the guide, if you can check it that would be great. |
@@ -7,7 +7,7 @@ import ListItemIcon from '@mui/material/ListItemIcon'; | |||
import ListItemText from '@mui/material/ListItemText'; | |||
import { INCOTERM_OPTIONS } from '../services/static-data'; | |||
|
|||
function EditIncoterm(props: GridRenderEditCellParams<string | null>) { | |||
function EditIncoterm(props: GridRenderEditCellParams<any, string | null>) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the motivation behind inverting generics' order?
I feel like in GridRenderEditCellParams
specifically the V
generic is the most useful, and because R
is now first - we have to pass any
everywhere. The DX feels a bit awkward
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's mainly for consistency. Others were typed in a similar manner.
packages/grid/x-data-grid-premium/src/hooks/features/rowGrouping/gridRowGroupingUtils.ts
Outdated
Show resolved
Hide resolved
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Fixes #5603