From b092929bba77ab386e8095052ab0635da96f0656 Mon Sep 17 00:00:00 2001 From: Daniel Lockyer Date: Wed, 9 Oct 2024 12:02:40 +0100 Subject: [PATCH] Lazy-minified assets at first request - right now, we minify the assets on boot. This is wasteful because they're not even needed - this commit implements a change which lazy-minifies these assets and allows for cache invalidation when the theme changes - it also introduces some middleware that each asset calls to ensure that the assets are minified before serving --- ghost/core/core/bridge.js | 14 +++++--- .../assets-minification/AdminAuthAssets.js | 33 ++++++++----------- .../AssetsMinificationBase.js | 24 ++++++++++++++ .../assets-minification/CardAssets.js | 14 +++++++- ghost/core/core/frontend/web/site.js | 9 ++--- ghost/core/core/server/web/admin/app.js | 3 +- 6 files changed, 68 insertions(+), 29 deletions(-) diff --git a/ghost/core/core/bridge.js b/ghost/core/core/bridge.js index eaa6ec488b..0ace590f5c 100644 --- a/ghost/core/core/bridge.js +++ b/ghost/core/core/bridge.js @@ -51,6 +51,10 @@ class Bridge { return themeEngine.getActive(); } + ensureAdminAuthAssetsMiddleware() { + return adminAuthAssets.serveMiddleware(); + } + async activateTheme(loadedTheme, checkedTheme) { let settings = { locale: settingsCache.get('locale') @@ -60,15 +64,17 @@ class Bridge { try { themeEngine.setActive(settings, loadedTheme, checkedTheme); + logging.info('Invalidating assets for regeneration'); + const cardAssetConfig = this.getCardAssetConfig(); debug('reload card assets config', cardAssetConfig); - await cardAssets.load(cardAssetConfig); + cardAssets.invalidate(cardAssetConfig); // TODO: is this in the right place? // rebuild asset files - await commentCountsAssets.load(); - await adminAuthAssets.load(); - await memberAttributionAssets.load(); + commentCountsAssets.invalidate(); + adminAuthAssets.invalidate(); + memberAttributionAssets.invalidate(); } catch (err) { logging.error(new errors.InternalServerError({ message: tpl(messages.activateFailed, {theme: loadedTheme.name}), diff --git a/ghost/core/core/frontend/services/assets-minification/AdminAuthAssets.js b/ghost/core/core/frontend/services/assets-minification/AdminAuthAssets.js index 7007211716..075205d00f 100644 --- a/ghost/core/core/frontend/services/assets-minification/AdminAuthAssets.js +++ b/ghost/core/core/frontend/services/assets-minification/AdminAuthAssets.js @@ -1,7 +1,7 @@ // const debug = require('@tryghost/debug')('comments-counts-assets'); const Minifier = require('@tryghost/minifier'); const path = require('path'); -const fs = require('fs').promises; +const fs = require('fs'); const logging = require('@tryghost/logging'); const config = require('../../../shared/config'); const urlUtils = require('../../../shared/url-utils'); @@ -16,6 +16,19 @@ module.exports = class AdminAuthAssets extends AssetsMinificationBase { this.dest = options.dest || path.join(config.getContentPath('public'), 'admin-auth'); this.minifier = new Minifier({src: this.src, dest: this.dest}); + + try { + // TODO: don't do this synchronously + fs.mkdirSync(this.dest, {recursive: true}); + fs.copyFileSync(path.join(this.src, 'index.html'), path.join(this.dest, 'index.html')); + } catch (error) { + if (error.code === 'EACCES') { + logging.error('Ghost was not able to write admin-auth asset files due to permissions.'); + return; + } + + throw error; + } } /** @@ -41,23 +54,6 @@ module.exports = class AdminAuthAssets extends AssetsMinificationBase { }; } - /** - * @private - * @returns {Promise} - */ - async copyStatic() { - try { - await fs.copyFile(path.join(this.src, 'index.html'), path.join(this.dest, 'index.html')); - } catch (error) { - if (error.code === 'EACCES') { - logging.error('Ghost was not able to write admin-auth asset files due to permissions.'); - return; - } - - throw error; - } - } - /** * Minify, move into the destination directory, and clear existing asset files. * @@ -69,6 +65,5 @@ module.exports = class AdminAuthAssets extends AssetsMinificationBase { const replacements = this.generateReplacements(); await this.clearFiles(); await this.minify(globs, {replacements}); - await this.copyStatic(); } }; \ No newline at end of file diff --git a/ghost/core/core/frontend/services/assets-minification/AssetsMinificationBase.js b/ghost/core/core/frontend/services/assets-minification/AssetsMinificationBase.js index 842a5a3269..7a9d8b8cde 100644 --- a/ghost/core/core/frontend/services/assets-minification/AssetsMinificationBase.js +++ b/ghost/core/core/frontend/services/assets-minification/AssetsMinificationBase.js @@ -6,10 +6,16 @@ const logging = require('@tryghost/logging'); module.exports = class AssetsMinificationBase { minifier; + ready = false; + constructor(options = {}) { this.options = options; } + invalidate() { + this.ready = false; + } + generateGlobs() { throw new errors.InternalServerError({ message: 'generateGlobs not implemented' @@ -50,6 +56,24 @@ module.exports = class AssetsMinificationBase { } throw error; + } finally { + this.ready = true; } } + + serveMiddleware() { + const self = this; + /** + * @param {import('express').Request} req + * @param {import('express').Response} res + * @param {import('express').NextFunction} next + */ + return async function serveMiddleware(req, res, next) { + if (!self.ready) { + await self.load(); + } + + next(); + }; + } }; diff --git a/ghost/core/core/frontend/services/assets-minification/CardAssets.js b/ghost/core/core/frontend/services/assets-minification/CardAssets.js index 3632f7dc80..96ad4c2fe0 100644 --- a/ghost/core/core/frontend/services/assets-minification/CardAssets.js +++ b/ghost/core/core/frontend/services/assets-minification/CardAssets.js @@ -55,9 +55,21 @@ module.exports = class CardAssets extends AssetsMinificationBase { } hasFile(type) { + if (this.files.length) { + return this.files.indexOf(`cards.min.${type}`) > -1; + } + return Object.keys(this.generateGlobs()).indexOf(`cards.min.${type}`) > -1; } + invalidate(cardAssetConfig) { + if (cardAssetConfig) { + this.config = cardAssetConfig; + } + + return super.invalidate(); + } + /** * A theme can declare which cards it supports, and we'll do the rest * @@ -68,7 +80,7 @@ module.exports = class CardAssets extends AssetsMinificationBase { this.config = cardAssetConfig; } - debug('loading with config', cardAssetConfig); + debug('loading with config', this.config); await this.clearFiles(); diff --git a/ghost/core/core/frontend/web/site.js b/ghost/core/core/frontend/web/site.js index 8ffafeb3aa..637e3a8f83 100644 --- a/ghost/core/core/frontend/web/site.js +++ b/ghost/core/core/frontend/web/site.js @@ -16,6 +16,7 @@ const membersService = require('../../server/services/members'); const offersService = require('../../server/services/offers'); const customRedirects = require('../../server/services/custom-redirects'); const linkRedirects = require('../../server/services/link-redirection'); +const {cardAssets, commentCountsAssets, memberAttributionAssets} = require('../services/assets-minification'); const siteRoutes = require('./routes'); const shared = require('../../server/web/shared'); const errorHandler = require('@tryghost/mw-error-handler'); @@ -72,14 +73,14 @@ module.exports = function setupSiteApp(routerConfig) { siteApp.use(mw.servePublicFile('static', 'public/ghost.min.css', 'text/css', config.get('caching:publicAssets:maxAge'))); // Card assets - siteApp.use(mw.servePublicFile('built', 'public/cards.min.css', 'text/css', config.get('caching:publicAssets:maxAge'))); - siteApp.use(mw.servePublicFile('built', 'public/cards.min.js', 'application/javascript', config.get('caching:publicAssets:maxAge'))); + siteApp.use(cardAssets.serveMiddleware(), mw.servePublicFile('built', 'public/cards.min.css', 'text/css', config.get('caching:publicAssets:maxAge'))); + siteApp.use(cardAssets.serveMiddleware(), mw.servePublicFile('built', 'public/cards.min.js', 'application/javascript', config.get('caching:publicAssets:maxAge'))); // Comment counts - siteApp.use(mw.servePublicFile('built', 'public/comment-counts.min.js', 'application/javascript', config.get('caching:publicAssets:maxAge'))); + siteApp.use(commentCountsAssets.serveMiddleware(), mw.servePublicFile('built', 'public/comment-counts.min.js', 'application/javascript', config.get('caching:publicAssets:maxAge'))); // Member attribution - siteApp.use(mw.servePublicFile('built', 'public/member-attribution.min.js', 'application/javascript', config.get('caching:publicAssets:maxAge'))); + siteApp.use(memberAttributionAssets.serveMiddleware(), mw.servePublicFile('built', 'public/member-attribution.min.js', 'application/javascript', config.get('caching:publicAssets:maxAge'))); // Serve site images using the storage adapter siteApp.use(STATIC_IMAGE_URL_PREFIX, mw.handleImageSizes, storage.getStorage('images').serve()); diff --git a/ghost/core/core/server/web/admin/app.js b/ghost/core/core/server/web/admin/app.js index 02280dbb9d..090e3eb16d 100644 --- a/ghost/core/core/server/web/admin/app.js +++ b/ghost/core/core/server/web/admin/app.js @@ -9,6 +9,7 @@ const shared = require('../shared'); const errorHandler = require('@tryghost/mw-error-handler'); const sentry = require('../../../shared/sentry'); const redirectAdminUrls = require('./middleware/redirect-admin-urls'); +const bridge = require('../../../bridge'); /** * @@ -38,7 +39,7 @@ module.exports = function setupAdminApp() { // request to the Admin API /users/me/ endpoint to check if the user is logged in. // // Used by comments-ui to add moderation options to front-end comments when logged in. - adminApp.use('/auth-frame', function authFrameMw(req, res, next) { + adminApp.use('/auth-frame', bridge.ensureAdminAuthAssetsMiddleware(), function authFrameMw(req, res, next) { // only render content when we have an Admin session cookie, // otherwise return a 204 to avoid JS and API requests being made unnecessarily try {