mirror of https://github.com/apache/superset.git
fix(sqllab): rendering performance regression by resultset (#25091)
This commit is contained in:
parent
e4b54c3c01
commit
72150ebadf
|
@ -20,11 +20,12 @@ import React from 'react';
|
|||
import configureStore from 'redux-mock-store';
|
||||
import thunk from 'redux-thunk';
|
||||
import { render, screen, waitFor } from 'spec/helpers/testing-library';
|
||||
import SouthPane from 'src/SqlLab/components/SouthPane';
|
||||
import SouthPane, { SouthPaneProps } from 'src/SqlLab/components/SouthPane';
|
||||
import '@testing-library/jest-dom/extend-expect';
|
||||
import { STATUS_OPTIONS } from 'src/SqlLab/constants';
|
||||
import { initialState, table, defaultQueryEditor } from 'src/SqlLab/fixtures';
|
||||
import { denormalizeTimestamp } from '@superset-ui/core';
|
||||
import { Store } from 'redux';
|
||||
|
||||
const mockedProps = {
|
||||
queryEditorId: defaultQueryEditor.id,
|
||||
|
@ -42,6 +43,8 @@ const mockedEmptyProps = {
|
|||
defaultQueryLimit: 100,
|
||||
};
|
||||
|
||||
jest.mock('src/SqlLab/components/SqlEditorLeftBar', () => jest.fn());
|
||||
|
||||
const latestQueryProgressMsg = 'LATEST QUERY MESSAGE - LCly_kkIN';
|
||||
|
||||
const middlewares = [thunk];
|
||||
|
@ -100,14 +103,14 @@ const store = mockStore({
|
|||
},
|
||||
},
|
||||
});
|
||||
const setup = (props, store) =>
|
||||
const setup = (props: SouthPaneProps, store: Store) =>
|
||||
render(<SouthPane {...props} />, {
|
||||
useRedux: true,
|
||||
...(store && { store }),
|
||||
});
|
||||
|
||||
describe('SouthPane', () => {
|
||||
const renderAndWait = (props, store) =>
|
||||
const renderAndWait = (props: SouthPaneProps, store: Store) =>
|
||||
waitFor(async () => setup(props, store));
|
||||
|
||||
it('Renders an empty state for results', async () => {
|
|
@ -16,8 +16,8 @@
|
|||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
import React, { createRef } from 'react';
|
||||
import { useDispatch, useSelector } from 'react-redux';
|
||||
import React, { createRef, useMemo } from 'react';
|
||||
import { shallowEqual, useDispatch, useSelector } from 'react-redux';
|
||||
import shortid from 'shortid';
|
||||
import Alert from 'src/components/Alert';
|
||||
import Tabs from 'src/components/Tabs';
|
||||
|
@ -105,11 +105,26 @@ const SouthPane = ({
|
|||
defaultQueryLimit,
|
||||
}: SouthPaneProps) => {
|
||||
const dispatch = useDispatch();
|
||||
|
||||
const { editorQueries, dataPreviewQueries, databases, offline, user } =
|
||||
useSelector(({ user, sqlLab }: SqlLabRootState) => {
|
||||
const { databases, offline, queries, tables } = sqlLab;
|
||||
const dataPreviewQueries = tables
|
||||
const user = useSelector(({ user }: SqlLabRootState) => user, shallowEqual);
|
||||
const { databases, offline, queries, tables } = useSelector(
|
||||
({ sqlLab: { databases, offline, queries, tables } }: SqlLabRootState) => ({
|
||||
databases,
|
||||
offline,
|
||||
queries,
|
||||
tables,
|
||||
}),
|
||||
shallowEqual,
|
||||
);
|
||||
const editorQueries = useMemo(
|
||||
() =>
|
||||
Object.values(queries).filter(
|
||||
({ sqlEditorId }) => sqlEditorId === queryEditorId,
|
||||
),
|
||||
[queries, queryEditorId],
|
||||
);
|
||||
const dataPreviewQueries = useMemo(
|
||||
() =>
|
||||
tables
|
||||
.filter(
|
||||
({ dataPreviewQueryId, queryEditorId: qeId }) =>
|
||||
dataPreviewQueryId &&
|
||||
|
@ -119,18 +134,13 @@ const SouthPane = ({
|
|||
.map(({ name, dataPreviewQueryId }) => ({
|
||||
...queries[dataPreviewQueryId || ''],
|
||||
tableName: name,
|
||||
}));
|
||||
const editorQueries = Object.values(queries).filter(
|
||||
({ sqlEditorId }) => sqlEditorId === queryEditorId,
|
||||
);
|
||||
return {
|
||||
editorQueries,
|
||||
dataPreviewQueries,
|
||||
databases,
|
||||
offline: offline ?? false,
|
||||
user,
|
||||
};
|
||||
});
|
||||
})),
|
||||
[queries, queryEditorId, tables],
|
||||
);
|
||||
const latestQuery = useMemo(
|
||||
() => editorQueries.find(({ id }) => id === latestQueryId),
|
||||
[editorQueries, latestQueryId],
|
||||
);
|
||||
|
||||
const activeSouthPaneTab =
|
||||
useSelector<SqlLabRootState, string>(
|
||||
|
@ -148,11 +158,6 @@ const SouthPane = ({
|
|||
);
|
||||
|
||||
const renderResults = () => {
|
||||
let latestQuery;
|
||||
if (editorQueries.length > 0) {
|
||||
// get the latest query
|
||||
latestQuery = editorQueries.find(({ id }) => id === latestQueryId);
|
||||
}
|
||||
let results;
|
||||
if (latestQuery) {
|
||||
if (latestQuery?.extra?.errors) {
|
||||
|
|
|
@ -30,6 +30,7 @@ import {
|
|||
defaultQueryEditor,
|
||||
} from 'src/SqlLab/fixtures';
|
||||
import SqlEditorLeftBar from 'src/SqlLab/components/SqlEditorLeftBar';
|
||||
import ResultSet from 'src/SqlLab/components/ResultSet';
|
||||
import { api } from 'src/hooks/apiResources/queryApi';
|
||||
import { getExtensionsRegistry } from '@superset-ui/core';
|
||||
import setupExtensions from 'src/setup/setupExtensions';
|
||||
|
@ -46,6 +47,7 @@ jest.mock('src/components/AsyncAceEditor', () => ({
|
|||
),
|
||||
}));
|
||||
jest.mock('src/SqlLab/components/SqlEditorLeftBar', () => jest.fn());
|
||||
jest.mock('src/SqlLab/components/ResultSet', () => jest.fn());
|
||||
|
||||
fetchMock.get('glob:*/api/v1/database/*/function_names/', {
|
||||
function_names: [],
|
||||
|
@ -56,10 +58,17 @@ fetchMock.post('glob:*/sqllab/execute/*', { result: [] });
|
|||
|
||||
let store;
|
||||
let actions;
|
||||
const latestQuery = {
|
||||
...queries[0],
|
||||
sqlEditorId: defaultQueryEditor.id,
|
||||
};
|
||||
const mockInitialState = {
|
||||
...initialState,
|
||||
sqlLab: {
|
||||
...initialState.sqlLab,
|
||||
queries: {
|
||||
[latestQuery.id]: { ...latestQuery, startDttm: new Date().getTime() },
|
||||
},
|
||||
databases: {
|
||||
1991: {
|
||||
allow_ctas: false,
|
||||
|
@ -77,6 +86,7 @@ const mockInitialState = {
|
|||
unsavedQueryEditor: {
|
||||
id: defaultQueryEditor.id,
|
||||
dbId: 1991,
|
||||
latestQueryId: latestQuery.id,
|
||||
},
|
||||
},
|
||||
};
|
||||
|
@ -107,7 +117,6 @@ const createStore = initState =>
|
|||
describe('SqlEditor', () => {
|
||||
const mockedProps = {
|
||||
queryEditor: initialState.sqlLab.queryEditors[0],
|
||||
latestQuery: queries[0],
|
||||
tables: [table],
|
||||
getHeight: () => '100px',
|
||||
editorQueries: [],
|
||||
|
@ -125,6 +134,8 @@ describe('SqlEditor', () => {
|
|||
SqlEditorLeftBar.mockImplementation(() => (
|
||||
<div data-test="mock-sql-editor-left-bar" />
|
||||
));
|
||||
ResultSet.mockClear();
|
||||
ResultSet.mockImplementation(() => <div data-test="mock-result-set" />);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
|
@ -153,15 +164,18 @@ describe('SqlEditor', () => {
|
|||
expect(await findByTestId('react-ace')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('avoids rerendering EditorLeftBar while typing', async () => {
|
||||
it('avoids rerendering EditorLeftBar and ResultSet while typing', async () => {
|
||||
const { findByTestId } = setup(mockedProps, store);
|
||||
const editor = await findByTestId('react-ace');
|
||||
const sql = 'select *';
|
||||
const renderCount = SqlEditorLeftBar.mock.calls.length;
|
||||
const renderCountForSouthPane = ResultSet.mock.calls.length;
|
||||
expect(SqlEditorLeftBar).toHaveBeenCalledTimes(renderCount);
|
||||
expect(ResultSet).toHaveBeenCalledTimes(renderCountForSouthPane);
|
||||
fireEvent.change(editor, { target: { value: sql } });
|
||||
// Verify the rendering regression
|
||||
expect(SqlEditorLeftBar).toHaveBeenCalledTimes(renderCount);
|
||||
expect(ResultSet).toHaveBeenCalledTimes(renderCountForSouthPane);
|
||||
});
|
||||
|
||||
it('renders sql from unsaved change', async () => {
|
||||
|
@ -198,10 +212,8 @@ describe('SqlEditor', () => {
|
|||
});
|
||||
|
||||
it('render a SouthPane', async () => {
|
||||
const { findByText } = setup(mockedProps, store);
|
||||
expect(
|
||||
await findByText(/run a query to display results/i),
|
||||
).toBeInTheDocument();
|
||||
const { findByTestId } = setup(mockedProps, store);
|
||||
expect(await findByTestId('mock-result-set')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('runs query action with ctas false', async () => {
|
||||
|
|
Loading…
Reference in New Issue