From 8c199fd44c017ff268a87bad0b0980e44b56a16e Mon Sep 17 00:00:00 2001 From: Thomas Ip Date: Tue, 25 Jun 2019 17:39:03 +0800 Subject: [PATCH] webpack: Use handlebars-loader to handle frontend templates. And remove the compile-handlebars-templates system. --- docs/development/using.md | 10 ++-- docs/subsystems/html-templates.md | 9 ++-- package.json | 1 + static/js/bundles/app.js | 3 +- static/js/templates.js | 36 +++++++------ static/templates/.gitignore | 1 - tools/compile-handlebars-templates | 84 ------------------------------ tools/minify-js | 7 --- tools/run-dev.py | 11 ++-- tools/webpack.assets.json | 11 ++-- tools/webpack.config.ts | 13 ++++- yarn.lock | 29 +++++++++++ 12 files changed, 80 insertions(+), 135 deletions(-) delete mode 100644 static/templates/.gitignore delete mode 100755 tools/compile-handlebars-templates diff --git a/docs/development/using.md b/docs/development/using.md index ff34d898fb..90ffd9909e 100644 --- a/docs/development/using.md +++ b/docs/development/using.md @@ -21,12 +21,10 @@ interactive console. When you make a change, here's a guide for what you need to do in order to see your change take effect in Development: -* If you change JavaScript, CSS, or Jinja2 backend templates (under -`templates/`), you'll just need to reload the browser window to see -changes take effect. The Handlebars frontend HTML templates -(`static/templates`) are automatically recompiled by the -`tools/compile-handlebars-templates` job, which runs as part of -`tools/run-dev.py`. +* If you change CSS files, your changes will appear immediately via hot module +replacement. If you change JavaScript or Handlebars templates, the browser +window will be reloaded automatically. For Jinja2 backend templates, you'll +need to reload the browser manually to see changes take effect. * If you change Python code used by the main Django/Tornado server processes, these services are run on top of Django's [manage.py diff --git a/docs/subsystems/html-templates.md b/docs/subsystems/html-templates.md index 633625f62c..92b165cc0e 100644 --- a/docs/subsystems/html-templates.md +++ b/docs/subsystems/html-templates.md @@ -77,11 +77,10 @@ The second argument to `templates.render` is the context. ### Toolchain -Handlebars is in our `package.json` and thus ends up in -`node_modules`; and then we have a script, -`tools/compile-handlebars-templates`, which is responsible for -compiling the templates, both in production and as they change in a -development environment. +Handlebars is in our `package.json` and thus ends up in `node_modules`; We use +handlebars-loader to load and compile templates during the webpack bundling +stage. In the development environment, webpack will trigger a browser reload +whenever a template is changed. ### Translation diff --git a/package.json b/package.json index c2e6331d19..b4b0625fb5 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ "flatpickr": "4.5.7", "font-awesome": "4.7.0", "handlebars": "4.1.2", + "handlebars-loader": "1.7.1", "i18next": "3.4.4", "imports-loader": "0.8.0", "jquery": "3.4.1", diff --git a/static/js/bundles/app.js b/static/js/bundles/app.js index e7bb651b9d..870493aa44 100644 --- a/static/js/bundles/app.js +++ b/static/js/bundles/app.js @@ -15,7 +15,7 @@ import "blueimp-md5/js/md5.js"; import "clipboard/dist/clipboard.js"; import "string.prototype.codepointat/codepointat.js"; import "winchan/winchan.js"; -import "handlebars/dist/handlebars.runtime.js"; +import "handlebars/dist/cjs/handlebars.runtime.js"; import "to-markdown/dist/to-markdown.js"; import "flatpickr/dist/flatpickr.js"; import "flatpickr/dist/plugins/confirmDate/confirmDate.js"; @@ -25,7 +25,6 @@ import "generated/emoji/emoji_codes.js"; import "generated/pygments_data.js"; // Import App JS -import "templates/compiled.js"; import "js/translations.js"; import "js/feature_flags.js"; import "js/loading.js"; diff --git a/static/js/templates.js b/static/js/templates.js index 7c891e0246..a15f673007 100644 --- a/static/js/templates.js +++ b/static/js/templates.js @@ -2,24 +2,30 @@ var templates = (function () { var exports = {}; -exports.render = function (name, arg) { - if (Handlebars.templates === undefined) { - throw new Error("Cannot find compiled templates!"); - } - if (Handlebars.templates[name] === undefined) { - throw new Error("Cannot find a template with this name: " + name - + ". If you are developing a new feature, this likely " - + "means you need to add the file static/templates/" - + name + ".handlebars"); - } +var template_context = require.context('../templates', true, /\.handlebars$/); +var template_paths = ['./', './settings/', './widgets/']; - // The templates should be compiled into compiled.js. In - // prod we build compiled.js as part of the deployment process, - // and for devs we have run_dev.py build compiled.js when templates - // change. - return Handlebars.templates[name](arg); +exports.render = function (name, arg) { + for (var i = 0; i < template_paths.length; i += 1) { + var template; + try { + template = template_context(template_paths[i] + name + '.handlebars'); + } catch (_e) { + continue; + } + return template(arg); + } + throw new Error('Cannot find template ' + name + '.handlebars anywhere ' + + 'under the static/templates/ folder.'); }; +// Below, we register Zulip-specific extensions to the handlebars API. +// +// IMPORTANT: When adding a new handlebars helper, update the +// knownHelpers array in the webpack config so that webpack knows your +// helper is registered at runtime and don't try to require them when +// bundling. + // We don't want to wait for DOM ready to register the Handlebars helpers // below. There's no need to, as they do not access the DOM. // Furthermore, waiting for DOM ready would introduce race conditions with diff --git a/static/templates/.gitignore b/static/templates/.gitignore deleted file mode 100644 index b48ee68ba5..0000000000 --- a/static/templates/.gitignore +++ /dev/null @@ -1 +0,0 @@ -/compiled.js diff --git a/tools/compile-handlebars-templates b/tools/compile-handlebars-templates deleted file mode 100755 index 5006d32443..0000000000 --- a/tools/compile-handlebars-templates +++ /dev/null @@ -1,84 +0,0 @@ -#!/usr/bin/env python3 - -import os -import glob -import subprocess -import sys -import time - -# check for the venv -from lib import sanity_check -sanity_check.check_venv(__file__) - -sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..')) -os.environ['DJANGO_SETTINGS_MODULE'] = 'zproject.settings' -from django.conf import settings - -from typing import Dict, List - -os.chdir(settings.DEPLOY_ROOT) -STATIC_PATH = 'static/' - -def get_templates(): - # type: () -> List[str] - return (glob.glob(os.path.join(STATIC_PATH, 'templates/*.handlebars')) + - glob.glob(os.path.join(STATIC_PATH, 'templates/settings/*.handlebars')) + - glob.glob(os.path.join(STATIC_PATH, 'templates/widgets/*.handlebars'))) - -def run(): - # type: () -> None - subprocess.check_call(['node', 'node_modules/.bin/handlebars'] + - get_templates() + - ['--output', os.path.join(STATIC_PATH, 'templates/compiled.js'), - '--known', 'if,unless,each,with']) - - -def add_error_stamp_file(file_path): - # type: (str) -> None - file_dir = os.path.dirname(file_path) - if not os.path.exists(file_dir): - os.makedirs(file_dir) - open(file_path, 'a').close() - - -def remove_error_stamp_file(file_path): - # type: (str) -> None - if os.path.exists(file_path): - os.remove(file_path) - - -def run_forever(): - # type: () -> None - # Keep polling for file changes, similar to how Django does it in - # django/utils/autoreload.py. If any of our templates change, rebuild - # compiled.js - mtimes = {} # type: Dict[str, float] - error_file_path = os.path.join(settings.DEPLOY_ROOT, - 'var/handlebars-templates/compile.error') - while True: - changed = False - for fn in get_templates(): - new_mtime = os.stat(fn).st_mtime - if new_mtime != mtimes.get(fn, None): - changed = True - mtimes[fn] = new_mtime - if changed: - print('Recompiling templates') - try: - run() - remove_error_stamp_file(error_file_path) - print('done') - except Exception: - add_error_stamp_file(error_file_path) - print('\n\n\n\033[91mPLEASE FIX!!\033[0m\n\n') - - time.sleep(0.200) - -if __name__ == '__main__': - if len(sys.argv) == 2 and sys.argv[1] == 'forever': - try: - run_forever() - except KeyboardInterrupt: - print(sys.argv[0], "exited after receiving KeyboardInterrupt") - else: - run() diff --git a/tools/minify-js b/tools/minify-js index fec8d9799b..379eb5100f 100755 --- a/tools/minify-js +++ b/tools/minify-js @@ -28,9 +28,6 @@ os.chdir(settings.DEPLOY_ROOT) STATIC_PATH = 'static/' -# Compile Handlebars templates -subprocess.check_call(['tools/compile-handlebars-templates']) - # Create webpack bundle subprocess.check_call(['tools/webpack']) @@ -81,10 +78,6 @@ if prev_deploy: else: changed_files = changed_files_tmp -# Always use the newly compiled handlebars templates and webpack bundle. -if prev_deploy: - changed_files.add(os.path.join(STATIC_PATH, 'templates/compiled.js')) - JS_SPECS = settings.JS_SPECS CLOSURE_BINARY = '/usr/bin/closure-compiler' if not os.path.exists(CLOSURE_BINARY): diff --git a/tools/run-dev.py b/tools/run-dev.py index 48364165ff..d8861a76eb 100755 --- a/tools/run-dev.py +++ b/tools/run-dev.py @@ -150,15 +150,12 @@ cmds = [['./manage.py', 'runserver'] + ['/srv/zulip-thumbor-venv/bin/thumbor', '-c', './zthumbor/thumbor.conf', '-p', '%s' % (thumbor_port,)]] if options.test: - # We just need to compile handlebars templates and webpack assets - # once at startup, not run a daemon, in test mode. Additionally, - # webpack-dev-server doesn't support running 2 copies on the same - # system, so this model lets us run the casper tests with a running - # development server. - subprocess.check_call(['./tools/compile-handlebars-templates']) + # We just need to compile webpack assets once at startup, not run a daemon, + # in test mode. Additionally, webpack-dev-server doesn't support running 2 + # copies on the same system, so this model lets us run the casper tests + # with a running development server. subprocess.check_call(['./tools/webpack', '--quiet', '--test']) else: - cmds.append(['./tools/compile-handlebars-templates', 'forever']) webpack_cmd = ['./tools/webpack', '--watch', '--port', str(webpack_port)] if options.minify: webpack_cmd.append('--minify') diff --git a/tools/webpack.assets.json b/tools/webpack.assets.json index d112b2e949..397c4d4afa 100644 --- a/tools/webpack.assets.json +++ b/tools/webpack.assets.json @@ -6,31 +6,28 @@ ], "archive": [ "xdate/src/xdate.js", - "handlebars/dist/handlebars.runtime.js", + "handlebars/dist/cjs/handlebars.runtime.js", "./static/js/archive.js", "./static/js/colorspace.js", "./static/js/floating_recipient_bar.js", "./static/js/timerender.js", "./static/js/templates.js", "./static/js/stream_color.js", - "./static/js/scroll_bar.js", - "./static/templates/compiled.js" + "./static/js/scroll_bar.js" ], "billing": [ "./static/js/billing/helpers.js", "./static/js/billing/billing.js", - "handlebars/dist/handlebars.runtime.js", + "handlebars/dist/cjs/handlebars.runtime.js", "./static/js/templates.js", - "./static/templates/compiled.js", "./static/js/loading.js", "./static/styles/portico/billing.scss" ], "upgrade": [ "./static/js/billing/helpers.js", "./static/js/billing/upgrade.js", - "handlebars/dist/handlebars.runtime.js", + "handlebars/dist/cjs/handlebars.runtime.js", "./static/js/templates.js", - "./static/templates/compiled.js", "./static/js/loading.js", "./static/styles/portico/billing.scss" ], diff --git a/tools/webpack.config.ts b/tools/webpack.config.ts index e2755e8785..487623bd45 100644 --- a/tools/webpack.config.ts +++ b/tools/webpack.config.ts @@ -78,6 +78,17 @@ export default (env?: string): webpack.Configuration => { }, ], }, + { + test: /\.handlebars$/, + loader: 'handlebars-loader', + options: { + // Tell webpack not to explicitly require these. + knownHelpers: ['if', 'unless', 'each', 'with', + // The ones below are defined in static/js/templates.js + 'partial', 'plural', 'eq', 'and', 'or', 'not', + 't', 'tr'], + }, + }, // load fonts and files { test: /\.(woff(2)?|ttf|eot|svg|otf|png)(\?v=\d+\.\d+\.\d+)?$/, @@ -137,7 +148,7 @@ export default (env?: string): webpack.Configuration => { { path: "../static/js/common.js" }, { path: "jquery/dist/jquery.js", name: ['$', 'jQuery'] }, { path: "underscore/underscore.js", name: '_' }, - { path: "handlebars/dist/handlebars.runtime.js", name: 'Handlebars' }, + { path: "handlebars/dist/cjs/handlebars.runtime.js", name: 'Handlebars' }, { path: "to-markdown/dist/to-markdown.js", name: 'toMarkdown' }, { path: "sortablejs/Sortable.js"}, { path: "winchan/winchan.js", name: 'WinChan'}, diff --git a/yarn.lock b/yarn.lock index 30e10fd61d..6d3d6405e9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1011,6 +1011,11 @@ async@^1.4.0, async@^1.5.2: resolved "https://registry.yarnpkg.com/async/-/async-1.5.2.tgz#ec6a61ae56480c0c3cb241c95618e20892f9672a" integrity sha1-7GphrlZIDAw8skHJVhjiCJL5Zyo= +async@~0.2.10: + version "0.2.10" + resolved "https://registry.yarnpkg.com/async/-/async-0.2.10.tgz#b6bbe0b0674b9d719708ca38de8c237cb526c3d1" + integrity sha1-trvgsGdLnXGXCMo43owjfLUmw9E= + asynckit@^0.4.0: version "0.4.0" resolved "https://registry.yarnpkg.com/asynckit/-/asynckit-0.4.0.tgz#c79ed97f7f34cb8f2ba1bc9790bcc366474b4b79" @@ -3641,6 +3646,11 @@ fast-levenshtein@~2.0.4: resolved "https://registry.yarnpkg.com/fast-levenshtein/-/fast-levenshtein-2.0.6.tgz#3d8a5c66883a16a30ca8643e851f19baa7797917" integrity sha1-PYpcZog6FqMMqGQ+hR8Zuqd5eRc= +fastparse@^1.0.0: + version "1.1.2" + resolved "https://registry.yarnpkg.com/fastparse/-/fastparse-1.1.2.tgz#91728c5a5942eced8531283c79441ee4122c35a9" + integrity sha512-483XLLxTVIwWK3QTrMGRqUfUpoOs/0hbQrl2oz4J0pAcm3A3bu84wxTFqGqkJzewCLdME38xJLJAxBABfQT8sQ== + faye-websocket@^0.10.0: version "0.10.0" resolved "https://registry.yarnpkg.com/faye-websocket/-/faye-websocket-0.10.0.tgz#4e492f8d04dfb6f89003507f6edbf2d501e7c6f4" @@ -4811,6 +4821,16 @@ handle-thing@^2.0.0: resolved "https://registry.yarnpkg.com/handle-thing/-/handle-thing-2.0.0.tgz#0e039695ff50c93fc288557d696f3c1dc6776754" integrity sha512-d4sze1JNC454Wdo2fkuyzCr6aHcbL6PGGuFAz0Li/NcOm1tCHGnWDRmJP85dh9IhQErTc2svWFEX5xHIOo//kQ== +handlebars-loader@1.7.1: + version "1.7.1" + resolved "https://registry.yarnpkg.com/handlebars-loader/-/handlebars-loader-1.7.1.tgz#07088f09d8a559344908f7c88c68c0ffdacc555d" + integrity sha512-Q+Z/hDPQzU8ZTlVnAe/0T1LHABlyhL7opNcSKcQDhmUXK2ByGTqib1Z2Tfv4Ic50WqDcLFWQcOb3mhjcBRbscQ== + dependencies: + async "~0.2.10" + fastparse "^1.0.0" + loader-utils "1.0.x" + object-assign "^4.1.0" + handlebars@4.1.2, handlebars@^4.1.2: version "4.1.2" resolved "https://registry.yarnpkg.com/handlebars/-/handlebars-4.1.2.tgz#b6b37c1ced0306b221e094fc7aca3ec23b131b67" @@ -6092,6 +6112,15 @@ loader-runner@^2.3.0: resolved "https://registry.yarnpkg.com/loader-runner/-/loader-runner-2.3.0.tgz#f482aea82d543e07921700d5a46ef26fdac6b8a2" integrity sha1-9IKuqC1UPgeSFwDVpG7yb9rGuKI= +loader-utils@1.0.x: + version "1.0.4" + resolved "https://registry.yarnpkg.com/loader-utils/-/loader-utils-1.0.4.tgz#13f56197f1523a305891248b4c7244540848426c" + integrity sha1-E/Vhl/FSOjBYkSSLTHJEVAhIQmw= + dependencies: + big.js "^3.1.3" + emojis-list "^2.0.0" + json5 "^0.5.0" + loader-utils@^1.0.1, loader-utils@^1.0.2, loader-utils@^1.1.0: version "1.1.0" resolved "https://registry.yarnpkg.com/loader-utils/-/loader-utils-1.1.0.tgz#c98aef488bcceda2ffb5e2de646d6a754429f5cd"