From 18a73b5653c6d6623f8c26af9ecec26ed0327700 Mon Sep 17 00:00:00 2001 From: Jesse Yang Date: Thu, 19 Mar 2020 18:49:12 -0700 Subject: [PATCH] Use manifest hooks for dev server proxy --- CONTRIBUTING.md | 39 ++++-- superset-frontend/package-lock.json | 115 +++++++----------- superset-frontend/package.json | 2 +- .../src/visualizations/presets/MainPreset.js | 2 +- superset-frontend/webpack.config.js | 68 +++++++---- superset-frontend/webpack.proxy-config.js | 78 ++++-------- 6 files changed, 145 insertions(+), 159 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 648847ec55188..4b6c69bd82c63 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -75,7 +75,7 @@ little bit helps, and credit will always be given. - [Creating a new language dictionary](#creating-a-new-language-dictionary) - [Tips](#tips) - [Adding a new datasource](#adding-a-new-datasource) - - [Creating a new visualization type](#creating-a-new-visualization-type) + - [Improving visualizations](#improving-visualizations) - [Adding a DB migration](#adding-a-db-migration) - [Merging DB migrations](#merging-db-migrations) - [SQL Lab Async](#sql-lab-async) @@ -389,7 +389,7 @@ Make sure your machine meets the [OS dependencies](https://superset.incubator.ap Developers should use a virtualenv. -``` +```bash pip install virtualenv ``` @@ -726,7 +726,7 @@ In TypeScript/JavaScript, the technique is similar: we import `t` (simple translation), `tn` (translation containing a number). ```javascript -import { t, tn } from "@superset-ui/translation"; +import { t, tn } from '@superset-ui/translation'; ``` ### Enabling language selection @@ -803,11 +803,32 @@ Then, [extract strings for the new language](#extracting-new-strings-for-transla This means it'll register MyDatasource and MyOtherDatasource in superset.my_models module in the source registry. -### Creating a new visualization type +### Improving visualizations + +Superset is working towards a plugin system where new visualizations can be installed as optional npm packages. To achieve this goal, +we are not accepting pull requests for new community-contributed visualization types at the moment. However, bugfixes for current visualizations are welcome. To edit the frontend code for visualizations, you will have to check out a copy of [apache-superset/superset-ui-plugins](https://github.com/apache-superset/superset-ui-plugins): + +```bash +git clone https://github.com/apache-superset/superset-ui-plugins.git +yarn && yarn build +``` + +Then use `npm link` to create a symlink of the source code in `superset-frontend/node_modules`: + +```bash +cd incubator-superset/superset-frontend +npm link ../../superset-ui-plugins/packages/superset-ui-[PLUGIN NAME] + +# Or to link all plugin packages: +# npm link ../../superset-ui-plugins/packages/* + +# Start developing +npm run dev-server +``` + +When plugin packages are linked with `npm link`, the dev server will automatically load files from the plugin's `/src` directory. -Here's an example as a Github PR with comments that describe what the -different sections of the code do: -https://github.com/apache/incubator-superset/pull/3013 +Note that every time you do `npm install`, you will lose the symlink(s) and may have to run `npm link` again. ### Adding a DB migration @@ -905,12 +926,14 @@ To do this, you'll need to: - Configure a results backend, here's a local `FileSystemCache` example, not recommended for production, but perfect for testing (stores cache in `/tmp`) + ```python from werkzeug.contrib.cache import FileSystemCache RESULTS_BACKEND = FileSystemCache('/tmp/sqllab') ``` -* Start up a celery worker +- Start up a celery worker + ```shell script celery worker --app=superset.tasks.celery_app:app -Ofair ``` diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index dc63e0d3c2e8c..7fad2dbc907c0 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -17367,6 +17367,17 @@ "readable-stream": "^2.0.0" } }, + "fs-extra": { + "version": "7.0.1", + "resolved": "https://registry.npmjs.org/fs-extra/-/fs-extra-7.0.1.tgz", + "integrity": "sha512-YJDaCJZEnBmcbw13fvdAM9AwNOJwOzrE4pqMqBq5nFiEqXUqHwlK4B+3pUw6JNvfSPtX05xFHtYy/1ni01eGCw==", + "dev": true, + "requires": { + "graceful-fs": "^4.1.2", + "jsonfile": "^4.0.0", + "universalify": "^0.1.0" + } + }, "fs-readdir-recursive": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/fs-readdir-recursive/-/fs-readdir-recursive-1.1.0.tgz", @@ -25683,6 +25694,15 @@ "resolved": "http://registry.npmjs.org/json5/-/json5-0.5.1.tgz", "integrity": "sha1-Hq3nrMASA0rYTiOWdn6tn6VJWCE=" }, + "jsonfile": { + "version": "4.0.0", + "resolved": "https://registry.npmjs.org/jsonfile/-/jsonfile-4.0.0.tgz", + "integrity": "sha1-h3Gq4HmbZAdrdmQPygWPnBDjPss=", + "dev": true, + "requires": { + "graceful-fs": "^4.1.6" + } + }, "jsprim": { "version": "1.4.1", "resolved": "https://registry.npmjs.org/jsprim/-/jsprim-1.4.1.tgz", @@ -25901,12 +25921,6 @@ "resolved": "https://registry.npmjs.org/lodash.get/-/lodash.get-4.4.2.tgz", "integrity": "sha1-LRd/ZS+jHpObRDjVNBSZ36OCXpk=" }, - "lodash.has": { - "version": "4.5.2", - "resolved": "https://registry.npmjs.org/lodash.has/-/lodash.has-4.5.2.tgz", - "integrity": "sha1-0Z9NwQlQWMzL4rDN9O4P5Ko3yGI=", - "dev": true - }, "lodash.isequal": { "version": "4.5.0", "resolved": "https://registry.npmjs.org/lodash.isequal/-/lodash.isequal-4.5.0.tgz", @@ -33951,6 +33965,12 @@ "resolved": "https://registry.npmjs.org/unist-util-visit-parents/-/unist-util-visit-parents-1.1.2.tgz", "integrity": "sha512-yvo+MMLjEwdc3RhhPYSximset7rwjMrdt9E41Smmvg25UQIenzrN83cRnF1JMzoMi9zZOQeYXHSDf7p+IQkW3Q==" }, + "universalify": { + "version": "0.1.2", + "resolved": "https://registry.npmjs.org/universalify/-/universalify-0.1.2.tgz", + "integrity": "sha512-rBJeI5CXAlmy1pV+617WB9J63U6XcazHHF2f2dbJix4XzpUF0RS3Zbj0FGIOCAva5P/d/GBOYaACQ1w+0azUkg==", + "dev": true + }, "unpipe": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/unpipe/-/unpipe-1.0.0.tgz", @@ -35277,69 +35297,6 @@ } } }, - "webpack-assets-manifest": { - "version": "3.1.1", - "resolved": "https://registry.npmjs.org/webpack-assets-manifest/-/webpack-assets-manifest-3.1.1.tgz", - "integrity": "sha512-JV9V2QKc5wEWQptdIjvXDUL1ucbPLH2f27toAY3SNdGZp+xSaStAgpoMcvMZmqtFrBc9a5pTS1058vxyMPOzRQ==", - "dev": true, - "requires": { - "chalk": "^2.0", - "lodash.get": "^4.0", - "lodash.has": "^4.0", - "mkdirp": "^0.5", - "schema-utils": "^1.0.0", - "tapable": "^1.0.0", - "webpack-sources": "^1.0.0" - }, - "dependencies": { - "ansi-styles": { - "version": "3.2.1", - "resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-3.2.1.tgz", - "integrity": "sha512-VT0ZI6kZRdTh8YyJw3SMbYm/u+NqfsAxEpWO0Pf9sq8/e94WxxOpPKx9FR1FlyCtOVDNOQ+8ntlqFxiRc+r5qA==", - "dev": true, - "requires": { - "color-convert": "^1.9.0" - } - }, - "chalk": { - "version": "2.4.1", - "resolved": "https://registry.npmjs.org/chalk/-/chalk-2.4.1.tgz", - "integrity": "sha512-ObN6h1v2fTJSmUXoS3nMQ92LbDK9be4TV+6G+omQlGJFdcUX5heKi1LZ1YnRMIgwTLEj3E24bT6tYni50rlCfQ==", - "dev": true, - "requires": { - "ansi-styles": "^3.2.1", - "escape-string-regexp": "^1.0.5", - "supports-color": "^5.3.0" - } - }, - "schema-utils": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/schema-utils/-/schema-utils-1.0.0.tgz", - "integrity": "sha512-i27Mic4KovM/lnGsy8whRCHhc7VicJajAjTrYg11K9zfZXnYIt4k5F+kZkwjnrhKzLic/HLU4j11mjsz2G/75g==", - "dev": true, - "requires": { - "ajv": "^6.1.0", - "ajv-errors": "^1.0.0", - "ajv-keywords": "^3.1.0" - } - }, - "supports-color": { - "version": "5.5.0", - "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-5.5.0.tgz", - "integrity": "sha512-QjVjwdXIt408MIiAqCX4oUKsgU2EqAGzs2Ppkm4aQYbjm+ZEWEcW4SfFNTr4uMNZma0ey4f5lgLrkB0aX0QMow==", - "dev": true, - "requires": { - "has-flag": "^3.0.0" - } - }, - "tapable": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/tapable/-/tapable-1.1.1.tgz", - "integrity": "sha512-9I2ydhj8Z9veORCw5PRm4u9uebCn0mcCa6scWoNcbZ6dAtoo2618u9UUzxgmsCOreJpqDDuv61LvwofW7hLcBA==", - "dev": true - } - } - }, "webpack-bundle-analyzer": { "version": "3.6.1", "resolved": "https://registry.npmjs.org/webpack-bundle-analyzer/-/webpack-bundle-analyzer-3.6.1.tgz", @@ -36771,6 +36728,26 @@ "uuid": "^3.3.2" } }, + "webpack-manifest-plugin": { + "version": "2.2.0", + "resolved": "https://registry.npmjs.org/webpack-manifest-plugin/-/webpack-manifest-plugin-2.2.0.tgz", + "integrity": "sha512-9S6YyKKKh/Oz/eryM1RyLVDVmy3NSPV0JXMRhZ18fJsq+AwGxUY34X54VNwkzYcEmEkDwNxuEOboCZEebJXBAQ==", + "dev": true, + "requires": { + "fs-extra": "^7.0.0", + "lodash": ">=3.5 <5", + "object.entries": "^1.1.0", + "tapable": "^1.0.0" + }, + "dependencies": { + "tapable": { + "version": "1.1.3", + "resolved": "https://registry.npmjs.org/tapable/-/tapable-1.1.3.tgz", + "integrity": "sha512-4WK/bYZmj8xLr+HUCODHGF1ZFzsYffasLUgEiMBY4fgtltdO6B4WJtlSbPaDTLpYTcGVwM2qLnFTICEcNxs3kA==", + "dev": true + } + } + }, "webpack-sources": { "version": "1.4.3", "resolved": "https://registry.npmjs.org/webpack-sources/-/webpack-sources-1.4.3.tgz", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index a5a161390ef41..4dc3cee578598 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -231,10 +231,10 @@ "typescript": "^3.8.3", "url-loader": "^1.0.1", "webpack": "^4.42.0", - "webpack-assets-manifest": "^3.1.1", "webpack-bundle-analyzer": "^3.6.1", "webpack-cli": "^3.3.11", "webpack-dev-server": "^3.10.3", + "webpack-manifest-plugin": "^2.2.0", "webpack-sources": "^1.4.3", "yargs": "12 - 15" }, diff --git a/superset-frontend/src/visualizations/presets/MainPreset.js b/superset-frontend/src/visualizations/presets/MainPreset.js index f543fe34bc688..12d39c096baeb 100644 --- a/superset-frontend/src/visualizations/presets/MainPreset.js +++ b/superset-frontend/src/visualizations/presets/MainPreset.js @@ -60,7 +60,7 @@ import { LineMultiChartPlugin, PieChartPlugin, TimePivotChartPlugin, -} from '@superset-ui/legacy-preset-chart-nvd3/lib'; +} from '@superset-ui/legacy-preset-chart-nvd3'; import { BoxPlotChartPlugin } from '@superset-ui/preset-chart-xy/esm/legacy'; import { DeckGLChartPreset } from '@superset-ui/legacy-preset-chart-deckgl'; diff --git a/superset-frontend/webpack.config.js b/superset-frontend/webpack.config.js index c26ef0351b277..d16f186acbf22 100644 --- a/superset-frontend/webpack.config.js +++ b/superset-frontend/webpack.config.js @@ -20,17 +20,17 @@ const fs = require('fs'); const path = require('path'); const webpack = require('webpack'); -const BundleAnalyzerPlugin = require('webpack-bundle-analyzer') - .BundleAnalyzerPlugin; +const { BundleAnalyzerPlugin } = require('webpack-bundle-analyzer'); const { CleanWebpackPlugin } = require('clean-webpack-plugin'); const CopyPlugin = require('copy-webpack-plugin'); const MiniCssExtractPlugin = require('mini-css-extract-plugin'); const OptimizeCSSAssetsPlugin = require('optimize-css-assets-webpack-plugin'); const SpeedMeasurePlugin = require('speed-measure-webpack-plugin'); const TerserPlugin = require('terser-webpack-plugin'); -const WebpackAssetsManifest = require('webpack-assets-manifest'); +const ManifestPlugin = require('webpack-manifest-plugin'); const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin'); const parsedArgs = require('yargs').argv; +const getProxyConfig = require('./webpack.proxy-config'); const packageConfig = require('./package.json'); // input dir @@ -60,14 +60,39 @@ if (isDevMode) { const plugins = [ // creates a manifest.json mapping of name to hashed output used in template files - new WebpackAssetsManifest({ - publicPath: true, + new ManifestPlugin({ + publicPath: output.publicPath, + seed: { app: 'superset' }, // This enables us to include all relevant files for an entry - entrypoints: true, + generate: (seed, files, entrypoints) => { + // Each entrypoint's chunk files in the format of + // { + // entry: { + // css: [], + // js: [] + // } + // } + const entryFiles = {}; + for (const [entry, chunks] of Object.entries(entrypoints)) { + entryFiles[entry] = { + css: chunks + .filter(x => x.endsWith('.css')) + .map(x => path.join(output.publicPath, x)), + js: chunks + .filter(x => x.endsWith('.js')) + .map(x => path.join(output.publicPath, x)), + }; + } + return { + ...seed, + // files: filePaths, + entrypoints: entryFiles, + }; + }, // Also write to disk when using devServer // instead of only keeping manifest.json in memory // This is required to make devServer work with flask. - writeToDisk: isDevMode, + writeToFileEmit: isDevMode, }), // create fresh dist/ upon build @@ -127,6 +152,8 @@ const babelLoader = { loader: 'babel-loader', options: { cacheDirectory: true, + // disable gzip compression for cache files + // faster when there are millions of small files cacheCompression: false, }, }; @@ -205,7 +232,7 @@ const config = { { test: /\.jsx?$/, // include source code for plugins, but exclude node_modules within them - exclude: [/superset-ui.*\/node_modules\/.*/], + exclude: [/superset-ui.*\/node_modules\//], include: [new RegExp(`${APP_DIR}/src`), /superset-ui.*\/src/], use: [babelLoader], }, @@ -277,28 +304,17 @@ const config = { devtool: false, }; -let proxyConfig = {}; -const requireModule = module.require; - -function loadProxyConfig() { - try { - delete require.cache[require.resolve('./webpack.proxy-config')]; - proxyConfig = requireModule('./webpack.proxy-config'); - } catch (e) { - if (e.code !== 'ENOENT') { - console.error('\n>> Error loading proxy config:'); - console.trace(e); - } - } -} +let proxyConfig = getProxyConfig(); if (isDevMode) { config.devtool = 'eval-cheap-module-source-map'; config.devServer = { - before() { - loadProxyConfig(); - // hot reloading proxy config - fs.watch('./webpack.proxy-config.js', loadProxyConfig); + before(app, server, compiler) { + // load proxy config when manifest updates + const hook = compiler.hooks.webpackManifestPluginAfterEmit; + hook.tap('ManifestPlugin', manifest => { + proxyConfig = getProxyConfig(manifest); + }); }, historyApiFallback: true, hot: true, diff --git a/superset-frontend/webpack.proxy-config.js b/superset-frontend/webpack.proxy-config.js index a797a5cee56cf..1d7999f6a0d36 100644 --- a/superset-frontend/webpack.proxy-config.js +++ b/superset-frontend/webpack.proxy-config.js @@ -1,4 +1,3 @@ -/* eslint-disable no-console */ /** * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -17,9 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -const fs = require('fs'); const zlib = require('zlib'); -const path = require('path'); // eslint-disable-next-line import/no-extraneous-dependencies const parsedArgs = require('yargs').argv; @@ -28,28 +25,8 @@ const backend = (supersetUrl || `http://localhost:${supersetPort}`).replace( '//+$/', '', ); // strip ending backslash -const MANIFEST_FILE = path.resolve( - __dirname, - '../superset/static/assets/manifest.json', -); -let manifestContent; let manifest; -function loadManifest() { - try { - const newContent = fs.readFileSync(MANIFEST_FILE, { encoding: 'utf-8' }); - if (!newContent || newContent === manifestContent) return; - manifestContent = newContent; - manifest = JSON.parse(manifestContent); - console.log(`${MANIFEST_FILE} loaded.`); - } catch (e) { - if (e.code !== 'ENOENT') { - console.error('\n>> Error loading manifest file:'); - console.trace(e); - } - } -} - function isHTML(res) { const CONTENT_TYPE_HEADER = 'content-type'; const contentType = res.getHeader @@ -63,10 +40,6 @@ function toDevHTML(originalHtml) { /(\s*)([\s\S]*)(<\/title>)/i, '$1[DEV] $2 $3', ); - // load manifest file only when needed - if (!manifest) { - loadManifest(); - } if (manifest) { const loaded = new Set(); // replace bundled asset files, HTML comment tags generated by Jinja macros @@ -152,32 +125,29 @@ function processHTML(proxyResponse, response) { }); } -// make sure the manifest file exists -fs.mkdirSync(path.dirname(MANIFEST_FILE), { recursive: true }); -fs.closeSync(fs.openSync(MANIFEST_FILE, 'as+')); -// watch it as webpack-dev-server updates it -fs.watch(MANIFEST_FILE, loadManifest); - -module.exports = { - context: '/', - target: backend, - hostRewrite: true, - changeOrigin: true, - cookieDomainRewrite: '', // remove cookie domain - selfHandleResponse: true, // so that the onProxyRes takes care of sending the response - onProxyRes(proxyResponse, request, response) { - try { - copyHeaders(proxyResponse, response); - if (isHTML(response)) { - processHTML(proxyResponse, response); - } else { - proxyResponse.pipe(response); +module.exports = newManifest => { + manifest = newManifest; + return { + context: '/', + target: backend, + hostRewrite: true, + changeOrigin: true, + cookieDomainRewrite: '', // remove cookie domain + selfHandleResponse: true, // so that the onProxyRes takes care of sending the response + onProxyRes(proxyResponse, request, response) { + try { + copyHeaders(proxyResponse, response); + if (isHTML(response)) { + processHTML(proxyResponse, response); + } else { + proxyResponse.pipe(response); + } + response.flushHeaders(); + } catch (e) { + response.setHeader('content-type', 'text/plain'); + response.write(`Error requesting ${request.path} from proxy:\n\n`); + response.end(e.stack); } - response.flushHeaders(); - } catch (e) { - response.setHeader('content-type', 'text/plain'); - response.write(`Error requesting ${request.path} from proxy:\n\n`); - response.end(e.stack); - } - }, + }, + }; };