refinedev / refine

A React Framework for building internal tools, admin panels, dashboards & B2B apps with unmatched flexibility.
https://refine.dev
MIT License
27.72k stars 2.15k forks source link

[BUG] MUI useDataGrid's dataGridProps type not compatible with DataGridPro #5997

Closed jamesdh closed 1 month ago

jamesdh commented 4 months ago

Describe the bug

Upon trying to upgrade from MUI DataGrid to DataGridPro, I'm getting the following TS errors:

TS2322: Type ... is not assignable to type DataGridProProps<any>
Types of property onFilterModelChange are incompatible.
Types of parameters details and details are incompatible.
Types of property reason are incompatible.

Steps To Reproduce

Create an extremely basic component w/ useDataGrid:

import React from 'react';
import {List, useDataGrid} from '@refinedev/mui';
import {DataGridPro} from '@mui/x-data-grid-pro';

const ListPage = () => {
  const { dataGridProps } = useDataGrid<any>({})
  return (
    <List>
       <DataGridPro {...dataGridProps} columns={[]} />
    </List>
  )
}

Expected behavior

The dataGridProps type returned from useDataGrid will also be compatible with DataGridPro.

Packages

Additional Context

No response

jamesdh commented 4 months ago

Downgrading @mui/x-data-grid-pro to 6.20.0 helped resolve this. It's a bit odd, as prior to upgrading to the Pro license, the regular @mui/x-data-grid was working fine w/ 7.5.1

aliemir commented 4 months ago

Hey @jamesdh sorry for the issue! I've investigated this a bit. Looks like this is just a type issue and caused by a faulty type from our useDataGrid hooks return type. We have onFilterModelChange as DataGridProps["onFilterModelChange"] in the return type (the function has two arguments model and details) but we're only using the model and doesn't even have details in the implementation 😅

We can update the return type to reflect our implementation (which still satisfies the onFilterModelChange prop of DataGrid components) and get rid of this error easily.

Here's the part defining the return type of the hook:

https://github.com/refinedev/refine/blob/77df5be781497681b1ef2718538d76ad41d76901/packages/mui/src/hooks/useDataGrid/index.ts#L33-L53

Instead of inferring it directly from the DataGridProps, we can define onFilterModelChange as:

onFilterModelChange: (model: GridFilterModel) => void;

If you want to contribute to Refine and help us fix the issue, check out the Contributing Guide to get started!

bhargavpshah98 commented 3 months ago

Hello @jamesdh @aliemir , I would like to contribute for working on this issue. Can you please assign it to me?

jamesdh commented 3 months ago

@bhargavpshah98 absolutely! Feel free to submit a PR for it!

aliemir commented 3 months ago

Hey @bhargavpshah98, assigned the issue! I think my comment above will be enough to get the task done but let us know if you need any help! 🙏

@jamesdh, did you had a chance to check my comment above about the solution, do you need any workaround for now, or does just casting the proper types resolves the issue in your codebase? 🙏

bhargavpshah98 commented 3 months ago

Hello, @aliemir , Thank you for assigning the task. Yeah, that sounds good. I will reach out if I need any help or guidance.

bhargavpshah98 commented 3 months ago

Hello @aliemir @jamesdh , I need some help regarding testing of the code and the changes I found in the codebase. Let me know the best way to connect with you.

jamesdh commented 3 months ago

@bhargavpshah98 the easiest way is to create a draft PR and ask for feedback on your changes directly.

Sergio16T commented 2 months ago

Hello @aliemir. Is this issue still open and available to pick up?

aliemir commented 2 months ago

Hey @Sergio16T, issue is still open and available 🚀 I can assign it to you if you're willing to work on it, let me know if any help is needed 🙏

Sergio16T commented 2 months ago

@aliemir Thank you! That sounds good, Please assign it to me.

Sergio16T commented 2 months ago

@aliemir PR is ready for review.