[data grid] Throw error if a field in the `columns` prop is used more than once

[kkirsche]

Duplicates

• [X] I have searched the existing issues

Latest version

• [X] I have tested the latest version

Steps to reproduce 🕹

Link to live example: https://codesandbox.io/s/mui-x-maximum-call-stack-example-tkob2v?file=/src/App.tsx

Steps:

1. Create columns
2. Apply flex to multiple columns
3. Forget to update one "field"

Current behavior 😯

RangeError: Maximum call stack size exceeded

Expected behavior 🤔

DuplicateColumnError: Multiple "title" fields detected

Context 🔦

Building large tables is error prone, so people will often copy existing column definitions of similar fields (e.g. createdAt and updatedAt) and may forget to update the field name.

Your environment 🌎

npx @mui/envinfo

``` System: OS: macOS 12.6.1 Binaries: Node: 18.12.1 - ~/.asdf/installs/nodejs/18.12.1/bin/node Yarn: 3.2.4 - ~/.asdf/installs/nodejs/18.12.1/bin/yarn npm: 9.1.1 - ~/.asdf/plugins/nodejs/shims/npm Browsers: Chrome: 107.0.5304.110 (in-use) Edge: Not Found Firefox: Not Found Safari: 16.1 npmPackages: @emotion/react: ^11.10.5 => 11.10.5 @emotion/styled: ^11.10.5 => 11.10.5 @mui/base: 5.0.0-alpha.106 @mui/core-downloads-tracker: 5.10.14 @mui/icons-material: ^5.10.14 => 5.10.14 @mui/lab: ^5.0.0-alpha.108 => 5.0.0-alpha.108 @mui/material: ^5.10.14 => 5.10.14 @mui/private-theming: 5.10.14 @mui/styled-engine: 5.10.14 @mui/system: ^5.10.14 => 5.10.14 @mui/types: 7.2.1 @mui/utils: 5.10.14 @mui/x-data-grid: ^5.17.11 => 5.17.11 @types/react: ^18.0.25 => 18.0.25 react: ^18.2.0 => 18.2.0 react-dom: ^18.2.0 => 18.2.0 typescript: ^4.8.4 => 4.8.4 ```

Order ID 💳 (optional)

55193

[m4theushw]

We don't check if the columns prop contains duplicated columns. The only error you see, when not using flex, comes from React: "Encountered two children with the same key...". To improve DX, a check on the columns provided and an error could be thrown before the maximum call stack error.

[kkirsche]

Well, it honestly depends on your intent. Do you want to support use cases with rendering the same field different ways?

This is simply intended to inform you that the DX is a bit misleading when flex is used on this not that this is necessarily the correct solution for the library.

[flaviendelangle]

We don't check if the columns prop contains duplicated columns

We could add a check during the generation of the lookup (that would not add any linear complexity).

We could also improve the wording on this page

To be closer to what we do for the rows on this page

---

We clearly don't support having 2 columns with the same field, the field is the unique identifier of the column so any state related to these two columns would be mixed up (filtering, sorting, dimensions, etc...)

[kkirsche]

We clearly don't support having 2 columns with the same field, the field is the unique identifier of the column so any state related to these two columns would be mixed up (filtering, sorting, dimensions, etc...)

That wasn't clear to me, to be fair. That's not clearly documented to my knowledge. And filtering / sorting don't impact it necessarily as you are still filtering on a singular value even if represented multiple ways.

Edit: Potentially important note is that to me I thought field was simply an accessor not a unique id. You can achieve what I'm saying with unique fields that use value getters to replicate it, but at a greater complexity to the user.

[flaviendelangle]

I said "We clearly don't support" to say "It is not going to be supported in the future", it's something that is used everywhere in our codebase (and we need an identifier for the column to handle column updates).

But I do agree that it is not clearly explained on the doc.

[kkirsche]

That's too bad, but good to understand. Thank you

[flaviendelangle]

As for how to create several columns with the same data The best solution depends on your needs

If you want to have the same column and then modify some fields, I would suggest to create a shared column definition with a value getter

const titleColumnsSharedProperties = {
  valueGetter: params => params.row.title,
  headerName: 'Title',
  width: 300,
  type: 'string',
  // ... any other property shared by all your title columns and that you want to have as default
}

const colums = [
  { field: 'title', ...titleColumnsSharedProperties },
  { field: titleBis', ...titleColumnsSharedProperties, width: 350 },
]

Or if you want most of the properties to differ between your columns, you can just add a valueGetter to the 2nd one

const colums = [
  { field: 'title', width: 300 },
  { field: titleBis', valueGetter: params => params.row.title, width: 350 },
]

---

For the sorting / filtering, you can disable it on the 2nd column.

But right now I don't see a way to share this state between the two columns. That's the first time we have this need reported to use, but I can understand the interest. I'm not sure how we could support it on our side and if we have competitors that do support it.

[m4theushw]

We don't explain in the docs because nobody reported trying to reuse the field across multiple columns. I think most users use the DataGrid tied to a DB table and this one already doesn't allow two columns with the same name. We should add the check and also update the docs adding something similar to what exists for row IDs in https://mui.com/x/react-data-grid/row-definition/#row-identifier.

[kkirsche]

All good, I just ran into this while I was building a generic Python library to handle all the server-side aspects of this with Flask, FastAPI and SQLAlchemy, and found that there wasn't a reason this had to be limited from the perspective of the models themselves in how SQLAlchemy reasons about databases. Coming from material-table, this just seemed like an odd restriction (though it doesn't really matter in practice to me).

[yay]

We don't explain in the docs because nobody reported trying to reuse the field across multiple columns.

@m4theushw I'm doing this now because field is a required field :) And one of the columns is using a valueGetter instead of the raw value from the field. So I'm forced to create a fake field name for it to work, but then filtering that column doesn't work. It would be nice if the field wasn't required by GridEnrichedColDef and filtering worked on the columns with valueGetter only.

[m4theushw]

@yay We can't remove the requirement to give a field for each column. It's how we refer to the columns internally. If there's no field, then the position of the column would have to be used, however, columns can be reordered while the columns prop remains the same.

It would be nice if the field wasn't required by GridEnrichedColDef and filtering worked on the columns with valueGetter only.

I didn't understand the problem about filtering. If a column has valueGetter, the filter operators will use the value provided by the value getter. Could you describe better the pain?

[kkirsche]

@yay We can't remove the requirement to give a field for each column. It's how we refer to the columns internally. If there's no field, then the position of the column would have to be used, however, columns can be reordered while the columns prop remains the same.

Well, you could. Simply (does not imply ease) provide a key or identifier attribute on the column definition object which falls back to field if not defined. If neither are defined, throw an error.

Edit to expand for neither being defined and better word choice to avoid confusion with word "field"