From dd69f1ab43f934ad74d04f048af1334b1bb8757c Mon Sep 17 00:00:00 2001 From: Faris Ansari Date: Thu, 29 Apr 2021 13:30:07 +0530 Subject: [PATCH] fix: Hash based file naming - For better HTTP caching and cache busting - assets.json is created under [app]/dist folder which contains the map of input file and output file name, this is used to get the correct path for bundled assets --- esbuild/index.js | 134 +++++++++++++++++++++------------- frappe/cache_manager.py | 3 + frappe/hooks.py | 16 ++-- frappe/templates/base.html | 8 +- frappe/utils/__init__.py | 16 ++++ frappe/utils/jinja_globals.py | 30 ++++---- frappe/utils/redis_wrapper.py | 8 +- frappe/www/app.html | 4 +- frappe/www/login.html | 2 +- 9 files changed, 136 insertions(+), 85 deletions(-) diff --git a/esbuild/index.js b/esbuild/index.js index 694417647e..4c096b70c1 100644 --- a/esbuild/index.js +++ b/esbuild/index.js @@ -13,6 +13,7 @@ let { app_list, assets_path, apps_path, + sites_path, get_app_path, get_public_path, get_cli_arg @@ -39,18 +40,26 @@ const NODE_PATHS = [].concat( } console.time(TOTAL_BUILD_TIME); - build_assets_for_apps(apps) - .then(() => { - console.timeEnd(TOTAL_BUILD_TIME); - console.log(); - if (WATCH_MODE) { - console.log('Watching for changes...') - } - }) - .catch(() => { - let error = chalk.white.bgRed(" ERROR "); - console.error(`${error} There were some problems during build`); - }); + await clean_dist_folders(apps); + + let result; + try { + result = await build_assets_for_apps(apps); + } catch (e) { + let error = chalk.white.bgRed(" ERROR "); + console.error(`${error} There were some problems during build`); + console.log(); + console.log(chalk.dim(e.stack)); + } + + if (!WATCH_MODE) { + log_built_assets(result.metafile); + console.timeEnd(TOTAL_BUILD_TIME); + console.log(); + await write_meta_file(result.metafile); + } else { + console.log("Watching for changes..."); + } })(); function build_assets_for_apps(apps) { @@ -117,48 +126,57 @@ function get_files_to_build(apps) { } function build_files({ files, outdir }) { - return esbuild - .build({ - entryPoints: files, - outdir, - sourcemap: true, - bundle: true, - metafile: true, - // minify: true, - nodePaths: NODE_PATHS, - define: { - "process.env.NODE_ENV": "'development'" - }, - plugins: [ - html_plugin, - ignore_assets, - vue(), - postCssPlugin({ - plugins: [require("autoprefixer")], - sassOptions: sass_options - }) - ], - - watch: WATCH_MODE - ? { - onRebuild(error, result) { - if (error) - console.error("watch build failed:", error); - else { - console.log(`${new Date().toLocaleTimeString()}: Compiled changes...`) - // log_build_meta(result.metafile); - } + return esbuild.build({ + entryPoints: files, + entryNames: "[dir]/[name].[hash]", + outdir, + sourcemap: true, + bundle: true, + metafile: true, + // minify: true, + nodePaths: NODE_PATHS, + define: { + "process.env.NODE_ENV": "'development'" + }, + plugins: [ + html_plugin, + ignore_assets, + vue(), + postCssPlugin({ + plugins: [require("autoprefixer")], + sassOptions: sass_options + }) + ], + + watch: WATCH_MODE + ? { + onRebuild(error, result) { + if (error) console.error("watch build failed:", error); + else { + console.log( + `${new Date().toLocaleTimeString()}: Compiled changes...` + ); } - } - : null - }) - .then(result => { - log_build_meta(result.metafile); + } + } + : null + }); +} + +async function clean_dist_folders(apps) { + for (let app of apps) { + let public_path = get_public_path(app); + await fs.promises.rmdir(path.resolve(public_path, "dist", "js"), { + recursive: true + }); + await fs.promises.rmdir(path.resolve(public_path, "dist", "css"), { + recursive: true }); + } } -function log_build_meta(metafile) { - let column_widths = [40, 20]; +function log_built_assets(metafile) { + let column_widths = [60, 20]; cliui.div( { text: chalk.cyan.bold("File"), @@ -217,3 +235,19 @@ function log_build_meta(metafile) { } console.log(cliui.toString()); } + +function write_meta_file(metafile) { + let out = {}; + for (let output in metafile.outputs) { + let info = metafile.outputs[output]; + let asset_path = "/" + path.relative(sites_path, output); + if (info.entryPoint) { + out[path.basename(info.entryPoint)] = asset_path; + } + } + + return fs.promises.writeFile( + path.resolve(assets_path, "frappe", "dist", "assets.json"), + JSON.stringify(out, null, 4) + ); +} diff --git a/frappe/cache_manager.py b/frappe/cache_manager.py index 4e0fe0cf44..7330c83102 100644 --- a/frappe/cache_manager.py +++ b/frappe/cache_manager.py @@ -13,6 +13,8 @@ common_default_keys = ["__default", "__global"] doctype_map_keys = ('energy_point_rule_map', 'assignment_rule_map', 'milestone_tracker_map', 'event_consumer_document_type_map') +bench_cache_keys = ('assets_json',) + global_cache_keys = ("app_hooks", "installed_apps", 'all_apps', "app_modules", "module_app", "system_settings", 'scheduler_events', 'time_zone', 'webhooks', 'active_domains', @@ -58,6 +60,7 @@ def clear_global_cache(): clear_doctype_cache() clear_website_cache() frappe.cache().delete_value(global_cache_keys) + frappe.cache().delete_value(bench_cache_keys) frappe.setup_module_map() def clear_defaults_cache(user=None): diff --git a/frappe/hooks.py b/frappe/hooks.py index f587c59b4a..3215f23e04 100644 --- a/frappe/hooks.py +++ b/frappe/hooks.py @@ -29,16 +29,16 @@ page_js = { # website app_include_js = [ - "/assets/frappe/dist/js/libs.bundle.js", - "/assets/frappe/dist/js/desk.bundle.js", - "/assets/frappe/dist/js/list.bundle.js", - "/assets/frappe/dist/js/form.bundle.js", - "/assets/frappe/dist/js/controls.bundle.js", - "/assets/frappe/dist/js/report.bundle.js", + "libs.bundle.js", + "desk.bundle.js", + "list.bundle.js", + "form.bundle.js", + "controls.bundle.js", + "report.bundle.js", ] app_include_css = [ - "/assets/frappe/dist/css/desk.bundle.css", - "/assets/frappe/dist/css/report.bundle.css", + "desk.bundle.css", + "report.bundle.css", ] doctype_js = { diff --git a/frappe/templates/base.html b/frappe/templates/base.html index e9b17eb221..5b321da412 100644 --- a/frappe/templates/base.html +++ b/frappe/templates/base.html @@ -30,11 +30,11 @@ {%- if theme.name != 'Standard' -%} {%- else -%} - + {{ include_style('website.bundle.css') }} {%- endif -%} {%- for link in web_include_css %} - + {{ include_style(link) }} {%- endfor -%} {%- endblock -%} @@ -96,11 +96,11 @@ {% block base_scripts %} - + {{ include_script('frappe-web.bundle.js') }} {% endblock %} {%- for link in web_include_js %} - + {{ include_script(link) }} {%- endfor -%} {%- block script %} diff --git a/frappe/utils/__init__.py b/frappe/utils/__init__.py index 251a095343..9ef054534a 100644 --- a/frappe/utils/__init__.py +++ b/frappe/utils/__init__.py @@ -765,6 +765,22 @@ def get_build_version(): # this is not a major problem so send fallback return frappe.utils.random_string(8) +def get_assets_json(): + if not hasattr(frappe.local, "assets_json"): + + assets_json = frappe.cache().get_value("assets_json", shared=True) + if not assets_json: + import json + assets_json = json.loads( + frappe.read_file("assets/frappe/dist/assets.json") + ) + frappe.cache().set_value("assets_json", assets_json, shared=True) + + frappe.local.assets_json = assets_json + + return frappe.local.assets_json + + def get_bench_relative_path(file_path): """Fixes paths relative to the bench root directory if exists and returns the absolute path diff --git a/frappe/utils/jinja_globals.py b/frappe/utils/jinja_globals.py index dff69bb1f8..dbd80f22bd 100644 --- a/frappe/utils/jinja_globals.py +++ b/frappe/utils/jinja_globals.py @@ -69,23 +69,21 @@ def web_blocks(blocks): return html -def script(path): - path = assets_url(path) - if '/public/' in path: - path = path.replace('/public/', '/dist/') + +def include_script(path): + if not path.startswith("/assets") and ".bundle." in path: + path = bundled_asset_path(path) return f'' -def style(path): - path = assets_url(path) - if '/public/' in path: - path = path.replace('/public/', '/dist/') - if path.endswith(('.scss', '.sass', '.less', '.styl')): - path = path.rsplit('.', 1)[0] + '.css' + +def include_style(path): + if not path.startswith("/assets") and ".bundle." in path: + path = bundled_asset_path(path) return f'' -def assets_url(path): - if not path.startswith('/'): - path = '/' + path - if not path.startswith('/assets'): - path = '/assets' + path - return path + +def bundled_asset_path(path): + from frappe.utils import get_assets_json + + bundled_assets = get_assets_json() + return bundled_assets.get(path) diff --git a/frappe/utils/redis_wrapper.py b/frappe/utils/redis_wrapper.py index 20bbb283a3..fe75e67d8e 100644 --- a/frappe/utils/redis_wrapper.py +++ b/frappe/utils/redis_wrapper.py @@ -28,7 +28,7 @@ class RedisWrapper(redis.Redis): return "{0}|{1}".format(frappe.conf.db_name, key).encode('utf-8') - def set_value(self, key, val, user=None, expires_in_sec=None): + def set_value(self, key, val, user=None, expires_in_sec=None, shared=False): """Sets cache value. :param key: Cache key @@ -36,7 +36,7 @@ class RedisWrapper(redis.Redis): :param user: Prepends key with User :param expires_in_sec: Expire value of this key in X seconds """ - key = self.make_key(key, user) + key = self.make_key(key, user, shared) if not expires_in_sec: frappe.local.cache[key] = val @@ -50,7 +50,7 @@ class RedisWrapper(redis.Redis): except redis.exceptions.ConnectionError: return None - def get_value(self, key, generator=None, user=None, expires=False): + def get_value(self, key, generator=None, user=None, expires=False, shared=False): """Returns cache value. If not found and generator function is given, it will call the generator. @@ -59,7 +59,7 @@ class RedisWrapper(redis.Redis): :param expires: If the key is supposed to be with an expiry, don't store it in frappe.local """ original_key = key - key = self.make_key(key, user) + key = self.make_key(key, user, shared) if key in frappe.local.cache: val = frappe.local.cache[key] diff --git a/frappe/www/app.html b/frappe/www/app.html index a1dfd7d460..164be40bdc 100644 --- a/frappe/www/app.html +++ b/frappe/www/app.html @@ -21,7 +21,7 @@ {% for include in include_css -%} - {{ style(include) }} + {{ include_style(include) }} {%- endfor -%} @@ -51,7 +51,7 @@ {% for include in include_js %} - {{ script(include) }} + {{ include_script(include) }} {% endfor %} {% include "templates/includes/app_analytics/google_analytics.html" %} diff --git a/frappe/www/login.html b/frappe/www/login.html index 24faea5941..90e235e679 100644 --- a/frappe/www/login.html +++ b/frappe/www/login.html @@ -53,7 +53,7 @@ {% endmacro %} {% block head_include %} - +{{ include_style('login.bundle.css') }} {% endblock %} {% macro logo_section() %}