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

[RFC] Improve grid column definition typing #7188

Closed
Tracked by #3287
cherniavskii opened this issue Dec 13, 2022 · 4 comments · Fixed by #7224
Closed
Tracked by #3287

[RFC] Improve grid column definition typing #7188

cherniavskii opened this issue Dec 13, 2022 · 4 comments · Fixed by #7224
Assignees
Labels
breaking change component: data grid This is the name of the generic UI component, not the React module! dx Related to developers' experience RFC Request For Comments typescript

Comments

@cherniavskii
Copy link
Member

What's the problem? 🤔

I usually use the GridColDef interface when defining grid columns:

const columns: GridColDef[] = [];

But I've noticed that it doesn't work with the actions column type. In this case, different interfaces should be used instead:

// This doesn't work:
// Type '{ field: string; type: string; getActions: () => JSX.Element[]; }' is not assignable to type 'GridColDef<any, any, any>'.
// Object literal may only specify known properties, and 'getActions' does not exist in type 'GridColDef<any, any, any>'.
const columns: GridColDef[] = [
  {
    field: 'actions',
    type: 'actions',
    getActions: () => [<GridActionsCellItem icon={<DeleteIcon />} label="Delete" />],
  },
];

// This works
const columns: GridEnrichedColDef[] = [
  {
    field: 'actions',
    type: 'actions',
    getActions: () => [<GridActionsCellItem icon={<DeleteIcon />} label="Delete" />],
  },
]

// This works
const columns: GridColumns = [
  {
    field: 'actions',
    type: 'actions',
    getActions: () => [<GridActionsCellItem icon={<DeleteIcon />} label="Delete" />],
  },
]

We use both GridColDef and GridColumns in our demos, so it's not clear which one should be used and what is the difference between them.
This is confusing - see #7115 (comment) for example.

What are the requirements? ❓

Ideally, we should expose a single interface that would work for every column type.

What are our options? 💡

Currently, the GridEnrichedColDef should work for every column type, but the interface name is not straightforward and we don't use it in our demos - so it's almost impossible to discover.

Proposed solution 🟢

  1. Rename GridEnrichedColDef to GridColDef
  2. Rename GridColDef to GridBaseColDef
  3. Remove the GridColumns interface and use GridColDef[] instead

Resources and benchmarks 🔗

No response

@cherniavskii cherniavskii added component: data grid This is the name of the generic UI component, not the React module! typescript dx Related to developers' experience RFC Request For Comments labels Dec 13, 2022
@m4theushw
Copy link
Member

+1 for the renaming proposal but it's important to also remove them from the exports and keep only the new GridColDef. We also export GridStateColDef, however, users will never need to use it in the columns prop.

@cjsilva-umich
Copy link

In this demo in the documentation it shows to use GridColDef as the type and getActions should work, but it doesn't, as shown above.

@cherniavskii
Copy link
Member Author

Which version of the package are you using?
The change was released in v6 and it seems to work as expected:
coldef

@cjsilva-umich
Copy link

@cherniavskii Didn't realize there was a new major version out. You're right, w/ v6 GridColDef no longer complains about getActions 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change component: data grid This is the name of the generic UI component, not the React module! dx Related to developers' experience RFC Request For Comments typescript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants