diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper/AceEditorWrapper.test.tsx b/superset-frontend/src/SqlLab/components/AceEditorWrapper/AceEditorWrapper.test.tsx index ea3c21640a877..e2abec9f8c767 100644 --- a/superset-frontend/src/SqlLab/components/AceEditorWrapper/AceEditorWrapper.test.tsx +++ b/superset-frontend/src/SqlLab/components/AceEditorWrapper/AceEditorWrapper.test.tsx @@ -18,16 +18,29 @@ */ 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', () => () => (
)); @@ -36,9 +49,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) => @@ -59,6 +74,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()); @@ -91,4 +110,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 c77a7daf128ac..06e67fb40433d 100644 --- a/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx +++ b/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx @@ -18,7 +18,7 @@ */ import { 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'; @@ -77,11 +77,20 @@ const AceEditorWrapper = ({ 'catalog', '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, @@ -145,12 +154,7 @@ const AceEditorWrapper = ({ editor.selection.on('changeCursor', () => { const cursor = editor.getCursorPosition(); - const { row, column } = cursorPosition; - // Prevent a maximum update depth exceeded error - // by skipping repeated updates to the same cursor position - if (cursor.row !== row && cursor.column !== column) { - onCursorPositionChange(cursor); - } + onCursorPositionChange(cursor); }); const { row, column } = cursorPosition; diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper/useAnnotations.ts b/superset-frontend/src/SqlLab/components/AceEditorWrapper/useAnnotations.ts index f640e3077961b..c64605c39583f 100644 --- a/superset-frontend/src/SqlLab/components/AceEditorWrapper/useAnnotations.ts +++ b/superset-frontend/src/SqlLab/components/AceEditorWrapper/useAnnotations.ts @@ -24,9 +24,12 @@ import { VALIDATION_DEBOUNCE_MS } from 'src/SqlLab/constants'; import { FetchValidationQueryParams, useQueryValidationsQuery, + ValidationResult, } from 'src/hooks/apiResources'; import { useDebounceValue } from 'src/hooks/useDebounceValue'; +const EMPTY = [] as ValidationResult[]; + export function useAnnotations(params: FetchValidationQueryParams) { const { sql, dbId, schema, templateParams } = params; const debouncedSql = useDebounceValue(sql, VALIDATION_DEBOUNCE_MS); @@ -73,7 +76,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 df88d6afca9bb..048ce3098c642 100644 --- a/superset-frontend/src/hooks/apiResources/queryValidations.ts +++ b/superset-frontend/src/hooks/apiResources/queryValidations.ts @@ -26,7 +26,7 @@ export type FetchValidationQueryParams = { templateParams?: string; }; -type ValidationResult = { +export type ValidationResult = { end_column: number | null; line_number: number | null; message: string | null;