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

Add more granular sCU functions to stateful components #5026

Open
captainsafia opened this issue Mar 30, 2020 · 5 comments
Open

Add more granular sCU functions to stateful components #5026

captainsafia opened this issue Mar 30, 2020 · 5 comments

Comments

@captainsafia
Copy link
Member

With the new model of each component within stateful-components mapping its own state from Redux (as opposed to a top-level component mapping the state and propagating it down), we have the flexibility to add more granular shouldComponentUpdate methods to each of the components.

For example, the CodeCell component almost never needs to re-render unless the cell_type changes. If the outputs or cell source changes, we should only update the Outputs and Editor components which are children of the CodeCell component, but not the CodeCell component itself.

TL;DR: We should only re-render at the most localized unit of change.

Specifically, we want to add an sCU method to the CodeCell, MarkdownCell, and RawCell to only update if the cell_type, ID, or contentRef changes

Some scenarios to validate when making this change:

  • Updating cell contents still does the right thing
  • Moving cells around still does the right thing
  • Toggling input and output visibility still does the right thing

Tagging this as new-contributor-friendly since it's relatively localized and straightforward to implement and test. Requires familiarity with React.

@captainsafia
Copy link
Member Author

Addendum that I clarified: we've already got pretty granular checks on components like Prompt and Input. The problem is specifically in our top-level cell components which re-render whenever any of the properties in the cell changes. Of course, when they re-render, it causes their children to re-render which defeats the purpose of the granular checks in child components.

@captainsafia captainsafia added this to the April 2020 Release milestone Apr 7, 2020
@captainsafia
Copy link
Member Author

cc: @ramantehlan @barshana-banerjee @kshitijsubedi

Would any of you be interested in taking a look at this?

@ramantehlan
Copy link
Member

Sure :)

@captainsafia
Copy link
Member Author

Thanks! Ping me on Slack if you need help.

@ramantehlan ramantehlan self-assigned this Apr 9, 2020
@captainsafia captainsafia removed this from the April 2020 Release milestone Apr 28, 2020
@stale
Copy link

stale bot commented Jan 9, 2022

This issue hasn't had any activity on it in the last 90 days. Unfortunately we don't get around to dealing with every issue that is opened. Instead of leaving issues open we're seeking to be transparent by closing issues that aren't being prioritized. If no other activity happens on this issue in one week, it will be closed.
It's more than likely that just by me posting about this, the maintainers will take a closer look at these long forgotten issues to help evaluate what to do next.
If you would like to see this issue get prioritized over others, there are multiple avenues 🗓:

  • Ask how you can help with this issue 👩🏿‍💻👨🏻‍💻
  • Help solve other issues the team is currently working on 👨🏾‍💻👩🏼‍💻
  • Donate to nteract so we can support developers to work on these features and bugs more regularly 💰🕐

Thank you!

@stale stale bot added the stale workflow: stale issue label Jan 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants