From 15c3c342680e041019eab9a327034c77f6456966 Mon Sep 17 00:00:00 2001 From: EugeneTorap Date: Wed, 28 Sep 2022 03:04:34 +0300 Subject: [PATCH] chore: refactor AceEditorWrapper to functional component (#21532) --- .../AceEditorWrapper.test.tsx | 31 -- .../components/AceEditorWrapper/index.tsx | 270 +++++++----------- .../components/SqlEditor/SqlEditor.test.jsx | 56 ++++ .../src/SqlLab/components/SqlEditor/index.jsx | 16 +- 4 files changed, 177 insertions(+), 196 deletions(-) diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper/AceEditorWrapper.test.tsx b/superset-frontend/src/SqlLab/components/AceEditorWrapper/AceEditorWrapper.test.tsx index 2bbc90d947..0fd5c7d3e8 100644 --- a/superset-frontend/src/SqlLab/components/AceEditorWrapper/AceEditorWrapper.test.tsx +++ b/superset-frontend/src/SqlLab/components/AceEditorWrapper/AceEditorWrapper.test.tsx @@ -23,11 +23,6 @@ import { render, waitFor } from 'spec/helpers/testing-library'; import { QueryEditor } from 'src/SqlLab/types'; import { Store } from 'redux'; import { initialState, defaultQueryEditor } from 'src/SqlLab/fixtures'; -import { - queryEditorSetSelectedText, - queryEditorSetFunctionNames, - addTable, -} from 'src/SqlLab/actions/sqlLab'; import AceEditorWrapper from 'src/SqlLab/components/AceEditorWrapper'; import { AsyncAceEditorProps } from 'src/components/AsyncAceEditor'; @@ -54,11 +49,6 @@ const setup = (queryEditor: QueryEditor, store?: Store) => render( { ); }); - it('renders sql from unsaved change', () => { - const expectedSql = 'SELECT updated_column\nFROM updated_table\nWHERE'; - const { getByTestId } = setup( - defaultQueryEditor, - mockStore({ - ...initialState, - sqlLab: { - ...initialState.sqlLab, - unsavedQueryEditor: { - id: defaultQueryEditor.id, - sql: expectedSql, - }, - }, - }), - ); - - expect(getByTestId('react-ace')).toHaveTextContent( - JSON.stringify({ value: expectedSql }).slice(1, -1), - ); - }); - it('renders current sql for unrelated unsaved changes', () => { const expectedSql = 'SELECT updated_column\nFROM updated_table\nWHERE'; const { getByTestId } = setup( diff --git a/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx b/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx index 6b70be228b..0583bd6343 100644 --- a/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx +++ b/superset-frontend/src/SqlLab/components/AceEditorWrapper/index.tsx @@ -16,10 +16,16 @@ * specific language governing permissions and limitations * under the License. */ -import React from 'react'; -import { connect } from 'react-redux'; +import React, { useState, useEffect, useRef } from 'react'; +import { useDispatch } from 'react-redux'; +import { usePrevious } from 'src/hooks/usePrevious'; import { areArraysShallowEqual } from 'src/reduxUtils'; import sqlKeywords from 'src/SqlLab/utils/sqlKeywords'; +import { + queryEditorSetSelectedText, + queryEditorSetFunctionNames, + addTable, +} from 'src/SqlLab/actions/sqlLab'; import { SCHEMA_AUTOCOMPLETE_SCORE, TABLE_AUTOCOMPLETE_SCORE, @@ -31,7 +37,7 @@ import { AceCompleterKeyword, FullSQLEditor as AceEditor, } from 'src/components/AsyncAceEditor'; -import { QueryEditor, SchemaOption, SqlLabRootState } from 'src/SqlLab/types'; +import { QueryEditor } from 'src/SqlLab/types'; type HotKey = { key: string; @@ -40,115 +46,91 @@ type HotKey = { func: () => void; }; -type OwnProps = { - queryEditor: QueryEditor; - extendedTables: Array<{ name: string; columns: any[] }>; +type AceEditorWrapperProps = { autocomplete: boolean; - onChange: (sql: string) => void; onBlur: (sql: string) => void; - database: any; - actions: { - queryEditorSetSelectedText: (edit: any, text: null | string) => void; - queryEditorSetFunctionNames: (queryEditor: object, dbId: number) => void; - addTable: ( - queryEditor: any, - database: any, - value: any, - schema: any, - ) => void; - }; - hotkeys: HotKey[]; - height: string; -}; - -type ReduxProps = { + onChange: (sql: string) => void; queryEditor: QueryEditor; - sql: string; - schemas: SchemaOption[]; - tables: any[]; - functionNames: string[]; + database: any; + extendedTables?: Array<{ name: string; columns: any[] }>; + height: string; + hotkeys: HotKey[]; }; -type Props = ReduxProps & OwnProps; +const AceEditorWrapper = ({ + autocomplete, + onBlur = () => {}, + onChange = () => {}, + queryEditor, + database, + extendedTables = [], + height, + hotkeys, +}: AceEditorWrapperProps) => { + const dispatch = useDispatch(); -interface State { - sql: string; - words: AceCompleterKeyword[]; -} + const { sql: currentSql } = queryEditor; + const functionNames = queryEditor.functionNames ?? []; + const schemas = queryEditor.schemaOptions ?? []; + const tables = queryEditor.tableOptions ?? []; -class AceEditorWrapper extends React.PureComponent { - static defaultProps = { - onBlur: () => {}, - onChange: () => {}, - schemas: [], - tables: [], - functionNames: [], - extendedTables: [], + const [sql, setSql] = useState(currentSql); + const [words, setWords] = useState([]); + + // The editor changeSelection is called multiple times in a row, + // faster than React reconciliation process, so the selected text + // needs to be stored out of the state to ensure changes to it + // get saved immediately + const currentSelectionCache = useRef(''); + + useEffect(() => { + // Making sure no text is selected from previous mount + dispatch(queryEditorSetSelectedText(queryEditor, null)); + if (queryEditor.dbId) { + dispatch(queryEditorSetFunctionNames(queryEditor, queryEditor.dbId)); + } + setAutoCompleter(); + }, []); + + const prevTables = usePrevious(tables) ?? []; + const prevSchemas = usePrevious(schemas) ?? []; + const prevExtendedTables = usePrevious(extendedTables) ?? []; + const prevSql = usePrevious(currentSql); + + useEffect(() => { + if ( + !areArraysShallowEqual(tables, prevTables) || + !areArraysShallowEqual(schemas, prevSchemas) || + !areArraysShallowEqual(extendedTables, prevExtendedTables) + ) { + setAutoCompleter(); + } + }, [tables, schemas, extendedTables]); + + useEffect(() => { + if (currentSql !== prevSql) { + setSql(currentSql); + } + }, [currentSql]); + + const onBlurSql = () => { + onBlur(sql); }; - private currentSelectionCache; + const onAltEnter = () => { + onBlur(sql); + }; - constructor(props: Props) { - super(props); - this.state = { - sql: props.sql, - words: [], - }; - - // The editor changeSelection is called multiple times in a row, - // faster than React reconciliation process, so the selected text - // needs to be stored out of the state to ensure changes to it - // get saved immediately - this.currentSelectionCache = ''; - this.onChange = this.onChange.bind(this); - } - - componentDidMount() { - // Making sure no text is selected from previous mount - this.props.actions.queryEditorSetSelectedText(this.props.queryEditor, null); - if (this.props.queryEditor.dbId) { - this.props.actions.queryEditorSetFunctionNames( - this.props.queryEditor, - this.props.queryEditor.dbId, - ); - } - this.setAutoCompleter(this.props); - } - - UNSAFE_componentWillReceiveProps(nextProps: Props) { - if ( - !areArraysShallowEqual(this.props.tables, nextProps.tables) || - !areArraysShallowEqual(this.props.schemas, nextProps.schemas) || - !areArraysShallowEqual( - this.props.extendedTables, - nextProps.extendedTables, - ) - ) { - this.setAutoCompleter(nextProps); - } - if (nextProps.sql !== this.props.sql) { - this.setState({ sql: nextProps.sql }); - } - } - - onBlur() { - this.props.onBlur(this.state.sql); - } - - onAltEnter() { - this.props.onBlur(this.state.sql); - } - - onEditorLoad(editor: any) { + const onEditorLoad = (editor: any) => { editor.commands.addCommand({ name: 'runQuery', bindKey: { win: 'Alt-enter', mac: 'Alt-enter' }, exec: () => { - this.onAltEnter(); + onAltEnter(); }, }); - this.props.hotkeys.forEach(keyConfig => { + hotkeys.forEach(keyConfig => { editor.commands.addCommand({ name: keyConfig.name, bindKey: { win: keyConfig.key, mac: keyConfig.key }, @@ -162,27 +144,23 @@ class AceEditorWrapper extends React.PureComponent { // Backspace trigger 1 character selection, ignoring if ( - selectedText !== this.currentSelectionCache && + selectedText !== currentSelectionCache.current && selectedText.length !== 1 ) { - this.props.actions.queryEditorSetSelectedText( - this.props.queryEditor, - selectedText, - ); + dispatch(queryEditorSetSelectedText(queryEditor, selectedText)); } - this.currentSelectionCache = selectedText; + currentSelectionCache.current = selectedText; }); - } + }; - onChange(text: string) { - this.setState({ sql: text }); - this.props.onChange(text); - } + const onChangeText = (text: string) => { + setSql(text); + onChange(text); + }; - setAutoCompleter(props: Props) { + const setAutoCompleter = () => { // Loading schema, table and column names as auto-completable words - const schemas = props.schemas || []; const schemaWords = schemas.map(s => ({ name: s.label, value: s.value, @@ -191,9 +169,6 @@ class AceEditorWrapper extends React.PureComponent { })); const columns = {}; - const tables = props.tables || []; - const extendedTables = props.extendedTables || []; - const tableWords = tables.map(t => { const tableName = t.value; const extendedTable = extendedTables.find(et => et.name === tableName); @@ -217,7 +192,7 @@ class AceEditorWrapper extends React.PureComponent { meta: 'column', })); - const functionWords = props.functionNames.map(func => ({ + const functionWords = functionNames.map(func => ({ name: func, value: func, score: SQL_FUNCTIONS_AUTOCOMPLETE_SCORE, @@ -227,11 +202,8 @@ class AceEditorWrapper extends React.PureComponent { const completer = { insertMatch: (editor: Editor, data: any) => { if (data.meta === 'table') { - this.props.actions.addTable( - this.props.queryEditor, - this.props.database, - data.value, - this.props.queryEditor.schema, + dispatch( + addTable(queryEditor, database, data.value, queryEditor.schema), ); } @@ -257,11 +229,11 @@ class AceEditorWrapper extends React.PureComponent { completer, })); - this.setState({ words }); - } + setWords(words); + }; - getAceAnnotations() { - const { validationResult } = this.props.queryEditor; + const getAceAnnotations = () => { + const { validationResult } = queryEditor; const resultIsReady = validationResult?.completed; if (resultIsReady && validationResult?.errors?.length) { const errors = validationResult.errors.map((err: any) => ({ @@ -273,42 +245,22 @@ class AceEditorWrapper extends React.PureComponent { return errors; } return []; - } - - render() { - return ( - - ); - } -} - -function mapStateToProps( - { sqlLab: { unsavedQueryEditor } }: SqlLabRootState, - { queryEditor }: OwnProps, -) { - const currentQueryEditor = { - ...queryEditor, - ...(queryEditor.id === unsavedQueryEditor.id && unsavedQueryEditor), }; - return { - queryEditor: currentQueryEditor, - sql: currentQueryEditor.sql, - schemas: currentQueryEditor.schemaOptions || [], - tables: currentQueryEditor.tableOptions, - functionNames: currentQueryEditor.functionNames, - }; -} -export default connect(mapStateToProps)( - AceEditorWrapper, -); + + return ( + + ); +}; + +export default AceEditorWrapper; diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx b/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx index 163c6408ad..e2003c8acb 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx +++ b/superset-frontend/src/SqlLab/components/SqlEditor/SqlEditor.test.jsx @@ -18,6 +18,7 @@ */ import React from 'react'; import { mount } from 'enzyme'; +import { render } from 'spec/helpers/testing-library'; import { supersetTheme, ThemeProvider } from '@superset-ui/core'; import { Provider } from 'react-redux'; import thunk from 'redux-thunk'; @@ -48,6 +49,13 @@ import { defaultQueryEditor, } from 'src/SqlLab/fixtures'; +jest.mock('src/components/AsyncAceEditor', () => ({ + ...jest.requireActual('src/components/AsyncAceEditor'), + FullSQLEditor: props => ( +
{JSON.stringify(props)}
+ ), +})); + const MOCKED_SQL_EDITOR_HEIGHT = 500; fetchMock.get('glob:*/api/v1/database/*', { result: [] }); @@ -79,6 +87,12 @@ const store = mockStore({ }, }); +const setup = (props = {}, store) => + render(, { + useRedux: true, + ...(store && { store }), + }); + describe('SqlEditor', () => { const mockedProps = { actions: { @@ -118,21 +132,61 @@ describe('SqlEditor', () => { const wrapper = buildWrapper(updatedProps); expect(wrapper.find(EmptyStateBig)).toExist(); }); + it('render a SqlEditorLeftBar', async () => { const wrapper = buildWrapper(); await waitForComponentToPaint(wrapper); expect(wrapper.find(SqlEditorLeftBar)).toExist(); }); + it('render an AceEditorWrapper', async () => { const wrapper = buildWrapper(); await waitForComponentToPaint(wrapper); expect(wrapper.find(AceEditorWrapper)).toExist(); }); + + it('renders sql from unsaved change', () => { + const expectedSql = 'SELECT updated_column\nFROM updated_table\nWHERE'; + const { getByTestId } = setup( + mockedProps, + mockStore({ + ...initialState, + sqlLab: { + ...initialState.sqlLab, + databases: { + dbid1: { + allow_ctas: false, + allow_cvas: false, + allow_dml: false, + allow_file_upload: false, + allow_run_async: false, + backend: 'postgresql', + database_name: 'examples', + expose_in_sqllab: true, + force_ctas_schema: null, + id: 1, + }, + }, + unsavedQueryEditor: { + id: defaultQueryEditor.id, + dbId: 'dbid1', + sql: expectedSql, + }, + }, + }), + ); + + expect(getByTestId('react-ace')).toHaveTextContent( + JSON.stringify({ value: expectedSql }).slice(1, -1), + ); + }); + it('render a SouthPane', async () => { const wrapper = buildWrapper(); await waitForComponentToPaint(wrapper); expect(wrapper.find(ConnectedSouthPane)).toExist(); }); + // TODO eschutho convert tests to RTL // eslint-disable-next-line jest/no-disabled-tests it.skip('does not overflow the editor window', async () => { @@ -146,6 +200,7 @@ describe('SqlEditor', () => { SQL_EDITOR_GUTTER_HEIGHT; expect(totalSize).toEqual(MOCKED_SQL_EDITOR_HEIGHT); }); + // eslint-disable-next-line jest/no-disabled-tests it.skip('does not overflow the editor window after resizing', async () => { const wrapper = buildWrapper(); @@ -159,6 +214,7 @@ describe('SqlEditor', () => { SQL_EDITOR_GUTTER_HEIGHT; expect(totalSize).toEqual(450); }); + it('render a Limit Dropdown', async () => { const defaultQueryLimit = 101; const updatedProps = { ...mockedProps, defaultQueryLimit }; diff --git a/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx b/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx index d6947c1ba5..3e8ee84f6c 100644 --- a/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx +++ b/superset-frontend/src/SqlLab/components/SqlEditor/index.jsx @@ -163,8 +163,13 @@ const SqlEditor = ({ const theme = useTheme(); const dispatch = useDispatch(); - const { database, latestQuery, hideLeftBar } = useSelector( - ({ sqlLab: { unsavedQueryEditor, databases, queries } }) => { + const { currentQueryEditor, database, latestQuery, hideLeftBar } = + useSelector(({ sqlLab: { unsavedQueryEditor, databases, queries } }) => { + const currentQueryEditor = { + ...queryEditor, + ...(queryEditor.id === unsavedQueryEditor.id && unsavedQueryEditor), + }; + let { dbId, latestQueryId, hideLeftBar } = queryEditor; if (unsavedQueryEditor.id === queryEditor.id) { dbId = unsavedQueryEditor.dbId || dbId; @@ -172,12 +177,12 @@ const SqlEditor = ({ hideLeftBar = unsavedQueryEditor.hideLeftBar || hideLeftBar; } return { + currentQueryEditor, database: databases[dbId], latestQuery: queries[latestQueryId], hideLeftBar, }; - }, - ); + }); const queryEditors = useSelector(({ sqlLab }) => sqlLab.queryEditors); @@ -608,11 +613,10 @@ const SqlEditor = ({ >