From 711525c8a06d6deab76cf76ab6220e467a29a3ca Mon Sep 17 00:00:00 2001 From: Ze Yu Date: Wed, 10 Jun 2020 18:46:33 +0800 Subject: [PATCH 1/5] Add more plugin hooks There are only 2 interfaces for plugins to modify a page's content, preRender and postRender. These plugin hooks do so by passing the processed page content thus far to the plugin, requiring the plugin to reparse the entire page content, often only to perform simple operations. Let's add 2 more plugin hooks to allow plugins to tap into the recursive dom handling processes at these respective stages. In the following commits, we adapt some of the plugins to use these new hooks, and observe a significant performance improvement of ~10%, which should likely scale as we add and adapt more plugins. --- src/Page.js | 76 ++++++++++++------- src/Site.js | 4 +- src/lib/markbind/src/parser.js | 20 +++-- .../markbind/src/parsers/componentParser.js | 16 +++- .../preprocessors/componentPreprocessor.js | 15 +++- test/unit/parsers/componentParser.test.js | 2 +- 6 files changed, 89 insertions(+), 44 deletions(-) diff --git a/src/Page.js b/src/Page.js index a8275e5b09..2d27226d89 100644 --- a/src/Page.js +++ b/src/Page.js @@ -10,6 +10,7 @@ const _ = {}; _.isString = require('lodash/isString'); _.isObject = require('lodash/isObject'); _.isArray = require('lodash/isArray'); +_.isFunction = require('lodash/isFunction'); const CyclicReferenceError = require('./lib/markbind/src/handlers/cyclicReferenceError.js'); const { ensurePosix } = require('./lib/markbind/src/utils'); @@ -141,10 +142,10 @@ class Page { */ this.globalOverride = pageConfig.globalOverride; /** - * Array of plugins used in this page. - * @type {Array} + * Plugin names and the corresponding plugins used in this page. + * @type {Object} */ - this.plugins = pageConfig.plugins; + this.plugins = pageConfig.plugins || {}; /** * @type {Object>} */ @@ -521,6 +522,7 @@ class Page { collectFrontMatter(includedPage) { const $ = cheerio.load(includedPage); const frontMatter = $('frontmatter'); + Object.keys(this.frontMatter).forEach(k => delete this.frontMatter[k]); if (frontMatter.text().trim()) { // Retrieves the front matter from either the first frontmatter element // or from a frontmatter element that includes from another file @@ -529,15 +531,16 @@ class Page { ? frontMatter.find('div')[0].children[0].data : frontMatter[0].children[0].data; const frontMatterWrapped = `${FRONT_MATTER_FENCE}\n${frontMatterData}\n${FRONT_MATTER_FENCE}`; + // Parse front matter data const parsedData = fm(frontMatterWrapped); - this.frontMatter = { ...parsedData.attributes }; - this.frontMatter.src = this.src; + parsedData.attributes.src = this.src; // Title specified in site.json will override title specified in front matter - this.frontMatter.title = (this.title || this.frontMatter.title || ''); + parsedData.attributes.title = (this.title || parsedData.attributes.title || ''); // Layout specified in site.json will override layout specified in the front matter - this.frontMatter.layout = (this.layout || this.frontMatter.layout || LAYOUT_DEFAULT_NAME); - this.frontMatter = { ...this.frontMatter, ...this.globalOverride, ...this.frontmatterOverride }; + parsedData.attributes.layout = (this.layout || parsedData.attributes.layout || LAYOUT_DEFAULT_NAME); + + Object.assign(this.frontMatter, parsedData.attributes, this.globalOverride, this.frontmatterOverride); } else { // Page is addressable but no front matter specified const defaultAttributes = { @@ -545,11 +548,7 @@ class Page { title: this.title || '', layout: LAYOUT_DEFAULT_NAME, }; - this.frontMatter = { - ...defaultAttributes, - ...this.globalOverride, - ...this.frontmatterOverride, - }; + Object.assign(this.frontMatter, defaultAttributes, this.globalOverride, this.frontmatterOverride); } this.title = this.frontMatter.title; } @@ -942,8 +941,12 @@ class Page { generate(builtFiles) { this.includedFiles = new Set([this.sourcePath]); this.headerIdMap = {}; // Reset for live reload + + const pluginConfig = this.getPluginConfig(); + const markbinder = new MarkBind({ variablePreprocessor: this.variablePreprocessor, + ...this.preparePluginHooks(pluginConfig), }); /** * @type {FileConfig} @@ -963,15 +966,15 @@ class Page { .then(result => this.generateExpressiveLayout(result, fileConfig, markbinder)) .then(result => Page.removePageHeaderAndFooter(result)) .then(result => Page.addContentWrapper(result)) - .then(result => this.collectPluginSources(result)) - .then(result => this.preRender(result)) + .then(result => this.collectPluginSources(result, pluginConfig)) + .then(result => this.preRender(result, pluginConfig)) .then(result => this.insertSiteNav((result))) .then(result => this.insertHeaderFile(result, fileConfig)) .then(result => this.insertFooterFile(result)) .then(result => Page.insertTemporaryStyles(result)) .then(result => markbinder.resolveBaseUrl(result, fileConfig)) .then(result => markbinder.render(result, this.sourcePath, fileConfig)) - .then(result => this.postRender(result)) + .then(result => this.postRender(result, pluginConfig)) .then(result => this.collectPluginsAssets(result)) .then(result => markbinder.processDynamicResources(this.sourcePath, result)) .then(result => MarkBind.unwrapIncludeSrc(result)) @@ -1033,7 +1036,7 @@ class Page { * Retrieves page config for plugins * @return {PluginConfig} pluginConfig */ - getPluginConfig() { + getPluginConfig(override = {}) { return { headingIndexingLevel: this.headingIndexingLevel, enableSearch: this.enableSearch, @@ -1042,13 +1045,27 @@ class Page { sourcePath: this.sourcePath, includedFiles: this.includedFiles, resultPath: this.resultPath, + ...override, }; } + preparePluginHooks(pluginConfig) { + // Prepare curried plugin node hooks + const preRenderNodeHooks = Object.entries(this.plugins) + .filter(([, plugin]) => _.isFunction(plugin.preRenderNode)) + .map(([name, plugin]) => node => plugin.preRenderNode(node, this.pluginsContext[name] || {}, + this.frontMatter, pluginConfig)); + const postRenderNodeHooks = Object.entries(this.plugins) + .filter(([, plugin]) => _.isFunction(plugin.postRenderNode)) + .map(([name, plugin]) => node => plugin.postRenderNode(node, this.pluginsContext[name] || {}, + this.frontMatter, pluginConfig)); + return { preRenderNodeHooks, postRenderNodeHooks }; + } + /** * Collects file sources provided by plugins for the page for live reloading */ - collectPluginSources(content) { + collectPluginSources(content, pluginConfig) { const self = this; Object.entries(self.plugins) @@ -1058,7 +1075,7 @@ class Page { } const result = plugin.getSources(content, self.pluginsContext[pluginName] || {}, - self.frontMatter, self.getPluginConfig()); + self.frontMatter, pluginConfig); let pageContextSources; let domTagSourcesMap; @@ -1132,12 +1149,12 @@ class Page { /** * Entry point for plugin pre-render */ - preRender(content) { + preRender(content, pluginConfig) { let preRenderedContent = content; Object.entries(this.plugins).forEach(([pluginName, plugin]) => { if (plugin.preRender) { preRenderedContent = plugin.preRender(preRenderedContent, this.pluginsContext[pluginName] || {}, - this.frontMatter, this.getPluginConfig()); + this.frontMatter, pluginConfig); } }); return preRenderedContent; @@ -1146,12 +1163,12 @@ class Page { /** * Entry point for plugin post-render */ - postRender(content) { + postRender(content, pluginConfig) { let postRenderedContent = content; Object.entries(this.plugins).forEach(([pluginName, plugin]) => { if (plugin.postRender) { postRenderedContent = plugin.postRender(postRenderedContent, this.pluginsContext[pluginName] || {}, - this.frontMatter, this.getPluginConfig()); + this.frontMatter, pluginConfig); } }); return postRenderedContent; @@ -1253,12 +1270,19 @@ class Page { return resolve(); } builtFiles.add(resultPath); + + const pluginConfig = this.getPluginConfig({ + sourcePath: dependency.asIfTo, + resultPath, + }); + /* * We create a local instance of Markbind for an empty dynamicIncludeSrc * so that we only recursively rebuild the file's included content */ const markbinder = new MarkBind({ variablePreprocessor: this.variablePreprocessor, + ...this.preparePluginHooks(pluginConfig), }); return fs.readFileAsync(dependency.to, 'utf-8') .then(result => markbinder.includeFile(dependency.to, result, { @@ -1267,8 +1291,8 @@ class Page { cwf: file, })) .then(result => Page.removeFrontMatter(result)) - .then(result => this.collectPluginSources(result)) - .then(result => this.preRender(result)) + .then(result => this.collectPluginSources(result, pluginConfig)) + .then(result => this.preRender(result, pluginConfig)) .then(result => markbinder.resolveBaseUrl(result, { baseUrlMap: this.baseUrlMap, rootPath: this.rootPath, @@ -1278,7 +1302,7 @@ class Page { rootPath: this.rootPath, headerIdMap: {}, })) - .then(result => this.postRender(result)) + .then(result => this.postRender(result, pluginConfig)) .then(result => this.collectPluginsAssets(result)) .then(result => markbinder.processDynamicResources(file, result)) .then(result => MarkBind.unwrapIncludeSrc(result)) diff --git a/src/Site.js b/src/Site.js index c39a87dcf2..4edf614819 100644 --- a/src/Site.js +++ b/src/Site.js @@ -262,7 +262,7 @@ class Site { disableHtmlBeautify: this.siteConfig.disableHtmlBeautify, globalOverride: this.siteConfig.globalOverride, pageTemplate: this.pageTemplate, - plugins: this.plugins || {}, + plugins: this.plugins, rootPath: this.rootPath, enableSearch: this.siteConfig.enableSearch, searchable: this.siteConfig.enableSearch && config.searchable, @@ -270,7 +270,7 @@ class Site { layoutsAssetPath: path.relative(path.dirname(resultPath), path.join(this.siteAssetsDestPath, LAYOUT_SITE_FOLDER_NAME)), layout: config.layout, - title: config.title || '', + title: config.title, titlePrefix: this.siteConfig.titlePrefix, headingIndexingLevel: this.siteConfig.headingIndexingLevel, variablePreprocessor: this.variablePreprocessor, diff --git a/src/lib/markbind/src/parser.js b/src/lib/markbind/src/parser.js index 30a83b3303..97edc0c164 100644 --- a/src/lib/markbind/src/parser.js +++ b/src/lib/markbind/src/parser.js @@ -29,11 +29,11 @@ const { } = require('./constants'); class Parser { - constructor(config) { - this.variablePreprocessor = config.variablePreprocessor; - this.dynamicIncludeSrc = []; - this.staticIncludeSrc = []; - this.missingIncludeSrc = []; + constructor(options) { + this.variablePreprocessor = options.variablePreprocessor; + this.preRenderNodeHooks = options.preRenderNodeHooks || []; + this.postRenderNodeHooks = options.postRenderNodeHooks || []; + this.resetIncludeSrces(); } getDynamicIncludeSrc() { @@ -48,6 +48,12 @@ class Parser { return _.clone(this.missingIncludeSrc); } + resetIncludeSrces() { + this.dynamicIncludeSrc = []; + this.staticIncludeSrc = []; + this.missingIncludeSrc = []; + } + processDynamicResources(context, html) { const $ = cheerio.load(html, { xmlMode: false, @@ -177,14 +183,12 @@ class Parser { }); } - componentParser.postParseComponents(node); - // If a fixed header is applied to this page, generate dummy spans as anchor points if (config.fixedHeader && isHeadingTag && node.attribs.id) { cheerio(node).append(cheerio.parseHTML(``)); } - return node; + return componentParser.postParseComponents(node, this.postRenderNodeHooks); } _trimNodes(node) { diff --git a/src/lib/markbind/src/parsers/componentParser.js b/src/lib/markbind/src/parsers/componentParser.js index e4665b59f3..7e03f65f68 100644 --- a/src/lib/markbind/src/parsers/componentParser.js +++ b/src/lib/markbind/src/parsers/componentParser.js @@ -4,6 +4,7 @@ const _ = {}; _.has = require('lodash/has'); const md = require('../lib/markdown-it'); +const utils = require('../utils'); const logger = require('../../../../util/logger'); cheerio.prototype.options.xmlMode = true; // Enable xml mode for self-closing tag @@ -471,17 +472,26 @@ function parseComponents(node) { } } -function postParseComponents(node) { +function postParseComponents(node, postRenderNodeHooks) { try { - switch (node.name) { + let element = node; + + switch (element.name) { case 'panel': - _assignPanelId(node); + _assignPanelId(element); break; default: break; } + + postRenderNodeHooks.forEach((hook) => { + element = hook(element); + }); + + return element; } catch (error) { logger.error(error); + return utils.createErrorNode(node, error); } } diff --git a/src/lib/markbind/src/preprocessors/componentPreprocessor.js b/src/lib/markbind/src/preprocessors/componentPreprocessor.js index a4b7a749fc..47fdfb9b33 100644 --- a/src/lib/markbind/src/preprocessors/componentPreprocessor.js +++ b/src/lib/markbind/src/preprocessors/componentPreprocessor.js @@ -343,19 +343,21 @@ function _preprocessBody(node) { function preProcessComponent(node, context, config, parser) { - const element = node; + let element = node; _preProcessAllComponents(element, context); switch (element.name) { case 'panel': - return _preProcessPanel(element, context, config, parser); + element = _preProcessPanel(element, context, config, parser); + break; case 'variable': return _preprocessVariables(); case 'import': return _preprocessImports(node, parser); case 'include': - return _preprocessInclude(element, context, config, parser); + element = _preprocessInclude(element, context, config, parser); + break; case 'body': _preprocessBody(element); // eslint-disable-next-line no-fallthrough @@ -364,8 +366,13 @@ function preProcessComponent(node, context, config, parser) { if (element.children && element.children.length > 0) { element.children = element.children.map(e => preProcessComponent(e, context, config, parser)); } - return element; } + + parser.preRenderNodeHooks.forEach((hook) => { + element = hook(element); + }); + + return element; } diff --git a/test/unit/parsers/componentParser.test.js b/test/unit/parsers/componentParser.test.js index debeba9417..e3d5ef2cff 100644 --- a/test/unit/parsers/componentParser.test.js +++ b/test/unit/parsers/componentParser.test.js @@ -16,7 +16,7 @@ const parseAndVerifyTemplate = (template, expectedTemplate, postParse = false) = expect(error).toBeFalsy(); if (postParse) { - dom.forEach(node => componentParser.postParseComponents(node)); + dom.forEach(node => componentParser.postParseComponents(node, [])); } else { dom.forEach(node => componentParser.parseComponents(node)); } From 8f2fe1142ab977cd6ae7023fd910a98872fd6fc6 Mon Sep 17 00:00:00 2001 From: Ze Yu Date: Thu, 28 May 2020 03:18:49 +0800 Subject: [PATCH 2/5] Rewrite anchor plugin to use new plugin hooks --- .../default/markbind-plugin-anchors.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/plugins/default/markbind-plugin-anchors.js b/src/plugins/default/markbind-plugin-anchors.js index 6dcdc900d5..ee3a34201e 100644 --- a/src/plugins/default/markbind-plugin-anchors.js +++ b/src/plugins/default/markbind-plugin-anchors.js @@ -2,19 +2,20 @@ const cheerio = module.parent.require('cheerio'); const CSS_FILE_NAME = 'markbind-plugin-anchors.css'; +const HEADING_PATTERN = new RegExp('^h[1-6]$'); + /** * Adds anchor links to headers */ module.exports = { getLinks: (content, pluginContext, frontMatter, utils) => [utils.buildStylesheet(CSS_FILE_NAME)], - postRender: (content) => { - const $ = cheerio.load(content, { xmlMode: false }); - $(':header').each((i, heading) => { - if ($(heading).attr('id')) { - $(heading).append( - ``); - } - }); - return $.html(); + postRenderNode: (node) => { + if (HEADING_PATTERN.test(node.name) && node.attribs.id) { + const anchorElement = cheerio.parseHTML( + ``, + ); + node.children = [...node.children, ...anchorElement]; + } + return node; }, }; From e9d3fe5b0008388eaf3f82270f1a6f81d831da16 Mon Sep 17 00:00:00 2001 From: Ze Yu Date: Wed, 10 Jun 2020 18:50:46 +0800 Subject: [PATCH 3/5] Rewrite puml plugin to use preRenderNode hook --- src/lib/markbind/src/utils/index.js | 4 +- .../default/markbind-plugin-plantuml.js | 37 +++++++------------ src/util/pluginUtil.js | 16 +++++--- 3 files changed, 25 insertions(+), 32 deletions(-) diff --git a/src/lib/markbind/src/utils/index.js b/src/lib/markbind/src/utils/index.js index b46bebf05f..c166e5f341 100644 --- a/src/lib/markbind/src/utils/index.js +++ b/src/lib/markbind/src/utils/index.js @@ -87,7 +87,7 @@ module.exports = { getTextContent(element) { const elements = element.children; if (!elements || !elements.length) { - return undefined; + return ''; } const elementStack = elements.slice(); @@ -103,6 +103,6 @@ module.exports = { } } - return text.join('').trim(); + return text.join(''); }, }; diff --git a/src/plugins/default/markbind-plugin-plantuml.js b/src/plugins/default/markbind-plugin-plantuml.js index 4862a991e1..281db8cffa 100644 --- a/src/plugins/default/markbind-plugin-plantuml.js +++ b/src/plugins/default/markbind-plugin-plantuml.js @@ -3,7 +3,6 @@ * Replaces tags with tags with the appropriate src attribute and generates the diagrams * by running the JAR executable */ -const cheerio = module.parent.require('cheerio'); const fs = require('fs'); const path = require('path'); @@ -17,9 +16,6 @@ const { ERR_PROCESSING, } = require('../../constants'); -// Tracks diagrams that have already been processed -const processedDiagrams = new Set(); - /** * Generates diagram and returns the file name of the diagram * @param fileName name of the file to be generated @@ -33,9 +29,6 @@ function generateDiagram(fileName, content, config) { const outputDir = path.join(path.dirname(resultPath), path.dirname(fileName)); // Path of the .puml file const outputFilePath = path.join(outputDir, path.basename(fileName)); - // Tracks built files to avoid accessing twice - if (processedDiagrams.has(outputFilePath)) { return fileName; } - processedDiagrams.add(outputFilePath); // Creates output dir if it doesn't exist if (!fs.existsSync(outputDir)) { @@ -76,24 +69,20 @@ function generateDiagram(fileName, content, config) { } module.exports = { - preRender: (content, pluginContext, frontmatter, config) => { - // Clear tags processed before for live reload - processedDiagrams.clear(); - // Processes all tags - const $ = cheerio.load(content, { xmlMode: true }); - $('puml').each((i, tag) => { - tag.name = 'pic'; - const { cwf } = tag.attribs; - const pumlContent = pluginUtil.getPluginContent($, tag, cwf); - - const filePath = pluginUtil.getFilePathForPlugin(tag.attribs, pumlContent); - - tag.attribs.src = generateDiagram(filePath, pumlContent, config); - tag.children = []; - }); - - return $.html(); + preRenderNode: (node, pluginContext, frontmatter, config) => { + if (node.name === 'puml') { + node.name = 'pic'; + const pumlContent = pluginUtil.getNodeSrcOrTextContent(node); + + const fileName = pluginUtil.getFileNameForPlugin(node.attribs, pumlContent); + + node.attribs.src = generateDiagram(fileName, pumlContent, config); + node.children = []; + } + + return node; }, + getSources: () => ({ tagMap: [['puml', 'src']], }), diff --git a/src/util/pluginUtil.js b/src/util/pluginUtil.js index e27e3f991e..8d8c12db0c 100644 --- a/src/util/pluginUtil.js +++ b/src/util/pluginUtil.js @@ -2,6 +2,7 @@ const cryptoJS = require('crypto-js'); const fs = require('fs-extra-promise'); const path = require('path'); const fsUtil = require('./fsUtil'); +const utils = require('../lib/markbind/src/utils'); const logger = require('./logger'); const { @@ -14,7 +15,7 @@ module.exports = { * Return based on given name if provided, or it will be based on src. * If both are not provided, a md5 generated hash is provided for consistency. */ - getFilePathForPlugin: (tagAttribs, content) => { + getFileNameForPlugin: (tagAttribs, content) => { if (tagAttribs.name !== undefined) { return `${fsUtil.removeExtension(tagAttribs.name)}.png`; } @@ -31,12 +32,15 @@ module.exports = { }, /** - * Returns the string content of the plugin. + * Returns the content of a node from a file pointed to by its 'src' attribute + * or the text it contains otherwise. + * This is only to be used during the preRender stage, where nodes have their 'cwf' + * attribute tagged to them. */ - getPluginContent: ($, element, cwf) => { - if (element.attribs.src !== undefined) { + getNodeSrcOrTextContent: (node) => { + if (node.attribs.src !== undefined) { // Path of the plugin content - const rawPath = path.resolve(path.dirname(cwf), element.attribs.src); + const rawPath = path.resolve(path.dirname(node.attribs.cwf), node.attribs.src); try { return fs.readFileSync(rawPath, 'utf8'); } catch (err) { @@ -46,6 +50,6 @@ module.exports = { } } - return $(element).text(); + return utils.getTextContent(node); }, }; From ec1598f54cfd0ed4a70348a150f4937cb5bd821a Mon Sep 17 00:00:00 2001 From: Ze Yu Date: Thu, 28 May 2020 16:02:43 +0800 Subject: [PATCH 4/5] Use preRenderNode in panel span heading plugin --- .../markbind-plugin-shorthandSyntax.js | 27 +++++++++---------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/src/plugins/default/markbind-plugin-shorthandSyntax.js b/src/plugins/default/markbind-plugin-shorthandSyntax.js index 4888bc3d7e..da3c5bc70b 100644 --- a/src/plugins/default/markbind-plugin-shorthandSyntax.js +++ b/src/plugins/default/markbind-plugin-shorthandSyntax.js @@ -1,22 +1,19 @@ -const cheerio = module.parent.require('cheerio'); - -// Convert panel headings: -function convertPanelHeadings($) { - $('panel>span[heading]').each((i, element) => { - $(element).attr('slot', 'header'); - $(element).addClass('card-title'); - $(element).removeAttr('heading'); - }); -} - /** * Converts shorthand syntax to proper Markbind syntax * @param content of the page */ module.exports = { - postRender: (content) => { - const $ = cheerio.load(content, { xmlMode: false }); - convertPanelHeadings($); - return $.html(); + preRenderNode: (node) => { + if (node.name === 'panel' && node.children) { + node.children.forEach((n) => { + if (n.name === 'span' && n.attribs.heading !== undefined) { + n.attribs.slot = 'header'; + n.attribs.class = n.attribs.class ? `${n.attribs.class} card-title` : 'card-title'; + delete n.attribs.heading; + } + }); + } + + return node; }, }; From 4183299299f2780b97e65cc60fe8647cf4a294ee Mon Sep 17 00:00:00 2001 From: Ze Yu Date: Wed, 10 Jun 2020 18:54:18 +0800 Subject: [PATCH 5/5] Use postRenderNode for copyCodeBlock plugin --- src/plugins/codeBlockCopyButtons.js | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/plugins/codeBlockCopyButtons.js b/src/plugins/codeBlockCopyButtons.js index 7c11b169d1..4173794835 100644 --- a/src/plugins/codeBlockCopyButtons.js +++ b/src/plugins/codeBlockCopyButtons.js @@ -9,13 +9,13 @@ const COPY_ICON = ' + const buttonHtml = `
${COPY_ICON} ${COPY_TO_CLIPBOARD}
`; - return html; + return cheerio.parseHTML(buttonHtml)[0]; } @@ -44,13 +44,10 @@ const copyCodeBlockScript = `