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

Solid Table: Examples from docs cause unnecessary rerenders of components when data updates #5019

Open
2 tasks done
raymondboswel opened this issue Aug 11, 2023 · 4 comments

Comments

@raymondboswel
Copy link

raymondboswel commented Aug 11, 2023

Describe the bug

When implementing a table as shown in the examples in the documentation site, a change to the underlying data causes every row and cell in the table to rerender (instead of updating in a fine grained manner). This is noticeably slow in a real world use case where table edits are supposed to navigate to the next row.

I've noticed that if I use Key instead of For elements, the rows don't rerender, but the cells still do. Only replacing flexRender with a manual <Switch / Match > implementation prevents the cells from rerendering. This however breaks other features, such as row selection & column ordering.

In the screenshot you can see the output of changing first name of a row in each case.

Your minimal, reproducible example

https://codesandbox.io/p/github/raymondboswel/solid-table-rendering-example

Steps to reproduce

  1. Open the codesandbox link.
  2. Open the devtools console
  3. Click the Update button for each table
  4. Notice that in the last case, the Cell component isn't re-executed.

Expected behavior

As a user I expect fine grained updates in the table, in order to have decent table performance.

How often does this bug happen?

Every time

Screenshots or Videos

image

Platform

Chrome / Linux

react-table version

8.9.3

TypeScript version

No response

Additional context

No response

Terms & Code of Conduct

  • I agree to follow this project's Code of Conduct
  • I understand that if my bug cannot be reliable reproduced in a debuggable environment, it will probably not be fixed and this issue may even be closed.
@sledgehammer999
Copy link

Your description of the bug sounds awfully similar to what I reported for Svelte in #4962.
Just a heads up. I am no JS/Solid/Svelte expert. I can't debug it more.

@raymondboswel
Copy link
Author

Hi @sledgehammer999 - I'm not sure what the fix is, but I think the cause is that flexRender recreates the components. Libraries with VDOM like React do that all the time anyways, but the VDOM buffers the actual DOM updates, so it's not super expensive, whereas Svelte & Solid write to the DOM when a component is created, so it's a lot more costly.

@sledgehammer999
Copy link

I thought that, Svelte at least, was able to tell exactly which values/components had changed and update only those elements directly. AFAICT, flexRender() for Svelte just returns a JS class(not an instance) which is a subclass/derived class of SvelteComponent.

I could be wrong though. In any case, sorry for hijacking your thread. You don't have to reply to this comment.

@AlexErrant
Copy link

I iterated on your example and got everything working (including row selection and column ordering) with <Dynamic/>, <Index/>, and <Key/>. (Nice job finding Key, btw.)

https://stackblitz.com/github/AlexErrant/solid-table-rendering-example

Notice how the table more granularly rerenders.


The Solid docs state:

When the list value changes in <For>, the entire list is re-rendered.

createSignal's setMyData changes the list value, so the entire table re-renders. So I use <Index/> in places where the index is stable, and <Key/> where it is not.

You can also use createStore instead of createSignal and retain the above granularity like so:

  const [data, setData] = createStore(initialData);
  const table = createSolidTable({
    get data() {
      return [...data];
    },

To be clear: this is not 100% optimal, per Table's docs:

When the data option changes reference (compared via Object.is), the table will reprocess the data. Any other data processing that relies on the core data model (such as grouping, sorting, filtering, etc) will also be reprocessed.

Make sure your data option is only changing when you want the table to reprocess. Providing an inline [] or constructing the data array as a new object every time you want to render the table will result in a lot of unnecessary re-processing. This can easily go unnoticed in smaller tables, but you will likely notice it in larger tables.

But at least my solution updates the UI without re-rendering the entire table.

(I'm confused because there are zero occurrences of Object.is in the Table codebase, even at the time that doc was written, and when building a demo app there are still no occurrences of Object.is in the build output so it isn't in a dependency... but whatever. Perhaps they meant ===.)


I believe that solid-table's flexRender should be removed. Since Solid has Dynamic, there's no reason for solid-table to export a wrapper for it...? Since this is a breaking change, we should update the wrapper to use Dynamic and remove it for v9. I will submit a PR for this @KevinVandy if you approve the concept. Also willing to work on examples/docs.

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

3 participants