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

Decide How To Manage Form State In A Performant Way #10551

Closed
Tracked by #8745
JamieB-gu opened this issue Feb 12, 2024 · 2 comments
Closed
Tracked by #8745

Decide How To Manage Form State In A Performant Way #10551

JamieB-gu opened this issue Feb 12, 2024 · 2 comments

Comments

@JamieB-gu
Copy link
Contributor

If we naively lift form state up to the reducer and do nothing else, the performance drops because the whole app re-renders on every key press (typing into the form). Two options to solve this:

  1. Manage the form state at the form component level.
  2. Manage the form state at the top level, and use context to pass it to the form to prevent intermediate components re-rendering.
@JamieB-gu JamieB-gu added this to the DCR for Apps milestone Feb 12, 2024
@github-project-automation github-project-automation bot moved this to Triage in WebX Team Feb 12, 2024
@JamieB-gu JamieB-gu moved this from Triage to This Sprint in WebX Team Feb 12, 2024
@arelra
Copy link
Member

arelra commented Feb 13, 2024

  1. The amount of lag caused by the performance degradation was surprising. It's worth checking if we have any unnecessary re-renders or computation in the discussion component tree. Possible suspects are:
    • chained useEffects that could be derived state instead
    • reassignment of objects which changes their reference and hence causes unnecessary triggering of useEffects
  2. I think I saw memo mentioned in another issue, memo is usually a last resort sticky-plaster. It is usually worth understanding why the re-renders are slow before reaching for memoisation.
  3. Another option is a composition 'trick' which passes components as props to avoid re-renders although this is getting into the esoteric territory of React performance optimisation.
    https://overreacted.io/before-you-memo/
    https://kentcdodds.com/blog/optimize-react-re-renders
    https://www.developerway.com/posts/react-re-renders-guide#part3.2
  4. It seems like we are looking for a heuristic to guide us on where to place state. I think rather than a binary 'local' or 'all at the top' we could perhaps expand this:
    • if state is not dependant or interacts with any other state then it should stay local to the component
    • if state is dependent or interacts with other state then it can move to the top of the discussion tree
  5. Context is a good option for state propagation down a complex tree but we should make it clear that in almost all other cases we favour prop drilling as it is more explicit

@cemms1 cemms1 moved this from This Sprint to In Progress in WebX Team Feb 14, 2024
@alinaboghiu alinaboghiu moved this from In Progress to This Sprint in WebX Team Feb 19, 2024
@alinaboghiu alinaboghiu moved this from This Sprint to Next Sprint in WebX Team Feb 27, 2024
@ioannakok ioannakok moved this from Next Sprint to Backlog in WebX Team Mar 13, 2024
@mxdvl
Copy link
Contributor

mxdvl commented Apr 19, 2024

Fixed by #10531

@mxdvl mxdvl closed this as completed Apr 19, 2024
@github-project-automation github-project-automation bot moved this from Backlog to Done in WebX Team Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

4 participants