AllenFang / react-bootstrap-table

A Bootstrap table built with React.js
https://allenfang.github.io/react-bootstrap-table/
MIT License
2.24k stars 782 forks source link

Potential performance issue... #1345

Open crowdwave opened 7 years ago

crowdwave commented 7 years ago

I noticed very slow performance on one of my pages that uses react bootstrap table.

Specifically, when I type text into a field on my page, it is very slow for the text to appear. Of course reactjs does a rerender with every keypress in the field.

So I wondered if maybe with every rerender that react-bootstrao-table was rerendering.

So I ran npm install why-did-you-update and on my page at the top I put:

import {whyDidYouUpdate} from 'why-did-you-update' whyDidYouUpdate(React)

The output in the console says that react-boostrap-table is doing alot of possibly unnecessary updates, and because it the table is complex, it slows the entire page down quite dramatically because the bootstrap table re-renders with every keypress in my input fields, even though the input fields have nothing to do with the react-bootstrap-table.

So I am wondering, is it possible that react-bootstrap-table is rerendering even when it does not get new props that require it to update?

And great work Allen - I love the table and use it for all my projects!

thanks!

AllenFang commented 7 years ago

@crowdwave, I'll improve it, it's a known issue from long time ago... and did you enable the row selection? and how many row in your table?

crowdwave commented 7 years ago

There is anywhere from a few hundred rows to 1,000 rows in the table, and seven columns.

Which option are you referring to when you ask about row selection? I have checkbox selection turned on.

Out of interest, how would you address this? Perhaps to use the deep-diff npm package to compare nextProps table data with existing table data in componentWillRecieveProps?

AllenFang commented 7 years ago

Which option are you referring to when you ask about row selection? I have checkbox selection turned on.

yes, that's what I said

Out of interest, how would you address this? Perhaps to use the deep-diff npm package to compare nextProps table data with existing table data in componentWillRecieveProps?

Yes, I know how to do it, but just too busy... so I still do some little or critical bug. At before, if a large table here and select a row, it'll be very slow so I enhanced once but still not good,

But to be honest, my component structure is very bad, actually, this repo is a practice for me to practice React.js but i've no idea why it growth well in last year... but just no time to do a large refactoring..

crowdwave commented 7 years ago

EDIT THIS IS WRONG: The actual solution to this problem is to use immutablejs and describing how to do that is not easily done. Below was my naive first solution but I found it did not work.

For historicial reasons, here is the wrong post:

For anyone else encountering this issue.

Any page on my application using a large or complex react-boostrap-table was taking a big performance hit because react-bootstrap-table rerenders the entire table on every change in a parent component.

This may not suit your needs, but I addressed this by putting react-bootstrap-table into a higher order component https://facebook.github.io/react/docs/higher-order-components.html that implements shouldComponentUpdate and examines the props.data that gets passed in, and if the data has not changed then it does not re-render react-bootstrap-table.

It is dependent on https://www.npmjs.com/package/deep-equal for comparing the inbound table data to the previous table data.

You should be cautious because I do not use refs and I read somewhere whilst implementing this that higher order components do not pass refs or something like that - so if you are using refs you might need to do some study.

In your component where you currently display BootstrapTable, change it to this:

import withBootstrapTable from './withBootstrapTable'
const BootstrapTableHOC = withBootstrapTable(BootstrapTable)

export default class MyExampleTableOfData extends React.Component {

  render() {
    return (
              <BootstrapTableHOC data={this.state.currentTableData}
                              ref="table">
                <TableHeaderColumn dataField="exampleDataY"
                                   dataAlign="left">Y</TableHeaderColumn>
                <TableHeaderColumn dataField="exampleDataX"
                                   width="150px"
                                   dataAlign="center">X</TableHeaderColumn>
              </BootstrapTableHOC>

    )
  }
}

And then in a separate file named "withBootstrapTable.jsx"

import React from 'react'
import PropTypes from 'prop-types'
import de from 'deep-equal'

const withBootstrapTable = (WrappedComponent) => class extends React.PureComponent {

  constructor(props) {
    super(props)
  }

  shouldComponentUpdate(nextProps) {
    return !de(this.props.data, nextProps.data)
  }

  render() {
    return <WrappedComponent {...this.props} />
  }
}

export default withBootstrapTable
sonagrigoryan commented 6 years ago

Hi. Are there any updates on this issue? I am experiencing really bad UI performance when trying to select a row in a table that has multiple (around 100) rows. Seems like it re-renders the entire table (even the rows that are hidden). Anything that I can try to improve this issue? Thanks

crowdwave commented 6 years ago

You need to provide alot more information about what your table is doing, show the code.

