-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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] Throw an error if incorrect column type is used #3757
[DataGrid] Throw an error if incorrect column type is used #3757
Conversation
packages/grid/x-data-grid-pro/src/tests/columns.DataGridPro.test.tsx
Outdated
Show resolved
Hide resolved
These are the results for the performance tests:
|
packages/grid/x-data-grid-pro/src/tests/columns.DataGridPro.test.tsx
Outdated
Show resolved
Hide resolved
if (!columnTypes[type]) { | ||
console.error( | ||
[ | ||
`MUI: The column type "${type}" you are using is not supported.`, |
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.
I wonder if providing the column field
would allow a developer to resolve the issue faster. Not sure.
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.
Another helper could be to print the available types. Such that they can compare the type they used and the correct ones
// Supported types are: "string", "number", "date", "dateTime", "boolean", "singleSelect", "actions"
`Supported types are: ${Object.keys(columnTypes).map(t => `"${t}"`).join(', ')}`
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.
Not sure either. Printing all of them is ok but can the list be too long?
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.
Is it possible to add types? In the example from the doc, there is not strictly speaking a modification of the available types. It's only a trick to simply colDef reading
The demo below defines a new column type: usdPrice that extends the native number column type.
const usdPrice: GridColTypeDef = {
type: 'number',
width: 130,
valueFormatter: ({ value }) => valueFormatter.format(Number(value)),
cellClassName: 'font-tabular-nums',
};
<DataGrid
columns={[
{ field: 'status', width: 130 },
{ field: 'subTotal', ...usdPrice },
{ field: 'total', ...usdPrice },
]}
rows={rows}
/>
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.
I was also thinking it is possible but it turns out, no -> https://codesandbox.io/s/columntypesgrid-material-demo-forked-hodui?file=/demo.tsx
You can extend existing column types but you can't create a new one. I'm wondering whether we should add this feature?
…st.tsx Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
packages/grid/x-data-grid-pro/src/tests/columns.DataGridPro.test.tsx
Outdated
Show resolved
Hide resolved
packages/grid/_modules_/grid/hooks/features/columns/gridColumnsUtils.ts
Outdated
Show resolved
Hide resolved
…taGrid-3749-throw-error-incorrect-col-type
Co-authored-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
packages/grid/_modules_/grid/hooks/features/columns/gridColumnsUtils.ts
Outdated
Show resolved
Hide resolved
…sUtils.ts Co-authored-by: Matheus Wichman <matheushw@outlook.com>
Fixes #3749