You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2022/09/28 00:04:45 UTC
[superset] branch master updated: chore: refactor AceEditorWrapper to functional component (#21532)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 15c3c34268 chore: refactor AceEditorWrapper to functional component (#21532)
15c3c34268 is described below
commit 15c3c342680e041019eab9a327034c77f6456966
Author: EugeneTorap <ev...@gmail.com>
AuthorDate: Wed Sep 28 03:04:34 2022 +0300
chore: refactor AceEditorWrapper to functional component (#21532)
---
.../AceEditorWrapper/AceEditorWrapper.test.tsx | 31 ---
.../SqlLab/components/AceEditorWrapper/index.tsx | 256 +++++++++------------
.../SqlLab/components/SqlEditor/SqlEditor.test.jsx | 56 +++++
.../src/SqlLab/components/SqlEditor/index.jsx | 16 +-
4 files changed, 170 insertions(+), 189 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(
<AceEditorWrapper
queryEditor={queryEditor}
- actions={{
- queryEditorSetSelectedText,
- queryEditorSetFunctionNames,
- addTable,
- }}
height="100px"
hotkeys={[]}
database={{}}
@@ -82,27 +72,6 @@ describe('AceEditorWrapper', () => {
);
});
- 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;
+ onChange: (sql: string) => void;
+ queryEditor: QueryEditor;
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[];
+ extendedTables?: Array<{ name: string; columns: any[] }>;
height: string;
+ hotkeys: HotKey[];
};
-type ReduxProps = {
- queryEditor: QueryEditor;
- sql: string;
- schemas: SchemaOption[];
- tables: any[];
- functionNames: string[];
-};
-
-type Props = ReduxProps & OwnProps;
-
-interface State {
- sql: string;
- words: AceCompleterKeyword[];
-}
-
-class AceEditorWrapper extends React.PureComponent<Props, State> {
- static defaultProps = {
- onBlur: () => {},
- onChange: () => {},
- schemas: [],
- tables: [],
- functionNames: [],
- extendedTables: [],
- };
-
- private currentSelectionCache;
-
- 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() {
+const AceEditorWrapper = ({
+ autocomplete,
+ onBlur = () => {},
+ onChange = () => {},
+ queryEditor,
+ database,
+ extendedTables = [],
+ height,
+ hotkeys,
+}: AceEditorWrapperProps) => {
+ const dispatch = useDispatch();
+
+ const { sql: currentSql } = queryEditor;
+ const functionNames = queryEditor.functionNames ?? [];
+ const schemas = queryEditor.schemaOptions ?? [];
+ const tables = queryEditor.tableOptions ?? [];
+
+ const [sql, setSql] = useState(currentSql);
+ const [words, setWords] = useState<AceCompleterKeyword[]>([]);
+
+ // 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
- this.props.actions.queryEditorSetSelectedText(this.props.queryEditor, null);
- if (this.props.queryEditor.dbId) {
- this.props.actions.queryEditorSetFunctionNames(
- this.props.queryEditor,
- this.props.queryEditor.dbId,
- );
+ dispatch(queryEditorSetSelectedText(queryEditor, null));
+ if (queryEditor.dbId) {
+ dispatch(queryEditorSetFunctionNames(queryEditor, queryEditor.dbId));
}
- this.setAutoCompleter(this.props);
- }
+ setAutoCompleter();
+ }, []);
+
+ const prevTables = usePrevious(tables) ?? [];
+ const prevSchemas = usePrevious(schemas) ?? [];
+ const prevExtendedTables = usePrevious(extendedTables) ?? [];
+ const prevSql = usePrevious(currentSql);
- UNSAFE_componentWillReceiveProps(nextProps: Props) {
+ useEffect(() => {
if (
- !areArraysShallowEqual(this.props.tables, nextProps.tables) ||
- !areArraysShallowEqual(this.props.schemas, nextProps.schemas) ||
- !areArraysShallowEqual(
- this.props.extendedTables,
- nextProps.extendedTables,
- )
+ !areArraysShallowEqual(tables, prevTables) ||
+ !areArraysShallowEqual(schemas, prevSchemas) ||
+ !areArraysShallowEqual(extendedTables, prevExtendedTables)
) {
- this.setAutoCompleter(nextProps);
+ setAutoCompleter();
}
- if (nextProps.sql !== this.props.sql) {
- this.setState({ sql: nextProps.sql });
+ }, [tables, schemas, extendedTables]);
+
+ useEffect(() => {
+ if (currentSql !== prevSql) {
+ setSql(currentSql);
}
- }
+ }, [currentSql]);
- onBlur() {
- this.props.onBlur(this.state.sql);
- }
+ const onBlurSql = () => {
+ onBlur(sql);
+ };
- onAltEnter() {
- this.props.onBlur(this.state.sql);
- }
+ const onAltEnter = () => {
+ onBlur(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<Props, State> {
// 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<Props, State> {
}));
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<Props, State> {
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<Props, State> {
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<Props, State> {
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<Props, State> {
return errors;
}
return [];
- }
+ };
- render() {
- return (
- <AceEditor
- keywords={this.state.words}
- onLoad={this.onEditorLoad.bind(this)}
- onBlur={this.onBlur.bind(this)}
- height={this.props.height}
- onChange={this.onChange}
- width="100%"
- editorProps={{ $blockScrolling: true }}
- enableLiveAutocompletion={this.props.autocomplete}
- value={this.state.sql}
- annotations={this.getAceAnnotations()}
- />
- );
- }
-}
+ return (
+ <AceEditor
+ keywords={words}
+ onLoad={onEditorLoad}
+ onBlur={onBlurSql}
+ height={height}
+ onChange={onChangeText}
+ width="100%"
+ editorProps={{ $blockScrolling: true }}
+ enableLiveAutocompletion={autocomplete}
+ value={sql}
+ annotations={getAceAnnotations()}
+ />
+ );
+};
-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<ReduxProps, {}, OwnProps>(mapStateToProps)(
- AceEditorWrapper,
-);
+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 => (
+ <div data-test="react-ace">{JSON.stringify(props)}</div>
+ ),
+}));
+
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(<SqlEditor {...props} />, {
+ 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 = ({
>
<div ref={northPaneRef} className="north-pane">
<AceEditorWrapper
- actions={actions}
autocomplete={autocompleteEnabled}
onBlur={setQueryEditorAndSaveSql}
onChange={onSqlChanged}
- queryEditor={queryEditor}
+ queryEditor={currentQueryEditor}
database={database}
extendedTables={tables}
height={`${aceEditorHeight}px`}