GriddleGriddle / Griddle

Simple Grid Component written in React
http://griddlegriddle.github.io/Griddle/
MIT License
2.5k stars 377 forks source link

Double rendering of all rows #762

Open Cremz opened 7 years ago

Cremz commented 7 years ago

Hi, I'm using an older version of Griddle (0.2.4) with some custom tweaks therefore i cannot update to newer versions. We are rendering a large number of rows (sometimes up to 200K) and i've noticed that scrolling is really bad. After checking out the code, it seems that each time we do any action, griddle renders twice. the main react component that has Griddle only renders once. I'm not sure what within the lifecycle of the Griddle component triggers another render. Any advice on what i should look for? I put console logs in componentDidUpdate of Griddle and i get two calls to that method. On initial page load i get calls to componentDidMount and componentDidUpdate one after the other, sometimes with as much as 5 seconds delay.

Also, is it normal that the gridRows get rendered when scrolling? each time i scroll, all rows get rerendered.

radziksh commented 6 years ago

@Cremz I had a very similar problem and found 2 reasons, 1) griddle version less than 1.10.1 has the following method in index.js that updated state: (raw)

_createClass (Griddle, [{
     key: 'componentWillReceiveProps',
     value: function componentWillReceiveProps (nextProps) {
       var data = nextProps.data,
           pageProperties = nextProps.pageProperties,
           sortProperties = nextProps.sortProperties;
       this.store.dispatch (actions.updateState ({data: data, pageProperties: pageProperties, sortProperties: sortProperties}));
     }

as you can see here, griddle whose version is less than 1.10.1 updates its internal state each time it receives new props (even if they are not different from the old ones). Most likely this is the reason for an additional re-render

but in griddle version >= 1.10.1 the state is updated only if the props have really changed:

  componentWillReceiveProps(nextProps) {
    const newState = _.pickBy(nextProps, (value, key) => {
      return this.props[key] !== value;
    })

    // Only update the state if something has changed.
    if (Object.keys(newState).length > 0) {
     this.store.dispatch(actions.updateState(newState));
    }
  }

maybe you should somehow rewrite this method in version 0.x. but this is one problem - the second will arise after solving the first. It is that if you send anonymous objects to griddle, then will be problem with this line:

return this.props[key] !== value;

it will return true because {} !== {} is true in javascript. for example instead of:

class SomeComponent extends React.Component{
  render(){
    return {
     <Griddle 
        data={props.data} 
       components={{
         Row: <SomeRow/>
       }} 
       storeKey="griddleStore" />
   }
  }
}

should be:

const components = {
     Row: <SomeRow/>
}
class SomeComponent extends React.Component{
  render(){
    return <Griddle 
        data={props.data} 
       components={components} 
       storeKey="griddleStore" />
  }
}

Only in this case there will be no additional re-renders.

see https://github.com/GriddleGriddle/Griddle/issues/767 https://github.com/GriddleGriddle/Griddle/pull/768

Cremz commented 6 years ago

thank you so much, i will try it out and post an update with the results