Build: optimize frontend build configs to improve superset-ui-plugin dev experience (#9326)

* Upgrade webpack, babel and React

* Upgrade all Babel related packages

Also remove babel-plugin-css-modules-transform that is not in use.

* Remvoe tslib as dependency

* Remove unnecesary packages

superset-frontend/babel.config.js

@@ -16,30 +16,36 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+const packageConfig = require('./package.json');
+
 module.exports = {
   sourceMaps: true,
-  sourceType: 'unambiguous',
+  sourceType: 'module',
   retainLines: true,
   presets: [
-    '@babel/preset-react',
     [
       '@babel/preset-env',
       {
         useBuiltIns: 'usage',
         corejs: 3,
         loose: true,
-        shippedProposals: true,
         modules: false,
-        targets: false,
+        shippedProposals: true,
+        targets: packageConfig.browserslist,
       },
     ],
+    [
+      '@babel/preset-react',
+      { development: process.env.BABEL_ENV === 'development' },
+    ],
   ],
   plugins: [
     'lodash',
     '@babel/plugin-syntax-dynamic-import',
     '@babel/plugin-proposal-class-properties',
-    'react-hot-loader/babel',
+    '@babel/plugin-proposal-optional-chaining',
     ['@babel/plugin-transform-runtime', { corejs: 3 }],
+    'react-hot-loader/babel',
   ],
   env: {
     // Setup a different config for tests as they run in node instead of a browser
@@ -52,8 +58,8 @@ module.exports = {
             corejs: 3,
             loose: true,
             shippedProposals: true,
-            targets: { node: 'current' },
             modules: 'commonjs',
+            targets: { node: 'current' },
           },
         ],
       ],

superset-frontend/jest.config.js

@@ -37,6 +37,10 @@ module.exports = {
       diagnostics: {
         warnOnly: true,
       },
+      tsConfig: {
+        jsx: 'react',
+        esModuleInterop: true,
+      },
     },
   },
 };

superset-frontend/package.json

@@ -12,7 +12,7 @@
     "test": "NODE_ENV=test jest",
     "cover": "NODE_ENV=test jest --coverage",
     "dev": "webpack --mode=development --colors --progress --debug --watch",
-    "dev-server": "node --max_old_space_size=4096 ./node_modules/webpack-dev-server/bin/webpack-dev-server.js --mode=development --progress",
+    "dev-server": "NODE_ENV=development BABEL_ENV=development node --max_old_space_size=4096 ./node_modules/webpack-dev-server/bin/webpack-dev-server.js --mode=development --progress",
     "prod": "node --max_old_space_size=4096 ./node_modules/webpack/bin/webpack.js --mode=production --colors --progress",
     "build-dev": "cross-env NODE_OPTIONS=--max_old_space_size=8192 NODE_ENV=development webpack --mode=development --colors --progress",
     "build": "cross-env NODE_OPTIONS=--max_old_space_size=8192 NODE_ENV=production webpack --mode=production --colors --progress",
@@ -120,7 +120,7 @@
     "omnibar": "^2.1.1",
     "prop-types": "^15.6.0",
     "re-resizable": "^4.3.1",
-    "react": "^16.9.0",
+    "react": "^16.13.0",
     "react-ace": "^5.10.0",
     "react-bootstrap": "^0.31.5",
     "react-bootstrap-dialog": "^0.10.0",
@@ -130,9 +130,9 @@
     "react-datetime": "^2.14.0",
     "react-dnd": "^2.5.4",
     "react-dnd-html5-backend": "^2.5.4",
-    "react-dom": "^16.9.0",
+    "react-dom": "^16.13.0",
     "react-gravatar": "^2.6.1",
-    "react-hot-loader": "^4.3.6",
+    "react-hot-loader": "^4.12.20",
     "react-json-tree": "^0.11.2",
     "react-jsonschema-form": "^1.2.0",
     "react-markdown": "^4.3.1",
