From 0ede559d5b9ac62f69d15eddcceac883cdabc3df Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Tue, 23 Nov 2021 12:47:46 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20card=20asset=20init/relo?= =?UTF-8?q?ad=20behaviour?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Card asset reloading was incorrectly only happening if the API version changed 🙈 - In addition, having an init function was redundant, as theme activation happens on boot - This meant that the card assets were being generated twice on boot - Instead, we now only generate them on theme activation, which covers the boot case and simplifies all the logic --- core/boot.js | 3 --- core/bridge.js | 11 +++++----- core/frontend/services/card-assets/index.js | 12 ----------- core/frontend/services/card-assets/service.js | 5 +++++ .../frontend/services/card-assets.test.js | 20 ------------------- 5 files changed, 11 insertions(+), 40 deletions(-) diff --git a/core/boot.js b/core/boot.js index f3ab630d0f..1c6ae7bde7 100644 --- a/core/boot.js +++ b/core/boot.js @@ -151,9 +151,6 @@ async function initFrontend() { const helperService = require('./frontend/services/helpers'); await helperService.init(); - const cardAssetService = require('./frontend/services/card-assets'); - await cardAssetService.init(); - debug('End: initFrontend'); } diff --git a/core/bridge.js b/core/bridge.js index 27af4c0d9d..2ded058cbb 100644 --- a/core/bridge.js +++ b/core/bridge.js @@ -16,6 +16,7 @@ const config = require('./shared/config'); const logging = require('@tryghost/logging'); const tpl = require('@tryghost/tpl'); const themeEngine = require('./frontend/services/theme-engine'); +const cardAssetService = require('./frontend/services/card-assets'); const routerManager = require('./frontend/services/routing').routerManager; const settingsCache = require('./shared/settings-cache'); @@ -65,6 +66,11 @@ class Bridge { themeEngine.setActive(settings, loadedTheme, checkedTheme); const currentGhostAPI = this.getActiveTheme().engine('ghost-api'); + const cardAssetConfig = this.getCardAssetConfig(); + + debug('reload card assets config', cardAssetConfig); + cardAssetService.load(cardAssetConfig); + if (previousGhostAPI !== undefined && (previousGhostAPI !== currentGhostAPI)) { events.emit('services.themes.api.changed'); this.reloadFrontend(); @@ -95,11 +101,6 @@ class Bridge { reloadFrontend() { const apiVersion = this.getFrontendApiVersion(); - const cardAssetConfig = this.getCardAssetConfig(); - - debug('reload card assets config', cardAssetConfig); - const cardAssetService = require('./frontend/services/card-assets'); - cardAssetService.load(cardAssetConfig); debug('reload frontend', apiVersion); const siteApp = require('./frontend/web/site'); diff --git a/core/frontend/services/card-assets/index.js b/core/frontend/services/card-assets/index.js index 81a3cbde81..340777cde1 100644 --- a/core/frontend/services/card-assets/index.js +++ b/core/frontend/services/card-assets/index.js @@ -1,16 +1,4 @@ -const debug = require('@tryghost/debug')('card-assets'); -const themeEngine = require('../theme-engine'); - const CardAssetService = require('./service'); let cardAssetService = new CardAssetService(); -const initFn = async () => { - const cardAssetConfig = themeEngine.getActive().config('card_assets'); - debug('initialising with config', cardAssetConfig); - - await cardAssetService.load(cardAssetConfig); -}; - module.exports = cardAssetService; - -module.exports.init = initFn; diff --git a/core/frontend/services/card-assets/service.js b/core/frontend/services/card-assets/service.js index 32fbe898bb..57300d7aa4 100644 --- a/core/frontend/services/card-assets/service.js +++ b/core/frontend/services/card-assets/service.js @@ -1,3 +1,4 @@ +const debug = require('@tryghost/debug')('card-assets'); const Minifier = require('@tryghost/minifier'); const _ = require('lodash'); const path = require('path'); @@ -98,10 +99,14 @@ class CardAssetService { this.config = cardAssetConfig; } + debug('loading with config', cardAssetConfig); + await this.clearFiles(); const globs = this.generateGlobs(); + debug('globs', globs); + this.files = await this.minify(globs) || []; } } diff --git a/test/unit/frontend/services/card-assets.test.js b/test/unit/frontend/services/card-assets.test.js index 0d4b7d7ef8..8e2a9d9ae9 100644 --- a/test/unit/frontend/services/card-assets.test.js +++ b/test/unit/frontend/services/card-assets.test.js @@ -5,30 +5,10 @@ const path = require('path'); const fs = require('fs').promises; const os = require('os'); -const cardAssetService = require('../../../../core/frontend/services/card-assets'); const CardAssetService = require('../../../../core/frontend/services/card-assets/service'); -const themeEngine = require('../../../../core/frontend/services/theme-engine'); const themeDefaults = require('../../../../core/frontend/services/theme-engine/config/defaults.json'); -describe('Card Asset Init', function () { - it('calls loader with config', function () { - sinon.stub(themeEngine, 'getActive').returns({ - config: function (key) { - if (key === 'card_assets') { - return 'random-test-value'; - } - } - }); - - let serviceStub = sinon.stub(cardAssetService, 'load'); - - cardAssetService.init(); - serviceStub.calledOnce.should.eql(true); - serviceStub.calledWith('random-test-value').should.eql(true); - }); -}); - describe('Card Asset Service', function () { let testDir, srcDir,