onColumnVisibilityChange behaves strange when clicked the show menu column showAll or hideAll button

[shakif95]

material-react-table version

v2.12.1

react & react-dom versions

v18.2.0

Describe the bug and the steps to reproduce it

I want to save the user-defined visible columns in my server, hence every time changing the visibility change I want to make an API call. So I used a state variable to keep track of these changes. When changing a specific column, the updater parameter sends all the column names and their visibility status. All works well when it is being handled by the material react table.

However, when managed with a state, for N columns updated function is called N times with each column changes. For example, given an initial state as {'name'; true, 'age'; true, 'salary': true} The function is called 3 times with these parameters for HIDE_ALL:

{'name'; false, 'age'; true, 'salary': true}
{'name'; true, 'age'; false, 'salary': true}
{'name'; true, 'age'; true, 'salary': false}

And end up with only the last property hidden. The expected parameter would be one call with the following object {'name'; false, 'age'; false, 'salary': false}

Minimal, Reproducible Example - (Optional, but Recommended)

Here is an example using custom states. https://stackblitz.com/edit/github-afi8ma?file=src%2FTS.tsx

Here is an example without using custom states. https://stackblitz.com/github/KevinVandy/material-react-table/tree/v2/apps/material-react-table-docs/examples/external-toolbar/sandbox?file=src%2FTS.tsx

Screenshots or Videos (Optional)

Do you intend to try to help solve this bug with your own PR?

Maybe, I'll investigate and start debugging

Terms

• [X] I understand that if my bug cannot be reliably reproduced in a debuggable environment, it will probably not be fixed and this issue may even be closed.

[LICF01]

I've had this issue too, and it's due to how react batches state updates, it works well if you pass the prev from the setColumnVisibility to the updater function like this

 setColumnVisibility((prev) => updater instanceof Function ? updater(prev) : updater)

[KevinVandy]

Yeah, looks like there is nothing we can patch here in MRT, just have to recommend the ^above fix.

[CaveSeal]

For the scenario where the "Hide all" button is clicked, the columns are toggled like this:

const handleToggleAllColumns = (value?: boolean) => {
    getAllLeafColumns()
      .filter((col) => col.columnDef.enableHiding !== false)
      .forEach((col) => col.toggleVisibility(value));
  };

I think it should be using table.toggleAllColumnsVisible which triggers a single state update, rather than above which triggers multiple state updates for each column.

[KevinVandy]

If that API respects the enable hiding column def option the same way, then sure.

[CaveSeal]

It appears to respect it. https://github.com/TanStack/table/blob/5edd993959967e84766602a9bba908fc6deb4b70/packages/table-core/src/features/ColumnVisibility.ts#L276

    table.toggleAllColumnsVisible = value => {
      value = value ?? !table.getIsAllColumnsVisible()

      table.setColumnVisibility(
        table.getAllLeafColumns().reduce(
          (obj, column) => ({
            ...obj,
            [column.id]: !value ? !column.getCanHide?.() : value,
          }),
          {}
        )
      )
    }

    // getCanHide called when attempting to hide a column
    column.getCanHide = () => {
      return (
        (column.columnDef.enableHiding ?? true) &&
        (table.options.enableHiding ?? true)
      )
    }

EDIT: I now see that toggleAllColumnsVisible and filtering out the columns where enableHiding is false are not the same and switching toggleAllColumnsVisible will break some edge cases.