I can't make suggestions without detailed information.

sonagrigoryan commented 6 years ago

This is the table I am using:

<div>
    <BootstrapTable data={ this.props.people } striped height="400" scrollTop="Bottom" selectRow={ this.selectedRowRules } options={ rowSortingOptions }>
        <TableHeaderColumn dataField="gKey" hidden isKey>GKey</TableHeaderColumn>
        <TableHeaderColumn dataField="id" hidden>ID</TableHeaderColumn>
        <TableHeaderColumn width="150" dataField="description">Description</TableHeaderColumn>
        <TableHeaderColumn width="150" dataField="time">Time</TableHeaderColumn>
        <TableHeaderColumn width="150" dataField="person">Person</TableHeaderColumn>
        <TableHeaderColumn width="100" dataField="status">Status</TableHeaderColumn>
        <TableHeaderColumn width="80" dataField="info">Info</TableHeaderColumn>
    </BootstrapTable>
</div>

The rules for select:

this.selectedRowRules = {
    mode: 'checkbox',
    clickToSelect: true,
    hideSelectColumn: false,
    selected: [],
    className: 'table-row-selected', // Important for eliminating the issue with bootstrap overriding styles for selection/hover/striped
    onSelect: this.props.updateSelectedRows,
};

Sorting options:

const rowSortingOptions = {
    defaultSortName: 'time',  // default sort column name
    defaultSortOrder: 'asc',             // default sort order
    sortIndicator: false,
};

Selecting the row, I just take the key of the row and store it. Nothing that can slow the performance. So, my problem is that whenever there are many rows in the table, selecting a row renders the entire table, not just that row. And since I have say 1000 rows, if I have updates quite often, the table just freezes the page and comes back in a bit.

Also, if the array that contains table data gets a burst of updates, it will just crash the page.

Any ideas what can be done for this?

Thanks in advance.

crowdwave commented 6 years ago

I can't give you an easy solution because although I solved it for myself, my solution is specific to how my code is structured. Hopefully I can point you in the right direction though and you might be able to work it out.

The key thing to understand is that you are correct - react-bootstrap-table rerenders every time it gets new props. This can be a huge performance hit.

So, I solved my problem, by ensuring that react-bootstrap-table updates only when the data passed in to the table actually changes. Caution - I'm not sure that this is the same problem as you are having. It might be, I don't know - you'll have to work that out.

There are some really important things to understand about preventing updates.

The core of the solution revolves around comparing the last set of data rendered to the new set of data that arrives into your component. The key thing to understand here is that comparing JavaScript objects/arrays is a really hard problem... if you have never looked into the detail of this then you do not understand how hard the problem actually is. So you need a very reliable way to compare the old data to the new data. My advice to you is do not attempt to do this comparision in any other way except to use the immutablejs library. immutablejs comparisons actually work cleanly, but the price is you need to understand and use immutablejs. I suggest this is a good thing to do anyway if you are a professional JavaScript programmer.

So now that you know how to properly compare old data to new data, you need to stop bootstrap-table from rendering if the data is the same. The key to this is the reactJS shouldComponentUpdate function. In the React lifecycle, this function is run prior to a component rendering, and if the function returns false then no render occurs. So it is in this function that you compare old data to new. If you got new data, then return true. If the data is the same, return false. And that is where the problem is solved - if you returned false then the component does not render and thus your react-bootstrap-table does not render over and over.

My solution was to create a HOC (higher order component) that wraps the bootstrap table and does nothing but ensure that the react-bootstrap-table is rendered only if the inbound data has changed.

I'm sorry this is not a complete solution and I am not in a position to fully explain HOCs - you'll have to do that yourself.

It's really pretty simple in the end once you understand the two things described above...... 1: that you must use immutablejs to compare old and new data 2: that you do that comparison in the shouldComponentUpdate() function

import React from 'react'
import immutable from 'immutable'

const withBootstrapTable = (WrappedComponent) => class extends React.Component {

  constructor(props) {
    super(props)
    // data starts out as an immutable list so that we can do an immutable equals later
    this.previousData = immutable.List()
  }

  shouldComponentUpdate(nextProps) {
    let doUpdate = !(this.previousData.equals(nextProps.data))
    return doUpdate
  }

  render() {
    let data = this.props.data.toJS()
    this.previousData = this.props.data
    if (this.props.data.size === 0) {
      return <WrappedComponent {...this.props} data={[]}/>
    } else {
      return <WrappedComponent {...this.props} data={data}/>
    }
  }
}

export default withBootstrapTable
sonagrigoryan commented 6 years ago

Thank you, @crowdwave . I will look into this.