You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2020/09/11 22:10:06 UTC
[GitHub] [incubator-superset] ktmud commented on a change in pull request #10837: feat: move ace-editor and mathjs to async modules
ktmud commented on a change in pull request #10837:
URL: https://github.com/apache/incubator-superset/pull/10837#discussion_r487171528
##########
File path: superset-frontend/src/components/AsyncEsmComponent.tsx
##########
@@ -0,0 +1,123 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React, { useEffect, useState, RefObject } from 'react';
+import Loading from './Loading';
+
+type PlaceholderProps = {
+ showLoadingForImport: boolean;
+ width?: string | number;
+ height?: string | number;
+} & {
+ [key: string]: any;
+};
+
+function DefaultPlaceholder({
+ width,
+ height,
+ showLoadingForImport,
+}: PlaceholderProps) {
+ return (
+ // since `width` defaults to 100%, we can display the placeholder once
+ // height is specified.
+ (height && (
+ <div style={{ width, height }}>
+ {showLoadingForImport && <Loading position="floating" />}
Review comment:
This is for displaying a loading spinner while the async module is still loading:
E.g.:
Normally users wouldn't see this because most places we used the `AsyncEsmComponent` have on-demand preloading.
##########
File path: superset-frontend/webpack.config.js
##########
@@ -55,9 +55,12 @@ const output = {
if (isDevMode) {
output.filename = '[name].[hash:8].entry.js';
output.chunkFilename = '[name].[hash:8].chunk.js';
-} else {
+} else if (nameChunks) {
output.filename = '[name].[chunkhash].entry.js';
output.chunkFilename = '[name].[chunkhash].chunk.js';
+} else {
+ output.filename = '[name].[chunkhash].entry.js';
+ output.chunkFilename = '[chunkhash].chunk.js';
Review comment:
Don't add names to `chunkFiles` (they are just single digit numbers anyway).
##########
File path: superset-frontend/src/components/AsyncAceEditor.tsx
##########
@@ -0,0 +1,166 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import {
+ Editor,
+ IEditSession,
+ Position,
+ TextMode as OrigTextMode,
+} from 'brace';
+import AceEditor, { AceEditorProps } from 'react-ace';
+import AsyncEsmComponent from 'src/components/AsyncEsmComponent';
+
+type TextMode = OrigTextMode & { $id: string };
+
+/**
+ * Async loaders to import brace modules. Must manually create call `import(...)`
+ * promises because webpack can only analyze asycn imports statically.
+ */
+const aceModuleLoaders = {
+ 'mode/sql': () => import('brace/mode/sql'),
+ 'mode/markdown': () => import('brace/mode/markdown'),
+ 'mode/css': () => import('brace/mode/css'),
+ 'mode/json': () => import('brace/mode/json'),
+ 'mode/yaml': () => import('brace/mode/yaml'),
+ 'mode/html': () => import('brace/mode/html'),
+ 'mode/javascript': () => import('brace/mode/javascript'),
+ 'theme/textmate': () => import('brace/theme/textmate'),
+ 'theme/github': () => import('brace/theme/github'),
+ 'ext/language_tools': () => import('brace/ext/language_tools'),
+};
+export type AceModule = keyof typeof aceModuleLoaders;
+
+export interface AceCompleterKeywordData {
+ name: string;
+ value: string;
+ score: number;
+ meta: string;
+}
+export interface AceCompleterKeyword extends AceCompleterKeywordData {
+ completer?: {
+ insertMatch: (editor: Editor, data: AceCompleterKeywordData) => void;
+ };
+}
+
+export type AsyncAceEditorProps = AceEditorProps & {
+ keywords?: AceCompleterKeyword[];
+};
+
+export type AceEditorMode = 'sql';
+export type AceEditorTheme = 'textmate' | 'github';
+export type AsyncAceEditorOptions = {
+ defaultMode?: AceEditorMode;
+ defaultTheme?: AceEditorTheme;
+ defaultTabSize?: number;
+};
+
+/**
+ * Get an async AceEditor with automatical loading of specified ace modules.
+ */
+export default function AsyncAceEditor(
+ aceModules: AceModule[],
+ { defaultMode, defaultTheme, defaultTabSize = 2 }: AsyncAceEditorOptions = {},
Review comment:
Set default `tabSize` for all AceEditor. Previously there were some places where `tabSize` was not set and would default to `4`.
##########
File path: superset-frontend/webpack.config.js
##########
@@ -232,16 +240,12 @@ const config = {
'jquery',
'core-js.*',
'@emotion.*',
- 'd3.*',
+ 'd3',
+ 'd3-(array|color|scale||interpolateformat|selection|collection|time|time-format)',
].join('|')})/`,
),
},
// bundle large libraries separately
- brace: {
- name: 'brace',
- test: /\/node_modules\/(brace|react-ace)\//,
- priority: 40,
- },
Review comment:
Different `brace` modes/themes now load separately and asynchronously.
##########
File path: superset-frontend/src/explore/components/controls/TextAreaControl.jsx
##########
@@ -73,18 +73,18 @@ export default class TextAreaControl extends React.Component {
}
renderEditor(inModal = false) {
const value = this.props.value || '';
+ const minLines = inModal ? 40 : this.props.minLines || 12;
if (this.props.language) {
return (
- <AceEditor
+ <TextAreaEditor
mode={this.props.language}
- theme="textmate"
style={{ border: '1px solid #CCC' }}
- minLines={inModal ? 40 : this.props.minLines}
+ minLines={minLines}
maxLines={inModal ? 1000 : this.props.maxLines}
- onChange={this.onAceChange.bind(this)}
+ onChange={this.onAceChangeDebounce}
width="100%"
+ height={`${minLines}em`}
editorProps={{ $blockScrolling: true }}
- enableLiveAutocompletion
Review comment:
Removed this prop according to a console warning from Ace.
##########
File path: superset-frontend/src/explore/components/controls/TextAreaControl.jsx
##########
@@ -73,18 +73,18 @@ export default class TextAreaControl extends React.Component {
}
renderEditor(inModal = false) {
const value = this.props.value || '';
+ const minLines = inModal ? 40 : this.props.minLines || 12;
if (this.props.language) {
return (
- <AceEditor
+ <TextAreaEditor
mode={this.props.language}
- theme="textmate"
style={{ border: '1px solid #CCC' }}
- minLines={inModal ? 40 : this.props.minLines}
+ minLines={minLines}
maxLines={inModal ? 1000 : this.props.maxLines}
- onChange={this.onAceChange.bind(this)}
+ onChange={this.onAceChangeDebounce}
Review comment:
Added `debounce` to text inputs in Explore view's Datasource editor.
##########
File path: superset-frontend/webpack.config.js
##########
@@ -214,13 +219,16 @@ const config = {
'react',
'react-dom',
'prop-types',
+ 'react-prop-types',
+ 'prop-types-extra',
'redux',
'react-redux',
'react-hot-loader',
'react-select',
'react-sortable-hoc',
'react-virtualized',
'react-table',
+ 'react-ace',
Review comment:
`react-ace` itself is quite small.
##########
File path: superset-frontend/src/components/AsyncAceEditor.tsx
##########
@@ -0,0 +1,166 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import {
+ Editor,
+ IEditSession,
+ Position,
+ TextMode as OrigTextMode,
+} from 'brace';
+import AceEditor, { AceEditorProps } from 'react-ace';
+import AsyncEsmComponent from 'src/components/AsyncEsmComponent';
+
+type TextMode = OrigTextMode & { $id: string };
+
+/**
+ * Async loaders to import brace modules. Must manually create call `import(...)`
+ * promises because webpack can only analyze asycn imports statically.
+ */
+const aceModuleLoaders = {
+ 'mode/sql': () => import('brace/mode/sql'),
+ 'mode/markdown': () => import('brace/mode/markdown'),
+ 'mode/css': () => import('brace/mode/css'),
+ 'mode/json': () => import('brace/mode/json'),
+ 'mode/yaml': () => import('brace/mode/yaml'),
+ 'mode/html': () => import('brace/mode/html'),
+ 'mode/javascript': () => import('brace/mode/javascript'),
+ 'theme/textmate': () => import('brace/theme/textmate'),
+ 'theme/github': () => import('brace/theme/github'),
+ 'ext/language_tools': () => import('brace/ext/language_tools'),
+};
+export type AceModule = keyof typeof aceModuleLoaders;
+
+export interface AceCompleterKeywordData {
+ name: string;
+ value: string;
+ score: number;
+ meta: string;
+}
+export interface AceCompleterKeyword extends AceCompleterKeywordData {
+ completer?: {
+ insertMatch: (editor: Editor, data: AceCompleterKeywordData) => void;
+ };
+}
+
+export type AsyncAceEditorProps = AceEditorProps & {
+ keywords?: AceCompleterKeyword[];
+};
+
+export type AceEditorMode = 'sql';
+export type AceEditorTheme = 'textmate' | 'github';
+export type AsyncAceEditorOptions = {
+ defaultMode?: AceEditorMode;
+ defaultTheme?: AceEditorTheme;
+ defaultTabSize?: number;
+};
+
+/**
+ * Get an async AceEditor with automatical loading of specified ace modules.
+ */
+export default function AsyncAceEditor(
+ aceModules: AceModule[],
+ { defaultMode, defaultTheme, defaultTabSize = 2 }: AsyncAceEditorOptions = {},
+) {
+ return AsyncEsmComponent(async () => {
+ const { default: ace } = await import('brace');
+ const { default: ReactAceEditor } = await import('react-ace');
+
+ await Promise.all(aceModules.map(x => aceModuleLoaders[x]()));
+
+ const inferredMode =
+ defaultMode ||
+ aceModules.find(x => x.startsWith('mode/'))?.replace('mode/', '');
+ const inferredTheme =
+ defaultTheme ||
+ aceModules.find(x => x.startsWith('theme/'))?.replace('theme/', '');
+
+ return React.forwardRef<AceEditor, AsyncAceEditorProps>(
+ function ExtendedAceEditor(
+ {
+ keywords,
+ mode = inferredMode,
+ theme = inferredTheme,
+ tabSize = defaultTabSize,
+ ...props
+ },
+ ref,
+ ) {
+ if (keywords) {
+ const langTools = ace.acequire('ace/ext/language_tools');
+ const completer = {
+ getCompletions: (
+ editor: AceEditor,
+ session: IEditSession,
+ pos: Position,
+ prefix: string,
+ callback: (error: null, wordList: object[]) => void,
+ ) => {
+ if ((session.getMode() as TextMode).$id === `ace/mode/${mode}`) {
Review comment:
Enable completion for only the related language.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org