[superset] branch master updated: fix(sqllab): rendering performance regression (#23695)

[ju...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

justinpark pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new c197bf9e6d fix(sqllab): rendering performance regression (#23695)
c197bf9e6d is described below

commit c197bf9e6db85a76d8118a4ec11a83ca2f6aad6d
Author: JUST.in DO IT <ju...@airbnb.com>
AuthorDate: Mon Apr 17 09:49:15 2023 -0700

    fix(sqllab): rendering performance regression (#23695)
---
 .../SqlLab/components/SqlEditor/SqlEditor.test.jsx | 49 +++++++++++++++++-----
 .../src/SqlLab/components/SqlEditor/index.jsx      |  3 +-
 2 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx b/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx
index 9dcc37fdd1..99551b9d41 100644
--- a/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx
+++ b/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx
@@ -32,6 +32,8 @@ import {
 import AceEditorWrapper from 'src/SqlLab/components/AceEditorWrapper';
 import SouthPane from 'src/SqlLab/components/SouthPane';
 import SqlEditor from 'src/SqlLab/components/SqlEditor';
+import { setupStore } from 'src/views/store';
+import { reducers } from 'src/SqlLab/reducers';
 import QueryProvider from 'src/views/QueryProvider';
 import waitForComponentToPaint from 'spec/helpers/waitForComponentToPaint';
 import {
@@ -40,16 +42,21 @@ import {
   table,
   defaultQueryEditor,
 } from 'src/SqlLab/fixtures';
+import SqlEditorLeftBar from 'src/SqlLab/components/SqlEditorLeftBar';
 
 jest.mock('src/components/AsyncAceEditor', () => ({
   ...jest.requireActual('src/components/AsyncAceEditor'),
-  FullSQLEditor: props => (
-    <div data-test="react-ace">{JSON.stringify(props)}</div>
+  FullSQLEditor: ({ onChange, onBlur, value }) => (
+    <textarea
+      data-test="react-ace"
+      onChange={evt => onChange(evt.target.value)}
+      onBlur={onBlur}
+    >
+      {value}
+    </textarea>
   ),
 }));
-jest.mock('src/SqlLab/components/SqlEditorLeftBar', () => () => (
-  <div data-test="mock-sql-editor-left-bar" />
-));
+jest.mock('src/SqlLab/components/SqlEditorLeftBar', () => jest.fn());
 
 const MOCKED_SQL_EDITOR_HEIGHT = 500;
 
@@ -59,7 +66,7 @@ fetchMock.post('glob:*/sqllab/execute/*', { result: [] });
 
 const middlewares = [thunk];
 const mockStore = configureStore(middlewares);
-const store = mockStore({
+const mockInitialState = {
   ...initialState,
   sqlLab: {
     ...initialState.sqlLab,
@@ -82,7 +89,8 @@ const store = mockStore({
       dbId: 'dbid1',
     },
   },
-});
+};
+const store = mockStore(mockInitialState);
 
 const setup = (props = {}, store) =>
   render(<SqlEditor {...props} />, {
@@ -103,6 +111,13 @@ describe('SqlEditor', () => {
     displayLimit: 100,
   };
 
+  beforeEach(() => {
+    SqlEditorLeftBar.mockClear();
+    SqlEditorLeftBar.mockImplementation(() => (
+      <div data-test="mock-sql-editor-left-bar" />
+    ));
+  });
+
   const buildWrapper = (props = {}) =>
     mount(
       <QueryProvider>
@@ -136,6 +151,21 @@ describe('SqlEditor', () => {
     expect(await findByTestId('react-ace')).toBeInTheDocument();
   });
 
+  it('avoids rerendering EditorLeftBar while typing', async () => {
+    const sqlLabStore = setupStore({
+      initialState: mockInitialState,
+      rootReducers: reducers,
+    });
+    const { findByTestId } = setup(mockedProps, sqlLabStore);
+    const editor = await findByTestId('react-ace');
+    const sql = 'select *';
+    const renderCount = SqlEditorLeftBar.mock.calls.length;
+    expect(SqlEditorLeftBar).toHaveBeenCalledTimes(renderCount);
+    fireEvent.change(editor, { target: { value: sql } });
+    // Verify the rendering regression
+    expect(SqlEditorLeftBar).toHaveBeenCalledTimes(renderCount);
+  });
+
   it('renders sql from unsaved change', async () => {
     const expectedSql = 'SELECT updated_column\nFROM updated_table\nWHERE';
     const { findByTestId } = setup(
@@ -167,9 +197,8 @@ describe('SqlEditor', () => {
       }),
     );
 
-    expect(await findByTestId('react-ace')).toHaveTextContent(
-      JSON.stringify({ value: expectedSql }).slice(1, -1),
-    );
+    const editor = await findByTestId('react-ace');
+    expect(editor).toHaveValue(expectedSql);
   });
 
   it('render a SouthPane', async () => {
diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx b/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx
index 83286e5881..d23c309920 100644
--- a/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx
+++ b/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx
@@ -26,7 +26,7 @@ import React, {
   useCallback,
 } from 'react';
 import { CSSTransition } from 'react-transition-group';
-import { useDispatch, useSelector } from 'react-redux';
+import { shallowEqual, useDispatch, useSelector } from 'react-redux';
 import PropTypes from 'prop-types';
 import Split from 'react-split';
 import { css, FeatureFlag, styled, t, useTheme } from '@superset-ui/core';
@@ -230,6 +230,7 @@ const SqlEditor = ({
         hideLeftBar,
       };
     },
+    shallowEqual,
   );
 
   const [height, setHeight] = useState(0);