mirror of
https://github.com/TryGhost/Ghost.git
synced 2025-02-03 23:00:14 -05:00
Removed 'router.created' event emmision from forntend routers
refs https://linear.app/tryghost/issue/CORE-104/decouple-frontend-routing-events-from-urlserver-events - The 'router.created' event should eventually be killed. For now the aim is to create a clear communication pathway between frontend's routing module and the URL service (similar to the frontend bridge concept on the "server" side)
This commit is contained in:
Naz
naz
parent
c5856d04e4
commit
597ec51afb
11 changed files with 62 additions and 73 deletions
|
|
@ -5,6 +5,7 @@ const ParentRouter = require('./ParentRouter');
|
|||
const controllers = require('./controllers');
|
||||
const middlewares = require('./middlewares');
|
||||
const RSSRouter = require('./RSSRouter');
|
||||
const bootstrap = require('./bootstrap');
|
||||
|
||||
// This emits its own routing events AND listens to settings.timezone.edited :(
|
||||
const events = require('../../../server/lib/common/events');
|
||||
|
|
@ -92,7 +93,7 @@ class CollectionRouter extends ParentRouter {
|
|||
// REGISTER: permalinks e.g. /:slug/, /podcast/:slug
|
||||
this.mountRoute(this.permalinks.getValue({withUrlOptions: true}), controllers.entry);
|
||||
|
||||
events.emit('router.created', this);
|
||||
bootstrap.internal.routerCreated(this);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -2,9 +2,7 @@ const debug = require('@tryghost/debug')('routing:static-pages-router');
|
|||
const urlUtils = require('../../../shared/url-utils');
|
||||
const ParentRouter = require('./ParentRouter');
|
||||
const controllers = require('./controllers');
|
||||
|
||||
// This emits its own routing events
|
||||
const events = require('../../../server/lib/common/events');
|
||||
const bootstrap = require('./bootstrap');
|
||||
|
||||
/**
|
||||
* @description Resource: pages
|
||||
|
|
@ -50,7 +48,7 @@ class StaticPagesRouter extends ParentRouter {
|
|||
// REGISTER: permalink for static pages
|
||||
this.mountRoute(this.permalinks.getValue({withUrlOptions: true}), controllers.entry);
|
||||
|
||||
events.emit('router.created', this);
|
||||
bootstrap.internal.routerCreated(this);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -5,9 +5,7 @@ const RSSRouter = require('./RSSRouter');
|
|||
const controllers = require('./controllers');
|
||||
const middlewares = require('./middlewares');
|
||||
const ParentRouter = require('./ParentRouter');
|
||||
|
||||
// This emits its own routing events
|
||||
const events = require('../../../server/lib/common/events');
|
||||
const bootstrap = require('./bootstrap');
|
||||
|
||||
/**
|
||||
* @description Template routes allow you to map individual URLs to specific template files within a Ghost theme
|
||||
|
|
@ -64,7 +62,7 @@ class StaticRoutesRouter extends ParentRouter {
|
|||
this.router().param('page', middlewares.pageParam);
|
||||
this.mountRoute(urlUtils.urlJoin(this.route.value, 'page', ':page(\\d+)'), controllers[this.controller]);
|
||||
|
||||
events.emit('router.created', this);
|
||||
bootstrap.internal.routerCreated(this);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -100,7 +98,7 @@ class StaticRoutesRouter extends ParentRouter {
|
|||
// REGISTER: static route
|
||||
this.mountRoute(this.route.value, controllers.static);
|
||||
|
||||
events.emit('router.created', this);
|
||||
bootstrap.internal.routerCreated(this);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -5,9 +5,7 @@ const RSSRouter = require('./RSSRouter');
|
|||
const urlUtils = require('../../../shared/url-utils');
|
||||
const controllers = require('./controllers');
|
||||
const middlewares = require('./middlewares');
|
||||
|
||||
// This emits its own routing events
|
||||
const events = require('../../../server/lib/common/events');
|
||||
const bootstrap = require('./bootstrap');
|
||||
|
||||
/**
|
||||
* @description Taxonomies are groupings of posts based on a common relation.
|
||||
|
|
@ -60,7 +58,7 @@ class TaxonomyRouter extends ParentRouter {
|
|||
this.mountRoute(urlUtils.urlJoin(this.permalinks.value, 'edit'), this._redirectEditOption.bind(this));
|
||||
}
|
||||
|
||||
events.emit('router.created', this);
|
||||
bootstrap.internal.routerCreated(this);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
7
core/frontend/services/routing/bootstrap.js
vendored
7
core/frontend/services/routing/bootstrap.js
vendored
|
|
@ -128,6 +128,13 @@ module.exports.internal = {
|
|||
},
|
||||
getResourceById: (resourceId) => {
|
||||
return _urlService.getResourceById(resourceId);
|
||||
},
|
||||
routerCreated: (router) => {
|
||||
// NOTE: this event should be become an "internal frontend even"
|
||||
// and should not be consumed by the modules outside the frontend
|
||||
events.emit('router.created', this);
|
||||
|
||||
_urlService.onRouterAddedType(router);
|
||||
}
|
||||
};
|
||||
module.exports.init = init;
|
||||
|
|
|
|||
|
|
@ -35,9 +35,6 @@ class UrlService {
|
|||
* @private
|
||||
*/
|
||||
_listeners() {
|
||||
this._onRouterAddedListener = this._onRouterAddedType.bind(this);
|
||||
events.on('router.created', this._onRouterAddedListener);
|
||||
|
||||
this._onThemeChangedListener = this._onThemeChangedListener.bind(this);
|
||||
events.on('services.themes.api.changed', this._onThemeChangedListener);
|
||||
|
||||
|
|
@ -77,9 +74,8 @@ class UrlService {
|
|||
/**
|
||||
* @description Router was created, connect it with a url generator.
|
||||
* @param {ExpressRouter} router
|
||||
* @private
|
||||
*/
|
||||
_onRouterAddedType(router) {
|
||||
onRouterAddedType(router) {
|
||||
// CASE: there are router types which do not generate resource urls
|
||||
// e.g. static route router
|
||||
// we are listening on the general `router.created` event - every router throws this event
|
||||
|
|
@ -87,7 +83,7 @@ class UrlService {
|
|||
return;
|
||||
}
|
||||
|
||||
debug('router.created');
|
||||
debug('Registering route: ', router.name);
|
||||
|
||||
let urlGenerator = new UrlGenerator(router, this.queue, this.resources, this.urls, this.urlGenerators.length);
|
||||
this.urlGenerators.push(urlGenerator);
|
||||
|
|
@ -307,7 +303,6 @@ class UrlService {
|
|||
if (!options.keepListeners) {
|
||||
this._onQueueStartedListener && this.queue.removeListener('started', this._onQueueStartedListener);
|
||||
this._onQueueEndedListener && this.queue.removeListener('ended', this._onQueueEndedListener);
|
||||
this._onRouterAddedListener && events.removeListener('router.created', this._onRouterAddedListener);
|
||||
this._onThemeChangedListener && events.removeListener('services.themes.api.changed', this._onThemeChangedListener);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -5,7 +5,6 @@ const rewire = require('rewire');
|
|||
const testUtils = require('../utils');
|
||||
const configUtils = require('../utils/configUtils');
|
||||
const models = require('../../core/server/models');
|
||||
const events = require('../../core/server/lib/common/events');
|
||||
const UrlService = rewire('../../core/frontend/services/url/UrlService');
|
||||
|
||||
/**
|
||||
|
|
@ -111,10 +110,10 @@ describe('Integration: services/url/UrlService', function () {
|
|||
}
|
||||
});
|
||||
|
||||
events.emit('router.created', router1);
|
||||
events.emit('router.created', router2);
|
||||
events.emit('router.created', router3);
|
||||
events.emit('router.created', router4);
|
||||
urlService.onRouterAddedType(router1);
|
||||
urlService.onRouterAddedType(router2);
|
||||
urlService.onRouterAddedType(router3);
|
||||
urlService.onRouterAddedType(router4);
|
||||
|
||||
// We can't use our url service utils here, because this is a local copy of the urlService, not the singletone
|
||||
urlService.init();
|
||||
|
|
@ -315,11 +314,11 @@ describe('Integration: services/url/UrlService', function () {
|
|||
}
|
||||
});
|
||||
|
||||
events.emit('router.created', router1);
|
||||
events.emit('router.created', router2);
|
||||
events.emit('router.created', router3);
|
||||
events.emit('router.created', router4);
|
||||
events.emit('router.created', router5);
|
||||
urlService.onRouterAddedType(router1);
|
||||
urlService.onRouterAddedType(router2);
|
||||
urlService.onRouterAddedType(router3);
|
||||
urlService.onRouterAddedType(router4);
|
||||
urlService.onRouterAddedType(router5);
|
||||
|
||||
// We can't use our url service utils here, because this is a local copy of the urlService, not the singletone
|
||||
urlService.init();
|
||||
|
|
@ -513,11 +512,11 @@ describe('Integration: services/url/UrlService', function () {
|
|||
}
|
||||
});
|
||||
|
||||
events.emit('router.created', router1);
|
||||
events.emit('router.created', router2);
|
||||
events.emit('router.created', router3);
|
||||
events.emit('router.created', router4);
|
||||
events.emit('router.created', router5);
|
||||
urlService.onRouterAddedType(router1);
|
||||
urlService.onRouterAddedType(router2);
|
||||
urlService.onRouterAddedType(router3);
|
||||
urlService.onRouterAddedType(router4);
|
||||
urlService.onRouterAddedType(router5);
|
||||
|
||||
// We can't use our url service utils here, because this is a local copy of the urlService, not the singletone
|
||||
urlService.init();
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
const should = require('should');
|
||||
const sinon = require('sinon');
|
||||
const express = require('../../../../../core/shared/express')._express;
|
||||
const settingsCache = require('../../../../../core/shared/settings-cache');
|
||||
const bootstrap = require('../../../../../core/frontend/services/routing/bootstrap');
|
||||
const events = require('../../../../../core/server/lib/common/events');
|
||||
const controllers = require('../../../../../core/frontend/services/routing/controllers');
|
||||
const CollectionRouter = require('../../../../../core/frontend/services/routing/CollectionRouter');
|
||||
|
|
@ -15,6 +15,7 @@ describe('UNIT - services/routing/CollectionRouter', function () {
|
|||
beforeEach(function () {
|
||||
sinon.stub(events, 'emit');
|
||||
sinon.stub(events, 'on');
|
||||
sinon.stub(bootstrap.internal, 'routerCreated');
|
||||
|
||||
sinon.spy(CollectionRouter.prototype, 'mountRoute');
|
||||
sinon.spy(CollectionRouter.prototype, 'mountRouter');
|
||||
|
|
@ -43,10 +44,8 @@ describe('UNIT - services/routing/CollectionRouter', function () {
|
|||
collectionRouter.templates.should.eql([]);
|
||||
collectionRouter.getPermalinks().getValue().should.eql('/:slug/');
|
||||
|
||||
events.emit.calledOnce.should.be.true();
|
||||
events.emit.calledWith('router.created', collectionRouter).should.be.true();
|
||||
|
||||
events.on.calledTwice.should.be.false();
|
||||
bootstrap.internal.routerCreated.calledOnce.should.be.true();
|
||||
bootstrap.internal.routerCreated.calledWith(collectionRouter).should.be.true();
|
||||
|
||||
collectionRouter.mountRoute.callCount.should.eql(3);
|
||||
express.Router.param.callCount.should.eql(2);
|
||||
|
|
@ -92,10 +91,8 @@ describe('UNIT - services/routing/CollectionRouter', function () {
|
|||
collectionRouter.templates.should.eql([]);
|
||||
collectionRouter.getPermalinks().getValue().should.eql('/blog/:year/:slug/');
|
||||
|
||||
events.emit.calledOnce.should.be.true();
|
||||
events.emit.calledWith('router.created', collectionRouter).should.be.true();
|
||||
|
||||
events.on.calledTwice.should.be.false();
|
||||
bootstrap.internal.routerCreated.calledOnce.should.be.true();
|
||||
bootstrap.internal.routerCreated.calledWith(collectionRouter).should.be.true();
|
||||
|
||||
collectionRouter.mountRoute.callCount.should.eql(3);
|
||||
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
const should = require('should');
|
||||
const sinon = require('sinon');
|
||||
const events = require('../../../../../core/server/lib/common/events');
|
||||
const bootstrap = require('../../../../../core/frontend/services/routing/bootstrap');
|
||||
const controllers = require('../../../../../core/frontend/services/routing/controllers');
|
||||
const StaticRoutesRouter = require('../../../../../core/frontend/services/routing/StaticRoutesRouter');
|
||||
const configUtils = require('../../../../utils/configUtils');
|
||||
|
|
@ -15,8 +15,7 @@ describe('UNIT - services/routing/StaticRoutesRouter', function () {
|
|||
});
|
||||
|
||||
beforeEach(function () {
|
||||
sinon.stub(events, 'emit');
|
||||
sinon.stub(events, 'on');
|
||||
sinon.stub(bootstrap.internal, 'routerCreated');
|
||||
|
||||
sinon.spy(StaticRoutesRouter.prototype, 'mountRoute');
|
||||
sinon.spy(StaticRoutesRouter.prototype, 'mountRouter');
|
||||
|
|
@ -42,8 +41,8 @@ describe('UNIT - services/routing/StaticRoutesRouter', function () {
|
|||
|
||||
staticRoutesRouter.templates.should.eql(['test']);
|
||||
|
||||
events.emit.calledOnce.should.be.true();
|
||||
events.emit.calledWith('router.created', staticRoutesRouter).should.be.true();
|
||||
bootstrap.internal.routerCreated.calledOnce.should.be.true();
|
||||
bootstrap.internal.routerCreated.calledWith(staticRoutesRouter).should.be.true();
|
||||
|
||||
staticRoutesRouter.mountRoute.callCount.should.eql(1);
|
||||
|
||||
|
|
@ -52,7 +51,7 @@ describe('UNIT - services/routing/StaticRoutesRouter', function () {
|
|||
staticRoutesRouter.mountRoute.args[0][1].should.eql(controllers.static);
|
||||
});
|
||||
|
||||
it('initialise with data+filter', function () {
|
||||
it('initialize with data+filter', function () {
|
||||
const staticRoutesRouter = new StaticRoutesRouter('/about/', {
|
||||
data: {query: {}, router: {}},
|
||||
filter: 'tag:test'
|
||||
|
|
@ -64,8 +63,8 @@ describe('UNIT - services/routing/StaticRoutesRouter', function () {
|
|||
should.not.exist(staticRoutesRouter.getFilter());
|
||||
staticRoutesRouter.templates.should.eql([]);
|
||||
|
||||
events.emit.calledOnce.should.be.true();
|
||||
events.emit.calledWith('router.created', staticRoutesRouter).should.be.true();
|
||||
bootstrap.internal.routerCreated.calledOnce.should.be.true();
|
||||
bootstrap.internal.routerCreated.calledWith(staticRoutesRouter).should.be.true();
|
||||
|
||||
staticRoutesRouter.mountRoute.callCount.should.eql(1);
|
||||
|
||||
|
|
@ -109,8 +108,8 @@ describe('UNIT - services/routing/StaticRoutesRouter', function () {
|
|||
});
|
||||
|
||||
describe('channels', function () {
|
||||
describe('initialise', function () {
|
||||
it('initialise with controller+data+filter', function () {
|
||||
describe('initialize', function () {
|
||||
it('initialize with controller+data+filter', function () {
|
||||
const staticRoutesRouter = new StaticRoutesRouter('/channel/', {
|
||||
controller: 'channel',
|
||||
data: {query: {}, router: {}},
|
||||
|
|
@ -124,8 +123,8 @@ describe('UNIT - services/routing/StaticRoutesRouter', function () {
|
|||
staticRoutesRouter.templates.should.eql([]);
|
||||
should.exist(staticRoutesRouter.data);
|
||||
|
||||
events.emit.calledOnce.should.be.true();
|
||||
events.emit.calledWith('router.created', staticRoutesRouter).should.be.true();
|
||||
bootstrap.internal.routerCreated.calledOnce.should.be.true();
|
||||
bootstrap.internal.routerCreated.calledWith(staticRoutesRouter).should.be.true();
|
||||
|
||||
staticRoutesRouter.mountRoute.callCount.should.eql(2);
|
||||
|
||||
|
|
@ -138,7 +137,7 @@ describe('UNIT - services/routing/StaticRoutesRouter', function () {
|
|||
staticRoutesRouter.mountRoute.args[1][1].should.eql(controllers.channel);
|
||||
});
|
||||
|
||||
it('initialise with controller+filter', function () {
|
||||
it('initialize with controller+filter', function () {
|
||||
const staticRoutesRouter = new StaticRoutesRouter('/channel/', {
|
||||
controller: 'channel',
|
||||
filter: 'tag:test'
|
||||
|
|
@ -151,8 +150,8 @@ describe('UNIT - services/routing/StaticRoutesRouter', function () {
|
|||
|
||||
staticRoutesRouter.templates.should.eql([]);
|
||||
|
||||
events.emit.calledOnce.should.be.true();
|
||||
events.emit.calledWith('router.created', staticRoutesRouter).should.be.true();
|
||||
bootstrap.internal.routerCreated.calledOnce.should.be.true();
|
||||
bootstrap.internal.routerCreated.calledWith(staticRoutesRouter).should.be.true();
|
||||
|
||||
staticRoutesRouter.mountRoute.callCount.should.eql(2);
|
||||
|
||||
|
|
@ -165,7 +164,7 @@ describe('UNIT - services/routing/StaticRoutesRouter', function () {
|
|||
staticRoutesRouter.mountRoute.args[1][1].should.eql(controllers.channel);
|
||||
});
|
||||
|
||||
it('initialise with controller+data', function () {
|
||||
it('initialize with controller+data', function () {
|
||||
const staticRoutesRouter = new StaticRoutesRouter('/channel/', {
|
||||
controller: 'channel',
|
||||
data: {query: {}, router: {}}
|
||||
|
|
@ -174,7 +173,7 @@ describe('UNIT - services/routing/StaticRoutesRouter', function () {
|
|||
should.not.exist(staticRoutesRouter.getFilter());
|
||||
});
|
||||
|
||||
it('initialise on subdirectory with controller+data+filter', function () {
|
||||
it('initialize on subdirectory with controller+data+filter', function () {
|
||||
configUtils.set('url', 'http://localhost:2366/blog/');
|
||||
|
||||
const staticRoutesRouter = new StaticRoutesRouter('/channel/', {
|
||||
|
|
|
|||
|
|
@ -1,8 +1,7 @@
|
|||
const should = require('should');
|
||||
const sinon = require('sinon');
|
||||
const _ = require('lodash');
|
||||
const settingsCache = require('../../../../../core/shared/settings-cache');
|
||||
const events = require('../../../../../core/server/lib/common/events');
|
||||
const bootstrap = require('../../../../../core/frontend/services/routing/bootstrap');
|
||||
const controllers = require('../../../../../core/frontend/services/routing/controllers');
|
||||
const TaxonomyRouter = require('../../../../../core/frontend/services/routing/TaxonomyRouter');
|
||||
const RESOURCE_CONFIG_V2 = require('../../../../../core/frontend/services/routing/config/v2');
|
||||
|
|
@ -17,8 +16,7 @@ describe('UNIT - services/routing/TaxonomyRouter', function () {
|
|||
beforeEach(function () {
|
||||
sinon.stub(settingsCache, 'get').withArgs('permalinks').returns('/:slug/');
|
||||
|
||||
sinon.stub(events, 'emit');
|
||||
sinon.stub(events, 'on');
|
||||
sinon.stub(bootstrap.internal, 'routerCreated');
|
||||
|
||||
sinon.spy(TaxonomyRouter.prototype, 'mountRoute');
|
||||
sinon.spy(TaxonomyRouter.prototype, 'mountRouter');
|
||||
|
|
@ -43,8 +41,8 @@ describe('UNIT - services/routing/TaxonomyRouter', function () {
|
|||
taxonomyRouter.taxonomyKey.should.eql('tag');
|
||||
taxonomyRouter.getPermalinks().getValue().should.eql('/tag/:slug/');
|
||||
|
||||
events.emit.calledOnce.should.be.true();
|
||||
events.emit.calledWith('router.created', taxonomyRouter).should.be.true();
|
||||
bootstrap.internal.routerCreated.calledOnce.should.be.true();
|
||||
bootstrap.internal.routerCreated.calledWith(taxonomyRouter).should.be.true();
|
||||
|
||||
taxonomyRouter.mountRouter.callCount.should.eql(1);
|
||||
taxonomyRouter.mountRouter.args[0][0].should.eql('/tag/:slug/');
|
||||
|
|
|
|||
|
|
@ -58,9 +58,8 @@ describe('Unit: services/url/UrlService', function () {
|
|||
urlService.queue.addListener.args[0][0].should.eql('started');
|
||||
urlService.queue.addListener.args[1][0].should.eql('ended');
|
||||
|
||||
events.on.calledTwice.should.be.true();
|
||||
events.on.args[0][0].should.eql('router.created');
|
||||
events.on.args[1][0].should.eql('services.themes.api.changed');
|
||||
events.on.calledOnce.should.be.true();
|
||||
events.on.args[0][0].should.eql('services.themes.api.changed');
|
||||
});
|
||||
|
||||
it('fn: _onQueueStarted', function () {
|
||||
|
|
@ -73,8 +72,8 @@ describe('Unit: services/url/UrlService', function () {
|
|||
urlService.hasFinished().should.be.true();
|
||||
});
|
||||
|
||||
it('fn: _onRouterAddedType', function () {
|
||||
urlService._onRouterAddedType({getPermalinks: sinon.stub().returns({})});
|
||||
it('fn: onRouterAddedType', function () {
|
||||
urlService.onRouterAddedType({getPermalinks: sinon.stub().returns({})});
|
||||
urlService.urlGenerators.length.should.eql(1);
|
||||
});
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue