1

Here is the codesandbox for this question: https://codesandbox.io/s/rdg-grouping-81b1s

I am using React-Data-Grid to render a table. I render a ReactDataGrid with two columns, and When you click on the text GROUP in a header cell you group by that column.

To be able to have a custom header cell with that text GROUP, I use the property headerRenderer in the object defining the columns.

The value passed to this property is a function that takes an onClick handler as parameter, and returns a functional React component that uses that onClick handler.

The onClick parameter is just a method on the original React component, and it is bound in the component's constructor.

As you can see, I am using this headerRenderer property twice, once for each column. However, for the first column, I bind the parameter function to the React component again. For the second column I do not, and this generates an error when I try to click the GROUP text for this column. See error image further below.

My question is: why do I have to bind given that I've already bound the function in the constructor?

import React from 'react';
import './App.css';
import ReactDataGrid from 'react-data-grid';
import { Data } from 'react-data-grid-addons';

const HeaderRowRenderer = function(props) {
  return (
    <div
      style={{
        backgroundColor: 'red',
        paddingLeft: 10,
        height: '100%',
        padding: 0,
        display: 'flex',
        flexDirection: 'row',
        alignItems: 'center',
        justifyContent: 'space-between',
      }}
    >
      <span>{props.column.name}</span>
      <span onClick={props.onClick}>GROUP</span>
    </div>
  );
};

const HeaderRenderer = function(groupBy, onClick) {
  return function(props) {
    return (
      <HeaderRowRenderer
        {...props}
        onClick={function() {
          onClick(groupBy);
        }}
      />
    );
  };
};

const rows = [{ productname: 'Beef', quantity: 5 }, { productname: 'Veggies', quantity: 10 }];

class App extends React.Component {
  columns = [
    {
      key: 'productname',
      name: 'Product',
      width: 200,
      headerRenderer: HeaderRenderer('productname', this.groupBy.bind(this)),
    },
    {
      key: 'quantity',
      name: 'Quantity',
      headerRenderer: HeaderRenderer('quantity', this.groupBy),
    },
  ];

  constructor(props) {
    super(props);
    this.state = {
      groupBy: new Set([]),
    };
    this.groupBy = this.groupBy.bind(this);
  }

  groupBy(group) {
    const newSet = new Set(this.state.groupBy);
    if (newSet.has(group)) {
      newSet.delete(group);
    } else {
      newSet.add(group);
    }
    this.setState({ groupBy: newSet });
  }

  render() {
    const groupBy = Array.from(this.state.groupBy);
    // const rows = this.props.orderItems;

    const groupedRows = Data.Selectors.getRows({
      rows: rows,
      groupBy,
    });
    return (
      <div>
        <ReactDataGrid
          columns={this.columns}
          rowGetter={i => groupedRows[i]}
          rowsCount={groupedRows.length}
          minHeight={650}
        />
      </div>
    );
  }
}

export default App;

I looked at the code for React-Data-Grid, and I believe that the headerRenderer prop is called as below:

  getCell() {
    const { height, column, rowType } = this.props;
    const renderer = this.props.renderer || SimpleCellRenderer;
    if (isElement(renderer)) {
      // if it is a string, it's an HTML element, and column is not a valid property, so only pass height
      if (typeof renderer.type === 'string') {
        return React.cloneElement(renderer, { height });
      }
      return React.cloneElement(renderer, { column, height });
    }
    return React.createElement(renderer, { column, rowType });
  }

I'm not very familiar with the ways in which a function that was bound using bind and then is passed around can lose this boundedness. Does this happen as a result of React.cloneElement, or what could be the cause of it?

Error

evianpring
  • 3,316
  • 1
  • 25
  • 54
  • Use arrow functions in event handlers, not standard functions. So insead of `onClick={ function(..) { ... }`, which will have "whatever is the execution context when the function runs", use `onClick={ (...) => { ... } }`, which will have "whatever the execution context is when the function got declared", e.g. `this` inside the function points to the same thing that it points to outside the function. – Mike 'Pomax' Kamermans Jul 10 '19 at 21:37
  • Actually @Mike'Pomax'Kamermans I just switched over to regular functions because it wasn't working with arrow functions and I was trying to avoid pitfalls related to the "whatever the execution context is". Here is a codesandbox for the example if you think you can make it work with that change: https://codesandbox.io/s/rdg-grouping-81b1s – evianpring Jul 10 '19 at 21:39
  • Please turn that into a [mcve] first - people have plenty of time to help you debug your problem after you remove all the code that is non-essential, they generally don't have time to first read through your whole codebase =) – Mike 'Pomax' Kamermans Jul 10 '19 at 21:41
  • @Mike'Pomax'Kamermans That is exactly what I did. The only non-essential code in that example is a few lines of styles (but that's to make the codesandbox more useable). Everything else I wrote specifically from scratch to make this example. I want to understand why bind doesn't behave as expected. It's also more code than usual because this issue is specifically arising in React-Data-Grid. But that is the minimal code for a simple table in this lib. – evianpring Jul 10 '19 at 21:43
  • 1
    for one, what is `this` supposed to be in a static property? (looking at your `columns` definition. That said, this is certainly not minimal yet: remove the duplication, make groupBy simply set _any_ state value that render can show a silly `

    state changed!

    ` for, etc. There's still a lot of code here that has nothing to do with the problem you're asking about. (mcve's usually require removing all semblance of what your real code does, and purely drilling down to the one thing that's not working for you)
    – Mike 'Pomax' Kamermans Jul 10 '19 at 21:45
  • Well there you go @Mike'Pomax'Kamermans I think the answer is related to me defining `columns` as a static property. When I move the definition into the constructor it works. – evianpring Jul 10 '19 at 21:48
  • 1
    Its a timing issue. When you define a "static" property like that, it is translated / compiled to the constructor (as in it gets defined in the constructor). Its not truly static. But that assignment in the constructor is probably happening before you bind the function to the class. – John Ruddell Jul 10 '19 at 21:50
  • 1
    @JohnRuddell just for correctness, that isn't a static field definition, it's a class field definition. As you say, it's merely just syntactic sugar for writing constructor code as that's where it ends up. Your suggestion is most likely correct though, attempting to bind `this` in that way isn't reliable. – James Jul 10 '19 at 22:51
  • @James I know, thats why i wrote "static" in quotes, then also said `Its not truly static`. Its an instance variable assigned to `this` on the class in the constructor. Some compilers let you write properties on your class this way :) – John Ruddell Jul 10 '19 at 23:03

0 Answers0