fix(sqllab): remove set state on component update lifecycle (#21771)

superset-frontend/src/SqlLab/components/SouthPane/SouthPane.test.jsx

@@ -25,60 +25,25 @@ import SouthPaneContainer from 'src/SqlLab/components/SouthPane/state';
 import ResultSet from 'src/SqlLab/components/ResultSet';
 import '@testing-library/jest-dom/extend-expect';
 import { STATUS_OPTIONS } from 'src/SqlLab/constants';
-import { initialState } from 'src/SqlLab/fixtures';
+import { initialState, table, defaultQueryEditor } from 'src/SqlLab/fixtures';
 import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
 
 const NOOP = () => {};
 
 const mockedProps = {
-  editorQueries: [
-    {
-      cached: false,
-      changedOn: Date.now(),
-      db: 'main',
-      dbId: 1,
-      id: 'LCly_kkIN',
-      startDttm: Date.now(),
-    },
-    {
-      cached: false,
-      changedOn: 1559238500401,
-      db: 'main',
-      dbId: 1,
-      id: 'lXJa7F9_r',
-      startDttm: 1559238500401,
-    },
-    {
-      cached: false,
-      changedOn: 1559238506925,
-      db: 'main',
-      dbId: 1,
-      id: '2g2_iRFMl',
-      startDttm: 1559238506925,
-    },
-    {
-      cached: false,
-      changedOn: 1559238516395,
-      db: 'main',
-      dbId: 1,
-      id: 'erWdqEWPm',
-      startDttm: 1559238516395,
-    },
-  ],
+  queryEditorId: defaultQueryEditor.id,
   latestQueryId: 'LCly_kkIN',
-  dataPreviewQueries: [],
   actions: {},
   activeSouthPaneTab: '',
   height: 1,
   displayLimit: 1,
   databases: {},
-  offline: false,
+  defaultQueryLimit: 100,
 };
 
 const mockedEmptyProps = {
-  editorQueries: [],
+  queryEditorId: 'random_id',
   latestQueryId: '',
-  dataPreviewQueries: [],
   actions: {
     queryEditorSetAndSaveSql: NOOP,
     cloneQueryToNewTab: NOOP,
@@ -90,15 +55,66 @@ const mockedEmptyProps = {
   activeSouthPaneTab: '',
   height: 100,
   databases: '',
-  offline: false,
   displayLimit: 100,
   user: UserWithPermissionsAndRoles,
   defaultQueryLimit: 100,
 };
 
 const middlewares = [thunk];
 const mockStore = configureStore(middlewares);
-const store = mockStore(initialState);
+const store = mockStore({
+  ...initialState,
+  sqlLab: {
+    ...initialState,
+    offline: false,
+    tables: [
+      {
+        ...table,
+        dataPreviewQueryId: '2g2_iRFMl',
+        queryEditorId: defaultQueryEditor.id,
+      },
+    ],
+    databases: {},
+    queries: {
+      LCly_kkIN: {
+        cached: false,
+        changedOn: Date.now(),
+        db: 'main',
+        dbId: 1,
+        id: 'LCly_kkIN',
+        startDttm: Date.now(),
+        sqlEditorId: defaultQueryEditor.id,
+      },
+      lXJa7F9_r: {
+        cached: false,
+        changedOn: 1559238500401,
+        db: 'main',
+        dbId: 1,
+        id: 'lXJa7F9_r',
+        startDttm: 1559238500401,
+        sqlEditorId: defaultQueryEditor.id,
+      },
+      '2g2_iRFMl': {
+        cached: false,
+        changedOn: 1559238506925,
+        db: 'main',
+        dbId: 1,
+        id: '2g2_iRFMl',
+        startDttm: 1559238506925,
+        sqlEditorId: defaultQueryEditor.id,
+      },
+      erWdqEWPm: {
+        cached: false,
+        changedOn: 1559238516395,
+        db: 'main',
+        dbId: 1,
+        id: 'erWdqEWPm',
+        startDttm: 1559238516395,
+        sqlEditorId: defaultQueryEditor.id,
+      },
+    },
+  },
+});
 const setup = (overrides = {}) => (
   <SouthPaneContainer store={store} {...mockedProps} {...overrides} />
 );
@@ -117,9 +133,14 @@ describe('SouthPane - Enzyme', () => {
   it('should pass latest query down to ResultSet component', () => {
     wrapper = getWrapper().dive();
     expect(wrapper.find(ResultSet)).toExist();
-    expect(wrapper.find(ResultSet).props().query.id).toEqual(
+    // for editorQueries
+    expect(wrapper.find(ResultSet).first().props().query.id).toEqual(
       mockedProps.latestQueryId,
     );
+    // for dataPreviewQueries
+    expect(wrapper.find(ResultSet).last().props().query.id).toEqual(
+      '2g2_iRFMl',
+    );
   });
 });
 

superset-frontend/src/SqlLab/components/SouthPane/index.tsx

@@ -41,7 +41,8 @@ const TAB_HEIGHT = 140;
     editorQueries are queries executed by users passed from SqlEditor component
     dataPrebiewQueries are all queries executed for preview of table data (from SqlEditorLeft)
 */
-interface SouthPanePropTypes {
+export interface SouthPanePropTypes {
+  queryEditorId: string;
   editorQueries: any[];
   latestQueryId?: string;
   dataPreviewQueries: any[];

superset-frontend/src/SqlLab/components/SouthPane/state.ts

@@ -19,14 +19,36 @@
 import { connect } from 'react-redux';
 import { bindActionCreators, Dispatch } from 'redux';
 import * as Actions from 'src/SqlLab/actions/sqlLab';
-import SouthPane from '.';
+import { SqlLabRootState } from 'src/SqlLab/types';
+import SouthPane, { SouthPanePropTypes } from '.';
 
-function mapStateToProps({ sqlLab }: Record<string, any>) {
+function mapStateToProps(
+  { sqlLab }: SqlLabRootState,
+  { queryEditorId }: SouthPanePropTypes,
+) {
+  const { databases, activeSouthPaneTab, offline, user, queries, tables } =
+    sqlLab;
+  const dataPreviewQueries = tables
+    .filter(
+      ({ dataPreviewQueryId, queryEditorId: qeId }) =>
+        dataPreviewQueryId &&
+        queryEditorId === qeId &&
+        queries[dataPreviewQueryId],
+    )
+    .map(({ name, dataPreviewQueryId }) => ({
+      ...queries[dataPreviewQueryId],
+      tableName: name,
+    }));
+  const editorQueries = Object.values(queries).filter(
+    ({ sqlEditorId }) => sqlEditorId === queryEditorId,
+  );
   return {
-    activeSouthPaneTab: sqlLab.activeSouthPaneTab,
-    databases: sqlLab.databases,
-    offline: sqlLab.offline,
-    user: sqlLab.user,
+    editorQueries,
+    dataPreviewQueries,
+    activeSouthPaneTab,
+    databases,
+    offline,
+    user,
   };
 }
 

superset-frontend/src/SqlLab/components/SqlEditor/index.jsx

@@ -137,8 +137,6 @@ const StyledSidebar = styled.div`
 const propTypes = {
   actions: PropTypes.object.isRequired,
   tables: PropTypes.array.isRequired,
-  editorQueries: PropTypes.array.isRequired,
-  dataPreviewQueries: PropTypes.array.isRequired,
   queryEditor: PropTypes.object.isRequired,
   defaultQueryLimit: PropTypes.number.isRequired,
   maxRow: PropTypes.number.isRequired,
@@ -150,8 +148,6 @@ const propTypes = {
 const SqlEditor = ({
   actions,
   tables,
-  editorQueries,
-  dataPreviewQueries,
   queryEditor,
   defaultQueryLimit,
   maxRow,
@@ -622,9 +618,8 @@ const SqlEditor = ({
           {renderEditorBottomBar(hotkeys)}
         </div>
         <ConnectedSouthPane
-          editorQueries={editorQueries}
+          queryEditorId={queryEditor.id}
           latestQueryId={latestQuery?.id}
-          dataPreviewQueries={dataPreviewQueries}
           actions={actions}
           height={southPaneHeight}
           displayLimit={displayLimit}

superset-frontend/src/SqlLab/components/TabbedSqlEditors/TabbedSqlEditors.test.jsx

@@ -30,7 +30,7 @@ import { supersetTheme, ThemeProvider } from '@superset-ui/core';
 import { EditableTabs } from 'src/components/Tabs';
 import TabbedSqlEditors from 'src/SqlLab/components/TabbedSqlEditors';
 import SqlEditor from 'src/SqlLab/components/SqlEditor';
-import { table, initialState } from 'src/SqlLab/fixtures';
+import { initialState } from 'src/SqlLab/fixtures';
 import { newQueryTabName } from 'src/SqlLab/utils/newQueryTabName';
 import QueryProvider from 'src/views/QueryProvider';
 
@@ -43,12 +43,6 @@ describe('TabbedSqlEditors', () => {
   const mockStore = configureStore(middlewares);
   const store = mockStore(initialState);
 
-  const tabHistory = ['dfsadfs', 'newEditorId'];
-
-  const tables = [
-    { ...table, dataPreviewQueryId: 'B1-VQU1zW', queryEditorId: 'newEditorId' },
-  ];
-
   const queryEditors = [
     {
       autorun: false,
@@ -61,14 +55,6 @@ describe('TabbedSqlEditors', () => {
       name: 'Untitled Query',
     },
   ];
-  const queries = {
-    'B1-VQU1zW': {
-      id: 'B1-VQU1zW',
-      sqlEditorId: 'newEditorId',
-      tableName: 'ab_user',
-      state: 'success',
-    },
-  };
   const mockedProps = {
     actions: {},
     databases: {},
@@ -146,20 +132,6 @@ describe('TabbedSqlEditors', () => {
       );
     });
   });
-  describe('UNSAFE_componentWillReceiveProps', () => {
-    beforeEach(() => {
-      wrapper = getWrapper();
-      wrapper.setProps({ queryEditors, queries, tabHistory, tables });
-    });
-    it('should update queriesArray and dataPreviewQueries', () => {
-      expect(wrapper.state().queriesArray.slice(-1)[0]).toBe(
-        queries['B1-VQU1zW'],
-      );
-      expect(wrapper.state().dataPreviewQueries.slice(-1)[0]).toEqual(
-        queries['B1-VQU1zW'],
-      );
-    });
-  });
   it('should removeQueryEditor', () => {
     wrapper = getWrapper();
     sinon.stub(wrapper.instance().props.actions, 'removeQueryEditor');

superset-frontend/src/SqlLab/components/TabbedSqlEditors/index.jsx

@@ -24,7 +24,6 @@ import { bindActionCreators } from 'redux';
 import URI from 'urijs';
 import { styled, t } from '@superset-ui/core';
 import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags';
-import { areArraysShallowEqual } from 'src/reduxUtils';
 import { Tooltip } from 'src/components/Tooltip';
 import { detectOS } from 'src/utils/common';
 import * as Actions from 'src/SqlLab/actions/sqlLab';
@@ -72,8 +71,6 @@ class TabbedSqlEditors extends React.PureComponent {
     const sqlLabUrl = '/superset/sqllab';
     this.state = {
       sqlLabUrl,
-      queriesArray: [],
-      dataPreviewQueries: [],
     };
     this.removeQueryEditor = this.removeQueryEditor.bind(this);
     this.duplicateQueryEditor = this.duplicateQueryEditor.bind(this);
@@ -186,37 +183,6 @@ class TabbedSqlEditors extends React.PureComponent {
     }
   }
 
-  UNSAFE_componentWillReceiveProps(nextProps) {
-    const nextActiveQeId =
-      nextProps.tabHistory[nextProps.tabHistory.length - 1];
-    const queriesArray = Object.values(nextProps.queries).filter(
-      query => query.sqlEditorId === nextActiveQeId,
-    );
-    if (!areArraysShallowEqual(queriesArray, this.state.queriesArray)) {
-      this.setState({ queriesArray });
-    }
-
-    const dataPreviewQueries = [];
-    nextProps.tables.forEach(table => {
-      const queryId = table.dataPreviewQueryId;
-      if (
-        queryId &&
-        nextProps.queries[queryId] &&
-        table.queryEditorId === nextActiveQeId
-      ) {
-        dataPreviewQueries.push({
-          ...nextProps.queries[queryId],
-          tableName: table.name,
-        });
-      }
-    });
-    if (
-      !areArraysShallowEqual(dataPreviewQueries, this.state.dataPreviewQueries)
-    ) {
-      this.setState({ dataPreviewQueries });
-    }
-  }
-
   popNewTab() {
     // Clean the url in browser history
     window.history.replaceState({}, document.title, this.state.sqlLabUrl);
@@ -298,8 +264,6 @@ class TabbedSqlEditors extends React.PureComponent {
         <SqlEditor
           tables={this.props.tables.filter(xt => xt.queryEditorId === qe.id)}
           queryEditor={qe}
-          editorQueries={this.state.queriesArray}
-          dataPreviewQueries={this.state.dataPreviewQueries}
           actions={this.props.actions}
           defaultQueryLimit={this.props.defaultQueryLimit}
           maxRow={this.props.maxRow}