Skip to content
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] Add support for generic types in GridRowParams, GridCellParams, GridRenderCellParams #2436

Merged
merged 40 commits into from
Oct 20, 2021

Conversation

ZeeshanTamboli
Copy link
Member

@ZeeshanTamboli ZeeshanTamboli commented Aug 23, 2021

Exploring generics for some row and cell types.

Related to issue #1329 (maybe closes this?), issue #545

By passing generic types, we can type value, formattedValue, getValue return type and row in GridRenderCellParams.

For onRowXXXX handlers, row is typed:

Screenshot (7)

Need early feedback and how we can go further.

TODO:

  • test_static is failing. I tried auto generating docs, it created wrong docs with generics. I think we need to support generics in the docs api script.

@ZeeshanTamboli ZeeshanTamboli added typescript component: data grid This is the name of the generic UI component, not the React module! labels Aug 23, 2021
@ZeeshanTamboli ZeeshanTamboli self-assigned this Aug 23, 2021
@ZeeshanTamboli ZeeshanTamboli changed the title [DataGrid] Add option for generic types in GridRowParams, GridCellParams, GridRenderCellParams [DataGrid] Add support for generic types in GridRowParams, GridCellParams, GridRenderCellParams Aug 23, 2021
}

/**
* GridCellParams containing api.
*/
export interface GridRenderCellParams extends GridCellParams {
export interface GridRenderCellParams<T = any, RowType = any> extends GridCellParams<T, RowType> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export interface GridRenderCellParams<T = any, RowType = any> extends GridCellParams<T, RowType> {
-export interface GridRenderCellParams<T = any, RowType = any> extends GridCellParams<T, RowType> {
+export interface GridRenderCellParams<T extends GridCellValue = GridCellValue, RowType = any> extends GridCellParams<T, RowType> {

@flaviendelangle flaviendelangle changed the base branch from master to next August 26, 2021 08:04
@flaviendelangle
Copy link
Member

In another PR, we should add the RowType generic to GridApi

@ZeeshanTamboli
Copy link
Member Author

I have to still work on fixing the script to correctly generate the correct docs for generic types. Will be working on it.

@ZeeshanTamboli
Copy link
Member Author

ZeeshanTamboli commented Sep 6, 2021

I have to still work on fixing the script to correctly generate the correct docs for generic types. Will be working on it.

Looks like typedoc does not support documenting generic types in the way we want to do. I looked into the buildApi script and it seems we have the script for generics but it does not work as expected.

eg: For GridCellParams with row supporting generics to type row data:

 export interface GridCellParams<
  T = GridCellValue,
  RowType extends GridRowDefaultData = GridRowDefaultData,
> {
 --
 --
  value: T;
  /**
   * The cell value formatted with the column valueFormatter.
   */
  formattedValue: T;
  /**
   * The row model of the row that the current cell belongs to.
   */
  row: GridRowModel<RowType>;
 --
 --
}

Script which executes for reference type:
https://github.com/mui-org/material-ui-x/blob/e6cae75d7a49e889d063dba7af741503e2b240d6/docs/scripts/buildApi.ts#L49-L56

The type.type here for row is a reference which makes sense. But it is not detecting type.typeArguments for GridRowModel<RowType>. It is undefined.

The script is instead generating,
| row | RowType | The row model of the row that the current cell belongs to. |

Same is happening for other types and for some it is picking only default generic type in the docs.

I checked into core repo for reference but there is nothing I can find to help me.

@m4theushw Do you have any idea? You can try running docs:api script in this PR and see the generated docs.

@ZeeshanTamboli ZeeshanTamboli marked this pull request as ready for review September 6, 2021 10:35
@m4theushw
Copy link
Member

I added some TS tests and fixed the docs. In the end, I had to use a bunch of any because we though that we were doing the right thing but with the tests I realized that developers wouldn't be able to use the generics.

@ZeeshanTamboli
Copy link
Member Author

Thanks @m4theushw for pushing it further. I have also pushed two commits to improve the docs by leveraging generics.

@ZeeshanTamboli ZeeshanTamboli merged commit 20de1b9 into mui:next Oct 20, 2021
@ZeeshanTamboli ZeeshanTamboli deleted the issue/1329-gridColDef-generics branch October 20, 2021 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: data grid This is the name of the generic UI component, not the React module! typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants