Skip to content

Commit

Permalink
fix(sqllab): rendering performance regression (apache#23695)
Browse files Browse the repository at this point in the history
  • Loading branch information
justinpark authored and sebastianliebscher committed Apr 28, 2023
1 parent a4f7e55 commit cf0533d
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;

Expand All @@ -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,
Expand All @@ -82,7 +89,8 @@ const store = mockStore({
dbId: 'dbid1',
},
},
});
};
const store = mockStore(mockInitialState);

const setup = (props = {}, store) =>
render(<SqlEditor {...props} />, {
Expand All @@ -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>
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 () => {
Expand Down
3 changes: 2 additions & 1 deletion superset-frontend/src/SqlLab/components/SqlEditor/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -230,6 +230,7 @@ const SqlEditor = ({
hideLeftBar,
};
},
shallowEqual,
);

const [height, setHeight] = useState(0);
Expand Down

0 comments on commit cf0533d

Please sign in to comment.