Revert "fix(sqllab): perf regression on apache#21532 refactor (apache…

…#21632)"

This reverts commit 8d1b7ec.

superset-frontend/spec/helpers/testing-library.tsx

@@ -47,7 +47,7 @@ type Options = Omit<RenderOptions, 'queries'> & {
   store?: Store;
 };
 
-export function createWrapper(options?: Options) {
+function createWrapper(options?: Options) {
   const {
     useDnd,
     useRedux,

superset-frontend/src/SqlLab/components/AceEditorWrapper/AceEditorWrapper.test.tsx

@@ -48,7 +48,7 @@ jest.mock('src/components/AsyncAceEditor', () => ({
 const setup = (queryEditor: QueryEditor, store?: Store) =>
   render(
     <AceEditorWrapper
-      queryEditorId={queryEditor.id}
+      queryEditor={queryEditor}
       height="100px"
       hotkeys={[]}
       database={{}}

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

@@ -37,7 +37,7 @@ import {
   AceCompleterKeyword,
   FullSQLEditor as AceEditor,
 } from 'src/components/AsyncAceEditor';
-import useQueryEditor from 'src/SqlLab/hooks/useQueryEditor';
+import { QueryEditor } from 'src/SqlLab/types';
 
 type HotKey = {
   key: string;
@@ -50,7 +50,7 @@ type AceEditorWrapperProps = {
   autocomplete: boolean;
   onBlur: (sql: string) => void;
   onChange: (sql: string) => void;
-  queryEditorId: string;
+  queryEditor: QueryEditor;
   database: any;
   extendedTables?: Array<{ name: string; columns: any[] }>;
   height: string;
@@ -61,25 +61,15 @@ const AceEditorWrapper = ({
   autocomplete,
   onBlur = () => {},
   onChange = () => {},
-  queryEditorId,
+  queryEditor,
   database,
   extendedTables = [],
   height,
   hotkeys,
 }: AceEditorWrapperProps) => {
   const dispatch = useDispatch();
 
-  const queryEditor = useQueryEditor(queryEditorId, [
-    'id',
-    'dbId',
-    'sql',
-    'functionNames',
-    'schemaOptions',
-    'tableOptions',
-    'validationResult',
-    'schema',
-  ]);
-  const currentSql = queryEditor.sql ?? '';
+  const { sql: currentSql } = queryEditor;
   const functionNames = queryEditor.functionNames ?? [];
   const schemas = queryEditor.schemaOptions ?? [];
   const tables = queryEditor.tableOptions ?? [];

superset-frontend/src/SqlLab/components/QueryLimitSelect/QueryLimitSelect.test.tsx

@@ -51,7 +51,7 @@ const defaultQueryLimit = 100;
 const setup = (props?: Partial<QueryLimitSelectProps>, store?: Store) =>
   render(
     <QueryLimitSelect
-      queryEditorId={defaultQueryEditor.id}
+      queryEditor={defaultQueryEditor}
       maxRow={100000}
       defaultQueryLimit={defaultQueryLimit}
       {...props}
@@ -67,20 +67,12 @@ describe('QueryLimitSelect', () => {
     const queryLimit = 10;
     const { getByText } = setup(
       {
-        queryEditorId: defaultQueryEditor.id,
-      },
-      mockStore({
-        ...initialState,
-        sqlLab: {
-          ...initialState.sqlLab,
-          queryEditors: [
-            {
-              ...defaultQueryEditor,
-              queryLimit,
-            },
-          ],
+        queryEditor: {
+          ...defaultQueryEditor,
+          queryLimit,
         },
-      }),
+      },
+      mockStore(initialState),
     );
     expect(getByText(queryLimit)).toBeInTheDocument();
   });
@@ -137,9 +129,7 @@ describe('QueryLimitSelect', () => {
         {
           type: 'QUERY_EDITOR_SET_QUERY_LIMIT',
           queryLimit: LIMIT_DROPDOWN[expectedIndex],
-          queryEditor: {
-            id: defaultQueryEditor.id,
-          },
+          queryEditor: defaultQueryEditor,
         },
       ]),
     );

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

@@ -17,16 +17,16 @@
  * under the License.
  */
 import React from 'react';
-import { useDispatch } from 'react-redux';
+import { useSelector, useDispatch } from 'react-redux';
 import { styled, useTheme } from '@superset-ui/core';
 import { AntdDropdown } from 'src/components';
 import { Menu } from 'src/components/Menu';
 import Icons from 'src/components/Icons';
+import { SqlLabRootState, QueryEditor } from 'src/SqlLab/types';
 import { queryEditorSetQueryLimit } from 'src/SqlLab/actions/sqlLab';
-import useQueryEditor from 'src/SqlLab/hooks/useQueryEditor';
 
 export interface QueryLimitSelectProps {
-  queryEditorId: string;
+  queryEditor: QueryEditor;
   maxRow: number;
   defaultQueryLimit: number;
 }
@@ -79,12 +79,19 @@ function renderQueryLimit(
 }
 
 const QueryLimitSelect = ({
-  queryEditorId,
+  queryEditor,
   maxRow,
   defaultQueryLimit,
 }: QueryLimitSelectProps) => {
-  const queryEditor = useQueryEditor(queryEditorId, ['id', 'queryLimit']);
-  const queryLimit = queryEditor.queryLimit || defaultQueryLimit;
+  const queryLimit = useSelector<SqlLabRootState, number>(
+    ({ sqlLab: { unsavedQueryEditor } }) => {
+      const updatedQueryEditor = {
+        ...queryEditor,
+        ...(unsavedQueryEditor.id === queryEditor.id && unsavedQueryEditor),
+      };
+      return updatedQueryEditor.queryLimit || defaultQueryLimit;
+    },
+  );
   const dispatch = useDispatch();
   const setQueryLimit = (updatedQueryLimit: number) =>
     dispatch(queryEditorSetQueryLimit(queryEditor, updatedQueryLimit));

superset-frontend/src/SqlLab/components/RunQueryActionButton/RunQueryActionButton.test.tsx

@@ -41,13 +41,13 @@ jest.mock('src/components/Select/AsyncSelect', () => () => (
 ));
 
 const defaultProps = {
-  queryEditorId: defaultQueryEditor.id,
+  queryEditor: defaultQueryEditor,
   allowAsync: false,
   dbId: 1,
   queryState: 'ready',
-  runQuery: () => {},
+  runQuery: jest.fn(),
   selectedText: null,
-  stopQuery: () => {},
+  stopQuery: jest.fn(),
   overlayCreateAsMenu: null,
 };
 
@@ -57,104 +57,95 @@ const setup = (props?: Partial<Props>, store?: Store) =>
     ...(store && { store }),
   });
 
-it('renders a single Button', () => {
-  const { getByRole } = setup({}, mockStore(initialState));
-  expect(getByRole('button')).toBeInTheDocument();
-});
+describe('RunQueryActionButton', () => {
+  beforeEach(() => {
+    defaultProps.runQuery.mockReset();
+    defaultProps.stopQuery.mockReset();
+  });
 
-it('renders a label for Run Query', () => {
-  const { getByText } = setup({}, mockStore(initialState));
-  expect(getByText('Run')).toBeInTheDocument();
-});
+  it('renders a single Button', () => {
+    const { getByRole } = setup({}, mockStore(initialState));
+    expect(getByRole('button')).toBeInTheDocument();
+  });
 
-it('renders a label for Selected Query', () => {
-  const { getByText } = setup(
-    {},
-    mockStore({
-      ...initialState,
-      sqlLab: {
-        ...initialState.sqlLab,
-        unsavedQueryEditor: {
-          id: defaultQueryEditor.id,
-          selectedText: 'select * from\n-- this is comment\nwhere',
-        },
-      },
-    }),
-  );
-  expect(getByText('Run selection')).toBeInTheDocument();
-});
+  it('renders a label for Run Query', () => {
+    const { getByText } = setup({}, mockStore(initialState));
+    expect(getByText('Run')).toBeInTheDocument();
+  });
 
-it('disable button when sql from unsaved changes is empty', () => {
-  const { getByRole } = setup(
-    {},
-    mockStore({
-      ...initialState,
-      sqlLab: {
-        ...initialState.sqlLab,
-        unsavedQueryEditor: {
-          id: defaultQueryEditor.id,
-          sql: '',
+  it('renders a label for Selected Query', () => {
+    const { getByText } = setup(
+      {},
+      mockStore({
+        ...initialState,
+        sqlLab: {
+          ...initialState.sqlLab,
+          unsavedQueryEditor: {
+            id: defaultQueryEditor.id,
+            selectedText: 'FROM',
+          },
         },
-      },
-    }),
-  );
-  const button = getByRole('button');
-  expect(button).toBeDisabled();
-});
+      }),
+    );
+    expect(getByText('Run selection')).toBeInTheDocument();
+  });
 
-it('disable button when selectedText only contains blank contents', () => {
-  const { getByRole } = setup(
-    {},
-    mockStore({
-      ...initialState,
-      sqlLab: {
-        ...initialState.sqlLab,
-        unsavedQueryEditor: {
-          id: defaultQueryEditor.id,
-          selectedText: '-- this is comment\n\n     \t',
+  it('disable button when sql from unsaved changes is empty', () => {
+    const { getByRole } = setup(
+      {},
+      mockStore({
+        ...initialState,
+        sqlLab: {
+          ...initialState.sqlLab,
+          unsavedQueryEditor: {
+            id: defaultQueryEditor.id,
+            sql: '',
+          },
         },
-      },
-    }),
-  );
-  const button = getByRole('button');
-  expect(button).toBeDisabled();
-});
+      }),
+    );
+    const button = getByRole('button');
+    expect(button).toBeDisabled();
+  });
 
-it('enable default button for unrelated unsaved changes', () => {
-  const { getByRole } = setup(
-    {},
-    mockStore({
-      ...initialState,
-      sqlLab: {
-        ...initialState.sqlLab,
-        unsavedQueryEditor: {
-          id: `${defaultQueryEditor.id}-other`,
-          sql: '',
+  it('enable default button for unrelated unsaved changes', () => {
+    const { getByRole } = setup(
+      {},
+      mockStore({
+        ...initialState,
+        sqlLab: {
+          ...initialState.sqlLab,
+          unsavedQueryEditor: {
+            id: `${defaultQueryEditor.id}-other`,
+            sql: '',
+          },
         },
-      },
-    }),
-  );
-  const button = getByRole('button');
-  expect(button).toBeEnabled();
-});
+      }),
+    );
+    const button = getByRole('button');
+    expect(button).toBeEnabled();
+  });
 
-it('dispatch runQuery on click', async () => {
-  const runQuery = jest.fn();
-  const { getByRole } = setup({ runQuery }, mockStore(initialState));
-  const button = getByRole('button');
-  expect(runQuery).toHaveBeenCalledTimes(0);
-  fireEvent.click(button);
-  await waitFor(() => expect(runQuery).toHaveBeenCalledTimes(1));
-});
+  it('dispatch runQuery on click', async () => {
+    const { getByRole } = setup({}, mockStore(initialState));
+    const button = getByRole('button');
+    expect(defaultProps.runQuery).toHaveBeenCalledTimes(0);
+    fireEvent.click(button);
+    await waitFor(() => expect(defaultProps.runQuery).toHaveBeenCalledTimes(1));
+  });
 
-it('dispatch stopQuery on click while running state', async () => {
-  const stopQuery = jest.fn();
-  const { getByRole } = setup(
-    { queryState: 'running', stopQuery },
-    mockStore(initialState),
-  );
-  const button = getByRole('button');
-  expect(stopQuery).toHaveBeenCalledTimes(0);
-  fireEvent.click(button);
-  await waitFor(() => expect(stopQuery).toHaveBeenCalledTimes(1));
+  describe('on running state', () => {
+    it('dispatch stopQuery on click', async () => {
+      const { getByRole } = setup(
+        { queryState: 'running' },
+        mockStore(initialState),
+      );
+      const button = getByRole('button');
+      expect(defaultProps.stopQuery).toHaveBeenCalledTimes(0);
+      fireEvent.click(button);
+      await waitFor(() =>
+        expect(defaultProps.stopQuery).toHaveBeenCalledTimes(1),
+      );
+    });
+  });
 });