diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper/AceEditorWrapper.test.tsx b/superset-frontend/src/SqlLab/components/AceEditorWrapper/AceEditorWrapper.test.tsx index 72daff7602b24..b911253705992 100644 --- a/superset-frontend/src/SqlLab/components/AceEditorWrapper/AceEditorWrapper.test.tsx +++ b/superset-frontend/src/SqlLab/components/AceEditorWrapper/AceEditorWrapper.test.tsx @@ -19,16 +19,29 @@ import React from 'react'; import configureStore from 'redux-mock-store'; import thunk from 'redux-thunk'; -import { render, waitFor } from 'spec/helpers/testing-library'; +import reducerIndex from 'spec/helpers/reducerIndex'; +import { render, waitFor, createStore } from 'spec/helpers/testing-library'; import { QueryEditor } from 'src/SqlLab/types'; import { Store } from 'redux'; import { initialState, defaultQueryEditor } from 'src/SqlLab/fixtures'; import AceEditorWrapper from 'src/SqlLab/components/AceEditorWrapper'; -import { AsyncAceEditorProps } from 'src/components/AsyncAceEditor'; +import { + AsyncAceEditorProps, + FullSQLEditor, +} from 'src/components/AsyncAceEditor'; +import { + queryEditorSetCursorPosition, + queryEditorSetDb, +} from 'src/SqlLab/actions/sqlLab'; +import fetchMock from 'fetch-mock'; const middlewares = [thunk]; const mockStore = configureStore(middlewares); +fetchMock.get('glob:*/api/v1/database/*/function_names/', { + function_names: [], +}); + jest.mock('src/components/Select/Select', () => () => (
)); @@ -37,9 +50,11 @@ jest.mock('src/components/Select/AsyncSelect', () => () => ( )); jest.mock('src/components/AsyncAceEditor', () => ({ - FullSQLEditor: (props: AsyncAceEditorProps) => ( -
{JSON.stringify(props)}
- ), + FullSQLEditor: jest + .fn() + .mockImplementation((props: AsyncAceEditorProps) => ( +
{JSON.stringify(props)}
+ )), })); const setup = (queryEditor: QueryEditor, store?: Store) => @@ -60,6 +75,10 @@ const setup = (queryEditor: QueryEditor, store?: Store) => ); describe('AceEditorWrapper', () => { + beforeEach(() => { + (FullSQLEditor as any as jest.Mock).mockClear(); + }); + it('renders ace editor including sql value', async () => { const { getByTestId } = setup(defaultQueryEditor, mockStore(initialState)); await waitFor(() => expect(getByTestId('react-ace')).toBeInTheDocument()); @@ -92,4 +111,19 @@ describe('AceEditorWrapper', () => { JSON.stringify({ value: defaultQueryEditor.sql }).slice(1, -1), ); }); + + it('skips rerendering for updating cursor position', () => { + const store = createStore(initialState, reducerIndex); + setup(defaultQueryEditor, store); + + expect(FullSQLEditor).toHaveBeenCalled(); + const renderCount = (FullSQLEditor as any as jest.Mock).mock.calls.length; + const updatedCursorPosition = { row: 1, column: 9 }; + store.dispatch( + queryEditorSetCursorPosition(defaultQueryEditor, updatedCursorPosition), + ); + expect(FullSQLEditor).toHaveBeenCalledTimes(renderCount); + store.dispatch(queryEditorSetDb(defaultQueryEditor, 1)); + expect(FullSQLEditor).toHaveBeenCalledTimes(renderCount + 1); + }); }); diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx b/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx index 354d6b1c8fb81..b1fbbff55051d 100644 --- a/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx +++ b/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx @@ -18,7 +18,7 @@ */ import React, { useState, useEffect, useRef } from 'react'; import type { IAceEditor } from 'react-ace/lib/types'; -import { useDispatch } from 'react-redux'; +import { shallowEqual, useDispatch, useSelector } from 'react-redux'; import { css, styled, usePrevious, useTheme } from '@superset-ui/core'; import { Global } from '@emotion/react'; @@ -27,7 +27,7 @@ import { queryEditorSetSelectedText } from 'src/SqlLab/actions/sqlLab'; import { FullSQLEditor as AceEditor } from 'src/components/AsyncAceEditor'; import type { KeyboardShortcut } from 'src/SqlLab/components/KeyboardShortcutButton'; import useQueryEditor from 'src/SqlLab/hooks/useQueryEditor'; -import type { CursorPosition } from 'src/SqlLab/types'; +import { SqlLabRootState, type CursorPosition } from 'src/SqlLab/types'; import { useAnnotations } from './useAnnotations'; import { useKeywords } from './useKeywords'; @@ -76,11 +76,20 @@ const AceEditorWrapper = ({ 'sql', 'schema', 'templateParams', - 'cursorPosition', ]); + // Prevent a maximum update depth exceeded error + // by skipping access the unsaved query editor state + const cursorPosition = useSelector( + ({ sqlLab: { queryEditors } }) => { + const { cursorPosition } = { + ...queryEditors.find(({ id }) => id === queryEditorId), + }; + return cursorPosition ?? { row: 0, column: 0 }; + }, + shallowEqual, + ); const currentSql = queryEditor.sql ?? ''; - const cursorPosition = queryEditor.cursorPosition ?? { row: 0, column: 0 }; const [sql, setSql] = useState(currentSql); // The editor changeSelection is called multiple times in a row, diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper/useAnnotations.ts b/superset-frontend/src/SqlLab/components/AceEditorWrapper/useAnnotations.ts index f6924a243f715..04491fd58fadc 100644 --- a/superset-frontend/src/SqlLab/components/AceEditorWrapper/useAnnotations.ts +++ b/superset-frontend/src/SqlLab/components/AceEditorWrapper/useAnnotations.ts @@ -24,11 +24,14 @@ import { VALIDATION_DEBOUNCE_MS } from 'src/SqlLab/constants'; import { FetchValidationQueryParams, useQueryValidationsQuery, + ValidationResult, } from 'src/hooks/apiResources'; import { useDebounceValue } from 'src/hooks/useDebounceValue'; import { ClientErrorObject } from 'src/utils/getClientErrorObject'; import { t } from '@superset-ui/core'; +const EMPTY = [] as ValidationResult[]; + export function useAnnotations(params: FetchValidationQueryParams) { const { sql, dbId, schema, templateParams } = params; const debouncedSql = useDebounceValue(sql, VALIDATION_DEBOUNCE_MS); @@ -75,7 +78,7 @@ export function useAnnotations(params: FetchValidationQueryParams) { text: `The server failed to validate your query.\n${message}`, }, ] - : [], + : EMPTY, }; }, }, diff --git a/superset-frontend/src/hooks/apiResources/queryValidations.ts b/superset-frontend/src/hooks/apiResources/queryValidations.ts index 722c320049013..46c7e5274c188 100644 --- a/superset-frontend/src/hooks/apiResources/queryValidations.ts +++ b/superset-frontend/src/hooks/apiResources/queryValidations.ts @@ -25,7 +25,7 @@ export type FetchValidationQueryParams = { templateParams?: string; }; -type ValidationResult = { +export type ValidationResult = { end_column: number | null; line_number: number | null; message: string | null;