Moved frontend reloading logic into bridge

- following on from removing api versioning logic from the frontend, it's possible to make more sense of what's happening
- this commit first introduces a proper jsdoc'd object that gets passed through the frontent load & reload flow
- that object contains the urlService and optionally our routeSettings processed from routes.yaml
- additionally, we were passing around a start boolean, which told the routerManager whether to just init, or init+start
- with this refactor, we always pass in the routeSettings when we want to do init+start, so we no longer need a boolean

- The refactor itself moves logic from the reload function in site.js and urlservice + routesettings fetching logic from routes.js
    into the reloadFrontend function in bridge.js.
- This makes it clearer to see what happens when we call reloadFrontend.
- This commit also makes it clearer to see what is happening with the route settings, where they are needed and why
- Ideally we'd also clean up the weird dupliated logic and somewhat unnecessary routes.js file

core/boot.js

@@ -207,7 +207,8 @@ async function initExpressApps({frontend, backend, config}) {
 
     if (frontend) {
         // SITE + MEMBERS
-        const frontendApp = require('./server/web/parent/frontend')({});
+        const urlService = require('./server/services/url');
+        const frontendApp = require('./server/web/parent/frontend')({urlService});
         parentApp.use(vhost(config.getFrontendMountPath(), frontendApp));
     }
 

core/bridge.js

@@ -15,9 +15,12 @@ const errors = require('@tryghost/errors');
 const logging = require('@tryghost/logging');
 const tpl = require('@tryghost/tpl');
 const themeEngine = require('./frontend/services/theme-engine');
+const appService = require('./frontend/services/apps');
 const cardAssetService = require('./frontend/services/card-assets');
 const routerManager = require('./frontend/services/routing').routerManager;
 const settingsCache = require('./shared/settings-cache');
+const urlService = require('./server/services/url');
+const routeSettings = require('./server/services/route-settings');
 
 // Listen to settings.lang.edited, similar to the member service and models/base/listeners
 const events = require('./server/lib/common/events');
@@ -80,7 +83,23 @@ class Bridge {
     async reloadFrontend() {
         debug('reload frontend');
         const siteApp = require('./frontend/web/site');
-        await siteApp.reload();
+
+        const routerConfig = {
+            routeSettings: routeSettings.loadRouteSettingsSync(),
+            urlService
+        };
+
+        await siteApp.reload(routerConfig);
+
+        // re-initialize apps (register app routers, because we have re-initialized the site routers)
+        appService.init();
+
+        // connect routers and resources again
+        urlService.queue.start({
+            event: 'init',
+            tolerance: 100,
+            requiredSubscriberCount: 1
+        });
     }
 }
 

core/frontend/services/routing/router-manager.js

@@ -1,4 +1,4 @@
-const debug = require('@tryghost/debug')('routing');
+const debug = require('@tryghost/debug')('frontend:routing');
 const _ = require('lodash');
 const StaticRoutesRouter = require('./StaticRoutesRouter');
 const StaticPagesRouter = require('./StaticPagesRouter');
@@ -16,6 +16,9 @@ class RouterManager {
     constructor({registry}) {
         this.registry = registry;
         this.siteRouter = null;
+        /**
+         * @type {URLService}
+         */
         this.urlService = null;
     }
 
@@ -51,23 +54,20 @@ class RouterManager {
     }
 
     /**
-     * @description The `init` function will return the wrapped parent express router and will start creating all
-     *              routers if you pass the option "start: true".
+     * The `init` function will return the wrapped parent express router and will start creating all
+     * routers if you pass the option "start: true".
      *
      * CASES:
      *   - if Ghost starts, it will first init the site app with the wrapper router and then call `start`
      *     separately, because it could be that your blog goes into maintenance mode
      *   - if you change your route settings, we will re-initialize routing
      *
-     * @param {Object} options
-     * @param {Boolean} [options.start] - flag controlling if the frontend Routes should be reinitialized
-     * @param {Object} options.routerSettings - JSON configuration to build frontend Routes
-     * @param {Object} options.urlService - service providing resource URL utility functions such as owns, getUrlByResourceId, and getResourceById
-     * @returns {ExpressRouter}
+     * @param {RouterConfig} options
+     * @returns {import('express').Router}
      */
-    init({start = false, routerSettings, urlService}) {
+    init({routeSettings, urlService}) {
         this.urlService = urlService;
-        debug('routing init', start, routerSettings);
+        debug('routing init', routeSettings);
 
         this.registry.resetAllRouters();
         this.registry.resetAllRoutes();
@@ -79,8 +79,8 @@ class RouterManager {
         this.siteRouter = new ParentRouter('SiteRouter');
         this.registry.setRouter('siteRouter', this.siteRouter);
 
-        if (start) {
-            this.start(routerSettings);
+        if (routeSettings) {
+            this.start(routeSettings);
         }
 
         return this.siteRouter.router();
@@ -186,3 +186,25 @@ class RouterManager {
 }
 
 module.exports = RouterManager;
+
+/**
+ * @typedef {Object} RouterConfig
+ * @property {RouteSettings} [routeSettings] - JSON config representing routes
+ * @property {URLService} urlService - service providing resource URL utility functions such as owns, getUrlByResourceId, and getResourceById
+ */
+
+/**
+ * @typedef {Object} RouteSettings
+ * @property {Object} routes
+ * @property {Object} collections
+ * @property {Object} taxonomies
+ */
+
+/**
+ * @typedef {Object} URLService
+ * @property {Function} owns
+ * @property {Function} getUrlByResourceId
+ * @property {Function} getResourceById
+ * @property {Function} onRouterAddedType
+ * @property {Function} onRouterUpdated
+ */

core/frontend/web/routes.js

@@ -1,12 +1,12 @@
 const debug = require('@tryghost/debug')('frontend:routes');
-
 const routing = require('../services/routing');
-const urlService = require('../../server/services/url');
-const routeSettings = require('../../server/services/route-settings');
 
-module.exports = function siteRoutes(options = {}) {
-    debug('site Routes', options);
-    options.routerSettings = routeSettings.loadRouteSettingsSync();
-    options.urlService = urlService;
-    return routing.routerManager.init(options);
+/**
+ *
+ * @param {import('../services/routing/router-manager').RouterConfig} routerConfig
+ * @returns {import('express').Router}
+ */
+module.exports = function siteRoutes(routerConfig) {
+    debug('site Routes', routerConfig);
+    return routing.routerManager.init(routerConfig);
 };

core/frontend/web/site.js

@@ -8,10 +8,8 @@ const {MemberPageViewEvent} = require('@tryghost/member-events');
 const config = require('../../shared/config');
 const constants = require('@tryghost/constants');
 const storage = require('../../server/adapters/storage');
-const urlService = require('../../server/services/url');
 const urlUtils = require('../../shared/url-utils');
 const sitemapHandler = require('../services/sitemap/handler');
-const appService = require('../services/apps');
 const themeEngine = require('../services/theme-engine');
 const themeMiddleware = themeEngine.middleware;
 const membersService = require('../../server/services/members');
@@ -32,8 +30,13 @@ function SiteRouter(req, res, next) {
     router(req, res, next);
 }
 
-module.exports = function setupSiteApp(options = {}) {
-    debug('Site setup start', options);
+/**
+ *
+ * @param {import('../services/routing/router-manager').RouterConfig} routerConfig
+ * @returns {import('express').Application}
+ */
+module.exports = function setupSiteApp(routerConfig) {
+    debug('Site setup start', routerConfig);
 
     const siteApp = express('site');
 
@@ -136,7 +139,7 @@ module.exports = function setupSiteApp(options = {}) {
 
     debug('General middleware done');
 
-    router = siteRoutes(options);
+    router = siteRoutes(routerConfig);
     Object.setPrototypeOf(SiteRouter, router);
 
     // Set up Frontend routes (including private blogging routes)
@@ -158,18 +161,12 @@ module.exports = function setupSiteApp(options = {}) {
     return siteApp;
 };
 
-module.exports.reload = () => {
-    // https://github.com/expressjs/express/issues/2596
-    router = siteRoutes({start: true});
+/**
+ * see https://github.com/expressjs/express/issues/2596
+ * @param {import('../services/routing/router-manager').RouterConfig} routerConfig
+ */
+module.exports.reload = (routerConfig) => {
+    debug('reloading');
+    router = siteRoutes(routerConfig);
     Object.setPrototypeOf(SiteRouter, router);
-
-    // re-initialize apps (register app routers, because we have re-initialized the site routers)
-    appService.init();
-
-    // connect routers and resources again
-    urlService.queue.start({
-        event: 'init',
-        tolerance: 100,
-        requiredSubscriberCount: 1
-    });
 };

core/server/web/parent/frontend.js

@@ -1,15 +1,14 @@
 const debug = require('@tryghost/debug')('frontend');
 const express = require('../../../shared/express');
-
 const shared = require('../shared');
 
 /**
  *
- * @param {object} options
+ * @param {import('../../../frontend/services/routing/router-manager').RouterConfig} routerConfig
  * @returns {import('express').RequestHandler}
  */
-module.exports = (options) => {
-    debug('FrontendApp setup start', options);
+module.exports = (routerConfig) => {
+    debug('FrontendApp setup start', routerConfig);
 
     // FRONTEND
     const frontendApp = express('frontend');
@@ -19,7 +18,7 @@ module.exports = (options) => {
     frontendApp.use(shared.middleware.urlRedirects.frontendSSLRedirect);
 
     frontendApp.lazyUse('/members', require('../members'));
-    frontendApp.use('/', require('../../../frontend/web')(options));
+    frontendApp.use('/', require('../../../frontend/web')(routerConfig));
 
     return frontendApp;
 };