Merge pull request apache#75 from airbnb/chris--webpack-4-cherry-pick

[webpack 4] cherry pick
kristw · Aug 8, 2018 · 7b2585b · 7b2585b
2 parents 96ac68b + 3042407
commit 7b2585b
Show file tree

Hide file tree

Showing 18 changed files with 1,925 additions and 461 deletions.
diff --git a/superset/__init__.py b/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,
+    )
 
 
 #################################################################

diff --git a/superset/assets/.babelrc b/superset/assets/.babelrc
@@ -1,3 +1,4 @@
 {
-  "presets" : ["airbnb"],
+  "presets" : ["airbnb", "react", "env"],
+  "plugins": ["syntax-dynamic-import"],
 }
diff --git a/superset/assets/.eslintrc b/superset/assets/.eslintrc
@@ -1,6 +1,7 @@
 {
   "extends": "airbnb",
-  "parserOptions":{
+  "parser": "babel-eslint",
+  "parserOptions": {
     "ecmaFeatures": {
       "experimentalObjectRestSpread": true
     }

diff --git a/superset/assets/.istanbul.yml b/superset/assets/.istanbul.yml
@@ -3,7 +3,8 @@ instrumentation:
     root: './src'
     extensions: ['.js', '.jsx']
     excludes: [
-        'dist/**'
+        'dist/**',
+        'visualizations/index.js',
     ]
     embed-source: false
     variable: __coverage__

diff --git a/superset/assets/package.json b/superset/assets/package.json
@@ -10,12 +10,10 @@
   "scripts": {
     "test": "mocha --require ignore-styles --compilers js:babel-core/register --require spec/helpers/browser.js 'spec/**/*_spec.*'",
     "test:one": "mocha --require ignore-styles --compilers js:babel-core/register --require spec/helpers/browser.js",
-    "cover": "babel-node node_modules/.bin/babel-istanbul cover _mocha -- --require ignore-styles spec/helpers/browser.js '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",
+    "cover": "babel-node node_modules/.bin/babel-istanbul cover _mocha -- --compilers babel-core/register --require spec/helpers/browser.js --require ignore-styles 'spec/**/*_spec.*'",
+    "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"
@@ -40,6 +38,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",
@@ -129,13 +130,16 @@
   "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",
@@ -146,16 +150,16 @@
     "eslint-plugin-prettier": "^2.6.0",
     "eslint-plugin-react": "^7.0.1",
     "exports-loader": "^0.7.0",
-    "extract-text-webpack-plugin": "3.0.2",
-    "file-loader": "^0.11.1",
+    "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",
     "prettier": "^1.12.1",
@@ -166,10 +170,10 @@
     "style-loader": "^0.18.2",
     "transform-loader": "^0.2.3",
     "uglifyjs-webpack-plugin": "^1.1.0",
-    "url-loader": "^0.6.2",
-    "//": "Known issues with >=4",
-    "webpack": "^3.10.0",
-    "webpack-manifest-plugin": "2.0.3",
-    "webworkify-webpack": "2.1.2"
+    "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"
   }
 }
diff --git a/superset/assets/spec/helpers/browser.js b/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'];
 

diff --git a/superset/assets/spec/javascripts/chart/Chart_spec.jsx b/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();

diff --git a/superset/assets/src/chart/Chart.jsx b/superset/assets/src/chart/Chart.jsx
@@ -1,4 +1,4 @@
-/* eslint camelcase: 0 */
+/* eslint no-undef: 2 */
 import React from 'react';
 import PropTypes from 'prop-types';
 import Mustache from 'mustache';
@@ -10,7 +10,7 @@ import Loading from '../components/Loading';
 import { Logger, LOG_ACTIONS_RENDER_CHART } 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';
 
@@ -54,7 +54,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;
@@ -66,16 +70,23 @@ 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) {
-      const { formData } = this.props;
-      this.props.actions.runQuery(formData, false, this.props.timeout, this.props.chartId);
+      this.props.actions.runQuery(
+        this.props.formData,
+        false,
+        this.props.timeout,
+        this.props.chartId,
+      );
     } else {
       // when drag/dropping in a dashboard, a chart may be unmounted/remounted but still have data
-      this.renderViz();
+      this.renderVis();
     }
+
+    this.loadAsyncVis(this.props.vizType);
   }
 
   componentWillReceiveProps(nextProps) {
@@ -84,6 +95,10 @@ 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) {
@@ -97,10 +112,14 @@ class Chart extends React.PureComponent {
         prevProps.width !== this.props.width ||
         prevProps.lastRendered !== this.props.lastRendered)
     ) {
-      this.renderViz();
+      this.renderVis();
     }
   }
 
+  componentWillUnmount() {
+    this.visPromise = null;
+  }
+
   getFilters() {
     return this.props.getFilters();
   }
@@ -109,6 +128,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.warn(error); // eslint-disable-line
+        this.props.actions.chartRenderingFailed(error, this.props.chartId);
+      });
+  }
+
   addFilter(col, vals, merge = true, refresh = true) {
     this.props.addFilter(col, vals, merge, refresh);
   }
@@ -148,6 +183,7 @@ class Chart extends React.PureComponent {
     return this.props.datasource.verbose_map[metric] || metric;
   }
 
+  // eslint-disable-next-line camelcase
   render_template(s) {
     const context = {
       width: this.width(),
@@ -176,49 +212,56 @@ class Chart extends React.PureComponent {
     return null;
   }
 
-  renderViz() {
-    const { vizType, formData, queryResponse, setControlValue, chartId, chartStatus } = this.props;
-    const visRenderer = visMap[vizType];
-    const renderStart = Logger.getTimestamp();
-    try {
-      // Executing user-defined data mutator function
-      if (formData.js_data) {
-        queryResponse.data = sandboxedEval(formData.js_data)(queryResponse.data);
-      }
-      visRenderer(this, queryResponse, setControlValue);
-      if (chartStatus !== 'rendered') {
-        this.props.actions.chartRenderingSucceeded(chartId);
+  renderVis() {
+    const { chartStatus } = this.props;
+    const hasVisPromise = !!this.state.renderVis;
+    // check that we have the render function and data
+    if (hasVisPromise && ['success', 'rendered'].indexOf(chartStatus) > -1) {
+      const { vizType, formData, queryResponse, setControlValue, chartId } = this.props;
+      const renderStart = Logger.getTimestamp();
+
+      try {
+        // Executing user-defined data mutator function
+        if (formData.js_data) {
+          queryResponse.data = sandboxedEval(formData.js_data)(queryResponse.data);
+        }
+        // [re]rendering the visualization
+        this.state.renderVis(this, queryResponse, setControlValue);
+
+        if (chartStatus !== 'rendered') {
+          this.props.actions.chartRenderingSucceeded(chartId);
+        }
+
+        Logger.append(LOG_ACTIONS_RENDER_CHART, {
+          slice_id: chartId,
+          viz_type: vizType,
+          start_offset: renderStart,
+          duration: Logger.getTimestamp() - renderStart,
+        });
+      } catch (e) {
+        console.warn(e); // eslint-disable-line
+        this.props.actions.chartRenderingFailed(e, chartId);
       }
-      Logger.append(LOG_ACTIONS_RENDER_CHART, {
-        slice_id: chartId,
-        viz_type: vizType,
-        start_offset: renderStart,
-        duration: Logger.getTimestamp() - renderStart,
-      });
-      this.props.actions.chartRenderingSucceeded(chartId);
-    } catch (e) {
-      console.error(e); // eslint-disable-line no-console
-      this.props.actions.chartRenderingFailed(e, chartId);
     }
   }
 
   render() {
-    const isLoading = this.props.chartStatus === 'loading';
+    const isLoading = this.props.chartStatus === 'loading' || !this.state.renderVis;
 
     // this allows <Loading /> to be positioned in the middle of the chart
     const containerStyles = isLoading ? { height: this.height(), width: this.width() } : null;
     return (
       <div className={`chart-container ${isLoading ? 'is-loading' : ''}`} style={containerStyles}>
         {this.renderTooltip()}
-        {isLoading &&
-          <Loading size={75} />
-        }
-        {this.props.chartAlert &&
-        <StackTraceMessage
-          message={this.props.chartAlert}
-          queryResponse={this.props.queryResponse}
-        />
-        }
+
+        {isLoading && <Loading size={50} />}
+
+        {this.props.chartAlert && (
+          <StackTraceMessage
+            message={this.props.chartAlert}
+            queryResponse={this.props.queryResponse}
+          />
+        )}
 
         {!isLoading &&
           !this.props.chartAlert &&

diff --git a/superset/assets/src/components/Loading.jsx b/superset/assets/src/components/Loading.jsx
@@ -8,14 +8,14 @@ const defaultProps = {
   size: 50,
 };
 
-export default function Loading(props) {
+export default function Loading({ size }) {
   return (
     <img
       className="loading"
       alt="Loading..."
       src="/static/assets/images/loading.gif"
       style={{
-        width: Math.min(props.size, 50),
+        width: Math.min(size, 50),
         // height is auto
         padding: 0,
         margin: 0,