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

Editing peformance is rough for large lists #3415

Open
erquhart opened this issue Mar 12, 2020 · 16 comments · May be fixed by #6565
Open

Editing peformance is rough for large lists #3415

erquhart opened this issue Mar 12, 2020 · 16 comments · May be fixed by #6565

Comments

@erquhart
Copy link
Contributor

erquhart commented Mar 12, 2020

Describe the bug

When authoring in a large list, there are long pauses between keystrokes.

To Reproduce

  1. Go to https://www.netlifycms.org/admin/#/collections/updates/entries/releases
  2. Edit one of the list items

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.

@ottozaiser
Copy link

Hello! Do you guys recommend a workaround about this issue?

@lupelius
Copy link

lupelius commented Sep 2, 2020

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?

@magomimmo
Copy link

Hi @erezrokah,
is there any news on this issue? We're very close to deliver the project to the client, but this issue is so severe that will stop the delivery. If I remember well you're working on it.

My best

@erezrokah
Copy link
Contributor

Hi @magomimmo, no progress so far. I had to shift focus to other issues.

@magomimmo
Copy link

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.

@stevenpeniche
Copy link

@erezrokah @erquhart wondering if there's been any progress made on a fix for this?

@erezrokah
Copy link
Contributor

Hi @stevenpeniche, no progress yet. However, we would be happy to accept a contribution for this

@martinjagodic
Copy link
Member

Did anyone investigate, is the source of this issue in the Github API, or is it more of a React/DOM problem?

@erezrokah
Copy link
Contributor

erezrokah commented Jan 4, 2022

or is it more of a React/DOM problem?

For this specific issue, I think ⬆️. See the shouldComponentUpdate issue in the original description.

@herpster
Copy link

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.

@geotrev
Copy link

geotrev commented Sep 27, 2022

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?

@geotrev geotrev linked a pull request Sep 28, 2022 that will close this issue
7 tasks
@geotrev
Copy link

geotrev commented Sep 28, 2022

#6565

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. :)

@geotrev
Copy link

geotrev commented Oct 10, 2022

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 onChange handlers are debounced by 300ms.

This is probably not the most direct way to fix the performance issue but at least makes typing much nicer.

@geotrev
Copy link

geotrev commented Oct 16, 2022

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.

https://cdn.jsdelivr.net/gh/geotrev/netlify-cms@hotfix/cms-perf/packages/netlify-cms/dist/netlify-cms.js

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. :)

https://cdn.jsdelivr.net/gh/USER_NAME_HERE/netlify-cms@hotfix/cms-perf/packages/netlify-cms/dist/netlify-cms.js

Use it just like the docs recommend in your admin/index.html file.

@herpster
Copy link

@geotrev I can't thank you enough, you are just awesome!

@geotrev
Copy link

geotrev commented Jan 1, 2024

Update: the PR to fix this issue was pinned by the decap team and is now stable as of latest master.

🤞🏻 if this goes in, it'll be a late but sweet christmas present!

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

Successfully merging a pull request may close this issue.

9 participants