Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-render cell components without re-rendering the whole table.. Is this possible? #754

Closed
NickyYo opened this issue Jan 17, 2018 · 17 comments

Comments

@NickyYo
Copy link

NickyYo commented Jan 17, 2018

No description provided.

@gary-menzel
Copy link
Contributor

You need to provide more detail on what you are trying to do. ReactTable already applies appropriate keys at various levels to allow React to determine what needs to be re-rendered. But, essentially, React is designed to re-render what changes and forced rendering should be avoided.

@mreishus
Copy link

mreishus commented Jan 19, 2018

I ran into something that might be an example of this issue. I wanted to make a table with editable cells. I saw the storybook used contenteditable divs, but I wanted to try with input elements. So I used this as my renderEditable:

            <input
                type="text"
                className="form-control"
                disabled={isReadOnly}
                onChange={e => {
                    const tableData = [...this.state.tableData];
                    tableData[cellInfo.index][cellInfo.column.id] = e.target.value;
                    this.setState({ tableData });
                }}
                value={
                    tableData[cellInfo.index][cellInfo.column.id] != null
                        ? tableData[cellInfo.index][cellInfo.column.id]
                        : ""
                }
            />

With this, one keepress takes 30-40ms to process. The chrome devtools show various parts of react-table running updates:

chrome_2018-01-19_14-41-42

It's a table with 3 rows and 9 columns. I'm only updating one of the cells but it seems to spend time rerendering all of the cells. It might be because the data={tableData} I pass to <ReactTable ends up being a completely new object, meaning that a shallow compare would be different, so that kicks off everything under it rerendering. I haven't tried it, but maybe if the Tr and Td components did a deep equality comparison of their data in shouldComponentUpdate it might make my use case faster?

In my case, I'll probably have to go back to the contenteditable divs vs inputs, because the typing can be jerky for fast typists with a 40ms time to process a keystroke. My reason for trying inputs was that I wanted to be able to kick off an auto-save without necessarily waiting for a blur event.

@gary-menzel
Copy link
Contributor

This question of deep compare has been raised in other issues in the past and the author has determined it won't be added as that will also cause significant delays as the data set grows. The Td and Tr components are only "pure" components - so they don't have access to the React lifecycle.

In the example you provide, that is exactly why using a controlled component (i.e. the INPUT) is a bad idea (and why the DIV is used as it is not controlled by React). As far as kicking off an auto-save, you need to debounce the keypresses and only call save when they stop typing. This is also the same if you want to save the configuration of things like the columns sizing.

A new version may end up implementing other callbacks to say when some of these events have completed - but I suspect that version is a long way off at the moment due to other projects going on.

@mreishus
Copy link

Re: Deep compare - I suppose if the author has made a decision, then it's settled.

Re: The autosave, I didn't find an appropriate way to kick off an event when they stopped typing, even using debouncing. onBlur works fine, but onChange isn't fired for a contenteditable div, and I didn't want to require the user to click outside a field. I tried using onInput but I had problems with the caret jumping around. I might have to make a child contenteditable component that uses react lifecycle events to manage this: https://stackoverflow.com/questions/22677931/react-js-onchange-event-for-contenteditable

@gary-menzel
Copy link
Contributor

I should have been clearer.... if you use the INPUT (to get the onChange) then use an uncontrolled input (so the onChange is not actually updating the input field itself) and then debounce the function that you will use to save - and trigger the debounce function in the onChange). Although, in my own code, I am not saving until they blur (as round trips to a database are expensive even if you aren't using React).

Pushing the data back up the React stack and then back down into the props is going to be problematic in this instance.

Having said that, ReactTable is still very performant without the deep compare. Here is a "real time" example of cell updates that can be set to run very quickly.

https://codesandbox.io/s/wkxolyrpo5

It still happily runs at 50ms (even though it starts pushing the CPU hard).

@mreishus
Copy link

mreishus commented Jan 19, 2018

I'll try that. I rarely use uncontrolled inputs, so the idea didn't occur to me.

BTW, I looked at the source. I think that makeTemplateComponent makes stateless functional components, but it does not make pure components. So those will update even if a shallow compare of the props is the same. See facebook/react#5677

For complex components, defining shouldComponentUpdate (eg. pure render) will generally exceed the performance benefits of stateless components.

It might be worth trying those components as es6 classes extending PureComponent, assuming my reading of that issue is correct and that stateless functional components don't implement any special shouldComponentUpdate. I might try it if I can figure out how to modify a library :)

@gary-menzel
Copy link
Contributor

I think you will find it is best to redesign the library.

@NickyYo
Copy link
Author

NickyYo commented Jan 20, 2018

In my case, I wanted to apply a 'selected' className to a column of cells, see screenshot

screen shot 2018-01-20 at 14 09 45

This was quite easy to do because of this projects components extensibility, good work btw.

However, applying the className to cells causes everything to re-render.. This is just fricking crazy... all i want to do is apply a css class.. data in the table hasn't changed.. also, a table with 60 columns & rows took a good 300ms to process a css change..

Great project for extensibility but it's at the sacrifice of proper react life cylce management.

@mreishus
Copy link

I spent a little time trying solutions but I wasn't able to find anything that worked. I still have a test repository that I created to try out modifications on a react-table using controlled inputs, you can see my attempts here:

mreishus/react-table-test@3335224
mreishus/react-table-test@e496506

I will change to uncontrolled-inputs and stop working on this. Thanks for the explanation, it does seem to be a tough problem given the way that react handles shouldComponentUpdate and children.

@maxparelius
Copy link

I have had the same issues. This library is great in theory but fails to perform well under production environment. We should be given access to shouldComponentUpdate lifecycle methods in each cell so we can control when things re-render or not. This seems like an obvious need to me. Otherwise, this library is somewhat useless.

@texttechne
Copy link

@mreishus @maxparelius Instead of an uncontrolled component or contentEditable you can just create your own input component having its own state management. This way concerns are separated: React-Table does what it can do => shallow compare & you take care about rerendering your own component.

Works with your own custom components, mobx observers, ....

@mhafer
Copy link

mhafer commented Feb 15, 2019

I ended up using forceUpdate. Initially this table of 160 entries takes a few seconds to load, but every click after that is instant. The table is a nested component, on some of my other (shallower) tables they updated without calling forceUpdate.

  @action handleIncludeClick = (chk: boolean, id: string): void => {
       chk ? this.props.store.addAttendeeById(id) : this.props.store.removeAttendee(id);
       this.forceUpdate();
  } 

Header: "",
accessor:"included",
Cell: (row: any) => {
            const id: string = row.original.userId;
            return (
              <Checkbox
                checked={store.isIncluded(id)}
                onChange={(e: any, chk: boolean) => { this.handleIncludeClick(chk, id) }}
              />
            )
          }       

@SourceCipher
Copy link

If you enable pagination, you will not feel the delay (depends how many rows per page) even if you do a forceUpdate.

What we ended up doing because we wanted to control various states and updates constantly in each cell, we rendered each cell as a separate component which is controller via controller.

This way if you update table data for a specific row and specific cell, only that cell will re-render and nothing else. Its a hassle to do this but works great. I wish the v7 can handle all of that automatically

@Progue89
Copy link

Progue89 commented Dec 24, 2019

Can you provide an example for V7 where only the cell renders and not the full table. I am having issues when every time I make any change everything renders on the table

@kishan143-jaiswal
Copy link

Facing with the same issues , adding data or editing cell values is re-rendering whole table cells

@yyyyaaa
Copy link

yyyyaaa commented Jul 22, 2021

If you enable pagination, you will not feel the delay (depends how many rows per page) even if you do a forceUpdate.

What we ended up doing because we wanted to control various states and updates constantly in each cell, we rendered each cell as a separate component which is controller via controller.

This way if you update table data for a specific row and specific cell, only that cell will re-render and nothing else. Its a hassle to do this but works great. I wish the v7 can handle all of that automatically

Hi sorry to ping you about this but do you have any github gist of the solution mentioned? Appreciate it

@Viktor19931
Copy link

This issue still present in FixedSizeGrid
Any idea how to fix it ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests