[cherry-pick] Performance PRs

superset/__init__.py

@@ -47,23 +47,37 @@ def parse_manifest_json():
     global manifest
     try:
         with open(MANIFEST_FILE, 'r') as f:
-            manifest = json.load(f)
+            # the manifest inclues non-entry files
+            # we only need entries in templates
+            full_manifest = json.load(f)
+            manifest = full_manifest.get('entrypoints', {})
     except Exception:
         pass
 
 
-def get_manifest_file(filename):
+def get_js_manifest_files(filename):
     if app.debug:
         parse_manifest_json()
-    return '/static/assets/dist/' + manifest.get(filename, '')
+    entry_files = manifest.get(filename, {})
+    return entry_files.get('js', [])
+
+
+def get_css_manifest_files(filename):
+    if app.debug:
+        parse_manifest_json()
+    entry_files = manifest.get(filename, {})
+    return entry_files.get('css', [])
 
 
 parse_manifest_json()
 
 
 @app.context_processor
-def get_js_manifest():
-    return dict(js_manifest=get_manifest_file)
+def get_manifest():
+    return dict(
+        js_manifest=get_js_manifest_files,
+        css_manifest=get_css_manifest_files,
+    )
 
 
 #################################################################

superset/assets/.babelrc

@@ -1,3 +1,4 @@
 {
-  "presets" : ["airbnb"],
+  "presets" : ["airbnb", "react", "env"],
+  "plugins": ["syntax-dynamic-import"],
 }

superset/assets/.eslintrc

@@ -1,6 +1,7 @@
 {
   "extends": "airbnb",
-  "parserOptions":{
+  "parser": "babel-eslint",
+  "parserOptions": {
     "ecmaFeatures": {
       "experimentalObjectRestSpread": true
     }

superset/assets/package.json

@@ -10,11 +10,9 @@
   "scripts": {
     "test": "mocha --require ignore-styles --compilers js:babel-core/register --require spec/helpers/browser.js --recursive spec/**/*_spec.*",
     "cover": "babel-node node_modules/.bin/babel-istanbul cover _mocha -- --require ignore-styles spec/helpers/browser.js --recursive spec/**/*_spec.*",
-    "dev": "NODE_ENV=dev webpack --watch --colors --progress --debug --output-pathinfo --devtool eval-cheap-source-map",
-    "dev-slow": "NODE_ENV=dev webpack --watch --colors --progress --debug --output-pathinfo --devtool inline-source-map",
-    "dev-fast": "echo 'dev-fast in now replaced by dev'",
-    "prod": "NODE_ENV=production node --max_old_space_size=4096 ./node_modules/webpack/bin/webpack.js -p --colors --progress",
-    "build": "NODE_ENV=production webpack --colors --progress",
+    "dev": "webpack --mode=development --colors --progress --debug --watch",
+    "prod": "node --max_old_space_size=4096 webpack --mode=production --colors --progress",
+    "build": "webpack --mode=production --colors --progress",
     "lint": "eslint --ignore-path=.eslintignore --ext .js,.jsx .",
     "lint-fix": "eslint --fix --ignore-path=.eslintignore --ext .js,.jsx .",
     "sync-backend": "babel-node --presets env src/syncBackend.js"
@@ -39,6 +37,9 @@
   "bugs": {
     "url": "https://github.com/apache/incubator-superset/issues"
   },
+  "engines": {
+    "node": ">= 6.11.5 <7.0.0 || >= 8.9.0"
+  },
   "homepage": "http://superset.apache.org/",
   "dependencies": {
     "@data-ui/event-flow": "^0.0.8",
@@ -120,31 +121,34 @@
   "devDependencies": {
     "babel-cli": "^6.14.0",
     "babel-core": "^6.10.4",
+    "babel-eslint": "^8.2.2",
     "babel-istanbul": "^0.12.2",
-    "babel-loader": "^7.0.0",
+    "babel-loader": "^7.1.4",
     "babel-plugin-css-modules-transform": "^1.1.0",
+    "babel-plugin-dynamic-import-node": "^1.2.0",
+    "babel-plugin-syntax-dynamic-import": "^6.18.0",
     "babel-polyfill": "^6.23.0",
     "babel-preset-airbnb": "^2.1.1",
     "chai": "^4.0.2",
-    "clean-webpack-plugin": "^0.1.16",
+    "clean-webpack-plugin": "^0.1.19",
     "css-loader": "^0.28.0",
     "enzyme": "^2.0.0",
     "eslint": "^4.19.0",
     "eslint-config-airbnb": "^15.0.1",
     "eslint-plugin-import": "^2.2.0",
     "eslint-plugin-jsx-a11y": "^5.1.1",
     "eslint-plugin-react": "^7.0.1",
-    "exports-loader": "^0.6.3",
-    "extract-text-webpack-plugin": "3.0.0",
-    "file-loader": "^0.11.1",
+    "exports-loader": "^0.7.0",
+    "file-loader": "^1.1.11",
     "github-changes": "^1.0.4",
     "ignore-styles": "^5.0.1",
     "imports-loader": "^0.7.1",
     "istanbul": "^1.0.0-alpha",
     "jsdom": "9.12.0",
     "json-loader": "^0.5.4",
     "less": "^2.6.1",
-    "less-loader": "^4.0.3",
+    "less-loader": "^4.1.0",
+    "mini-css-extract-plugin": "^0.4.0",
     "mocha": "^3.2.0",
     "npm-check-updates": "^2.14.0",
     "react-addons-test-utils": "^15.6.2",
@@ -154,9 +158,10 @@
     "style-loader": "^0.18.2",
     "transform-loader": "^0.2.3",
     "uglifyjs-webpack-plugin": "^1.1.0",
-    "url-loader": "^0.6.2",
-    "webpack": "^3.8.1",
-    "webpack-manifest-plugin": "1.3.2",
-    "webworkify-webpack": "2.1.0"
+    "url-loader": "^1.0.1",
+    "webpack": "^4.6.0",
+    "webpack-assets-manifest": "^3.0.1",
+    "webpack-cli": "^2.0.10",
+    "webpack-sources": "^1.1.0"
   }
 }

superset/assets/spec/helpers/browser.js

@@ -3,7 +3,12 @@ import 'babel-polyfill';
 import chai from 'chai';
 import jsdom from 'jsdom';
 
-require('babel-register')();
+require('babel-register')({
+  // NOTE: If `dynamic-import-node` is in .babelrc alongside
+  // `syntax-dynamic-import` it breaks webpack's bundle splitting capability.
+  // So only load during runtime on the node-side (in tests)
+  plugins: ['dynamic-import-node'],
+});
 
 const exposedProperties = ['window', 'navigator', 'document'];
 

superset/assets/spec/javascripts/chart/Chart_spec.jsx

@@ -38,10 +38,10 @@ describe('Chart', () => {
       <Chart {...mockedProps} />,
     );
   });
-  describe('renderViz', () => {
+  describe('renderVis', () => {
     let stub;
     beforeEach(() => {
-      stub = sinon.stub(wrapper.instance(), 'renderViz');
+      stub = sinon.stub(wrapper.instance(), 'renderVis');
     });
     afterEach(() => {
       stub.restore();

superset/assets/src/chart/Chart.jsx

@@ -10,7 +10,7 @@ import Loading from '../components/Loading';
 import { Logger, LOG_ACTIONS_RENDER_EVENT } from '../logger';
 import StackTraceMessage from '../components/StackTraceMessage';
 import RefreshChartOverlay from '../components/RefreshChartOverlay';
-import visMap from '../visualizations';
+import visPromiseLookup from '../visualizations';
 import sandboxedEval from '../modules/sandbox';
 import './chart.css';
 
@@ -58,7 +58,11 @@ const defaultProps = {
 class Chart extends React.PureComponent {
   constructor(props) {
     super(props);
-    this.state = {};
+    // visualizations are lazy-loaded with promises that resolve to a renderVis function
+    this.state = {
+      renderVis: null,
+    };
+
     // these properties are used by visualizations
     this.annotationData = props.annotationData;
     this.containerId = props.containerId;
@@ -72,15 +76,20 @@ class Chart extends React.PureComponent {
     this.headerHeight = this.headerHeight.bind(this);
     this.height = this.height.bind(this);
     this.width = this.width.bind(this);
+    this.visPromise = null;
   }
 
   componentDidMount() {
     if (this.props.triggerQuery) {
-      this.props.actions.runQuery(this.props.formData, false,
+      this.props.actions.runQuery(
+        this.props.formData,
+        false,
         this.props.timeout,
         this.props.chartKey,
       );
     }
+
+    this.loadAsyncVis(this.props.vizType);
   }
 
   componentWillReceiveProps(nextProps) {
@@ -89,23 +98,31 @@ class Chart extends React.PureComponent {
     this.selector = `#${this.containerId}`;
     this.formData = nextProps.formData;
     this.datasource = nextProps.datasource;
+    if (nextProps.vizType !== this.props.vizType) {
+      this.setState(() => ({ renderVis: null }));
+      this.loadAsyncVis(nextProps.vizType);
+    }
   }
 
   componentDidUpdate(prevProps) {
     if (
-        this.props.queryResponse &&
-        ['success', 'rendered'].indexOf(this.props.chartStatus) > -1 &&
-        !this.props.queryResponse.error && (
-        prevProps.annotationData !== this.props.annotationData ||
+      this.props.queryResponse &&
+      ['success', 'rendered'].indexOf(this.props.chartStatus) > -1 &&
+      !this.props.queryResponse.error &&
+      (prevProps.annotationData !== this.props.annotationData ||
         prevProps.queryResponse !== this.props.queryResponse ||
         prevProps.height !== this.props.height ||
         prevProps.width !== this.props.width ||
         prevProps.lastRendered !== this.props.lastRendered)
     ) {
-      this.renderViz();
+      this.renderVis();
     }
   }
 
+  componentWillUnmount() {
+    this.visPromise = null;
+  }
+
   getFilters() {
     return this.props.getFilters();
   }
@@ -114,6 +131,22 @@ class Chart extends React.PureComponent {
     this.setState({ tooltip });
   }
 
+  loadAsyncVis(visType) {
+    this.visPromise = visPromiseLookup[visType];
+
+    this.visPromise()
+      .then((renderVis) => {
+        // ensure Component is still mounted
+        if (this.visPromise) {
+          this.setState({ renderVis }, this.renderVis);
+        }
+      })
+      .catch((error) => {
+        console.error(error); // eslint-disable-line
+        this.props.actions.chartRenderingFailed(error, this.props.chartKey);
+      });
+  }
+
   addFilter(col, vals, merge = true, refresh = true) {
     this.props.addFilter(col, vals, merge, refresh);
   }
@@ -185,69 +218,77 @@ class Chart extends React.PureComponent {
     return null;
   }
 
-  renderViz() {
-    const viz = visMap[this.props.vizType];
-    const fd = this.props.formData;
-    const qr = this.props.queryResponse;
-    const renderStart = Logger.getTimestamp();
-    try {
-      // Executing user-defined data mutator function
-      if (fd.js_data) {
-        qr.data = sandboxedEval(fd.js_data)(qr.data);
+  renderVis() {
+    // check that we have the render function and data
+    if (this.state.renderVis && ['success', 'rendered'].indexOf(this.props.chartStatus) > -1) {
+      const fd = this.props.formData;
+      const qr = this.props.queryResponse;
+      const renderStart = Logger.getTimestamp();
+
+      try {
+        // Executing user-defined data mutator function
+        if (fd.js_data) {
+          qr.data = sandboxedEval(fd.js_data)(qr.data);
+        }
+        // [re]rendering the visualization
+        this.state.renderVis(this, qr, this.props.setControlValue);
+
+        Logger.append(LOG_ACTIONS_RENDER_EVENT, {
+          label: this.props.chartKey,
+          vis_type: this.props.vizType,
+          start_offset: renderStart,
+          duration: Logger.getTimestamp() - renderStart,
+        });
+
+        this.props.actions.chartRenderingSucceeded(this.props.chartKey);
+      } catch (e) {
+        console.error(e); // eslint-disable-line
+        this.props.actions.chartRenderingFailed(e, this.props.chartKey);
       }
-      // [re]rendering the visualization
-      viz(this, qr, this.props.setControlValue);
-      Logger.append(LOG_ACTIONS_RENDER_EVENT, {
-        label: this.props.chartKey,
-        vis_type: this.props.vizType,
-        start_offset: renderStart,
-        duration: Logger.getTimestamp() - renderStart,
-      });
-      this.props.actions.chartRenderingSucceeded(this.props.chartKey);
-    } catch (e) {
-      this.props.actions.chartRenderingFailed(e, this.props.chartKey);
     }
   }
 
   render() {
-    const isLoading = this.props.chartStatus === 'loading';
+    const isLoading = this.props.chartStatus === 'loading' || !this.state.renderVis;
     return (
       <div className={`token col-md-12 ${isLoading ? 'is-loading' : ''}`}>
         {this.renderTooltip()}
-        {isLoading &&
-          <Loading size={25} />
-        }
-        {this.props.chartAlert &&
-        <StackTraceMessage
-          message={this.props.chartAlert}
-          queryResponse={this.props.queryResponse}
-        />
-        }
+
+        {isLoading && <Loading size={25} />}
+
+        {this.props.chartAlert && (
+          <StackTraceMessage
+            message={this.props.chartAlert}
+            queryResponse={this.props.queryResponse}
+          />
+        )}
 
         {!isLoading &&
           !this.props.chartAlert &&
           this.props.refreshOverlayVisible &&
           !this.props.errorMessage &&
-          this.container &&
-          <RefreshChartOverlay
-            height={this.height()}
-            width={this.width()}
-            onQuery={this.props.onQuery}
-            onDismiss={this.props.onDismissRefreshOverlay}
-          />
-        }
-        {!isLoading && !this.props.chartAlert &&
-          <ChartBody
-            containerId={this.containerId}
-            vizType={this.props.vizType}
-            height={this.height}
-            width={this.width}
-            faded={this.props.refreshOverlayVisible && !this.props.errorMessage}
-            ref={(inner) => {
-              this.container = inner;
-            }}
-          />
-        }
+          this.container && (
+            <RefreshChartOverlay
+              height={this.height()}
+              width={this.width()}
+              onQuery={this.props.onQuery}
+              onDismiss={this.props.onDismissRefreshOverlay}
+            />
+          )}
+
+        {!isLoading &&
+          !this.props.chartAlert && (
+            <ChartBody
+              containerId={this.containerId}
+              vizType={this.props.vizType}
+              height={this.height}
+              width={this.width}
+              faded={this.props.refreshOverlayVisible && !this.props.errorMessage}
+              ref={(inner) => {
+                this.container = inner;
+              }}
+            />
+          )}
       </div>
     );
   }