[DataGrid] Narrow down `GridColDef['type']` #11270

cherniavskii · 2023-12-01T13:58:45Z

Changelog

The GridColDef['type'] has been narrowed down to only accept the built-in column types.
TypeScript users need to use the GridColDef interface when defining columns:

// 🛑 `type` is inferred as `string` and is too wide
const columns = [{ type: 'number', field: 'id' }];
<DataGrid columns={columns} />;

// ✅ `type` is `'number'`
const columns: GridColDef[] = [{ type: 'number', field: 'id' }];
<DataGrid columns={columns} />;

// ✅ Alternalively, `as const` can be used to narrow down the type
const columns = [{ type: 'number' as const, field: 'id' }];
<DataGrid columns={columns} />;

mui-bot · 2023-12-01T14:03:12Z

Deploy preview: https://deploy-preview-11270--material-ui-x.netlify.app/

Updated pages:

docs/data/migration/migration-data-grid-v6/migration-data-grid-v6.md

Generated by 🚫 dangerJS against e2b2a5b

cherniavskii · 2023-12-01T14:53:38Z

docs/data/migration/migration-data-grid-v6/migration-data-grid-v6.md

+  ```tsx
+  // 🛑 `type` is casted to `string` which is too wide
+  const columns = [{ type: 'number', id: 'field' }];
+  <DataGrid columns={columns} />;


I don't like the error here though:

I thought I could improve this by making GridColDef a union discriminated by the type field, but type is optional and it doesn't seem to work as expected from discriminated union :/

Any ideas how we can improve this for TS users?

If you want autocomplete without this problem, you can use field: GridColType | string instead of field: GridColType.
But of course you are loosing the actual type checking.

Otherwise, people need to set type: 'number' as const or columns: GridColDef.

I just realized that the code snippet above was wrong.
What I wanted to show here is this error:

Otherwise, people need to set type: 'number' as const or columns: GridColDef.

Yes, this is fine, my concern is the error when the type is inferred as string.
I find this part confusing:

Types of property 'type' are incompatible.
Type 'string' is not assignable to type '"singleSelect"'.

Ideally, it should be:

Types of property 'type' are incompatible.
Type 'string' is not assignable to type "string" | "number" | "date" | "dateTime" | "boolean" | "singleSelect" | "actions".

Oh right,
AFAIK, it's due to the structure of your type
You are doing:
GridColDef = { type: 'number', ...rest } | { type: 'string', ...rest }
To have the correct autocompletion you would need to have something like:
GridColDef<T extends 'number' | 'string'> = { type: T' } & T extends 'string' ? { ...restString } : { ...restNumber }
But I suppose it's not a trivial change.

Other than that, they might be some hack you could do, but I don't know them :p

Yeah, I guess this is just the nature of unions in TS.
When columns is not explicitly typed, TS requires it to be compatible with all the subtypes of the GridColDef union.
Here's a minimal TS playground reproducing the issue: https://www.typescriptlang.org/play?#code/JYOwLgpgTgZghgYwgAgOJWAEwEJwM4QDCA9gDYAiEMyA3gFDKPJgCeADhAFzIDkeYGEAHMeAbgZMYwCKUzd+goeIC+dOqEixEKdFgCCCMMGIg8JClVoTGrDtx6IjJvGOvIpMucgWglboRBgBk6m3AAUAJTIALwAfMhwICwA2gC6Kmq2OhiY5pTU0Wg5uAR5lgA+RfqGxqZlMOJ0mBAIpHBQKAjOYO4g4V2kAK4AtqFVuWT5UXEJSY1dpj0DI6YxtMzsXLw+wjwANO7Ssvb4mKc8yMriMCBhy6N4EUA

flaviendelangle · 2023-12-04T14:33:54Z

packages/grid/x-data-grid-pro/src/hooks/features/detailPanel/gridDetailPanelToggleColDef.tsx

@@ -9,7 +9,7 @@ export const GRID_DETAIL_PANEL_TOGGLE_FIELD = '__detail_panel_toggle__';
 export const GRID_DETAIL_PANEL_TOGGLE_COL_DEF: GridColDef = {
  ...GRID_STRING_COL_DEF,
  field: GRID_DETAIL_PANEL_TOGGLE_FIELD,
-  type: 'detailPanelToggle',
+  type: 'string',


Does it make sense to have a type dedicated to all the interactive columns (all the toggles, the actions, etc...)
Because "string" is not super coherent IMHO

They're extending the string column definition anyway, it's just that they don't render the string as cell content 🙂
Adding another type would make it appear in the autocomplete for the users, I would rather not add it until there are more reasons to do so.

I think we've already debated on it, but I'll repeat anyway here for the record: I think we should add a "custom" type for the columns that don't fit with any real datatype. It's more explicit than having our special columns be "string". I don't mind that it appears on autocomplete, we can add a note about it on our types table, some users may also have custom columns that don't represent a datatype.

We can also assign it more sensible default behaviors, such as ignoring it for filtering & quickfiltering purposes, and that can improve performance a bit.

edit: btw if we extend GRID_STRING_COL_DEF, the type: 'string' here is redundant with the same field declared in GRID_STRING_COL_DEF

But regardless if we add a different "custom" type, we should probably do something like this for our custom columns:

I think we've already debated on it, but I'll repeat anyway here for the record: I think we should add a "custom" type for the columns that don't fit with any real datatype. It's more explicit than having our special columns be "string"

Yes that's basically what I had in mind

I've added the custom column type, but did not create GRID_CUSTOM_COL_DEF:

Every custom column type has a different set of flags and there's no general rule for that: some columns have the column menu, some columns have row grouping disabled, etc. It's better to have all the flags explicitly provided in each column definition.

Not all custom columns extend the string column definition - see

mui-x/packages/grid/x-data-grid/src/colDef/gridCheckboxSelectionColDef.tsx

Lines 10 to 12 in 8dff180

export const GRID_CHECKBOX_SELECTION_COL_DEF: GridColDef = {

...GRID_BOOLEAN_COL_DEF,

field: GRID_CHECKBOX_SELECTION_FIELD,

What do you think?

Sure, that makes sense 👍

flaviendelangle · 2023-12-04T14:36:36Z

docs/data/migration/migration-data-grid-v6/migration-data-grid-v6.md

+  ```tsx
+  // 🛑 `type` is casted to `string` which is too wide
+  const columns = [{ type: 'number', id: 'field' }];
+  <DataGrid columns={columns} />;