@@ -161,19 +161,20 @@
   },
   "devDependencies": {
     "@babel/cli": "^7.8.4",
-    "@babel/core": "^7.5.5",
-    "@babel/node": "^7.5.5",
-    "@babel/plugin-proposal-class-properties": "^7.7.4",
-    "@babel/plugin-syntax-dynamic-import": "^7.2.0",
+    "@babel/core": "^7.8.7",
+    "@babel/node": "^7.8.7",
+    "@babel/plugin-proposal-class-properties": "^7.8.3",
+    "@babel/plugin-proposal-optional-chaining": "^7.8.3",
+    "@babel/plugin-syntax-dynamic-import": "^7.8.3",
     "@babel/plugin-transform-runtime": "^7.8.3",
     "@babel/preset-env": "^7.8.7",
     "@babel/preset-react": "^7.8.3",
-    "@babel/preset-typescript": "^7.8.3",
     "@babel/register": "^7.8.6",
-    "@types/jest": "^23.3.5",
+    "@hot-loader/react-dom": "^16.13.0",
+    "@types/jest": "^25.1.4",
     "@types/jquery": "^3.3.32",
-    "@types/react": "^16.4.18",
-    "@types/react-dom": "^16.0.9",
+    "@types/react": "^16.9.23",
+    "@types/react-dom": "^16.9.5",
     "@types/react-json-tree": "^0.6.11",
     "@types/react-redux": "^7.1.7",
     "@types/react-select": "^3.0.10",
@@ -181,15 +182,11 @@
     "@types/yargs": "12 - 15",
     "@typescript-eslint/eslint-plugin": "^2.20.0",
     "@typescript-eslint/parser": "^2.20.0",
-    "babel-core": "^7.0.0-bridge.0",
-    "babel-eslint": "^10.0.3",
-    "babel-jest": "^24.8.0",
-    "babel-loader": "^8.0.4",
-    "babel-plugin-css-modules-transform": "^1.1.0",
-    "babel-plugin-dynamic-import-node": "^1.2.0",
+    "babel-eslint": "^10.1.0",
+    "babel-jest": "^25.1.0",
+    "babel-loader": "^8.0.6",
+    "babel-plugin-dynamic-import-node": "^2.3.0",
     "babel-plugin-lodash": "^3.3.4",
-    "babel-plugin-typescript-to-proptypes": "^1.3.2",
-    "babel-preset-airbnb": "^4.0.1",
     "cache-loader": "^1.2.2",
     "clean-webpack-plugin": "^3.0.0",
     "copy-webpack-plugin": "^5.1.1",
@@ -214,7 +211,7 @@
     "fork-ts-checker-webpack-plugin": "^0.4.9",
     "ignore-styles": "^5.0.1",
     "imports-loader": "^0.7.1",
-    "jest": "^24.8.0",
+    "jest": "^25.1.0",
     "jsdom": "9.12.0",
     "less": "^3.9.0",
     "less-loader": "^5.0.0",
@@ -230,17 +227,16 @@
     "terser-webpack-plugin": "^1.1.0",
     "thread-loader": "^1.2.0",
     "transform-loader": "^0.2.3",
-    "ts-jest": "^24.0.2",
-    "ts-loader": "^5.4.5",
-    "tslib": "^1.10.0",
-    "typescript": "^3.8.2",
+    "ts-jest": "^25.2.1",
+    "ts-loader": "^6.2.1",
+    "typescript": "^3.8.3",
     "url-loader": "^1.0.1",
     "webpack": "^4.42.0",
-    "webpack-assets-manifest": "^3.0.1",
+    "webpack-assets-manifest": "^3.1.1",
     "webpack-bundle-analyzer": "^3.6.1",
-    "webpack-cli": "^3.1.1",
-    "webpack-dev-server": "^3.1.14",
-    "webpack-sources": "^1.1.0",
+    "webpack-cli": "^3.3.11",
+    "webpack-dev-server": "^3.10.3",
+    "webpack-sources": "^1.4.3",
     "yargs": "12 - 15"
   },
   "optionalDependencies": {

superset-frontend/spec/javascripts/explore/components/SaveModal_spec.jsx

@@ -196,12 +196,20 @@ describe('SaveModal', () => {
 
     describe('should always reload or redirect', () => {
       let wrapper;
+      let windowLocation;
+
       beforeEach(() => {
         wrapper = getWrapper();
+        windowLocation = window.location;
+        // To bypass "TypeError: Cannot redefine property: assign"
+        Object.defineProperty(window, 'location', {
+          value: { ...windowLocation, assign: () => {} },
+        });
         sinon.stub(window.location, 'assign');
       });
       afterEach(() => {
         window.location.assign.restore();
+        Object.defineProperty(window, 'location', windowLocation);
       });
 
       it('Save & go to dashboard', done => {

superset-frontend/src/SqlLab/App.jsx

@@ -20,7 +20,7 @@ import React from 'react';
 import { createStore, compose, applyMiddleware } from 'redux';
 import { Provider } from 'react-redux';
 import thunkMiddleware from 'redux-thunk';
-import { hot } from 'react-hot-loader';
+import { hot } from 'react-hot-loader/root';
 
 import {
   initFeatureFlags,
@@ -112,4 +112,4 @@ const Application = () => (
   </Provider>
 );
 
-export default hot(module)(Application);
+export default hot(Application);

superset-frontend/src/addSlice/App.jsx

@@ -17,7 +17,7 @@
  * under the License.
  */
 import React from 'react';
-import { hot } from 'react-hot-loader';
+import { hot } from 'react-hot-loader/root';
 import setupApp from '../setup/setupApp';
 import setupPlugins from '../setup/setupPlugins';
 import AddSliceContainer from './AddSliceContainer';
@@ -32,4 +32,4 @@ const bootstrapData = JSON.parse(
 
 const App = () => <AddSliceContainer datasources={bootstrapData.datasources} />;
 
-export default hot(module)(App);
+export default hot(App);

superset-frontend/src/dashboard/App.jsx

@@ -20,7 +20,7 @@ import React from 'react';
 import thunk from 'redux-thunk';
 import { createStore, applyMiddleware, compose } from 'redux';
 import { Provider } from 'react-redux';
-import { hot } from 'react-hot-loader';
+import { hot } from 'react-hot-loader/root';
 
 import { initFeatureFlags } from 'src/featureFlags';
 import { initEnhancer } from '../reduxUtils';
@@ -51,4 +51,4 @@ const App = () => (
   </Provider>
 );
 
-export default hot(module)(App);
+export default hot(App);

superset-frontend/src/explore/App.jsx

@@ -17,18 +17,10 @@
  * under the License.
  */
 import React from 'react';
-import { hot } from 'react-hot-loader';
-import { createStore, applyMiddleware, compose } from 'redux';
+import { hot } from 'react-hot-loader/root';
 import { Provider } from 'react-redux';
-import thunk from 'redux-thunk';
-
-import { initFeatureFlags } from 'src/featureFlags';
-import { initEnhancer } from '../reduxUtils';
-import logger from '../middleware/loggerMiddleware';
 import ToastPresenter from '../messageToasts/containers/ToastPresenter';
 import ExploreViewContainer from './components/ExploreViewContainer';
-import getInitialState from './reducers/getInitialState';
-import rootReducer from './reducers/index';
 
 import setupApp from '../setup/setupApp';
 import setupPlugins from '../setup/setupPlugins';
@@ -38,20 +30,7 @@ import '../../stylesheets/reactable-pagination.less';
 setupApp();
 setupPlugins();
 
-const exploreViewContainer = document.getElementById('app');
-const bootstrapData = JSON.parse(
-  exploreViewContainer.getAttribute('data-bootstrap'),
-);
-initFeatureFlags(bootstrapData.common.feature_flags);
-const initState = getInitialState(bootstrapData);
-
-const store = createStore(
-  rootReducer,
-  initState,
-  compose(applyMiddleware(thunk, logger), initEnhancer(false)),
-);
-
-const App = () => (
+const App = ({ store }) => (
   <Provider store={store}>
     <div>
       <ExploreViewContainer />
@@ -60,4 +39,4 @@ const App = () => (
   </Provider>
 );
 
-export default hot(module)(App);
+export default hot(App);

superset-frontend/src/explore/index.jsx

@@ -18,6 +18,27 @@
  */
 import React from 'react';
 import ReactDOM from 'react-dom';
+import { createStore, applyMiddleware, compose } from 'redux';
+import thunk from 'redux-thunk';
+import logger from '../middleware/loggerMiddleware';
+import { initFeatureFlags } from '../featureFlags';
+import { initEnhancer } from '../reduxUtils';
+import getInitialState from './reducers/getInitialState';
+import rootReducer from './reducers/index';
+
 import App from './App';
 
-ReactDOM.render(<App />, document.getElementById('app'));
+const exploreViewContainer = document.getElementById('app');
+const bootstrapData = JSON.parse(
+  exploreViewContainer.getAttribute('data-bootstrap'),
+);
+initFeatureFlags(bootstrapData.common.feature_flags);
+const initState = getInitialState(bootstrapData);
+
+const store = createStore(
+  rootReducer,
+  initState,
+  compose(applyMiddleware(thunk, logger), initEnhancer(false)),
+);
+
+ReactDOM.render(<App store={store} />, document.getElementById('app'));

superset-frontend/src/profile/App.jsx

@@ -17,7 +17,7 @@
  * under the License.
  */
 import React from 'react';
-import { hot } from 'react-hot-loader';
+import { hot } from 'react-hot-loader/root';
 import thunk from 'redux-thunk';
 import { createStore, applyMiddleware, compose, combineReducers } from 'redux';
 import { Provider } from 'react-redux';
@@ -50,4 +50,4 @@ const Application = () => (
   </Provider>
 );
 
-export default hot(module)(Application);
+export default hot(Application);

superset-frontend/src/setup/setupApp.ts

@@ -49,7 +49,9 @@ function toggleCheckbox(apiUrlPrefix: string, selector: string) {
 
 export default function setupApp() {
   $(document).ready(function() {
-    $(':checkbox[data-checkbox-api-prefix]').change(function() {
+    $(':checkbox[data-checkbox-api-prefix]').change(function(
+      this: HTMLElement,
+    ) {
       const $this = $(this);
       const prefix = $this.data('checkbox-api-prefix');
       const id = $this.attr('id');

superset-frontend/src/welcome/App.jsx

@@ -17,7 +17,7 @@
  * under the License.
  */
 import React from 'react';
-import { hot } from 'react-hot-loader';
+import { hot } from 'react-hot-loader/root';
 import thunk from 'redux-thunk';
 import { createStore, applyMiddleware, compose, combineReducers } from 'redux';
 import { Provider } from 'react-redux';
@@ -72,4 +72,4 @@ const App = () => (
   </Provider>
 );
 
-export default hot(module)(App);
+export default hot(App);

superset-frontend/tsconfig.json

@@ -3,10 +3,10 @@
     "allowJs": true,
     "allowSyntheticDefaultImports": true,
     "baseUrl": ".",
-    "esModuleInterop": true,
+    "esModuleInterop": false,
     "forceConsistentCasingInFileNames": true,
-    "importHelpers": true,
-    "jsx": "react",
+    "importHelpers": false,
+    "jsx": "preserve",
     "lib": ["dom", "esnext"],
     "module": "esnext",
     "moduleResolution": "node",
@@ -20,7 +20,13 @@
     "sourceMap": true,
     "strictNullChecks": true,
     "suppressImplicitAnyIndexErrors": true,
-    "target": "es5"
+    "target": "esnext"
   },
-  "include": ["./src/**/*", "./spec/**/*"]
+  "include": [
+    "./src/**/*",
+    "./spec/**/*",
+    "./node_modules/*superset-ui*/**/src/**/*",
+    "./node_modules/*superset-ui*/**/types/**/*",
+    "./node_modules/*superset-ui*/**/node_modules/**/*.d.ts"
+  ]
 }

superset-frontend/webpack.config.js

@@ -18,7 +18,6 @@
  * under the License.
  */
 const fs = require('fs');
-const os = require('os');
 const path = require('path');
 const webpack = require('webpack');
 const BundleAnalyzerPlugin = require('webpack-bundle-analyzer')
@@ -32,6 +31,7 @@ const TerserPlugin = require('terser-webpack-plugin');
 const WebpackAssetsManifest = require('webpack-assets-manifest');
 const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin');
 const parsedArgs = require('yargs').argv;
+const packageConfig = require('./package.json');
 
 // input dir
 const APP_DIR = path.resolve(__dirname, './');
@@ -84,6 +84,7 @@ const plugins = [
 
   // runs type checking on a separate process to speed up the build
   new ForkTsCheckerWebpackPlugin({
+    eslint: true,
     checkSyntacticErrors: true,
   }),
 
@@ -96,10 +97,7 @@ const plugins = [
     { copyUnmodified: true },
   ),
 ];
-if (isDevMode) {
-  // Enable hot module replacement
-  plugins.push(new webpack.HotModuleReplacementPlugin());
-} else {
+if (!isDevMode) {
   // text loading (webpack 4+)
   plugins.push(
     new MiniCssExtractPlugin({
@@ -110,45 +108,29 @@ if (isDevMode) {
   plugins.push(new OptimizeCSSAssetsPlugin());
 }
 
-const BABEL_JAVASCRIPT_OPTIONS = {
-  presets: [
-    [
-      '@babel/preset-env',
-      {
-        useBuiltIns: 'usage',
-        corejs: 3,
-        loose: true,
-        shippedProposals: true,
-        modules: false,
-        targets: false,
-      },
-    ],
-    '@babel/preset-react',
-  ],
-  plugins: [
-    'lodash',
-    'react-hot-loader/babel',
-    '@babel/plugin-proposal-object-rest-spread',
-    '@babel/plugin-proposal-class-properties',
-    '@babel/plugin-syntax-dynamic-import',
-  ],
-};
-
-const BABEL_TYPESCRIPT_OPTIONS = {
-  presets: BABEL_JAVASCRIPT_OPTIONS.presets.concat([
-    '@babel/preset-typescript',
-  ]),
-  plugins: BABEL_JAVASCRIPT_OPTIONS.plugins.concat([
-    'babel-plugin-typescript-to-proptypes',
-  ]),
-};
-
-const PREAMBLE = ['babel-polyfill', path.join(APP_DIR, '/src/preamble.js')];
+const PREAMBLE = [path.join(APP_DIR, '/src/preamble.js')];
+if (isDevMode) {
+  // A Superset webpage normally includes two JS bundles in dev, `theme.js` and
+  // the main entrypoint. Only the main entry should have the dev server client,
+  // otherwise the websocket client will initialize twice, creating two sockets.
+  // Ref: https://github.com/gaearon/react-hot-loader/issues/141
+  PREAMBLE.unshift(
+    `webpack-dev-server/client?http://localhost:${devserverPort}`,
+  );
+}
 
 function addPreamble(entry) {
   return PREAMBLE.concat([path.join(APP_DIR, entry)]);
 }
 
+const babelLoader = {
+  loader: 'babel-loader',
+  options: {
+    cacheDirectory: true,
+    cacheCompression: false,
+  },
+};
+
 const config = {
   node: {
     fs: 'empty',
@@ -165,6 +147,13 @@ const config = {
     showSavedQuery: [path.join(APP_DIR, '/src/showSavedQuery/index.jsx')],
   },
   output,
+  stats: 'minimal',
+  performance: {
+    assetFilter(assetFilename) {
+      // don't throw size limit warning on geojson and font files
+      return !/\.(map|geojson|woff2)$/.test(assetFilename);
+    },
+  },
   optimization: {
     splitChunks: {
       chunks: 'all',
@@ -174,7 +163,7 @@ const config = {
         default: false,
         major: {
           name: 'vendors-major',
-          test: /[\\/]node_modules\/(brace|react[-]dom|@superset[-]ui\/translation)[\\/]/,
+          test: /\/node_modules\/(brace|react|react-dom|@superset-ui\/translation|webpack.*|@babel.*)\//,
         },
       },
     },
@@ -182,6 +171,7 @@ const config = {
   resolve: {
     alias: {
       src: path.resolve(APP_DIR, './src'),
+      'react-dom': '@hot-loader/react-dom',
     },
     extensions: ['.ts', '.tsx', '.js', '.jsx'],
     symlinks: false,
@@ -199,57 +189,29 @@ const config = {
       {
         test: /\.tsx?$/,
         use: [
-          { loader: 'cache-loader' },
-          {
-            loader: 'thread-loader',
-            options: {
-              // there should be 1 cpu for the fork-ts-checker-webpack-plugin
-              workers: os.cpus().length - 1,
-            },
-          },
+          'thread-loader',
+          babelLoader,
           {
             loader: 'ts-loader',
             options: {
               // transpile only in happyPack mode
               // type checking is done via fork-ts-checker-webpack-plugin
               happyPackMode: true,
+              transpileOnly: true,
             },
           },
         ],
       },
       {
         test: /\.jsx?$/,
-        exclude: /node_modules/,
-        include: APP_DIR,
-        loader: 'babel-loader',
-      },
-      {
-        // handle symlinked modules
-        // for debugging @superset-ui packages via npm link
-        test: /\.jsx?$/,
-        include: /node_modules\/[@]superset[-]ui.+\/src/,
-        use: [
-          {
-            loader: 'babel-loader',
-            options: BABEL_JAVASCRIPT_OPTIONS,
-          },
-        ],
-      },
-      {
-        // handle symlinked modules
-        // for debugging @superset-ui packages via npm link
-        test: /\.tsx?$/,
-        include: /node_modules\/[@]superset[-]ui.+\/src/,
-        use: [
-          {
-            loader: 'babel-loader',
-            options: BABEL_TYPESCRIPT_OPTIONS,
-          },
-        ],
+        // include source code for plugins, but exclude node_modules within them
+        exclude: [/superset-ui.*\/node_modules\/.*/],
+        include: [new RegExp(`${APP_DIR}/src`), /superset-ui.*\/src/],
+        use: [babelLoader],
       },
       {
         test: /\.css$/,
-        include: [APP_DIR, /superset[-]ui.+\/src/],
+        include: [APP_DIR, /superset-ui.+\/src/],
         use: [
           isDevMode ? 'style-loader' : MiniCssExtractPlugin.loader,
           {
@@ -331,8 +293,7 @@ function loadProxyConfig() {
 }
 
 if (isDevMode) {
-  config.devtool = 'cheap-module-eval-source-map';
-
+  config.devtool = 'eval-cheap-module-source-map';
   config.devServer = {
     before() {
       loadProxyConfig();
@@ -341,6 +302,8 @@ if (isDevMode) {
     },
     historyApiFallback: true,
     hot: true,
+    injectClient: false,
+    injectHot: true,
     inline: true,
     stats: 'minimal',
     overlay: true,
@@ -355,6 +318,24 @@ if (isDevMode) {
     ],
     contentBase: path.join(process.cwd(), '../static/assets'),
   };
+
+  // find all the symlinked plugins and use their source code for imports
+  let hasSymlink = false;
+  for (const [pkg, version] of Object.entries(packageConfig.dependencies)) {
+    const srcPath = `./node_modules/${pkg}/src`;
+    if (/superset-ui/.test(pkg) && fs.existsSync(srcPath)) {
+      console.log(
+        `[Superset Plugin] Use symlink source for ${pkg} @ ${version}`,
+      );
+      // only allow exact match so imports like `@superset-ui/plugin-name/lib`
+      // and `@superset-ui/plugin-name/esm` can still work.
+      config.resolve.alias[`${pkg}$`] = `${pkg}/src`;
+      hasSymlink = true;
+    }
+  }
+  if (hasSymlink) {
+    console.log(''); // pure cosmetic new line
+  }
 } else {
   config.optimization.minimizer = [
     new TerserPlugin({

superset-frontend/webpack.proxy-config.js

@@ -68,6 +68,7 @@ function toDevHTML(originalHtml) {
     loadManifest();
   }
   if (manifest) {
+    const loaded = new Set();
     // replace bundled asset files, HTML comment tags generated by Jinja macros
     // in superset/templates/superset/partials/asset_bundle.html
     html = html.replace(
@@ -77,6 +78,13 @@ function toDevHTML(originalHtml) {
           return `<!-- DEV bundle: ${bundleName} ${assetType} START -->\n  ${(
             manifest.entrypoints[bundleName][assetType] || []
           )
+            .filter(chunkFilePath => {
+              if (loaded.has(chunkFilePath)) {
+                return false;
+              }
+              loaded.add(chunkFilePath);
+              return true;
+            })
             .map(chunkFilePath =>
               assetType === 'css'
                 ? `<link rel="stylesheet" type="text/css" href="${chunkFilePath}" />`