PF4 Table attribute onExpand doesn't reflect a handler update

[idevat]

This is a simplified use case:

import React from "react";

import {
  Table,
  TableHeader,
  TableBody,
  compoundExpand,
} from "@patternfly/react-table";

let counter = 0;

const expandChangeHandler = (expands, handlerCounter) => {
  console.log("create handler", handlerCounter, expands);
  return (e, rowIndex, colIndex, isOpen) => {
    console.log("--- CLICK ---\nusing handler", handlerCounter, expands);
    const newExpands = expands.map(
      (row, i) => {
        if (Math.trunc(rowIndex / 2) !== i) {
          return row;
        }
        return !isOpen;
      },
    );
    console.log("returning from handler", handlerCounter, newExpands);
    return newExpands;
  };
};

const ClusterListView = ({ clusterList }) => {
  const [expands, setExpands] = React.useState([false, false]);

  const rows = clusterList.reduce((allRows, cluster, i) => [
    ...allRows,
    {
      isOpen: expands[i],
      cells: [
        { title: cluster.name },
        { title: cluster.nodeList.length, props: { isOpen: expands[i] } },
      ],
    },
    {
      parent: i * 2,
      compoundParent: 1,
      cells: [{ title: "Nodes", props: { colSpan: 2 } }],
    },
  ], []);

  const columns = [
    "Clusters",
    { title: "Nodes", cellTransforms: [compoundExpand] },
  ];

  console.log("component state", expands);
  const expandChange = expandChangeHandler(expands, counter++);
  return (
    <React.Fragment>
      <Table
        aria-label="Cluster list"
        rows={rows}
        cells={columns}
        onExpand={(...args) => setExpands(expandChange(...args))}
      >
        <TableHeader />
        <TableBody />
      </Table>
      <hr />
      <div onClick={e => setExpands(expandChange(e, 0, 1, expands[0]))}>
        {`${clusterList[0].name} Nodes`}
      </div>
      <div onClick={e => setExpands(expandChange(e, 2, 1, expands[1]))}>
        {`${clusterList[1].name} Nodes`}
      </div>
    </React.Fragment>
  );
};

export default () => {
  const clusterList = [
    { name: "cluster-1", nodeList: ["one", "two"] },
    { name: "cluster-2", nodeList: ["one", "two", "three"] },
  ];
  return <ClusterListView clusterList={clusterList} />;
};

• First click on cluster-1 nodes (i.e. click on number 2 in the cell) - everything works fine:
  • handler variables are: handlerCounter=0 and expands=[false, false] (console: using handler)
  • the handler returns [true, false] as expected (console: returning from handler)
  • component is re-rendered, expands state is [true, false] (console: component state)
  • the handler seems to be regenerated with: handlerCounter=1 and expands=[true, false] (console: create handler)
  • an area with nodes of cluster-1 is expanded

• Then click on cluster-2 nodes (i.e. click on number 3 in the cell) - the result is not as expected.
  • handler variables are not as expected (console: using handler):
    • expected: handlerCounter=1 and expands=[true, false]
    • but was: handlerCounter=0 and expands=[false, false]
  • so handler returns [false, true] (console: returning from handler)
  • and the are with nodes of cluster-1 is collapsed (it shouldn't be)

• When I try the same scenario outside of table component it works fine:
  • I click on div element cluster-1 Nodes and then on div element cluster-2 Nodes (both elements are bellow the table).
  • Table is expanded as expected and also in the console every value is as expected.

Version of @patternfly/react-table is 2.5.11.

[rachael-phillips]

@idevat are you blocked by this currently or do you have a workaround?

[idevat]

Unfortunately, I have no workaround. I can do it just as I described in #2358. When I encountered the issue, I was doing simplification to discover the root cause of the issue. But I haven't tried to find alternative approaches yet. So, there could be a workaround, but I don't know about it.

[karelhala]

@idevat this is not really a problem of tables, but how React is working with computed set state. If you look at the documentation [1] of how to use seState with computed state from previous value you will see the solution right away.

This is really easy to fix instead of creating callback with expands from state you can use arrow function in setExpands and use expands as argument. Here you are 100 % certain that you will use the correct state. I know that this is a bit confusing so let me update the code you posted

import React from "react";

import {
  Table,
  TableHeader,
  TableBody,
  compoundExpand,
} from "@patternfly/react-table";

let counter = 0;

const expandChangeHandler = (expands, handlerCounter) => {
  console.log("create handler", handlerCounter, expands);
  return (e, rowIndex, colIndex, isOpen) => {
    console.log("--- CLICK ---\nusing handler", handlerCounter, expands);
    const newExpands = expands.map(
      (row, i) => {
        if (Math.trunc(rowIndex / 2) !== i) {
          return row;
        }
        return !isOpen;
      },
    );
    console.log("returning from handler", handlerCounter, newExpands);
    return newExpands;
  };
};

const ClusterListView = ({ clusterList }) => {
  const [expands, setExpands] = React.useState([false, false]);

  const rows = clusterList.reduce((allRows, cluster, i) => [
    ...allRows,
    {
      isOpen: expands[i],
      cells: [
        { title: cluster.name },
        { title: cluster.nodeList.length, props: { isOpen: expands[i] } },
      ],
    },
    {
      parent: i * 2,
      compoundParent: 1,
      cells: [{ title: "Nodes", props: { colSpan: 2 } }],
    },
  ], []);

  const columns = [
    "Clusters",
    { title: "Nodes", cellTransforms: [compoundExpand] },
  ];

  console.log("component state", expands);
  return (
    <React.Fragment>
      <Table
        aria-label="Cluster list"
        rows={rows}
        cells={columns}
        onExpand={(...args) => setExpands((expands) => expandChangeHandler(expands, counter++)(...args))}
      >
        <TableHeader />
        <TableBody />
      </Table>
      <hr />
      <div onClick={e => setExpands((expands) => expandChangeHandler(expands, counter++)(e, 0, 1, expands[0]))}>
        {`${clusterList[0].name} Nodes`}
      </div>
      <div onClick={e => setExpands((expands) => expandChangeHandler(expands, counter++)(e, 2, 1, expands[1]))}>
        {`${clusterList[1].name} Nodes`}
      </div>
    </React.Fragment>
  );
};

export default () => {
  const clusterList = [
    { name: "cluster-1", nodeList: ["one", "two"] },
    { name: "cluster-2", nodeList: ["one", "two", "three"] },
  ];
  return <ClusterListView clusterList={clusterList} />;
};

[1] https://reactjs.org/docs/hooks-reference.html#functional-updates

[idevat]

Thanks! This elegantly solves the issue.

I was too concentrated on the fact that the handler behaves differently in the Table than in the div.

Just notice, that it is a problem of table because otherwise it wouldn't work in the case of div. So it is true that Table attribute onExpand doesn't reflect a handler update. But with your solution no update of handler is required (because previous state is passed as an argument and therefore a closure is not necessary). So I'm closing this issue.