GriddleGriddle / Griddle

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

Inline editing - performance issues #728

Closed andreme closed 6 years ago

andreme commented 7 years ago

I was looking at using Immutable records to improve performance and while digging into it I was wondering why the rowDataSelector uses a find instead of the lookup array?

Original

export const rowDataSelector = (state, { griddleKey }) => {
  return state.get('data')
    .find(r => r.get('griddleKey') === griddleKey).toJSON();
};

Updated

export const rowDataSelector = (state, { griddleKey }) => {
  return state.get('data').get(state.getIn(['lookup', griddleKey.toString()])).toJSON();
};
andreme commented 7 years ago

I'm also fairly sure that there is a bug in GRIDDLE_UPDATE_STATE,

const newState = _.omit(action.newState, data);

should be

const newState = _.omit(action.newState, 'data');

?

ryanlanciaux commented 7 years ago

Thanks a ton for looking into this! Everything should be using the lookup where possible so maybe that was something that was missed in the transition.

There very well could be a bug with the _.omit and update state.

andreme commented 7 years ago

I've fixed these things in #729

andreme commented 7 years ago

One reason that Rows and Cells get re-rendered is the use of Immutable.toJSON() in rowDataSelector, rowPropertiesSelector, cellPropertiesSelector. .toJSON() generates a new JS object every time it's called, breaking reacts shallow compare for props.