perf(dashboard): decouple redux props from dashboard components #16421
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #16421 +/- ##
==========================================
- Coverage 76.64% 76.54% -0.10%
==========================================
Files 1000 1000
Lines 53489 53494 +5
Branches 6816 6819 +3
==========================================
- Hits 40996 40949 -47
- Misses 12257 12307 +50
- Partials 236 238 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
villebro
approved these changes
Aug 24, 2021
| @@ -152,7 +152,7 @@ const FilterFocusHighlight = React.forwardRef( | |||
| }, | |||
| ); | |||
|
|
|||
| class ChartHolder extends React.Component { | |||
| export class ChartHolder extends React.Component { | |||
There was a problem hiding this comment.
is this export needed somewhere?
| @@ -77,7 +78,7 @@ Click here to edit [markdown](https://bit.ly/1dQOfRK)`; | |||
|
|
|||
| const MARKDOWN_ERROR_MESSAGE = t('This markdown component has an error.'); | |||
|
|
|||
| class Markdown extends React.PureComponent { | |||
| export class Markdown extends React.PureComponent { | |||
|
LGTM ! |
junlincc
added
#bug:performance
junlincc
added
test_priority:NA
and removed
need:merge
amitmiran137
pushed a commit
to nielsen-oss/superset
that referenced
this pull request
Aug 29, 2021
* upstream/master: fix: create example DB if needed (apache#16451) fix(native-filters): add handle undefined control value gracefully (apache#16468) Revert "chore: Changes the DatabaseSelector to use the new Select component (apache#16334)" (apache#16478) fix(explore): JS error for creating new metrics from columns (apache#16477) fix: queryEditor bug (apache#16452) docs: make code snippet usable with required imports (apache#16473) perf(dashboard): decouple redux props from dashboard components (apache#16421) perf(dashboard): reduce number of rerenders of Charts (apache#16444)
This was referenced Oct 14, 2021
opus-42
pushed a commit
to opus-42/incubator-superset
that referenced
this pull request
Nov 14, 2021
…he#16421) * perf(dashboard): decouple redux props from dashboard components * Lint fix * Dont make copy of filters object * Remove unnecessary exports
QAlexBall
pushed a commit
to QAlexBall/superset
that referenced
this pull request
Dec 28, 2021
…he#16421) * perf(dashboard): decouple redux props from dashboard components * Lint fix * Dont make copy of filters object * Remove unnecessary exports
mistercrunch
added
🏷️ bot
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
In DashboardComponent (a parent component of dashboard components such as ChartHolder, Tabs, Column etc.) we had a large mapStateToProps function, which maps redux state to props. A lot of those props were used only in some components, but since they were passed to every component, each change triggered a rerender of all components instead of the ones that actually use the changed prop. This PR decouples those props from components that don't need them.
For example -
directPathToChildis used only byChartHolderandTabs, but any change of that prop triggered a rerender of all components on dashboard.This PR is the first of a series of PRs that will come soon - all related to reducing dashboard rerenders.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
No changes
TESTING INSTRUCTIONS
Every dashboard functionality should work as it did before
ADDITIONAL INFORMATION
CC @junlincc @jinghua-qa