0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-02-03 23:00:14 -05:00

Refactored async helpers to use async/await (#14781)

- async/await makes the code easier to read
- also means we can get rid of usages of Promise, and some unnecessary usages of Bluebird
- this refactor is done for the 4 async helpers and the code that wraps async helpers so the whole pipeline is async/await and more understandable
This commit is contained in:
Hannah Wolfe 2022-05-11 09:51:38 +01:00 committed by GitHub
parent 961e300e77
commit 412d290a30
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 168 additions and 179 deletions

View file

@ -6,8 +6,6 @@
//
// Converts normal HTML into AMP HTML with Amperize module and uses a cache to return it from
// there if available. The cacheId is a combination of `updated_at` and the `slug`.
const Promise = require('bluebird');
const {DateTime, Interval} = require('luxon');
const errors = require('@tryghost/errors');
const logging = require('@tryghost/logging');
@ -170,19 +168,16 @@ function getAmperizeHTML(html, post) {
return Promise.resolve(amperizeCache[post.id].amp);
}
function ampContent() {
module.exports = async function amp_content() { // eslint-disable-line camelcase
let sanitizeHtml = require('sanitize-html');
let cheerio = require('cheerio');
let amperizeHTML = {
amperize: getAmperizeHTML(this.html, this)
};
return Promise.props(amperizeHTML).then((result) => {
try {
const response = await getAmperizeHTML(this.html, this);
let $ = null;
// our Amperized HTML
ampHTML = result.amperize || '';
ampHTML = response ?? '';
// Use cheerio to traverse through HTML and make little clean-ups
$ = cheerio.load(ampHTML);
@ -208,9 +203,12 @@ function ampContent() {
});
return new SafeString(cleanHTML);
});
}
} catch (error) {
logging.error(error);
module.exports = ampContent;
// Return an empty safe string
return new SafeString('');
}
};
module.exports.async = true;

View file

@ -9,7 +9,6 @@ const errors = require('@tryghost/errors');
const tpl = require('@tryghost/tpl');
const _ = require('lodash');
const Promise = require('bluebird');
const jsonpath = require('jsonpath');
const messages = {
@ -46,7 +45,6 @@ const pathAliases = {
/**
* ## Is Browse
* Is this a Browse request or a Read request?
* @param {Object} resource
* @param {Object} options
* @returns {boolean}
*/
@ -123,9 +121,9 @@ function parseOptions(globals, data, options) {
* ## Get
* @param {Object} resource
* @param {Object} options
* @returns {Promise}
* @returns {Promise<any>}
*/
module.exports = function get(resource, options) {
module.exports = async function get(resource, options) {
options = options || {};
options.hash = options.hash || {};
options.data = options.data || {};
@ -141,13 +139,13 @@ module.exports = function get(resource, options) {
if (!options.fn) {
data.error = tpl(messages.mustBeCalledAsBlock, {helperName: 'get'});
logging.warn(data.error);
return Promise.resolve();
return;
}
if (!RESOURCES[resource]) {
data.error = tpl(messages.invalidResource);
logging.warn(data.error);
return Promise.resolve(options.inverse(self, {data: data}));
return options.inverse(self, {data: data});
}
const controllerName = RESOURCES[resource].alias;
@ -158,34 +156,35 @@ module.exports = function get(resource, options) {
apiOptions = parseOptions(ghostGlobals, this, apiOptions);
apiOptions.context = {member: data.member};
// @TODO: https://github.com/TryGhost/Ghost/issues/10548
return controller[action](apiOptions).then(function success(result) {
try {
const response = await controller[action](apiOptions);
// prepare data properties for use with handlebars
if (result[resource] && result[resource].length) {
result[resource].forEach(prepareContextResource);
if (response[resource] && response[resource].length) {
response[resource].forEach(prepareContextResource);
}
// used for logging details of slow requests
returnedRowsCount = result[resource] && result[resource].length;
returnedRowsCount = response[resource] && response[resource].length;
// block params allows the theme developer to name the data using something like
// `{{#get "posts" as |result pageInfo|}}`
const blockParams = [result[resource]];
if (result.meta && result.meta.pagination) {
result.pagination = result.meta.pagination;
blockParams.push(result.meta.pagination);
const blockParams = [response[resource]];
if (response.meta && response.meta.pagination) {
response.pagination = response.meta.pagination;
blockParams.push(response.meta.pagination);
}
// Call the main template function
return options.fn(result, {
return options.fn(response, {
data: data,
blockParams: blockParams
});
}).catch(function error(err) {
logging.error(err);
data.error = err.message;
} catch (error) {
logging.error(error);
data.error = error.message;
return options.inverse(self, {data: data});
}).finally(function () {
} finally {
const totalMs = Date.now() - start;
const logLevel = config.get('logging:slowHelper:level');
const threshold = config.get('logging:slowHelper:threshold');
@ -200,7 +199,7 @@ module.exports = function get(resource, options) {
}
}));
}
});
}
};
module.exports.async = true;

View file

@ -96,7 +96,7 @@ function getMembersHelper(data) {
* Also see how the root object gets created, https://github.com/wycats/handlebars.js/blob/v4.0.6/lib/handlebars/runtime.js#L259
*/
// We use the name ghost_head to match the helper for consistency:
module.exports = function ghost_head(options) { // eslint-disable-line camelcase
module.exports = async function ghost_head(options) { // eslint-disable-line camelcase
debug('begin');
// if server error page do nothing
@ -118,128 +118,126 @@ module.exports = function ghost_head(options) { // eslint-disable-line camelcase
debug('preparation complete, begin fetch');
/**
* @TODO:
* - getMetaData(dataRoot, dataRoot) -> yes that looks confusing!
* - there is a very mixed usage of `data.context` vs. `root.context` vs `root._locals.context` vs. `this.context`
* - NOTE: getMetaData won't live here anymore soon, see https://github.com/TryGhost/Ghost/issues/8995
* - therefore we get rid of using `getMetaData(this, dataRoot)`
* - dataRoot has access to *ALL* locals, see function description
* - it should not break anything
*/
return getMetaData(dataRoot, dataRoot)
.then(function handleMetaData(meta) {
debug('end fetch');
try {
/**
* @TODO:
* - getMetaData(dataRoot, dataRoot) -> yes that looks confusing!
* - there is a very mixed usage of `data.context` vs. `root.context` vs `root._locals.context` vs. `this.context`
* - NOTE: getMetaData won't live here anymore soon, see https://github.com/TryGhost/Ghost/issues/8995
* - therefore we get rid of using `getMetaData(this, dataRoot)`
* - dataRoot has access to *ALL* locals, see function description
* - it should not break anything
*/
const meta = await getMetaData(dataRoot, dataRoot);
debug('end fetch');
if (context) {
// head is our main array that holds our meta data
if (meta.metaDescription && meta.metaDescription.length > 0) {
head.push('<meta name="description" content="' + escapeExpression(meta.metaDescription) + '" />');
}
// no output in head if a publication icon is not set
if (settingsCache.get('icon')) {
head.push('<link rel="icon" href="' + favicon + '" type="image/' + iconType + '" />');
}
head.push('<link rel="canonical" href="' +
escapeExpression(meta.canonicalUrl) + '" />');
head.push('<meta name="referrer" content="' + referrerPolicy + '" />');
// don't allow indexing of preview URLs!
if (_.includes(context, 'preview')) {
head.push(writeMetaTag('robots', 'noindex,nofollow', 'name'));
}
// show amp link in post when 1. we are not on the amp page and 2. amp is enabled
if (_.includes(context, 'post') && !_.includes(context, 'amp') && settingsCache.get('amp')) {
head.push('<link rel="amphtml" href="' +
escapeExpression(meta.ampUrl) + '" />');
}
if (meta.previousUrl) {
head.push('<link rel="prev" href="' +
escapeExpression(meta.previousUrl) + '" />');
}
if (meta.nextUrl) {
head.push('<link rel="next" href="' +
escapeExpression(meta.nextUrl) + '" />');
}
if (!_.includes(context, 'paged') && useStructuredData) {
head.push('');
head.push.apply(head, finaliseStructuredData(meta));
head.push('');
if (meta.schema) {
head.push('<script type="application/ld+json">\n' +
JSON.stringify(meta.schema, null, ' ') +
'\n </script>\n');
}
}
if (context) {
// head is our main array that holds our meta data
if (meta.metaDescription && meta.metaDescription.length > 0) {
head.push('<meta name="description" content="' + escapeExpression(meta.metaDescription) + '" />');
}
head.push('<meta name="generator" content="Ghost ' +
escapeExpression(safeVersion) + '" />');
// Ghost analytics tag
if (labs.isSet('membersActivity')) {
const postId = (dataRoot && dataRoot.post) ? dataRoot.post.id : '';
head.push(writeMetaTag('ghost-analytics-id', postId, 'name'));
// no output in head if a publication icon is not set
if (settingsCache.get('icon')) {
head.push('<link rel="icon" href="' + favicon + '" type="image/' + iconType + '" />');
}
head.push('<link rel="alternate" type="application/rss+xml" title="' +
escapeExpression(meta.site.title) + '" href="' +
escapeExpression(meta.rssUrl) + '" />');
head.push('<link rel="canonical" href="' + escapeExpression(meta.canonicalUrl) + '" />');
head.push('<meta name="referrer" content="' + referrerPolicy + '" />');
// no code injection for amp context!!!
if (!_.includes(context, 'amp')) {
head.push(getMembersHelper(options.data));
// @TODO do this in a more "frameworky" way
if (cardAssetService.hasFile('js')) {
head.push(`<script defer src="${getAssetUrl('public/cards.min.js')}"></script>`);
}
if (cardAssetService.hasFile('css')) {
head.push(`<link rel="stylesheet" type="text/css" href="${getAssetUrl('public/cards.min.css')}">`);
}
if (!_.isEmpty(globalCodeinjection)) {
head.push(globalCodeinjection);
}
if (!_.isEmpty(postCodeInjection)) {
head.push(postCodeInjection);
}
if (!_.isEmpty(tagCodeInjection)) {
head.push(tagCodeInjection);
}
// don't allow indexing of preview URLs!
if (_.includes(context, 'preview')) {
head.push(writeMetaTag('robots', 'noindex,nofollow', 'name'));
}
// AMP template has style injected directly because there can only be one <style amp-custom> tag
if (options.data.site.accent_color && !_.includes(context, 'amp')) {
const accentColor = escapeExpression(options.data.site.accent_color);
const styleTag = `<style>:root {--ghost-accent-color: ${accentColor};}</style>`;
const existingScriptIndex = _.findLastIndex(head, str => str.match(/<\/(style|script)>/));
if (existingScriptIndex !== -1) {
head[existingScriptIndex] = head[existingScriptIndex] + styleTag;
} else {
head.push(styleTag);
}
// show amp link in post when 1. we are not on the amp page and 2. amp is enabled
if (_.includes(context, 'post') && !_.includes(context, 'amp') && settingsCache.get('amp')) {
head.push('<link rel="amphtml" href="' +
escapeExpression(meta.ampUrl) + '" />');
}
debug('end');
return new SafeString(head.join('\n ').trim());
})
.catch(function handleError(err) {
logging.error(err);
if (meta.previousUrl) {
head.push('<link rel="prev" href="' +
escapeExpression(meta.previousUrl) + '" />');
}
// Return what we have so far (currently nothing)
return new SafeString(head.join('\n ').trim());
});
if (meta.nextUrl) {
head.push('<link rel="next" href="' +
escapeExpression(meta.nextUrl) + '" />');
}
if (!_.includes(context, 'paged') && useStructuredData) {
head.push('');
head.push.apply(head, finaliseStructuredData(meta));
head.push('');
if (meta.schema) {
head.push('<script type="application/ld+json">\n' +
JSON.stringify(meta.schema, null, ' ') +
'\n </script>\n');
}
}
}
head.push('<meta name="generator" content="Ghost ' +
escapeExpression(safeVersion) + '" />');
// Ghost analytics tag
if (labs.isSet('membersActivity')) {
const postId = (dataRoot && dataRoot.post) ? dataRoot.post.id : '';
head.push(writeMetaTag('ghost-analytics-id', postId, 'name'));
}
head.push('<link rel="alternate" type="application/rss+xml" title="' +
escapeExpression(meta.site.title) + '" href="' +
escapeExpression(meta.rssUrl) + '" />');
// no code injection for amp context!!!
if (!_.includes(context, 'amp')) {
head.push(getMembersHelper(options.data));
// @TODO do this in a more "frameworky" way
if (cardAssetService.hasFile('js')) {
head.push(`<script defer src="${getAssetUrl('public/cards.min.js')}"></script>`);
}
if (cardAssetService.hasFile('css')) {
head.push(`<link rel="stylesheet" type="text/css" href="${getAssetUrl('public/cards.min.css')}">`);
}
if (!_.isEmpty(globalCodeinjection)) {
head.push(globalCodeinjection);
}
if (!_.isEmpty(postCodeInjection)) {
head.push(postCodeInjection);
}
if (!_.isEmpty(tagCodeInjection)) {
head.push(tagCodeInjection);
}
}
// AMP template has style injected directly because there can only be one <style amp-custom> tag
if (options.data.site.accent_color && !_.includes(context, 'amp')) {
const accentColor = escapeExpression(options.data.site.accent_color);
const styleTag = `<style>:root {--ghost-accent-color: ${accentColor};}</style>`;
const existingScriptIndex = _.findLastIndex(head, str => str.match(/<\/(style|script)>/));
if (existingScriptIndex !== -1) {
head[existingScriptIndex] = head[existingScriptIndex] + styleTag;
} else {
head.push(styleTag);
}
}
debug('end');
return new SafeString(head.join('\n ').trim());
} catch (error) {
logging.error(error);
// Return what we have so far (currently nothing)
return new SafeString(head.join('\n ').trim());
}
};
module.exports.async = true;

View file

@ -9,7 +9,6 @@ const {checks} = require('../services/data');
const logging = require('@tryghost/logging');
const tpl = require('@tryghost/tpl');
const get = require('lodash/get');
const Promise = require('bluebird');
const moment = require('moment');
const messages = {
@ -55,29 +54,25 @@ const buildApiOptions = function buildApiOptions(options, post) {
* @param {*} data
* @returns {Promise<any>}
*/
const fetch = function fetch(options, data) {
const fetch = async function fetch(options, data) {
const self = this;
const apiOptions = buildApiOptions(options, this);
// @TODO: https://github.com/TryGhost/Ghost/issues/10548
const controller = api.postsPublic || api.posts;
try {
const response = await api.postsPublic.browse(apiOptions);
return controller
.browse(apiOptions)
.then(function handleSuccess(result) {
const related = result.posts[0];
const related = response.posts[0];
if (related) {
return options.fn(related, {data: data});
} else {
return options.inverse(self, {data: data});
}
})
.catch(function handleError(err) {
logging.error(err);
data.error = err.message;
if (related) {
return options.fn(related, {data: data});
} else {
return options.inverse(self, {data: data});
});
}
} catch (error) {
logging.error(error);
data.error = error.message;
return options.inverse(self, {data: data});
}
};
// If prevNext method is called without valid post data then we must return a promise, if there is valid post data
@ -87,7 +82,7 @@ const fetch = function fetch(options, data) {
* @param {*} options
* @returns {Promise<any>}
*/
module.exports = function prevNext(options) {
module.exports = async function prevNext(options) {
options = options || {};
const data = createFrame(options.data);
@ -97,16 +92,16 @@ module.exports = function prevNext(options) {
if (!options.fn || !options.inverse) {
data.error = tpl(messages.mustBeCalledAsBlock, {helperName: options.name});
logging.warn(data.error);
return Promise.resolve();
return;
}
if (context.includes('preview')) {
return Promise.resolve(options.inverse(this, {data: data}));
return options.inverse(this, {data: data});
}
// Guard against trying to execute prev/next on pages, or other resources
if (!checks.isPost(this) || this.page) {
return Promise.resolve(options.inverse(this, {data: data}));
return options.inverse(this, {data: data});
}
// With the guards out of the way, attempt to build the apiOptions, and then fetch the data

View file

@ -1,4 +1,3 @@
const Promise = require('bluebird');
const errors = require('@tryghost/errors');
const logging = require('@tryghost/logging');
@ -6,31 +5,31 @@ const {hbs} = require('../handlebars');
// Register an async handlebars helper for a given handlebars instance
function asyncHelperWrapper(hbsInstance, name, fn) {
hbsInstance.registerAsyncHelper(name, function returnAsync(context, options, cb) {
hbsInstance.registerAsyncHelper(name, async function returnAsync(context, options, cb) {
// Handle the case where we only get context and cb
if (!cb) {
cb = options;
options = undefined;
}
// Wrap the function passed in with a Promise.resolve so it can return either a promise or a value
Promise.resolve(fn.call(this, context, options)).then(function asyncHelperSuccess(result) {
cb(result);
}).catch(function asyncHelperError(err) {
const wrappedErr = errors.utils.isGhostError(err) ? err : new errors.IncorrectUsageError({
err: err,
try {
const response = await fn.call(this, context, options);
cb(response);
} catch (error) {
const wrappedErr = errors.utils.isGhostError(error) ? error : new errors.IncorrectUsageError({
err: error,
context: 'registerAsyncThemeHelper: ' + name,
errorDetails: {
originalError: err
originalError: error
}
});
const result = process.env.NODE_ENV === 'development' ? wrappedErr : '';
const response = process.env.NODE_ENV === 'development' ? wrappedErr : '';
logging.error(wrappedErr);
cb(new hbsInstance.SafeString(result));
});
cb(new hbsInstance.SafeString(response));
}
});
}