[DataGrid] Extending built-in column types doesn't work

[cherniavskii]

Duplicates

• I have searched the existing issues

Latest version

• I have tested the latest version

Current behavior 😯

When extending default column type, all column definition properties are lost.

Expected behavior 🤔

Extending default column type should preserve all default properties

Steps to reproduce 🕹

Steps:

1. Go to https://codesandbox.io/s/datagrid-v5-quick-start-forked-m9etpu?file=/src/App.tsx
2. See that default properties (like valueFormatter) are not used

Context 🔦

Discovered in different issue - see #3928 (comment)
After rewriting flex implementation, this issue caused layout issues because minWidth/maxWidth were undefined and clamp util function returns NaN when one of it's arguments is undefined

Your environment 🌎

`npx @mui/envinfo`

  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.

Order ID 💳 (optional)

No response

[flaviendelangle]

        columnTypes={{
          date: {
            extendType: "date"
          }
        }}

But is it valid ? You make a type extend itself.

[cherniavskii]

But is it valid ?

I think it's useful and users seem to use it - see #3928 (comment)
Easy way to overwrite default values in one place.
And also, it's an easy fix, so why not?

[flaviendelangle]

In that case, should we just support upsert instead of insert on the columnTypes prop ?

        columnTypes={{
          date: {
            // ... extend the type default properties
          }
        }}

And I would ignore the extendType key if the column type already exist.
Otherwise I feel like we could have strange things, if people start do to stuff like:

        columnTypes={{
          date: {
            extendType: "boolean"
          }
        }}

[m4theushw]

We can't change the way columnTypes work until v6. We need to continue to support inserting new column types. It will be more complex to add a check and warn if the user tries to do a insert than to fix #4113. For v6, we can go back to discussion #242 of removing columnTypes.

And I would ignore the extendType key if the column type already exist.

In this case you automatically extend the existing type or create a column with zero attributes?

Otherwise I feel like we could have strange things, if people start do to stuff like:

Note that the key of the new column doesn't matter internally, it's only a new column. One edge case with extendType that is not clear how to handle is:

columnTypes={{
  date: {
    extendType: 'date'
  },
  otherDate: {
    // should it extend the overridden 'date' or the built-in 'date'?
    // I'm favor of extending the built-in because relaying on the order of the keys is not reliable
    extendType: 'date' 
  },
}}

[flaviendelangle]

In this case you automatically extend the existing type or create a column with zero attributes?

I would extend the existing type

Basically for each (key, value) of props.columnTypes

1. If the key does not match an existing columnType not exist, we create a new columnType by merging props.columnTypes[key] and the column type defined by props.columnTypes[key].extendType
2. If the key exist, we just add props.columnTypes[key] to the already existing column type

---

But I also agree that the whole columnType abstraction brings complexity and maybe dropping in one day is better (not before v6 of course).

[cherniavskii]

@flaviendelangle right, #4114 fixes it exactly how you described