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] Throw an error if incorrect column type is used #3757

Merged

Conversation

DanailH
Copy link
Member

@DanailH DanailH commented Jan 27, 2022

Fixes #3749

@DanailH DanailH added bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! labels Jan 27, 2022
@DanailH DanailH self-assigned this Jan 27, 2022
@mui-bot
Copy link

mui-bot commented Jan 27, 2022

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 187.2 396.7 218.3 268.52 84.375
Sort 100k rows ms 326.4 698.3 550.9 536.26 132.867
Select 100k rows ms 131.3 303.4 187 192.42 59.781
Deselect 100k rows ms 87.4 217.7 195.2 159.96 46.17

Generated by 🚫 dangerJS against e20dd31

if (!columnTypes[type]) {
console.error(
[
`MUI: The column type "${type}" you are using is not supported.`,
Copy link
Member

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.

Copy link
Member

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(', ')}`

Copy link
Member Author

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?

Copy link
Member

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}
/>

Copy link
Member Author

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?

DanailH and others added 2 commits February 1, 2022 15:01
…st.tsx

Co-authored-by: Olivier Tassinari <olivier.tassinari@gmail.com>
Co-authored-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
DanailH and others added 3 commits February 3, 2022 14:32
@DanailH DanailH merged commit faf2ed0 into mui:master Feb 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DataGrid] Wrong error when columns.type is unknown
6 participants