0

I have a React component that renders a list of files. Sometimes the list is quite long, and as pagination isn't ideal from a UI perspective in this case, the list of files becomes quite slow during re-rendering (e.g. when dragging and dropping files to reorder them).

One thing that is contributing to the slowness is that in the loop that runs once for each file, there are a handful of bind() calls:

render() {
    return (
        <...>
        {this.props.files.map((file, index) => {
            return (
                <tr
                    key={`${index}#${file.name}`}
                    onDragStart={this.onDragStart.bind(this, file, index)}
                    onDragEnter={this.onDragEnter.bind(this, file)}
                    onDragOver={this.onDragOver.bind(this, file)}
                    onDragLeave={this.onDragLeave.bind(this, file)}
                    onDrop={this.onDrop.bind(this, file, index)}
                />
            );
        })}
        </...>
    );
}

These binds are necessary so the drag and drop handlers know which file is being dragged and where it's being dropped. With all these binds running once for each of the hundreds of files (even if the resulting elements are optimised out and never actually get rendered), I guess it is no surprise that things are a bit sluggish.

I am wondering whether there is any better way of doing this, to somehow pass the necessary per-iteration data to those functions without having to create a unique bind for each function in each iteration.

I have one possible solution which I will post as my own answer, however I would appreciate feedback as to whether this solution is better or worse, and whether it has any drawbacks.

Malvineous
  • 25,144
  • 16
  • 116
  • 151

1 Answers1

0

So my solution was to bind the functions once in the constructor as is the normal practice, then place the per-iteration data on the <tr/> DOM element itself.

When the functions are called, the browser passes an Event object which contains the currentTarget property pointing to the DOM node that has the event handler attached, allowing the per-iteration data to be extracted again.

This allows the same functions (bound only once in the constructor) to be used over and over again through multiple renders with no additional binding.

The only drawback with this method is that objects cannot be attached as DOM attributes, only strings. In my case I dropped the file object and stuck with the numeric index, and used it to look up the file object only when it was needed.

constructor() {
    // Functions are now bound only once during construction
    this.onDragStart = this.onDragStart.bind(this);
    this.onDragEnter = this.onDragEnter.bind(this);
    this.onDragOver = this.onDragOver.bind(this);
    this.onDragLeave = this.onDragLeave.bind(this);
    this.onDrop = this.onDrop.bind(this);
}

onDragStart(event) {
    // 'index' is recovered from the DOM node
    const index = parseInt(event.currentTarget.dataset.index);
    console.log('Event with index', index);

    // Get back the 'file' object (unique to my code, but showing that
    // I could not pass an object through this method and thus had to
    // retrieve it again.)
    const file = (index >= 0) ? this.props.files[index] : null;
}
// Same for other onXXX functions

// No more binds!
render() {
    return (
        <...>
        {this.props.files.map((file, index) => {
            return (
                <tr
                    key={`${index}#${file.name}`}
                    data-index={index}
                    onDragStart={this.onDragStart}
                    onDragEnter={this.onDragEnter}
                    onDragOver={this.onDragOver}
                    onDragLeave={this.onDragLeave}
                    onDrop={this.onDrop}
                />
            );
        })}
        </...>
    );
}
Malvineous
  • 25,144
  • 16
  • 116
  • 151