-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Editing peformance is rough for large lists #3415
Comments
Hello! Do you guys recommend a workaround about this issue? |
this is still happening, neither this one or #2009 seems to be fixed, my project needs a massive refactor for working around this putting each entry in its own file, when can we expect a fix please? |
Hi @erezrokah, My best |
Hi @magomimmo, no progress so far. I had to shift focus to other issues. |
Thanks @erezrokah, we'll see what we can do about it. Eventually we'll ask you some suggestions on the best way you think it's needed to fix it and what are the minimum requirement from you're side. |
@erezrokah @erquhart wondering if there's been any progress made on a fix for this? |
Hi @stevenpeniche, no progress yet. However, we would be happy to accept a contribution for this |
Did anyone investigate, is the source of this issue in the Github API, or is it more of a React/DOM problem? |
For this specific issue, I think ⬆️. See the |
Any news on this in 2022? I would like to use the CMS for a professional project, but the delay in input is so drastic that it makes little sense to pursue it further. |
I just filed #6563 because I didn't see this issue sooner. I see why, it's quite old! Off the bat, it seems like a lot of child components are re-rendering even when their props are unchanged. Could a quick win here be to memoize each list item component so only "real" prop changes (e.g. changed field values) result in re-renders? |
Here's a PR on an incremental fix to at least improve perf when object or list widgets are collapsed. I haven't had the time to dig through the redux store / overall component reconciliation strategy, but my basic thesis here is: if a module is collapsed/visually hidden, its content doesn't need to render, thus saving time on reconciliation. There is a noticeable improvement on the kitchen sink example with this fix. If this seems reasonable I can go ahead and update tests/get it ready for approval. :) |
I must be the only one posting about this... anywho, i went ahead and updated all the input components so they use a contained state variable for their values, and then their This is probably not the most direct way to fix the performance issue but at least makes typing much nicer. |
I committed the dist file of the CMS in my fork so others can get this fix sooner, if needed. I don't plan to delete the fork/branch so until Netlify fixes it, have at it. If any of y'all have issues, feel free to post here.
The exact changes in that dist are viewable in my fix PR (see above comment). Alternatively, just fork my branch from this repo and update the above URL with your gh username. :)
Use it just like the docs recommend in your |
@geotrev I can't thank you enough, you are just awesome! |
Update: the PR to fix this issue was pinned by the decap team and is now stable as of latest 🤞🏻 if this goes in, it'll be a late but sweet christmas present! |
Describe the bug
When authoring in a large list, there are long pauses between keystrokes.
To Reproduce
Expected behavior
Updates should be snappy.
Applicable Versions:
CMS 2.10.31
Additional context
This is a long known issue, just needed to be documented.
shouldComponentUpdate
is currently set to always update on lists because the list widget doesn't know if it's children require updates from whatever state change is coming down.I'm certain we can be smarter about this logic. If we can get a quick easy win, let's at least prioritize it. If it's likely to be a bigger lift, let's just get some thoughts down and prioritize later.
An improvement here will likely improve perf across the editor, or at least pave the way for general perf improvement.
The text was updated successfully, but these errors were encountered: