Skip to content
This repository has been archived by the owner on Sep 9, 2024. It is now read-only.

Performance: Improve input speed for widgets #34

Closed
geotrev opened this issue Oct 12, 2022 · 3 comments
Closed

Performance: Improve input speed for widgets #34

geotrev opened this issue Oct 12, 2022 · 3 comments
Labels
enhancement New feature or request
Milestone

Comments

@geotrev
Copy link

geotrev commented Oct 12, 2022

Really cool to see this fork! It looks promising so far and I just want to pull over a commonly reported perf issue from NCMS as it was still an issue at the time of the fork... I'm happy to implement the change as well since I already did it over there. lol

Problem

Currently, typing in input fields works fine so long as a collection doesn't contain a lot of inputs and/or lists.

At a certain point when you have enough lists and inputs, the input delay basically ruins the experience. Having opened lists while typing multiplies the performance issue.

Cause

I believe the issue is that for each keystroke to any widget receiving text input, the redux store is recalculating its state and populating it to every single component. This change can take up to 300ms or more per key stroke for large data sets (think: kitchen sink with multiples of each widget, including multiple lists with widgets inside).

Possible fix

Debounce props.onChange in widgets that receive text input.

Affected widgets:

  • Number
  • String
  • Text
  • Markdown
  • Code
  • DateTime
  • Color

Technical change should be straightforward although a bit tedious. Instead of always calling props.onChange, instead use a piece of internal state to quickly update the input value, and debounce the props.onChange handler (onChange is probably not a good name for this prop anymore, should be something like handleCollectionUpdate maybe?).

Tangent optimizations

Another thing I noticed is that collapsed List or Object widgets are still rendering their DOM, even though they're visually hidden. Completely unrendering those portions of the component would at least improve on reconciliation speed, although I'm sure it's negligible.

Alternatives / thoughts

I'm not sure if it will be possible to instead debounce the actual redux store update (but still continuously calculate the value so the draft state is still correct), but that would be the "smarter" approach. I still think regardless of that change, the inputs should maintain their state internally to ensure state is as fast as possible a la native html.

Another consideration could be to rearchitect the app so the collection data and UI state are more loosely coupled. As an example, any changes to the UI (via input or widget inclusion) trigger an update to the collection data, but the UI itself is only driven by the data on initial page load.

@KaneFreeman KaneFreeman added discussion enhancement New feature or request labels Oct 13, 2022
@KaneFreeman
Copy link
Collaborator

I would inclined to take a combined approach of debouncing the value to redux (maybe something like every 100ms) AND making the inputs uncontrolled (manage their own values instead of relying on redux to tell them).

I am working on a massive revision of the app right now (typescript conversion, removing immutable js, updating several dependencies, switching to a material UI). My goal is to have it far enough long to merge it into the main branch and start pushing to the @next tag on npm by the end of next week (right now the changes are in this branch: https://github.com/StaticJsCMS/static-cms/tree/feature/typescript-conversion). Once that is merged in, we can start looking into this. Feel free to open some PRs for it.

@KaneFreeman KaneFreeman reopened this Oct 13, 2022
@KaneFreeman KaneFreeman mentioned this issue Oct 13, 2022
11 tasks
@KaneFreeman KaneFreeman moved this to Todo in Static CMS Oct 13, 2022
@KaneFreeman
Copy link
Collaborator

I think I will actually take care of the decoupling of the widgets' ui from redux in the rewrite. I am already changing a lot of the internal struggle of them to be more uniform, so its a good chance to make this change.

@geotrev
Copy link
Author

geotrev commented Oct 15, 2022

Sounds good. :shipit:

@KaneFreeman KaneFreeman moved this from Todo to In Progress in Static CMS Oct 20, 2022
@KaneFreeman KaneFreeman moved this from In Progress to Todo in Static CMS Oct 24, 2022
@KaneFreeman KaneFreeman added this to the v2.0.0 milestone Jan 27, 2023
@github-project-automation github-project-automation bot moved this from Todo to Done in Static CMS Apr 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
Archived in project
Development

No branches or pull requests

2 participants