From c36a7e3adae0ef27c3f16f890a396e2463c63bdd Mon Sep 17 00:00:00 2001 From: Jianchao Yang Date: Tue, 17 Mar 2020 15:37:07 -0700 Subject: [PATCH] chore: allow webpack-dev-server proxy to any destination (#9296) One of the pain points in developing Superset frontend code is the lack of testing data. Local installation often do not have enough examples setup to test all edge cases. This change allows `webpack-dev-server` to proxy to any remote Superset service, but the same time replaces frontend asset references in HTML with links to local development version. This allows developers to test with production data locally, tackling edge cases all while maintaining the productivity of editing the code locally. --- CONTRIBUTING.md | 4 + superset-frontend/.eslintrc.js | 2 +- superset-frontend/package.json | 5 +- superset-frontend/webpack.config.js | 81 ++++---- superset-frontend/webpack.proxy-config.js | 175 ++++++++++++++++++ superset/__init__.py | 2 +- superset/extensions.py | 34 ++-- superset/templates/superset/add_slice.html | 4 +- superset/templates/superset/base.html | 33 ++-- superset/templates/superset/basic.html | 18 +- .../superset/models/savedquery/show.html | 4 +- .../{_script_tag.html => asset_bundle.html} | 18 +- superset/templates/superset/welcome.html | 4 +- superset/viz.py | 4 +- 14 files changed, 286 insertions(+), 102 deletions(-) create mode 100644 superset-frontend/webpack.proxy-config.js rename superset/templates/superset/partials/{_script_tag.html => asset_bundle.html} (61%) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cef83d11c..648847ec5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -496,6 +496,10 @@ npm run dev-server -- --devserverPort=9001 # Run the dev server proxying to a Flask server on a non-default port npm run dev-server -- --supersetPort=8081 + +# Or proxy it to a remote backend so you can test frontend changes without +# starting the backend locally +npm run dev-server -- --superset=https://superset-dev.example.com ``` Alternatively you can use one of the following commands. diff --git a/superset-frontend/.eslintrc.js b/superset-frontend/.eslintrc.js index d73326abc..714ce6ebd 100644 --- a/superset-frontend/.eslintrc.js +++ b/superset-frontend/.eslintrc.js @@ -38,7 +38,7 @@ module.exports = { 'prettier', 'prettier/@typescript-eslint', ], - plugins: ['@typescript-eslint', 'prettier', 'react'], + plugins: ['@typescript-eslint/eslint-plugin', 'prettier', 'react'], rules: { '@typescript-eslint/ban-ts-ignore': 0, '@typescript-eslint/camelcase': [ diff --git a/superset-frontend/package.json b/superset-frontend/package.json index 06e56c19f..a30eb10f6 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -177,6 +177,7 @@ "@types/react-redux": "^7.1.7", "@types/react-select": "^3.0.10", "@types/react-table": "^7.0.2", + "@types/yargs": "12 - 15", "@typescript-eslint/eslint-plugin": "^2.20.0", "@typescript-eslint/parser": "^2.20.0", "babel-core": "^7.0.0-bridge.0", @@ -216,7 +217,6 @@ "less": "^3.9.0", "less-loader": "^5.0.0", "mini-css-extract-plugin": "^0.4.0", - "minimist": "^1.2.0", "optimize-css-assets-webpack-plugin": "^5.0.1", "po2json": "^0.4.5", "prettier": "^1.19.1", @@ -238,7 +238,8 @@ "webpack-bundle-analyzer": "^3.4.1", "webpack-cli": "^3.1.1", "webpack-dev-server": "^3.1.14", - "webpack-sources": "^1.1.0" + "webpack-sources": "^1.1.0", + "yargs": "12 - 15" }, "optionalDependencies": { "fsevents": "^2.0.7" diff --git a/superset-frontend/webpack.config.js b/superset-frontend/webpack.config.js index acdec7dd7..e4f909943 100644 --- a/superset-frontend/webpack.config.js +++ b/superset-frontend/webpack.config.js @@ -1,3 +1,4 @@ +/* eslint-disable no-console */ /** * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -16,6 +17,7 @@ * specific language governing permissions and limitations * under the License. */ +const fs = require('fs'); const os = require('os'); const path = require('path'); const webpack = require('webpack'); @@ -29,9 +31,7 @@ const SpeedMeasurePlugin = require('speed-measure-webpack-plugin'); const TerserPlugin = require('terser-webpack-plugin'); const WebpackAssetsManifest = require('webpack-assets-manifest'); const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin'); - -// Parse command-line arguments -const parsedArgs = require('minimist')(process.argv.slice(2)); +const parsedArgs = require('yargs').argv; // input dir const APP_DIR = path.resolve(__dirname, './'); @@ -41,13 +41,23 @@ const BUILD_DIR = path.resolve(__dirname, '../superset/static/assets'); const { mode = 'development', devserverPort = 9000, - supersetPort = 8088, measure = false, analyzeBundle = false, } = parsedArgs; - const isDevMode = mode !== 'production'; +const output = { + path: BUILD_DIR, + publicPath: '/static/assets/', // necessary for lazy-loaded chunks +}; +if (isDevMode) { + output.filename = '[name].[hash:8].entry.js'; + output.chunkFilename = '[name].[hash:8].chunk.js'; +} else { + output.filename = '[name].[chunkhash].entry.js'; + output.chunkFilename = '[name].[chunkhash].chunk.js'; +} + const plugins = [ // creates a manifest.json mapping of name to hashed output used in template files new WebpackAssetsManifest({ @@ -86,7 +96,6 @@ const plugins = [ { copyUnmodified: true }, ), ]; - if (isDevMode) { // Enable hot module replacement plugins.push(new webpack.HotModuleReplacementPlugin()); @@ -101,19 +110,6 @@ if (isDevMode) { plugins.push(new OptimizeCSSAssetsPlugin()); } -const output = { - path: BUILD_DIR, - publicPath: '/static/assets/', // necessary for lazy-loaded chunks -}; - -if (isDevMode) { - output.filename = '[name].[hash:8].entry.js'; - output.chunkFilename = '[name].[hash:8].chunk.js'; -} else { - output.filename = '[name].[chunkhash].entry.js'; - output.chunkFilename = '[name].[chunkhash].chunk.js'; -} - const PREAMBLE = ['babel-polyfill', path.join(APP_DIR, '/src/preamble.js')]; function addPreamble(entry) { @@ -292,27 +288,50 @@ const config = { 'react/lib/ReactContext': true, }, plugins, - devtool: isDevMode ? 'cheap-module-eval-source-map' : false, - devServer: { + 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); + } + } +} + +if (isDevMode) { + config.devtool = 'cheap-module-eval-source-map'; + + config.devServer = { + before() { + loadProxyConfig(); + // hot reloading proxy config + fs.watch('./webpack.proxy-config.js', loadProxyConfig); + }, historyApiFallback: true, hot: true, - index: '', // This line is needed to enable root proxying inline: true, stats: 'minimal', overlay: true, port: devserverPort, // Only serves bundled files from webpack-dev-server // and proxy everything else to Superset backend - proxy: { - context: () => true, - '/': `http://localhost:${supersetPort}`, - target: `http://localhost:${supersetPort}`, - }, + proxy: [ + // functions are called for every request + () => { + return proxyConfig; + }, + ], contentBase: path.join(process.cwd(), '../static/assets'), - }, -}; - -if (!isDevMode) { + }; +} else { config.optimization.minimizer = [ new TerserPlugin({ cache: '.terser-plugin-cache/', diff --git a/superset-frontend/webpack.proxy-config.js b/superset-frontend/webpack.proxy-config.js new file mode 100644 index 000000000..b2a7cda4e --- /dev/null +++ b/superset-frontend/webpack.proxy-config.js @@ -0,0 +1,175 @@ +/* eslint-disable no-console */ +/** + * 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. + */ +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; + +const { supersetPort = 8088, superset: supersetUrl = null } = parsedArgs; +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 + ? res.getHeader(CONTENT_TYPE_HEADER) + : res.headers[CONTENT_TYPE_HEADER]; + return contentType.includes('text/html'); +} + +function toDevHTML(originalHtml) { + let html = originalHtml.replace( + /(\s*)([\s\S]*)(<\/title>)/i, + '$1[DEV] $2 $3', + ); + // load manifest file only when needed + if (!manifest) { + loadManifest(); + } + if (manifest) { + // replace bundled asset files, HTML comment tags generated by Jinja macros + // in superset/templates/superset/partials/asset_bundle.html + html = html.replace( + /<!-- Bundle (css|js) (.*?) START -->[\s\S]*?<!-- Bundle \1 \2 END -->/gi, + (match, assetType, bundleName) => { + if (bundleName in manifest.entrypoints) { + return `<!-- DEV bundle: ${bundleName} ${assetType} START -->\n ${( + manifest.entrypoints[bundleName][assetType] || [] + ) + .map(chunkFilePath => + assetType === 'css' + ? `<link rel="stylesheet" type="text/css" href="${chunkFilePath}" />` + : `<script src="${chunkFilePath}"></script>`, + ) + .join( + '\n ', + )}\n <!-- DEV bundle: ${bundleName} ${assetType} END -->`; + } + return match; + }, + ); + } + return html; +} + +function copyHeaders(originalResponse, response) { + response.statusCode = originalResponse.statusCode; + response.statusMessage = originalResponse.statusMessage; + if (response.setHeader) { + let keys = Object.keys(originalResponse.headers); + if (isHTML(originalResponse)) { + keys = keys.filter( + key => key !== 'content-encoding' && key !== 'content-length', + ); + } + keys.forEach(key => { + let value = originalResponse.headers[key]; + if (key === 'set-cookie') { + // remove cookie domain + value = Array.isArray(value) ? value : [value]; + value = value.map(x => x.replace(/Domain=[^;]+?/i, '')); + } else if (key === 'location') { + // set redirects to use local URL + value = (value || '').replace(backend, ''); + } + response.setHeader(key, value); + }); + } else { + response.headers = originalResponse.headers; + } +} + +/** + * Manipulate HTML server response to replace asset files with + * local webpack-dev-server build. + */ +function processHTML(proxyResponse, response) { + let body = Buffer.from([]); + let originalResponse = proxyResponse; + + // decode GZIP response + if (originalResponse.headers['content-encoding'] === 'gzip') { + const gunzip = zlib.createGunzip(); + originalResponse.pipe(gunzip); + originalResponse = gunzip; + } + + originalResponse + .on('data', data => { + body = Buffer.concat([body, data]); + }) + .on('end', () => { + response.end(toDevHTML(body.toString())); + }); +} + +// 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); + } + response.flushHeaders(); + } catch (e) { + response.setHeader('content-type', 'text/plain'); + response.write(`Error requesting ${request.path} from proxy:\n\n`); + response.end(e.stack); + } + }, +}; diff --git a/superset/__init__.py b/superset/__init__.py index feffead96..cc92f3d9c 100644 --- a/superset/__init__.py +++ b/superset/__init__.py @@ -43,7 +43,7 @@ app: Flask = current_app cache = LocalProxy(lambda: cache_manager.cache) conf = LocalProxy(lambda: current_app.config) get_feature_flags = feature_flag_manager.get_feature_flags -get_css_manifest_files = manifest_processor.get_css_manifest_files +get_manifest_files = manifest_processor.get_manifest_files is_feature_enabled = feature_flag_manager.is_feature_enabled jinja_base_context = jinja_context_manager.base_context results_backend = LocalProxy(lambda: results_backend_manager.results_backend) diff --git a/superset/extensions.py b/superset/extensions.py index 5d7eed008..0b7f39bae 100644 --- a/superset/extensions.py +++ b/superset/extensions.py @@ -82,11 +82,18 @@ class UIManifestProcessor: @app.context_processor def get_manifest(): # pylint: disable=unused-variable + loaded_chunks = set() + + def get_files(bundle, asset_type="js"): + files = self.get_manifest_files(bundle, asset_type) + filtered_files = [f for f in files if f not in loaded_chunks] + for f in filtered_files: + loaded_chunks.add(f) + return filtered_files + return dict( - loaded_chunks=set(), - get_unloaded_chunks=self.get_unloaded_chunks, - js_manifest=self.get_js_manifest_files, - css_manifest=self.get_css_manifest_files, + js_manifest=lambda bundle: get_files(bundle, "js"), + css_manifest=lambda bundle: get_files(bundle, "css"), ) def parse_manifest_json(self): @@ -99,28 +106,13 @@ class UIManifestProcessor: except Exception: # pylint: disable=broad-except pass - def get_js_manifest_files(self, filename): + def get_manifest_files(self, bundle, asset_type): if self.app.debug: self.parse_manifest_json() - entry_files = self.manifest.get(filename, {}) - return entry_files.get("js", []) - - def get_css_manifest_files(self, filename): - if self.app.debug: - self.parse_manifest_json() - entry_files = self.manifest.get(filename, {}) - return entry_files.get("css", []) - - @staticmethod - def get_unloaded_chunks(files, loaded_chunks): - filtered_files = [f for f in files if f not in loaded_chunks] - for f in filtered_files: - loaded_chunks.add(f) - return filtered_files + return self.manifest.get(bundle, {}).get(asset_type, []) APP_DIR = os.path.dirname(__file__) - appbuilder = AppBuilder(update_perms=False) cache_manager = CacheManager() celery_app = celery.Celery() diff --git a/superset/templates/superset/add_slice.html b/superset/templates/superset/add_slice.html index fa77c0333..364a89a69 100644 --- a/superset/templates/superset/add_slice.html +++ b/superset/templates/superset/add_slice.html @@ -31,7 +31,5 @@ {% block tail_js %} {{ super() }} - {% with filename="addSlice" %} - {% include "superset/partials/_script_tag.html" %} - {% endwith %} + {{ js_bundle("addSlice") }} {% endblock %} diff --git a/superset/templates/superset/base.html b/superset/templates/superset/base.html index 5dbadf9c7..5da253073 100644 --- a/superset/templates/superset/base.html +++ b/superset/templates/superset/base.html @@ -17,25 +17,20 @@ under the License. #} {% extends "appbuilder/baselayout.html" %} +{% from 'superset/partials/asset_bundle.html' import css_bundle, js_bundle with context %} - {% block head_css %} - {{super()}} - <link rel="icon" type="image/png" href="/static/assets/images/favicon.png"> - {% for entry in get_unloaded_chunks(css_manifest('theme'), loaded_chunks) %} - <link rel="stylesheet" type="text/css" href="{{ entry }}" /> - {% endfor %} - {% endblock %} +{% block head_css %} + {{ super() }} + <link rel="icon" type="image/png" href="/static/assets/images/favicon.png"> + {{ css_bundle("theme") }} +{% endblock %} - {% block head_js %} - {{super()}} - {% with filename="theme" %} - {% include "superset/partials/_script_tag.html" %} - {% endwith %} - {% endblock %} +{% block head_js %} + {{ super() }} + {{ js_bundle("theme") }} +{% endblock %} - {% block tail_js %} - {{super()}} - {% with filename="preamble" %} - {% include "superset/partials/_script_tag.html" %} - {% endwith %} - {% endblock %} +{% block tail_js %} + {{ super() }} + {{ js_bundle("preamble") }} +{% endblock %} diff --git a/superset/templates/superset/basic.html b/superset/templates/superset/basic.html index 922ca0c9f..3fa2591df 100644 --- a/superset/templates/superset/basic.html +++ b/superset/templates/superset/basic.html @@ -18,6 +18,7 @@ #} {% import 'appbuilder/general/lib.html' as lib %} +{% from 'superset/partials/asset_bundle.html' import css_bundle, js_bundle with context %} {% set favicons = appbuilder.app.config['FAVICONS'] %} @@ -45,22 +46,15 @@ <link rel="stylesheet" type="text/css" href="/static/appbuilder/css/flags/flags16.css" /> <link rel="stylesheet" type="text/css" href="/static/appbuilder/css/font-awesome.min.css"> - {% for entry in get_unloaded_chunks(css_manifest('theme'), loaded_chunks) %} - <link rel="stylesheet" type="text/css" href="{{ entry }}" /> - {% endfor %} + {{ css_bundle("theme") }} {% if entry %} - {% set entry_files = css_manifest(entry) %} - {% for entry in get_unloaded_chunks(entry_files, loaded_chunks) %} - <link rel="stylesheet" type="text/css" href="{{ entry }}" /> - {% endfor %} + {{ css_bundle(entry) }} {% endif %} {% endblock %} - {% with filename="theme" %} - {% include "superset/partials/_script_tag.html" %} - {% endwith %} + {{ js_bundle("theme") }} <input type="hidden" @@ -105,9 +99,7 @@ </div> {% block tail_js %} {% if entry %} - {% with filename=entry %} - {% include "superset/partials/_script_tag.html" %} - {% endwith %} + {{ js_bundle(entry) }} {% endif %} {% endblock %} </body> diff --git a/superset/templates/superset/models/savedquery/show.html b/superset/templates/superset/models/savedquery/show.html index 9ffedeedb..10da57212 100644 --- a/superset/templates/superset/models/savedquery/show.html +++ b/superset/templates/superset/models/savedquery/show.html @@ -29,7 +29,5 @@ {% block tail_js %} {{ super() }} - {% with filename="showSavedQuery" %} - {% include "superset/partials/_script_tag.html" %} - {% endwith %} + {{ js_bundle("showSavedQuery") }} {% endblock %} diff --git a/superset/templates/superset/partials/_script_tag.html b/superset/templates/superset/partials/asset_bundle.html similarity index 61% rename from superset/templates/superset/partials/_script_tag.html rename to superset/templates/superset/partials/asset_bundle.html index 6a94c1612..281ac74c7 100644 --- a/superset/templates/superset/partials/_script_tag.html +++ b/superset/templates/superset/partials/asset_bundle.html @@ -16,8 +16,20 @@ specific language governing permissions and limitations under the License. #} -{% block partial_js %} - {% for entry in get_unloaded_chunks(js_manifest(filename), loaded_chunks) %} +{% macro js_bundle(filename) %} + {# HTML comment is needed for webpack-dev-server to replace assets + with development version #} + <!-- Bundle js {{ filename }} START --> + {% for entry in js_manifest(filename) %} <script src="{{ entry }}"></script> {% endfor %} -{% endblock %} + <!-- Bundle js {{ filename }} END --> +{% endmacro %} + +{% macro css_bundle(filename) %} + <!-- Bundle css {{ filename }} START --> + {% for entry in css_manifest(filename) %} + <link rel="stylesheet" type="text/css" href="{{ entry }}" /> + {% endfor %} + <!-- Bundle css {{ filename }} END --> +{% endmacro %} diff --git a/superset/templates/superset/welcome.html b/superset/templates/superset/welcome.html index 5f051bbdd..35c25705a 100644 --- a/superset/templates/superset/welcome.html +++ b/superset/templates/superset/welcome.html @@ -22,7 +22,5 @@ {% endblock %} {% block tail_js %} - {% with filename="welcome" %} - {% include "superset/partials/_script_tag.html" %} - {% endwith %} + {{ js_bundle("welcome") }} {% endblock %} diff --git a/superset/viz.py b/superset/viz.py index 13bde2ae9..afdfa9f5a 100644 --- a/superset/viz.py +++ b/superset/viz.py @@ -45,7 +45,7 @@ from geopy.point import Point from markdown import markdown from pandas.tseries.frequencies import to_offset -from superset import app, cache, get_css_manifest_files, security_manager +from superset import app, cache, get_manifest_files, security_manager from superset.constants import NULL_STRING from superset.exceptions import NullValueException, SpatialException from superset.models.helpers import QueryResult @@ -786,7 +786,7 @@ class MarkupViz(BaseViz): code = self.form_data.get("code", "") if markup_type == "markdown": code = markdown(code) - return dict(html=code, theme_css=get_css_manifest_files("theme")) + return dict(html=code, theme_css=get_manifest_files("theme", "css")) class SeparatorViz(MarkupViz):