search

mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.52k stars 1.31k forks source link

[data grid] Is `valueOptions` required to be set on `singleSelect` column types? #14601

Open sydneyjodon-wk opened 1 month ago

sydneyjodon-wk commented 1 month ago

The problem in depth

Hello 👋

My team has a singleSelect column type that we overrode the edit cell for to make it multi-select and therefore didn't set valueOptions because we no longer needed it. This has seemed to work fine, but we just ran into an error being thrown when trying to export to excel:

Cannot read properties of undefined (reading 'address')

Click the "download as excel" option to repro in this Codesandbox: https://codesandbox.io/p/sandbox/datagrid-singleselect-without-valueoptions-hydgjz?file=%2Fsrc%2FDemo.tsx

If I add a valueOptions, it works again!

I understand that my usage could probably qualify more as a type: 'string' column, but valueOptions is marked as optional and it seemed to work without setting it until I ran into this error. So my question is: Is this particular case an issue with the download to excel or is valueOptions actually required on singleSelect columns? If it is the later, would it be possible to change the type definition to reflect that it is required to avoid this type of thing where things are assumed to work without it, but then some case comes up where it throws. Or maybe there's some other issue that I'm missing that's causing this?

Thank you!

Your environment

See codesandbox: https://codesandbox.io/p/sandbox/datagrid-singleselect-without-valueoptions-hydgjz?file=%2Fsrc%2FDemo.tsx

Search keywords: valueOptions required singleSelect Order ID: 82849

michelengelen commented 1 month ago

Hey @sydneyjodon-wk ... is there a specific reason why you need to keep the column type as 'singleSelect' ... it works perfectly fine when you set it to 'string'

michelengelen commented 1 month ago

But yes, I do agree that the valueOptions property should not be optional. I'll mark this as a bug and add it to the board for the team to discuss.

sydneyjodon-wk commented 1 month ago

Yep! I know it works fine as string, I'm just worried about our consumers doing something similar and using 'singleSelect' without valueOptions assuming that it works and then having it throw unexpectedly. Thanks for updating that!