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
2025-01-06 22:40:14 -05:00 · 2024-10-09 12:02:40 +01:00 · 2024-10-09 12:02:40 +01:00 · b092929bba
commit b092929bba
parent 0862989b2e
6 changed files with 68 additions and 29 deletions
--- 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}),
--- 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<void>}
-     */
-    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();
    }
 };
--- 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();
+        };
+    }
 };
--- 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();

--- 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());
--- 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 {