If you want autocomplete without this problem, you can use field: GridColType | string instead of field: GridColType.
But of course you are loosing the actual type checking.

Otherwise, people need to set type: 'number' as const or columns: GridColDef.

romgrk

I've left my opinion about the custom columns type, I disagree with making those "string" but I'm not going to block the PR on that.

However if you could standardize these, either convert all to "string" or remove all (last option prefered):

cherniavskii added 2 commits November 30, 2023 23:25

narrow down GridColDef['type']

4d70474

fix TS errors

aca5fe7

cherniavskii added breaking change component: data grid This is the name of the generic UI component, not the React module! v7.x labels Dec 1, 2023

cherniavskii commented Dec 1, 2023

View reviewed changes

cherniavskii added 4 commits December 1, 2023 18:31

fix typo

82b922d

fix failing unit test

a4c4edf

Merge branch 'next' into grid-col-def-type

87914b8

fix migration guide formatting

1025fbd

cherniavskii marked this pull request as ready for review December 1, 2023 19:46

cherniavskii requested review from romgrk, MBilalShafi and michelengelen December 1, 2023 19:46

flaviendelangle reviewed Dec 4, 2023

View reviewed changes

cherniavskii added 2 commits December 4, 2023 19:56

fix the code snipped in the migration docs

fa958bf

update wording

128126a

romgrk approved these changes Dec 5, 2023

View reviewed changes

cherniavskii added 5 commits December 6, 2023 17:22

Merge branch 'next' into grid-col-def-type

ec5bd2b

remove unnecessary type property

b9f4938

remove unnecessary __default__ column type

16a8a4f

add "custom" column type and use it for non-standard columns

7566d37

fix TS error

e2b2a5b

cherniavskii enabled auto-merge (squash) December 6, 2023 18:05

cherniavskii merged commit f65114a into mui:next Dec 6, 2023
15 checks passed

cherniavskii deleted the grid-col-def-type branch December 6, 2023 19:41

mbiggs-gresham mentioned this pull request Jan 16, 2024

[data grid] How can we use multiple custom column types in v7? #11696

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DataGrid] Narrow down `GridColDef['type']` #11270

[DataGrid] Narrow down `GridColDef['type']` #11270

cherniavskii commented Dec 1, 2023 •

edited

mui-bot commented Dec 1, 2023 •

edited

cherniavskii Dec 1, 2023

cherniavskii Dec 1, 2023

flaviendelangle Dec 4, 2023

cherniavskii Dec 4, 2023

flaviendelangle Dec 5, 2023

cherniavskii Dec 5, 2023

flaviendelangle Dec 4, 2023

cherniavskii Dec 4, 2023

romgrk Dec 4, 2023 •

edited

romgrk Dec 4, 2023 •

edited

flaviendelangle Dec 6, 2023

cherniavskii Dec 6, 2023

romgrk Dec 6, 2023

flaviendelangle Dec 4, 2023

romgrk left a comment

	export const GRID_CHECKBOX_SELECTION_COL_DEF: GridColDef = {
	...GRID_BOOLEAN_COL_DEF,
	field: GRID_CHECKBOX_SELECTION_FIELD,

[DataGrid] Narrow down GridColDef['type'] #11270

[DataGrid] Narrow down GridColDef['type'] #11270

Conversation

cherniavskii commented Dec 1, 2023 • edited

Changelog

mui-bot commented Dec 1, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romgrk Dec 4, 2023 • edited

Choose a reason for hiding this comment

romgrk Dec 4, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romgrk left a comment

Choose a reason for hiding this comment

[DataGrid] Narrow down `GridColDef['type']` #11270

[DataGrid] Narrow down `GridColDef['type']` #11270

cherniavskii commented Dec 1, 2023 •

edited

mui-bot commented Dec 1, 2023 •

edited

romgrk Dec 4, 2023 •

edited

romgrk Dec 4, 2023 •

edited