-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
fix(sqllab): remove set state on component update lifecycle #21771
fix(sqllab): remove set state on component update lifecycle #21771
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Can you add more details on how to reproduce this max-depth-call-stack error? |
Codecov Report
@@ Coverage Diff @@
## master #21771 +/- ##
==========================================
+ Coverage 66.85% 66.89% +0.04%
==========================================
Files 1800 1805 +5
Lines 68967 69064 +97
Branches 7339 7369 +30
==========================================
+ Hits 46107 46203 +96
+ Misses 20968 20953 -15
- Partials 1892 1908 +16
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@ktmud I updated the step to reproduce |
LGTM! Can we merge it? |
SUMMARY
Since the legacy TabbedSqlEditor component uses
UNSAFE_componentWillReceiveProps
lifecycle method to update the state foreditorQueries
anddataPreviewQueries
, which only consumes in the result panel.This can also violate the maximum update depth exceeded in some point.
This commit deprecates
UNSAFE_componentWillReceiveProps
implementation and moves to the SouthPane connector where it actually needs.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
![Screen Shot 2022-10-11 at 11 00 46 AM](https://user-images.githubusercontent.com/1392866/195172412-44f5ee8a-5fc7-4515-9e3e-0bdf84155c8f.png)
After:
No errors
TESTING INSTRUCTIONS
SQLLAB_BACKEND_PERSISTENCE
redux.sqlLab.tables
(remove the table data associated to the current queryEditor id) from the array) in order to trigger the following setState update during lifecycle period.superset/superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx
Lines 213 to 216 in 52d33b0
ADDITIONAL INFORMATION
cc: @ktmud