From 7040ac8577d7b543fa97362eedafce2444aa5530 Mon Sep 17 00:00:00 2001 From: Ze Yu Date: Sat, 30 May 2020 16:54:59 +0800 Subject: [PATCH 1/2] Reorganise variable processing (part 1) Processing variables involves a lot of boilerplate code with minor variations. This decentralizes variable processing, decreasing code maintainability and readability. Let's embark on a two stage refactor to improve variable processing. In this first refactor, we create the variablePreprocessor abstraction, and completely migrate the use of userDefinedVariablesMap for site variables to variablePreprocessor, reducing boilerplate code relating to site variables. Additionally, we move all page variable processing related methods to variablePreprocessor from Parser, abstracting such methods into smaller units of functionality where possible, and enhancing in code documentation. In the second part, we will similarly refactor page variable processing methods to reduce more boilerplate, increasing code maintainability. --- src/Page.js | 46 +- src/Site.js | 49 +- src/lib/markbind/src/parser.js | 245 +-------- .../preprocessors/componentPreprocessor.js | 25 +- .../src/preprocessors/variablePreprocessor.js | 496 ++++++++++++++++++ test/unit/Site.test.js | 12 +- test/unit/parser.test.js | 55 +- 7 files changed, 588 insertions(+), 340 deletions(-) create mode 100644 src/lib/markbind/src/preprocessors/variablePreprocessor.js diff --git a/src/Page.js b/src/Page.js index c38cf5b2b8..a8275e5b09 100644 --- a/src/Page.js +++ b/src/Page.js @@ -80,7 +80,7 @@ class Page { * @property {string} pageTemplate template used for this page * @property {string} title * @property {string} titlePrefix https://markbind.org/userGuide/siteConfiguration.html#titleprefix - * @property {Object} userDefinedVariablesMap + * @property {VariablePreprocessor} variablePreprocessor * @property {string} sourcePath the source file for rendering this page * @property {string} tempPath the temp path for writing intermediate result * @property {string} resultPath the output path of this page @@ -170,9 +170,9 @@ class Page { */ this.titlePrefix = pageConfig.titlePrefix; /** - * @type {Object} + * @type {VariablePreprocessor} */ - this.userDefinedVariablesMap = pageConfig.userDefinedVariablesMap; + this.variablePreprocessor = pageConfig.variablePreprocessor; /** * The source file for rendering this page @@ -631,10 +631,9 @@ class Page { } // Set header file as an includedFile this.includedFiles.add(headerPath); - // Map variables - const parentSite = urlUtils.getParentSiteAbsolutePath(this.sourcePath, this.rootPath, this.baseUrlMap); - const userDefinedVariables = this.userDefinedVariablesMap[parentSite]; - return `${njUtil.renderRaw(headerContent, userDefinedVariables)}\n${pageData}`; + + const renderedHeader = this.variablePreprocessor.renderSiteVariables(this.sourcePath, headerContent); + return `${renderedHeader}\n${pageData}`; } /** @@ -661,10 +660,9 @@ class Page { const footerContent = fs.readFileSync(footerPath, 'utf8'); // Set footer file as an includedFile this.includedFiles.add(footerPath); - // Map variables - const parentSite = urlUtils.getParentSiteAbsolutePath(this.sourcePath, this.rootPath, this.baseUrlMap); - const userDefinedVariables = this.userDefinedVariablesMap[parentSite]; - return `${pageData}\n${njUtil.renderRaw(footerContent, userDefinedVariables)}`; + + const renderedFooter = this.variablePreprocessor.renderSiteVariables(this.sourcePath, footerContent); + return `${pageData}\n${renderedFooter}`; } /** @@ -695,10 +693,7 @@ class Page { } this.includedFiles.add(siteNavPath); - // Render variables - const parentSite = urlUtils.getParentSiteAbsolutePath(this.sourcePath, this.rootPath, this.baseUrlMap); - const userDefinedVariables = this.userDefinedVariablesMap[parentSite]; - const siteNavMappedData = njUtil.renderRaw(siteNavContent, userDefinedVariables); + const siteNavMappedData = this.variablePreprocessor.renderSiteVariables(this.sourcePath, siteNavContent); // Check navigation elements const $ = cheerio.load(siteNavMappedData); @@ -879,11 +874,9 @@ class Page { const headFileContent = fs.readFileSync(headFilePath, 'utf8'); // Set head file as an includedFile this.includedFiles.add(headFilePath); - // Map variables - const parentSite = urlUtils.getParentSiteAbsolutePath(this.sourcePath, this.rootPath, this.baseUrlMap); - const userDefinedVariables = this.userDefinedVariablesMap[parentSite]; - const headFileMappedData = njUtil.renderRaw(headFileContent, userDefinedVariables) - .trim(); + + const headFileMappedData = this.variablePreprocessor.renderSiteVariables(this.sourcePath, + headFileContent).trim(); // Split top and bottom contents const $ = cheerio.load(headFileMappedData, { xmlMode: false }); if ($('head-top').length) { @@ -941,7 +934,7 @@ class Page { * @typedef {Object} FileConfig * @property {Set} baseUrlMap the set of urls representing the sites' base directories * @property {string} rootPath - * @property {Object} userDefinedVariablesMap + * @property {VariablePreprocessor} variablePreprocessor * @property {Object} headerIdMap * @property {boolean} fixedHeader indicates whether the header of the page is fixed */ @@ -949,15 +942,15 @@ class Page { generate(builtFiles) { this.includedFiles = new Set([this.sourcePath]); this.headerIdMap = {}; // Reset for live reload - - const markbinder = new MarkBind(); + const markbinder = new MarkBind({ + variablePreprocessor: this.variablePreprocessor, + }); /** * @type {FileConfig} */ const fileConfig = { baseUrlMap: this.baseUrlMap, rootPath: this.rootPath, - userDefinedVariablesMap: this.userDefinedVariablesMap, headerIdMap: this.headerIdMap, fixedHeader: this.fixedHeader, }; @@ -1264,11 +1257,12 @@ class Page { * 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(); + const markbinder = new MarkBind({ + variablePreprocessor: this.variablePreprocessor, + }); return fs.readFileAsync(dependency.to, 'utf-8') .then(result => markbinder.includeFile(dependency.to, result, { baseUrlMap: this.baseUrlMap, - userDefinedVariablesMap: this.userDefinedVariablesMap, rootPath: this.rootPath, cwf: file, })) diff --git a/src/Site.js b/src/Site.js index f54dd9753b..f2d80903b5 100644 --- a/src/Site.js +++ b/src/Site.js @@ -6,7 +6,7 @@ const path = require('path'); const Promise = require('bluebird'); const ProgressBar = require('progress'); const walkSync = require('walk-sync'); -const MarkBind = require('./lib/markbind/src/parser'); +const VariablePreprocessor = require('./lib/markbind/src/preprocessors/variablePreprocessor'); const njUtil = require('./lib/markbind/src/utils/nunjuckUtils'); const injectHtmlParser2SpecialTags = require('./lib/markbind/src/patches/htmlparser2'); const injectMarkdownItSpecialTags = require( @@ -134,7 +134,9 @@ class Site { */ this.siteConfig = undefined; this.siteConfigPath = siteConfigPath; - this.userDefinedVariablesMap = {}; + + // Site wide variable preprocessor + this.variablePreprocessor = undefined; // Lazy reload properties this.onePagePath = onePagePath; @@ -271,7 +273,7 @@ class Site { title: config.title || '', titlePrefix: this.siteConfig.titlePrefix, headingIndexingLevel: this.siteConfig.headingIndexingLevel, - userDefinedVariablesMap: this.userDefinedVariablesMap, + variablePreprocessor: this.variablePreprocessor, sourcePath, resultPath, asset: { @@ -513,6 +515,7 @@ class Site { .map(x => path.resolve(this.rootPath, x)); this.baseUrlMap = new Set(candidates.map(candidate => path.dirname(candidate))); + this.variablePreprocessor = new VariablePreprocessor(this.rootPath, this.baseUrlMap); return Promise.resolve(); } @@ -523,12 +526,9 @@ class Site { collectUserDefinedVariablesMap() { // The key is the base directory of the site/subsites, // while the value is a mapping of user defined variables - this.userDefinedVariablesMap = {}; - const markbindVariable = { MarkBind: MARKBIND_LINK_HTML }; + this.variablePreprocessor.resetUserDefinedVariablesMap(); this.baseUrlMap.forEach((base) => { - const userDefinedVariables = {}; - Object.assign(userDefinedVariables, markbindVariable); const userDefinedVariablesPath = path.resolve(base, USER_VARIABLES_PATH); const userDefinedVariablesDir = path.dirname(userDefinedVariablesPath); let content; @@ -539,15 +539,17 @@ class Site { logger.warn(e.message); } - // This is to prevent the first nunjuck call from converting {{baseUrl}} to an empty string - // and let the baseUrl value be injected later. - userDefinedVariables.baseUrl = '{{baseUrl}}'; - this.userDefinedVariablesMap[base] = userDefinedVariables; + /* + This is to prevent the first nunjuck call from converting {{baseUrl}} to an empty string + and let the baseUrl value be injected later. + */ + this.variablePreprocessor.addUserDefinedVariable(base, 'baseUrl', '{{baseUrl}}'); + this.variablePreprocessor.addUserDefinedVariable(base, 'MarkBind', MARKBIND_LINK_HTML); const $ = cheerio.load(content); - $('variable,span').each(function () { - const name = $(this).attr('name') || $(this).attr('id'); - const variableSource = $(this).attr('from'); + $('variable,span').each((index, element) => { + const name = $(element).attr('name') || $(element).attr('id'); + const variableSource = $(element).attr('from'); if (variableSource !== undefined) { try { @@ -555,18 +557,13 @@ class Site { const jsonData = fs.readFileSync(variableFilePath); const varData = JSON.parse(jsonData); Object.entries(varData).forEach(([varName, varValue]) => { - // Process the content of the variable with nunjucks, in case it refers to other variables. - const variableValue = njUtil.renderRaw(varValue, userDefinedVariables, {}, false); - - userDefinedVariables[varName] = variableValue; + this.variablePreprocessor.renderAndAddUserDefinedVariable(base, varName, varValue); }); } catch (err) { logger.warn(`Error ${err.message}`); } } else { - // Process the content of the variable with nunjucks, in case it refers to other variables. - const html = njUtil.renderRaw($(this).html(), userDefinedVariables, {}, false); - userDefinedVariables[name] = html; + this.variablePreprocessor.renderAndAddUserDefinedVariable(base, name, $(element).html()); } }); }); @@ -691,7 +688,7 @@ class Site { const filePathArray = Array.isArray(filePaths) ? filePaths : [filePaths]; const uniquePaths = _.uniq(filePathArray); logger.info('Rebuilding affected source files'); - MarkBind.resetVariables(); + this.variablePreprocessor.resetVariables(); return new Promise((resolve, reject) => { this.regenerateAffectedPages(uniquePaths) .then(() => fs.removeAsync(this.tempPath)) @@ -709,7 +706,7 @@ class Site { const uniqueUrls = _.uniq(normalizedUrlArray); uniqueUrls.forEach(normalizedUrl => logger.info( `Building ${normalizedUrl} as some of its dependencies were changed since the last visit`)); - MarkBind.resetVariables(); + this.variablePreprocessor.resetVariables(); /* Lazy loading only builds the page being viewed, but the user may be quick enough @@ -729,7 +726,6 @@ class Site { } this.toRebuild.delete(normalizedUrl); - pageToRebuild.userDefinedVariablesMap = this.userDefinedVariablesMap; pageToRebuild.generate(new Set()) .then(() => { pageToRebuild.collectHeadingsAndKeywords(); @@ -1062,7 +1058,6 @@ class Site { } } - page.userDefinedVariablesMap = this.userDefinedVariablesMap; processingFiles.push(page.generate(builtFiles) .catch((err) => { logger.error(err); @@ -1281,9 +1276,7 @@ class Site { timeZoneName: 'short', }; const time = new Date().toLocaleTimeString(this.siteConfig.locale, options); - Object.keys(this.userDefinedVariablesMap).forEach((base) => { - this.userDefinedVariablesMap[base].timestamp = time; - }); + this.variablePreprocessor.addUserDefinedVariableForAllSites('timestamp', time); return Promise.resolve(); } } diff --git a/src/lib/markbind/src/parser.js b/src/lib/markbind/src/parser.js index 902185601a..c18c7fcf2c 100644 --- a/src/lib/markbind/src/parser.js +++ b/src/lib/markbind/src/parser.js @@ -1,5 +1,4 @@ const cheerio = require('cheerio'); -const fs = require('fs'); const htmlparser = require('htmlparser2'); require('./patches/htmlparser2'); const path = require('path'); const Promise = require('bluebird'); @@ -27,107 +26,16 @@ cheerio.prototype.options.decodeEntities = false; // Don't escape HTML entities const { ATTRIB_INCLUDE_PATH, ATTRIB_CWF, - IMPORTED_VARIABLE_PREFIX, } = require('./constants'); class Parser { - constructor(options) { - this._options = options || {}; - this._fileCache = {}; + constructor(config) { + this.variablePreprocessor = config.variablePreprocessor; this.dynamicIncludeSrc = []; this.staticIncludeSrc = []; this.missingIncludeSrc = []; } - /** - * Returns an object containing the imported variables for specified file - * @param file file name to get the imported variables for - */ - // eslint-disable-next-line class-methods-use-this - getImportedVariableMap(file) { - const innerVariables = {}; - Parser.FILE_ALIASES.get(file).forEach((actualPath, alias) => { - innerVariables[alias] = {}; - const variables = Parser.VARIABLE_LOOKUP.get(actualPath); - variables.forEach((value, name) => { - innerVariables[alias][name] = value; - }); - }); - return innerVariables; - } - - /** - * Extract page variables from a page - * @param filename for error printing - * @param data to extract variables from - * @param userDefinedVariables global variables - * @param includedVariables variables from parent include - */ - static extractPageVariables(fileName, data, userDefinedVariables, includedVariables) { - const fileDir = path.dirname(fileName); - const $ = cheerio.load(data); - const pageVariables = {}; - Parser.VARIABLE_LOOKUP.set(fileName, new Map()); - /** - * ed variables have not been processed yet, we replace such variables with itself first. - */ - const importedVariables = {}; - $('import[from]').each((index, element) => { - const variableNames = Object.keys(element.attribs).filter(name => name !== 'from' && name !== 'as'); - // If no namespace is provided, we use the smallest name as one... - const largestName = variableNames.sort()[0]; - // ... and prepend it with $__MARKBIND__ to reduce collisions. - const generatedAlias = IMPORTED_VARIABLE_PREFIX + largestName; - const hasAlias = _.hasIn(element.attribs, 'as'); - const alias = hasAlias ? element.attribs.as : generatedAlias; - importedVariables[alias] = new Proxy({}, { - get(obj, prop) { - return `{{${alias}.${prop}}}`; - }, - }); - variableNames.forEach((name) => { - importedVariables[name] = `{{${alias}.${name}}}`; - }); - }); - const setPageVariable = (variableName, rawVariableValue) => { - const otherVariables = { - ...importedVariables, - ...pageVariables, - ...userDefinedVariables, - ...includedVariables, - }; - const variableValue = njUtil.renderRaw(rawVariableValue, otherVariables, {}, false); - if (!pageVariables[variableName]) { - pageVariables[variableName] = variableValue; - Parser.VARIABLE_LOOKUP.get(fileName).set(variableName, variableValue); - } - }; - $('variable').each(function () { - const variableElement = $(this); - const variableName = variableElement.attr('name'); - const variableSource = $(this).attr('from'); - if (variableSource !== undefined) { - try { - const variableFilePath = path.resolve(fileDir, variableSource); - const jsonData = fs.readFileSync(variableFilePath); - const varData = JSON.parse(jsonData); - Object.entries(varData).forEach(([varName, varValue]) => { - setPageVariable(varName, varValue); - }); - } catch (err) { - logger.warn(`Error ${err.message}`); - } - } else { - if (!variableName) { - logger.warn(`Missing 'name' for variable in ${fileName}\n`); - return; - } - setPageVariable(variableName, md.renderInline(variableElement.html())); - } - }); - return { ...importedVariables, ...pageVariables }; - } - getDynamicIncludeSrc() { return _.clone(this.dynamicIncludeSrc); } @@ -140,82 +48,6 @@ class Parser { return _.clone(this.missingIncludeSrc); } - _renderIncludeFile(filePath, node, context, config, asIfAt = filePath) { - try { - this._fileCache[filePath] = this._fileCache[filePath] || fs.readFileSync(filePath, 'utf8'); - } catch (e) { - // Read file fail - const missingReferenceErrorMessage = `Missing reference in: ${node.attribs[ATTRIB_CWF]}`; - e.message += `\n${missingReferenceErrorMessage}`; - logger.error(e); - return utils.createErrorNode(node, e); - } - const fileContent = this._fileCache[filePath]; // cache the file contents to save some I/O - const parentSitePath = urlUtils.getParentSiteAbsolutePath(asIfAt, config.rootPath, config.baseUrlMap); - const userDefinedVariables = config.userDefinedVariablesMap[parentSitePath]; - // Extract included variables from the PARENT file - const includeVariables = Parser.extractIncludeVariables(node, context.variables); - // Extract page variables from the CHILD file - const pageVariables = Parser.extractPageVariables(asIfAt, fileContent, - userDefinedVariables, includeVariables); - const content = njUtil.renderRaw(fileContent, { - ...pageVariables, - ...includeVariables, - ...userDefinedVariables, - }, { - path: filePath, - }); - const childContext = _.cloneDeep(context); - childContext.cwf = asIfAt; - childContext.variables = includeVariables; - return { content, childContext, userDefinedVariables }; - } - - _extractInnerVariables(content, context, config) { - const { cwf } = context; - const $ = cheerio.load(content, { - xmlMode: false, - decodeEntities: false, - }); - const aliases = new Map(); - Parser.FILE_ALIASES.set(cwf, aliases); - $('import[from]').each((index, node) => { - const filePath = path.resolve(path.dirname(cwf), node.attribs.from); - const variableNames = Object.keys(node.attribs) - .filter(name => name !== 'from' && name !== 'as'); - // If no namespace is provided, we use the smallest name as one - const largestName = variableNames.sort()[0]; - // ... and prepend it with $__MARKBIND__ to reduce collisions. - const generatedAlias = IMPORTED_VARIABLE_PREFIX + largestName; - const alias = _.hasIn(node.attribs, 'as') - ? node.attribs.as - : generatedAlias; - aliases.set(alias, filePath); - this.staticIncludeSrc.push({ from: context.cwf, to: filePath }); - // Render inner file content - const { - content: renderedContent, - childContext, - userDefinedVariables, - } = this._renderIncludeFile(filePath, node, context, config); - this.extractInnerVariablesIfNotProcessed(renderedContent, childContext, config, filePath); - const innerVariables = this.getImportedVariableMap(filePath); - - Parser.VARIABLE_LOOKUP.get(filePath).forEach((value, variableName, map) => { - map.set(variableName, njUtil.renderRaw(value, { - ...userDefinedVariables, ...innerVariables, - })); - }); - }); - } - - extractInnerVariablesIfNotProcessed(content, childContext, config, filePathToExtract) { - if (!Parser.PROCESSED_INNER_VARIABLES.has(filePathToExtract)) { - Parser.PROCESSED_INNER_VARIABLES.add(filePathToExtract); - this._extractInnerVariables(content, childContext, config); - } - } - processDynamicResources(context, html) { const $ = cheerio.load(html, { xmlMode: false, @@ -377,6 +209,8 @@ class Parser { const context = {}; context.cwf = config.cwf || file; // current working file context.callStack = []; + // TODO make componentPreprocessor a class to avoid this + config.variablePreprocessor = this.variablePreprocessor; return new Promise((resolve, reject) => { const handler = new htmlparser.DomHandler((error, dom) => { if (error) { @@ -401,29 +235,21 @@ class Parser { decodeEntities: true, }); - const parentSitePath = urlUtils.getParentSiteAbsolutePath(file, config.rootPath, config.baseUrlMap); - const userDefinedVariables = config.userDefinedVariablesMap[parentSitePath]; - const pageVariables = Parser.extractPageVariables(file, content, userDefinedVariables, {}); - let fileContent = njUtil.renderRaw(content, { - ...pageVariables, - ...userDefinedVariables, - ...additionalVariables, - }); - this._extractInnerVariables(fileContent, context, config); - const innerVariables = this.getImportedVariableMap(context.cwf); - fileContent = njUtil.renderRaw(fileContent, { - ...userDefinedVariables, - ...additionalVariables, - ...innerVariables, - }); + const outerContentRendered = this.variablePreprocessor.renderOuterVariables(content, file, + additionalVariables); + this.variablePreprocessor.extractInnerVariables(outerContentRendered, context.cwf) + .forEach(src => this.staticIncludeSrc.push(src)); + const innerContentRendered = this.variablePreprocessor.renderInnerVariables(outerContentRendered, + file, context.cwf, + additionalVariables); const fileExt = utils.getExt(file); if (utils.isMarkdownFileExt(fileExt)) { context.source = 'md'; - parser.parseComplete(fileContent.toString()); + parser.parseComplete(innerContentRendered.toString()); } else if (fileExt === 'html') { context.source = 'html'; - parser.parseComplete(fileContent); + parser.parseComplete(innerContentRendered); } else { const error = new Error(`Unsupported File Extension: '${fileExt}'`); reject(error); @@ -545,54 +371,9 @@ class Parser { return node; } - static resetVariables() { - Parser.VARIABLE_LOOKUP.clear(); - Parser.FILE_ALIASES.clear(); - Parser.PROCESSED_INNER_VARIABLES.clear(); - } - static isText(node) { return node.type === 'text' || node.type === 'comment'; } - - /** - * Extract variables from an include element - * @param includeElement include element to extract variables from - * @param contextVariables variables defined by parent pages - */ - static extractIncludeVariables(includeElement, contextVariables) { - const includedVariables = { ...contextVariables }; - Object.keys(includeElement.attribs).forEach((attribute) => { - if (!attribute.startsWith('var-')) { - return; - } - const variableName = attribute.replace(/^var-/, ''); - if (!includedVariables[variableName]) { - includedVariables[variableName] = includeElement.attribs[attribute]; - } - }); - if (includeElement.children) { - includeElement.children.forEach((child) => { - if (child.name !== 'variable' && child.name !== 'span') { - return; - } - const variableName = child.attribs.name || child.attribs.id; - if (!variableName) { - logger.warn(`Missing reference in ${includeElement.attribs[ATTRIB_CWF]}\n` - + `Missing 'name' or 'id' in variable for ${includeElement.attribs.src} include.`); - return; - } - if (!includedVariables[variableName]) { - includedVariables[variableName] = cheerio.html(child.children); - } - }); - } - return includedVariables; - } } -Parser.VARIABLE_LOOKUP = new Map(); -Parser.FILE_ALIASES = new Map(); -Parser.PROCESSED_INNER_VARIABLES = new Set(); - module.exports = Parser; diff --git a/src/lib/markbind/src/preprocessors/componentPreprocessor.js b/src/lib/markbind/src/preprocessors/componentPreprocessor.js index b5debeb761..4d22aee9f2 100644 --- a/src/lib/markbind/src/preprocessors/componentPreprocessor.js +++ b/src/lib/markbind/src/preprocessors/componentPreprocessor.js @@ -7,7 +7,6 @@ const CyclicReferenceError = require('../handlers/cyclicReferenceError.js'); const utils = require('../utils'); const urlUtils = require('../utils/urls'); -const njUtil = require('../utils/nunjuckUtils'); const _ = {}; _.has = require('lodash/has'); @@ -241,16 +240,13 @@ function _preprocessInclude(node, context, config, parser) { const isIncludeSrcMd = _isHtmlIncludingMarkdown(element, context, filePath); - const { content, childContext, userDefinedVariables } - = parser._renderIncludeFile(actualFilePath, element, context, config, filePath); - childContext.source = isIncludeSrcMd ? 'md' : 'html'; - childContext.callStack.push(context.cwf); - parser.extractInnerVariablesIfNotProcessed(content, childContext, config, filePath); - - const innerVariables = parser.getImportedVariableMap(filePath); - const fileContent = njUtil.renderRaw(content, { - ...userDefinedVariables, ...innerVariables, - }); + const { variablePreprocessor } = config; + const { + renderedContent, + childContext, + includedSrces, + } = variablePreprocessor.renderIncludeFile(actualFilePath, element, context, filePath); + includedSrces.forEach(src => parser.staticIncludeSrc.push(src)); _deleteIncludeAttributes(element); @@ -261,7 +257,7 @@ function _preprocessInclude(node, context, config, parser) { if (hash) { // Keep scripts in the fileContent - const src = cheerio.parseHTML(fileContent, true); + const src = cheerio.parseHTML(renderedContent, true); const $ = cheerio.load(src); const hashContent = $(hash).html(); @@ -282,7 +278,7 @@ function _preprocessInclude(node, context, config, parser) { // optional includes of segments have now been handled, so delete the attribute if (isOptional) delete element.attribs.optional; } else { - actualContent = (fileContent && isTrim) ? fileContent.trim() : fileContent; + actualContent = (renderedContent && isTrim) ? renderedContent.trim() : renderedContent; } if (isIncludeSrcMd) { @@ -296,6 +292,9 @@ function _preprocessInclude(node, context, config, parser) { element.children = cheerio.parseHTML(childrenHtml, true); if (element.children && element.children.length > 0) { + childContext.source = isIncludeSrcMd ? 'md' : 'html'; + childContext.callStack.push(context.cwf); + if (childContext.callStack.length > CyclicReferenceError.MAX_RECURSIVE_DEPTH) { const error = new CyclicReferenceError(childContext.callStack); logger.error(error); diff --git a/src/lib/markbind/src/preprocessors/variablePreprocessor.js b/src/lib/markbind/src/preprocessors/variablePreprocessor.js new file mode 100644 index 0000000000..d946ef8245 --- /dev/null +++ b/src/lib/markbind/src/preprocessors/variablePreprocessor.js @@ -0,0 +1,496 @@ +const cheerio = require('cheerio'); +const fs = require('fs'); +const path = require('path'); + +const _ = {}; +_.clone = require('lodash/clone'); +_.cloneDeep = require('lodash/cloneDeep'); +_.has = require('lodash/has'); +_.isArray = require('lodash/isArray'); +_.isEmpty = require('lodash/isEmpty'); + +const md = require('../lib/markdown-it'); +const njUtil = require('../utils/nunjuckUtils'); +const urlUtils = require('../utils/urls'); +const logger = require('../../../../util/logger'); + +const { + ATTRIB_CWF, + IMPORTED_VARIABLE_PREFIX, +} = require('../constants'); + + +/** + * All variable extraction and rendering is done here. + * + * There are four broad categories of methods here + * 1. Site level variable extraction and storage methods + * These methods provide interfaces to store a particular (sub)site's variables, + * rendering them with the same (sub)site's previously extracted variables + * before doing so in {@link userDefinedVariablesMap}. + * + * 2. Site level variable rendering methods + * These methods use the processed variables in {@link userDefinedVariablesMap} + * to render provided string content that belongs to a provided content file path. + * Using the content file path, we are able to get the appropriate (sub)site's (that + * the content belongs to) variables and then render them with it. + * + * 3. Page level variable extraction and storage methods + * These methods are similar to (1), but extract variables from a page, + * declared by s or s. + * There are also some utility methods here facilitating extraction of s + * the are nested within an element, and inline variables ('var-xx') declared + * as attributes in the itself. + * + * 4. Combined page and site variable rendering methods + * These methods are similar to (2), but in addition to the site variables, they + * render the content with the page variables extracted from (3) as well. + */ +class VariablePreprocessor { + constructor(rootPath, baseUrlMap) { + /** + * Root site path + * @type {string} + */ + this.rootPath = rootPath; + + /** + * Set of sites' root paths, for resolving the provided file path in + * rendering methods to the appropriate (sub)site's root path. + * @type {Set} + */ + this.baseUrlMap = baseUrlMap; + + /** + * Map of sites' root paths to their variables + * @type {Object>} + */ + this.userDefinedVariablesMap = {}; + + /* + * Page level + */ + + /** + * Map of file paths to another object containing its variable names matched to values + * @type {Object>} + */ + this.fileVariableMap = {}; + + /** + * Map of file paths to another object containing its + * variable aliases matched to the respective imported file + * @type {Object>} + */ + this.fileVariableAliasesMap = {}; + + /** + * Set of files that were already processed + * @type {Set} + */ + this.processedFiles = new Set(); + } + + + /* + * -------------------------------------------------- + * Site level variable storage methods + */ + + + /** + * Adds a variable for a site with the specified name and value. + * @param site path of the site, as reflected by the Site instance's baseUrlMap + * @param name of the variable to add + * @param value of the variable + */ + addUserDefinedVariable(site, name, value) { + this.userDefinedVariablesMap[site] = this.userDefinedVariablesMap[site] || {}; + this.userDefinedVariablesMap[site][name] = value; + } + + /** + * Renders the variable in addition to adding it, unlike {@link addUserDefinedVariable}. + * This is to allow using previously declared site variables in site variables declared later on. + */ + renderAndAddUserDefinedVariable(site, name, value) { + const renderedVal = njUtil.renderRaw(value, this.userDefinedVariablesMap[site], {}, false); + this.addUserDefinedVariable(site, name, renderedVal); + } + + /** + * Version of {@link addUserDefinedVariable} that adds to all sites. + */ + addUserDefinedVariableForAllSites(name, value) { + Object.keys(this.userDefinedVariablesMap) + .forEach(base => this.addUserDefinedVariable(base, name, value)); + } + + resetUserDefinedVariablesMap() { + this.userDefinedVariablesMap = {}; + } + + /** + * Retrieves the appropriate **(sub)site** variables of the supplied file path. + * @param contentFilePath to look up. + * @return {*} The appropriate (closest upwards) site variables map + */ + getParentSiteVariables(contentFilePath) { + const parentSitePath = urlUtils.getParentSiteAbsolutePath(contentFilePath, this.rootPath, + this.baseUrlMap); + return this.userDefinedVariablesMap[parentSitePath]; + } + + /* + * -------------------------------------------------- + * Site level variable rendering methods + */ + + + /** + * Renders the supplied content using only the site variables belonging to the specified site of the file. + * @param contentFilePath of the specified content to render + * @param content string + */ + renderSiteVariables(contentFilePath, content) { + const parentSite = urlUtils.getParentSiteAbsolutePath(contentFilePath, this.rootPath, this.baseUrlMap); + return njUtil.renderRaw(content, this.userDefinedVariablesMap[parentSite]); + } + + + /* + * -------------------------------------------------- + * Page level variable storage methods + */ + + + resetVariables() { + this.fileVariableMap = {}; + this.fileVariableAliasesMap = {}; + this.processedFiles.clear(); + } + + /** + * Returns an object containing the ed variables for the specified file. + * For variables specified with aliases, we construct the sub object containing the + * names and values of the variables tied to the alias. + * @param file to get the imported variables for + */ + getImportedVariableMap(file) { + const innerVariables = {}; + const fileAliases = this.fileVariableAliasesMap[file]; + + Object.entries(fileAliases).forEach(([alias, actualPath]) => { + innerVariables[alias] = {}; + const variables = this.fileVariableMap[actualPath]; + + Object.entries(variables).forEach(([name, value]) => { + innerVariables[alias][name] = value; + }); + }); + + return innerVariables; + } + + /** + * Retrieves a semi-unique alias for the given array of variable names from an import element + * by prepending the smallest variable name with $__MARKBIND__. + * Uses the alias declared in the 'as' attribute if there is one. + * @param variableNames of the import element, not referenced from the 'as' attribute + * @param node of the import element + * @return {string} Alias for the import element + */ + static getAliasForImportVariables(variableNames, node) { + if (_.has(node.attribs, 'as')) { + return node.attribs.as; + } + const largestName = variableNames.sort()[0]; + return IMPORTED_VARIABLE_PREFIX + largestName; + } + + /** + * Extracts a pseudo imported variables for the supplied content. + * @param $ loaded cheerio object of the content + * @return {{}} pseudo imported variables of the content + */ + static extractPseudoImportedVariables($) { + const importedVariables = {}; + $('import[from]').each((index, element) => { + const variableNames = Object.keys(element.attribs).filter(name => name !== 'from' && name !== 'as'); + + // Add pseudo variables for the alias's variables + const alias = VariablePreprocessor.getAliasForImportVariables(variableNames, element); + importedVariables[alias] = new Proxy({}, { + get(obj, prop) { + return `{{${alias}.${prop}}}`; + }, + }); + + // Add pseudo variables for inline variables in the import element + variableNames.forEach((name) => { + importedVariables[name] = `{{${alias}.${name}}}`; + }); + }); + return importedVariables; + } + + /** + * Extract page variables from a page. + * @param fileName for error printing + * @param data to extract variables from + * @param includedVariables variables from parent include + */ + extractPageVariables(fileName, data, includedVariables) { + const fileDir = path.dirname(fileName); + const $ = cheerio.load(data); + + const importedVariables = VariablePreprocessor.extractPseudoImportedVariables($); + const pageVariables = {}; + const userDefinedVariables = this.getContentParentSiteVariables(fileName); + + const setPageVariableIfNotSetBefore = (variableName, rawVariableValue) => { + if (pageVariables[variableName]) { + return; + } + + pageVariables[variableName] = njUtil.renderRaw(rawVariableValue, { + ...importedVariables, + ...pageVariables, + ...userDefinedVariables, + ...includedVariables, + }, {}, false); + }; + + $('variable').each((index, node) => { + const variableSource = $(node).attr('from'); + if (variableSource !== undefined) { + try { + const variableFilePath = path.resolve(fileDir, variableSource); + const jsonData = fs.readFileSync(variableFilePath); + const varData = JSON.parse(jsonData); + Object.entries(varData).forEach(([varName, varValue]) => { + setPageVariableIfNotSetBefore(varName, varValue); + }); + } catch (err) { + logger.warn(`Error ${err.message}`); + } + } else { + const variableName = $(node).attr('name'); + if (!variableName) { + logger.warn(`Missing 'name' for variable in ${fileName}\n`); + return; + } + setPageVariableIfNotSetBefore(variableName, md.renderInline($(node).html())); + } + }); + + this.fileVariableMap[fileName] = pageVariables; + + return { ...importedVariables, ...pageVariables }; + } + + /** + * Extracts inner (nested) variables of the content, + * resolving the s with respect to the supplied contentFilePath. + * @param content to extract from + * @param contentFilePath of the content + * @return {[]} Array of files that were imported + */ + extractInnerVariables(content, contentFilePath) { + let staticIncludeSrcs = []; + const $ = cheerio.load(content, { + xmlMode: false, + decodeEntities: false, + }); + + const aliases = {}; + this.fileVariableAliasesMap[contentFilePath] = aliases; + + $('import[from]').each((index, node) => { + const filePath = path.resolve(path.dirname(contentFilePath), node.attribs.from); + staticIncludeSrcs.push({ from: contentFilePath, to: filePath }); + + const variableNames = Object.keys(node.attribs).filter(name => name !== 'from' && name !== 'as'); + const alias = VariablePreprocessor.getAliasForImportVariables(variableNames, node); + aliases[alias] = filePath; + + // Render inner file content and extract inner variables recursively + const { + content: renderedContent, + childContext, + } = this.renderIncludeFile(filePath, node, {}); + const userDefinedVariables = this.getContentParentSiteVariables(filePath); + staticIncludeSrcs = [ + ...staticIncludeSrcs, + ...this.extractInnerVariablesIfNotProcessed(renderedContent, childContext.cwf, filePath), + ]; + + // Re-render page variables with imported variables + const innerVariables = this.getImportedVariableMap(filePath); + const variables = this.fileVariableMap[filePath]; + Object.entries(variables).forEach(([variableName, value]) => { + variables[variableName] = njUtil.renderRaw(value, { + ...userDefinedVariables, ...innerVariables, + }); + }); + }); + + return staticIncludeSrcs; + } + + /** + * Extracts inner variables {@link extractInnerVariables} only if not processed before + */ + extractInnerVariablesIfNotProcessed(content, contentFilePath, filePathToExtract) { + if (!this.processedFiles.has(filePathToExtract)) { + this.processedFiles.add(filePathToExtract); + return this.extractInnerVariables(content, contentFilePath); + } + return []; + } + + /** + * Extracts variables specified as in include elements, + * adding them to the supplied variables if it does not already exist. + * @param includeElement to extract inline variables from + * @param variables to add the extracted variables to + */ + static extractIncludeInlineVariables(includeElement, variables) { + Object.keys(includeElement.attribs).forEach((attribute) => { + if (!attribute.startsWith('var-')) { + return; + } + const variableName = attribute.replace(/^var-/, ''); + if (!variables[variableName]) { + variables[variableName] = includeElement.attribs[attribute]; + } + }); + } + + /** + * Extracts variables specified as in include elements, + * adding them to the supplied variables if it does not already exist. + * @param includeElement to search child nodes for + * @param variables to add the extracted variables to + */ + static extractIncludeChildElementVariables(includeElement, variables) { + if (!(includeElement.children && includeElement.children.length)) { + return; + } + + includeElement.children.forEach((child) => { + if (child.name !== 'variable' && child.name !== 'span') { + return; + } + const variableName = child.attribs.name || child.attribs.id; + if (!variableName) { + // eslint-disable-next-line no-console + logger.warn(`Missing reference in ${includeElement.attribs[ATTRIB_CWF]}\n` + + `Missing 'name' or 'id' in variable for ${includeElement.attribs.src} include.`); + return; + } + if (!variables[variableName]) { + variables[variableName] = cheerio.html(child.children); + } + }); + } + + + /** + * Extract variables from an element. + * See https://markbind.org/userGuide/reusingContents.html#specifying-variables-in-an-include for usage. + * It is a subroutine for {@link renderIncludeFile} + * @param includeElement include element to extract variables from + * @param parentVariables defined by parent pages, which have greater priority + */ + static extractIncludeVariables(includeElement, parentVariables) { + const includedVariables = { ...parentVariables }; + VariablePreprocessor.extractIncludeInlineVariables(includeElement, includedVariables); + VariablePreprocessor.extractIncludeChildElementVariables(includeElement, includedVariables); + + return includedVariables; + } + + + /* + * -------------------------------------------------- + * Combined (both site and page variables) rendering methods + */ + + + /** + * Renders an include file with the supplied context, returning the rendered + * content and new context with respect to the child content. + * @param filePath of the included file source + * @param node of the include element + * @param context object containing the variables for the current context + * @param asIfAt where the included file should be rendered from + */ + renderIncludeFile(filePath, node, context, asIfAt = filePath) { + // Extract included variables from the include element, merging with the parent context variables + const includeVariables = VariablePreprocessor.extractIncludeVariables(node, context.variables); + + // Extract page variables from the **included** file + const fileContent = fs.readFileSync(filePath, 'utf8'); + const userDefinedVariables = this.getContentParentSiteVariables(asIfAt); + const pageVariables = this.extractPageVariables(asIfAt, fileContent, includeVariables); + + // Render the included content with all the variables + const content = njUtil.renderRaw(fileContent, { + ...pageVariables, + ...includeVariables, + ...userDefinedVariables, + }); + + const includedSrces = this.extractInnerVariablesIfNotProcessed(content, asIfAt, asIfAt); + const renderedContent = this.renderInnerVariables(content, asIfAt, asIfAt); + + const childContext = _.cloneDeep(context); + childContext.cwf = asIfAt; + childContext.variables = includeVariables; + + return { + includedSrces, + renderedContent, + childContext, + }; + } + + + /** + * Extracts the content page's variables, then renders it. + * @param content to render + * @param contentFilePath of the content to render + * @param additionalVariables if any + */ + renderOuterVariables(content, contentFilePath, additionalVariables = {}) { + const pageVariables = this.extractPageVariables(contentFilePath, content, {}); + const userDefinedVariables = this.getContentParentSiteVariables(contentFilePath); + + return njUtil.renderRaw(content, { + ...pageVariables, + ...userDefinedVariables, + ...additionalVariables, + }); + } + + /** + * Extracts the content's page's inner variables, then renders it. + * @param content to render + * @param contentFilePath of the content to render + * @param contentAsIfAtFilePath + * @param additionalVariables if any + */ + renderInnerVariables(content, contentFilePath, contentAsIfAtFilePath, additionalVariables = {}) { + const userDefinedVariables = this.getContentParentSiteVariables(contentFilePath); + const innerVariables = this.getImportedVariableMap(contentAsIfAtFilePath); + + return njUtil.renderRaw(content, { + ...userDefinedVariables, + ...additionalVariables, + ...innerVariables, + }); + } +} + +module.exports = VariablePreprocessor; diff --git a/test/unit/Site.test.js b/test/unit/Site.test.js index a6f8a4f74c..a41f4a4fa6 100644 --- a/test/unit/Site.test.js +++ b/test/unit/Site.test.js @@ -361,12 +361,12 @@ test('Site resolves variables referencing other variables', async () => { await site.collectBaseUrl(); await site.collectUserDefinedVariablesMap(); - const root = site.userDefinedVariablesMap[path.resolve('')]; + const root = site.variablePreprocessor.userDefinedVariablesMap[path.resolve('')]; // check all variables expect(root.level1).toEqual('variable'); expect(root.level2).toEqual('variable'); - const expectedTextSpan = 'Blue text'; + const expectedTextSpan = '<span style="color: blue">Blue text</span>'; expect(root.level3).toEqual(expectedTextSpan); expect(root.level4).toEqual(expectedTextSpan); }); @@ -391,10 +391,10 @@ test('Site read correct user defined variables', async () => { await site.collectBaseUrl(); await site.collectUserDefinedVariablesMap(); - const root = site.userDefinedVariablesMap[path.resolve('')]; - const sub = site.userDefinedVariablesMap[path.resolve('sub')]; - const subsub = site.userDefinedVariablesMap[path.resolve('sub/sub')]; - const othersub = site.userDefinedVariablesMap[path.resolve('otherSub/sub')]; + const root = site.variablePreprocessor.userDefinedVariablesMap[path.resolve('')]; + const sub = site.variablePreprocessor.userDefinedVariablesMap[path.resolve('sub')]; + const subsub = site.variablePreprocessor.userDefinedVariablesMap[path.resolve('sub/sub')]; + const othersub = site.variablePreprocessor.userDefinedVariablesMap[path.resolve('otherSub/sub')]; // check all baseUrls const baseUrl = '{{baseUrl}}'; diff --git a/test/unit/parser.test.js b/test/unit/parser.test.js index d03f261a47..dfdf38ca1e 100644 --- a/test/unit/parser.test.js +++ b/test/unit/parser.test.js @@ -1,6 +1,7 @@ const path = require('path'); const fs = require('fs'); -const MarkBind = require('../../src/lib/markbind/src/parser.js'); +const Parser = require('../../src/lib/markbind/src/parser.js'); +const VariablePreprocessor = require('../../src/lib/markbind/src/preprocessors/variablePreprocessor'); const { USER_VARIABLES_DEFAULT } = require('./utils/data'); jest.mock('fs'); @@ -9,11 +10,13 @@ jest.mock('../../src/util/logger'); afterEach(() => fs.vol.reset()); const ROOT_PATH = path.resolve(''); -const DEFAULT_USER_DEFINED_VARIABLES_MAP = {}; -DEFAULT_USER_DEFINED_VARIABLES_MAP[ROOT_PATH] = { - example: USER_VARIABLES_DEFAULT, - baseUrl: '{{baseUrl}}', -}; +function getNewDefaultVariablePreprocessor() { + const DEFAULT_VARIABLE_PREPROCESSOR = new VariablePreprocessor(ROOT_PATH, new Set([ROOT_PATH])); + DEFAULT_VARIABLE_PREPROCESSOR.addUserDefinedVariable(ROOT_PATH, 'example', USER_VARIABLES_DEFAULT); + DEFAULT_VARIABLE_PREPROCESSOR.addUserDefinedVariable(ROOT_PATH, 'baseUrl', '{{baseUrl}}'); + + return DEFAULT_VARIABLE_PREPROCESSOR; +} test('includeFile replaces with
', async () => { const indexPath = path.resolve('index.md'); @@ -36,11 +39,10 @@ test('includeFile replaces with
', async () => { fs.vol.fromJSON(json, ''); const baseUrlMap = new Set([ROOT_PATH]); - const markbinder = new MarkBind(); + const markbinder = new Parser({ variablePreprocessor: getNewDefaultVariablePreprocessor() }); const result = await markbinder.includeFile(indexPath, index, { baseUrlMap, rootPath: ROOT_PATH, - userDefinedVariablesMap: DEFAULT_USER_DEFINED_VARIABLES_MAP, }); const expected = [ @@ -76,11 +78,10 @@ test('includeFile replaces with
', async fs.vol.fromJSON(json, ''); const baseUrlMap = new Set([ROOT_PATH]); - const markbinder = new MarkBind(); + const markbinder = new Parser({ variablePreprocessor: getNewDefaultVariablePreprocessor() }); const result = await markbinder.includeFile(indexPath, index, { baseUrlMap, rootPath: ROOT_PATH, - userDefinedVariablesMap: DEFAULT_USER_DEFINED_VARIABLES_MAP, }); const expected = [ @@ -112,11 +113,10 @@ test('includeFile replaces with empty < fs.vol.fromJSON(json, ''); const baseUrlMap = new Set([ROOT_PATH]); - const markbinder = new MarkBind(); + const markbinder = new Parser({ variablePreprocessor: getNewDefaultVariablePreprocessor() }); const result = await markbinder.includeFile(indexPath, index, { baseUrlMap, rootPath: ROOT_PATH, - userDefinedVariablesMap: DEFAULT_USER_DEFINED_VARIABLES_MAP, }); const expected = [ @@ -151,11 +151,10 @@ test('includeFile replaces with
', async fs.vol.fromJSON(json, ''); const baseUrlMap = new Set([ROOT_PATH]); - const markbinder = new MarkBind(); + const markbinder = new Parser({ variablePreprocessor: getNewDefaultVariablePreprocessor() }); const result = await markbinder.includeFile(indexPath, index, { baseUrlMap, rootPath: ROOT_PATH, - userDefinedVariablesMap: DEFAULT_USER_DEFINED_VARIABLES_MAP, }); const expected = [ @@ -195,11 +194,10 @@ test('includeFile replaces with inline fs.vol.fromJSON(json, ''); const baseUrlMap = new Set([ROOT_PATH]); - const markbinder = new MarkBind(); + const markbinder = new Parser({ variablePreprocessor: getNewDefaultVariablePreprocessor() }); const result = await markbinder.includeFile(indexPath, index, { baseUrlMap, rootPath: ROOT_PATH, - userDefinedVariablesMap: DEFAULT_USER_DEFINED_VARIABLES_MAP, }); const expected = [ @@ -235,11 +233,10 @@ test('includeFile replaces with trimmed c fs.vol.fromJSON(json, ''); const baseUrlMap = new Set([ROOT_PATH]); - const markbinder = new MarkBind(); + const markbinder = new Parser({ variablePreprocessor: getNewDefaultVariablePreprocessor() }); const result = await markbinder.includeFile(indexPath, index, { baseUrlMap, rootPath: ROOT_PATH, - userDefinedVariablesMap: DEFAULT_USER_DEFINED_VARIABLES_MAP, }); const expected = [ @@ -278,15 +275,10 @@ test('includeFile replaces with error { - expect(e.message).toEqual(expectedErrorMessage); - }, - }); + const markbinder = new Parser({ variablePreprocessor: getNewDefaultVariablePreprocessor() }); const result = await markbinder.includeFile(indexPath, index, { baseUrlMap, rootPath: ROOT_PATH, - userDefinedVariablesMap: DEFAULT_USER_DEFINED_VARIABLES_MAP, }); const expected = [ @@ -321,11 +313,10 @@ test('includeFile replaces with
fs.vol.fromJSON(json, ''); const baseUrlMap = new Set([ROOT_PATH]); - const markbinder = new MarkBind(); + const markbinder = new Parser({ variablePreprocessor: getNewDefaultVariablePreprocessor() }); const result = await markbinder.includeFile(indexPath, index, { baseUrlMap, rootPath: ROOT_PATH, - userDefinedVariablesMap: DEFAULT_USER_DEFINED_VARIABLES_MAP, }); const expected = [ @@ -361,11 +352,10 @@ test('includeFile replaces with fs.vol.fromJSON(json, ''); const baseUrlMap = new Set([ROOT_PATH]); - const markbinder = new MarkBind(); + const markbinder = new Parser({ variablePreprocessor: getNewDefaultVariablePreprocessor() }); const result = await markbinder.includeFile(indexPath, index, { baseUrlMap, rootPath: ROOT_PATH, - userDefinedVariablesMap: DEFAULT_USER_DEFINED_VARIABLES_MAP, }); const expected = [ @@ -415,15 +405,10 @@ test('includeFile detects cyclic references for static cyclic includes', async ( `\t${indexPath}`, ].join('\n'); - const markbinder = new MarkBind({ - errorHandler: (e) => { - expect(e.message).toEqual(expectedErrorMessage); - }, - }); + const markbinder = new Parser({ variablePreprocessor: getNewDefaultVariablePreprocessor() }); const result = await markbinder.includeFile(indexPath, index, { baseUrlMap, rootPath: ROOT_PATH, - userDefinedVariablesMap: DEFAULT_USER_DEFINED_VARIABLES_MAP, }); const expected = `
${expectedErrorMessage}
`; @@ -432,7 +417,7 @@ test('includeFile detects cyclic references for static cyclic includes', async ( }); test('renderFile converts markdown headers to

', async () => { - const markbinder = new MarkBind(); + const markbinder = new Parser({ variablePreprocessor: getNewDefaultVariablePreprocessor() }); const rootPath = path.resolve(''); const headerIdMap = {}; const indexPath = path.resolve('index.md'); From d6a00b080d6c446c4b1b1fd067ddd42efd3d9781 Mon Sep 17 00:00:00 2001 From: Ze Yu Date: Wed, 20 May 2020 23:00:13 +0800 Subject: [PATCH 2/2] Reorganise variable processing (part 2) Processing page variables involves a two step process. The first step extracts and renders locally declared page variables in tags, using javascript proxies to force imported variables to re-render as themselves again, which decreases code clarity. The second step then extracts the imported variables, and stores such variables into an alias map, then re-rendering the page a second time. This process can be combined into a single pass to simplify the process greatly and remove the need for workarounds like proxies. This also increases rendering performance slightly, as only a single pass is made Moreover, variable tags in tags are erroneously recognised as page variables. Let's include a simple fix for this as well. --- src/Site.js | 2 - src/lib/markbind/src/constants.js | 5 - src/lib/markbind/src/parser.js | 12 +- .../preprocessors/componentPreprocessor.js | 15 +- .../src/preprocessors/variablePreprocessor.js | 434 ++++++++---------- test/functional/test_site/expected/index.html | 4 +- .../testImportVariables._include_.html | 2 +- .../expected/testImportVariables.html | 2 +- test/functional/test_site/index.md | 7 + 9 files changed, 207 insertions(+), 276 deletions(-) diff --git a/src/Site.js b/src/Site.js index f2d80903b5..c39a87dcf2 100644 --- a/src/Site.js +++ b/src/Site.js @@ -688,7 +688,6 @@ class Site { const filePathArray = Array.isArray(filePaths) ? filePaths : [filePaths]; const uniquePaths = _.uniq(filePathArray); logger.info('Rebuilding affected source files'); - this.variablePreprocessor.resetVariables(); return new Promise((resolve, reject) => { this.regenerateAffectedPages(uniquePaths) .then(() => fs.removeAsync(this.tempPath)) @@ -706,7 +705,6 @@ class Site { const uniqueUrls = _.uniq(normalizedUrlArray); uniqueUrls.forEach(normalizedUrl => logger.info( `Building ${normalizedUrl} as some of its dependencies were changed since the last visit`)); - this.variablePreprocessor.resetVariables(); /* Lazy loading only builds the page being viewed, but the user may be quick enough diff --git a/src/lib/markbind/src/constants.js b/src/lib/markbind/src/constants.js index 758777b634..3ff5b152fc 100644 --- a/src/lib/markbind/src/constants.js +++ b/src/lib/markbind/src/constants.js @@ -5,11 +5,6 @@ module.exports = { BOILERPLATE_FOLDER_NAME: '_markbind/boilerplates', - /* Imported global variables will be assigned a namespace. - * A prefix is appended to reduce clashes with other variables in the page. - */ - IMPORTED_VARIABLE_PREFIX: '$__MARKBIND__', - // src/lib/markbind/src/utils.js markdownFileExts: ['md', 'mbd', 'mbdf'], }; diff --git a/src/lib/markbind/src/parser.js b/src/lib/markbind/src/parser.js index c18c7fcf2c..30a83b3303 100644 --- a/src/lib/markbind/src/parser.js +++ b/src/lib/markbind/src/parser.js @@ -235,21 +235,15 @@ class Parser { decodeEntities: true, }); - const outerContentRendered = this.variablePreprocessor.renderOuterVariables(content, file, - additionalVariables); - this.variablePreprocessor.extractInnerVariables(outerContentRendered, context.cwf) - .forEach(src => this.staticIncludeSrc.push(src)); - const innerContentRendered = this.variablePreprocessor.renderInnerVariables(outerContentRendered, - file, context.cwf, - additionalVariables); + const renderedContent = this.variablePreprocessor.renderPage(file, content, additionalVariables); const fileExt = utils.getExt(file); if (utils.isMarkdownFileExt(fileExt)) { context.source = 'md'; - parser.parseComplete(innerContentRendered.toString()); + parser.parseComplete(renderedContent.toString()); } else if (fileExt === 'html') { context.source = 'html'; - parser.parseComplete(innerContentRendered); + parser.parseComplete(renderedContent); } else { const error = new Error(`Unsupported File Extension: '${fileExt}'`); reject(error); diff --git a/src/lib/markbind/src/preprocessors/componentPreprocessor.js b/src/lib/markbind/src/preprocessors/componentPreprocessor.js index 4d22aee9f2..a4b7a749fc 100644 --- a/src/lib/markbind/src/preprocessors/componentPreprocessor.js +++ b/src/lib/markbind/src/preprocessors/componentPreprocessor.js @@ -244,9 +244,7 @@ function _preprocessInclude(node, context, config, parser) { const { renderedContent, childContext, - includedSrces, } = variablePreprocessor.renderIncludeFile(actualFilePath, element, context, filePath); - includedSrces.forEach(src => parser.staticIncludeSrc.push(src)); _deleteIncludeAttributes(element); @@ -317,6 +315,16 @@ function _preprocessVariables() { return utils.createEmptyNode(); } +function _preprocessImports(node, parser) { + if (node.attribs.from) { + parser.staticIncludeSrc.push({ + from: node.attribs.cwf, + to: path.resolve(node.attribs.cwf, node.attribs.from), + }); + } + + return utils.createEmptyNode(); +} /* * Body @@ -343,8 +351,9 @@ function preProcessComponent(node, context, config, parser) { case 'panel': return _preProcessPanel(element, context, config, parser); case 'variable': - case 'import': return _preprocessVariables(); + case 'import': + return _preprocessImports(node, parser); case 'include': return _preprocessInclude(element, context, config, parser); case 'body': diff --git a/src/lib/markbind/src/preprocessors/variablePreprocessor.js b/src/lib/markbind/src/preprocessors/variablePreprocessor.js index d946ef8245..c260ddeb2f 100644 --- a/src/lib/markbind/src/preprocessors/variablePreprocessor.js +++ b/src/lib/markbind/src/preprocessors/variablePreprocessor.js @@ -9,14 +9,12 @@ _.has = require('lodash/has'); _.isArray = require('lodash/isArray'); _.isEmpty = require('lodash/isEmpty'); -const md = require('../lib/markdown-it'); const njUtil = require('../utils/nunjuckUtils'); const urlUtils = require('../utils/urls'); const logger = require('../../../../util/logger'); const { ATTRIB_CWF, - IMPORTED_VARIABLE_PREFIX, } = require('../constants'); @@ -66,29 +64,6 @@ class VariablePreprocessor { * @type {Object>} */ this.userDefinedVariablesMap = {}; - - /* - * Page level - */ - - /** - * Map of file paths to another object containing its variable names matched to values - * @type {Object>} - */ - this.fileVariableMap = {}; - - /** - * Map of file paths to another object containing its - * variable aliases matched to the respective imported file - * @type {Object>} - */ - this.fileVariableAliasesMap = {}; - - /** - * Set of files that were already processed - * @type {Set} - */ - this.processedFiles = new Set(); } @@ -148,13 +123,21 @@ class VariablePreprocessor { /** - * Renders the supplied content using only the site variables belonging to the specified site of the file. + * Renders content belonging to a specified file path with the appropriate site's variables, + * with an optional set of lower and higher priority variables than the site variables to be rendered. * @param contentFilePath of the specified content to render - * @param content string + * @param content string to render + * @param lowerPriorityVariables than the site variables, if any + * @param higherPriorityVariables than the site variables, if any */ - renderSiteVariables(contentFilePath, content) { - const parentSite = urlUtils.getParentSiteAbsolutePath(contentFilePath, this.rootPath, this.baseUrlMap); - return njUtil.renderRaw(content, this.userDefinedVariablesMap[parentSite]); + renderSiteVariables(contentFilePath, content, lowerPriorityVariables = {}, higherPriorityVariables = {}) { + const userDefinedVariables = this.getParentSiteVariables(contentFilePath); + + return njUtil.renderRaw(content, { + ...lowerPriorityVariables, + ...userDefinedVariables, + ...higherPriorityVariables, + }); } @@ -163,220 +146,170 @@ class VariablePreprocessor { * Page level variable storage methods */ - - resetVariables() { - this.fileVariableMap = {}; - this.fileVariableAliasesMap = {}; - this.processedFiles.clear(); - } - /** - * Returns an object containing the ed variables for the specified file. - * For variables specified with aliases, we construct the sub object containing the - * names and values of the variables tied to the alias. - * @param file to get the imported variables for + * Subroutine for {@link extractPageVariables}. + * Renders a variable declared via either a tag or json file + * and then adds it to {@link pageVariables}. + * + * @param pageVariables object to add the extracted page variables to + * @param elem "dom node" as parsed by htmlparser2 + * @param elemHtml as outputted by $(elem).html() + * @param filePath that the tag is from + * @param renderVariable callback to render the extracted variable with before storing */ - getImportedVariableMap(file) { - const innerVariables = {}; - const fileAliases = this.fileVariableAliasesMap[file]; - - Object.entries(fileAliases).forEach(([alias, actualPath]) => { - innerVariables[alias] = {}; - const variables = this.fileVariableMap[actualPath]; + static addVariable(pageVariables, elem, elemHtml, filePath, renderVariable) { + const variableSource = elem.attribs.from; + if (variableSource) { + const variableFilePath = path.resolve(path.dirname(filePath), variableSource); + if (!fs.existsSync(variableFilePath)) { + logger.error(`The file ${variableSource} specified in 'from' attribute for json variable in ${ + filePath} does not exist!\n`); + return; + } + const rawData = fs.readFileSync(variableFilePath); - Object.entries(variables).forEach(([name, value]) => { - innerVariables[alias][name] = value; - }); - }); + try { + const jsonData = JSON.parse(rawData); + Object.entries(jsonData).forEach(([name, value]) => { + pageVariables[name] = renderVariable(filePath, value); + }); + } catch (err) { + logger.warn(`Error in parsing json from ${variableFilePath}:\n${err.message}\n`); + } + } else { + const variableName = elem.attribs.name; + if (!variableName) { + logger.warn(`Missing 'name' for variable in ${filePath}\n`); + return; + } - return innerVariables; + pageVariables[variableName] = renderVariable(filePath, elemHtml); + } } /** - * Retrieves a semi-unique alias for the given array of variable names from an import element - * by prepending the smallest variable name with $__MARKBIND__. - * Uses the alias declared in the 'as' attribute if there is one. - * @param variableNames of the import element, not referenced from the 'as' attribute - * @param node of the import element - * @return {string} Alias for the import element + * Subroutine for {@link extractPageVariables}. + * Processes an element with a 'from' attribute. + * {@link extractPageVariables} is recursively called to extract the other page's variables in doing so. + * Then, all locally declared variables of the imported file are assigned under the alias (if any), + * and any inline variables specified in the element are also set in {@link pageImportedVariables}. + * + * @param pageImportedVariables object to add the extracted imported variables to + * @param elem "dom node" of the element as parsed by htmlparser2 + * @param filePath that the tag is from + * @param renderFrom callback to render the 'from' attribute with */ - static getAliasForImportVariables(variableNames, node) { - if (_.has(node.attribs, 'as')) { - return node.attribs.as; + addImportVariables(pageImportedVariables, elem, filePath, renderFrom) { + // render the 'from' file path for the edge case that a variable is used inside it + const importedFilePath = renderFrom(filePath, elem.attribs.from); + const resolvedFilePath = path.resolve(path.dirname(filePath), importedFilePath); + if (!fs.existsSync(resolvedFilePath)) { + logger.error(`The file ${importedFilePath} specified in 'from' attribute for import in ${ + filePath} does not exist!\n`); + return; } - const largestName = variableNames.sort()[0]; - return IMPORTED_VARIABLE_PREFIX + largestName; - } - /** - * Extracts a pseudo imported variables for the supplied content. - * @param $ loaded cheerio object of the content - * @return {{}} pseudo imported variables of the content - */ - static extractPseudoImportedVariables($) { - const importedVariables = {}; - $('import[from]').each((index, element) => { - const variableNames = Object.keys(element.attribs).filter(name => name !== 'from' && name !== 'as'); - - // Add pseudo variables for the alias's variables - const alias = VariablePreprocessor.getAliasForImportVariables(variableNames, element); - importedVariables[alias] = new Proxy({}, { - get(obj, prop) { - return `{{${alias}.${prop}}}`; - }, - }); - - // Add pseudo variables for inline variables in the import element - variableNames.forEach((name) => { - importedVariables[name] = `{{${alias}.${name}}}`; - }); + // recursively extract the imported page's variables first + const importedFileContent = fs.readFileSync(resolvedFilePath); + const { pageVariables: importedFilePageVariables } = this.extractPageVariables(resolvedFilePath, + importedFileContent); + + const alias = elem.attribs.as; + if (alias) { + // import everything under the alias if there is one + pageImportedVariables[alias] = importedFilePageVariables; + } + + // additionally, import the inline variables without an alias + Object.keys(elem.attribs).filter((attr) => { + const isReservedAttribute = attr === 'from' || attr === 'as'; + if (isReservedAttribute) { + return false; + } + + const isExistingAttribute = !!importedFilePageVariables[attr]; + if (!isExistingAttribute) { + logger.warn(`Invalid inline attribute ${attr} imported in ${filePath} from ${resolvedFilePath}\n`); + return false; + } + + return true; + }).forEach((name) => { + pageImportedVariables[name] = importedFilePageVariables[name]; }); - return importedVariables; } /** * Extract page variables from a page. - * @param fileName for error printing + * These include all locally declared s and variables ed from other pages. + * @param filePath for error printing * @param data to extract variables from - * @param includedVariables variables from parent include + * @param includeVariables from the parent include, if any, used during {@link renderIncludeFile} */ - extractPageVariables(fileName, data, includedVariables) { - const fileDir = path.dirname(fileName); - const $ = cheerio.load(data); - - const importedVariables = VariablePreprocessor.extractPseudoImportedVariables($); + extractPageVariables(filePath, data, includeVariables = {}) { const pageVariables = {}; - const userDefinedVariables = this.getContentParentSiteVariables(fileName); + const pageImportedVariables = {}; - const setPageVariableIfNotSetBefore = (variableName, rawVariableValue) => { - if (pageVariables[variableName]) { - return; - } + const $ = cheerio.load(data); - pageVariables[variableName] = njUtil.renderRaw(rawVariableValue, { - ...importedVariables, + /* + This is used to render extracted variables before storing. + Hence, variables can be used within other variables, subject to declaration order. + */ + const renderVariable = (contentFilePath, content) => { + const previousVariables = { + ...pageImportedVariables, ...pageVariables, - ...userDefinedVariables, - ...includedVariables, - }, {}, false); + ...includeVariables, + }; + + return this.renderSiteVariables(contentFilePath, content, previousVariables); }; - $('variable').each((index, node) => { - const variableSource = $(node).attr('from'); - if (variableSource !== undefined) { - try { - const variableFilePath = path.resolve(fileDir, variableSource); - const jsonData = fs.readFileSync(variableFilePath); - const varData = JSON.parse(jsonData); - Object.entries(varData).forEach(([varName, varValue]) => { - setPageVariableIfNotSetBefore(varName, varValue); - }); - } catch (err) { - logger.warn(`Error ${err.message}`); - } + // NOTE: Selecting both at once is important to respect variable/import declaration order + $('variable, import[from]').not('include > variable').each((index, elem) => { + if (elem.name === 'variable') { + VariablePreprocessor.addVariable(pageVariables, elem, $(elem).html(), filePath, renderVariable); } else { - const variableName = $(node).attr('name'); - if (!variableName) { - logger.warn(`Missing 'name' for variable in ${fileName}\n`); - return; - } - setPageVariableIfNotSetBefore(variableName, md.renderInline($(node).html())); + /* + NOTE: we pass renderVariable here as well but not for rendering ed variables again! + This is only for the edge case that variables are used in the 'from' attribute of + the which we must resolve first. + */ + this.addImportVariables(pageImportedVariables, elem, filePath, renderVariable); } }); - this.fileVariableMap[fileName] = pageVariables; - - return { ...importedVariables, ...pageVariables }; + return { pageImportedVariables, pageVariables }; } /** - * Extracts inner (nested) variables of the content, - * resolving the s with respect to the supplied contentFilePath. - * @param content to extract from - * @param contentFilePath of the content - * @return {[]} Array of files that were imported - */ - extractInnerVariables(content, contentFilePath) { - let staticIncludeSrcs = []; - const $ = cheerio.load(content, { - xmlMode: false, - decodeEntities: false, - }); - - const aliases = {}; - this.fileVariableAliasesMap[contentFilePath] = aliases; - - $('import[from]').each((index, node) => { - const filePath = path.resolve(path.dirname(contentFilePath), node.attribs.from); - staticIncludeSrcs.push({ from: contentFilePath, to: filePath }); - - const variableNames = Object.keys(node.attribs).filter(name => name !== 'from' && name !== 'as'); - const alias = VariablePreprocessor.getAliasForImportVariables(variableNames, node); - aliases[alias] = filePath; - - // Render inner file content and extract inner variables recursively - const { - content: renderedContent, - childContext, - } = this.renderIncludeFile(filePath, node, {}); - const userDefinedVariables = this.getContentParentSiteVariables(filePath); - staticIncludeSrcs = [ - ...staticIncludeSrcs, - ...this.extractInnerVariablesIfNotProcessed(renderedContent, childContext.cwf, filePath), - ]; - - // Re-render page variables with imported variables - const innerVariables = this.getImportedVariableMap(filePath); - const variables = this.fileVariableMap[filePath]; - Object.entries(variables).forEach(([variableName, value]) => { - variables[variableName] = njUtil.renderRaw(value, { - ...userDefinedVariables, ...innerVariables, - }); - }); - }); - - return staticIncludeSrcs; - } - - /** - * Extracts inner variables {@link extractInnerVariables} only if not processed before - */ - extractInnerVariablesIfNotProcessed(content, contentFilePath, filePathToExtract) { - if (!this.processedFiles.has(filePathToExtract)) { - this.processedFiles.add(filePathToExtract); - return this.extractInnerVariables(content, contentFilePath); - } - return []; - } - - /** - * Extracts variables specified as in include elements, - * adding them to the supplied variables if it does not already exist. + * Extracts variables specified as in include elements. * @param includeElement to extract inline variables from - * @param variables to add the extracted variables to */ - static extractIncludeInlineVariables(includeElement, variables) { + static extractIncludeInlineVariables(includeElement) { + const includeInlineVariables = {}; + Object.keys(includeElement.attribs).forEach((attribute) => { if (!attribute.startsWith('var-')) { return; } - const variableName = attribute.replace(/^var-/, ''); - if (!variables[variableName]) { - variables[variableName] = includeElement.attribs[attribute]; - } + const variableName = attribute.slice(4); + includeInlineVariables[variableName] = includeElement.attribs[attribute]; }); + + return includeInlineVariables; } /** - * Extracts variables specified as in include elements, - * adding them to the supplied variables if it does not already exist. + * Extracts variables specified as in include elements. * @param includeElement to search child nodes for - * @param variables to add the extracted variables to */ - static extractIncludeChildElementVariables(includeElement, variables) { + static extractIncludeChildElementVariables(includeElement) { if (!(includeElement.children && includeElement.children.length)) { - return; + return {}; } + const includeChildVariables = {}; includeElement.children.forEach((child) => { if (child.name !== 'variable' && child.name !== 'span') { @@ -384,15 +317,16 @@ class VariablePreprocessor { } const variableName = child.attribs.name || child.attribs.id; if (!variableName) { - // eslint-disable-next-line no-console - logger.warn(`Missing reference in ${includeElement.attribs[ATTRIB_CWF]}\n` - + `Missing 'name' or 'id' in variable for ${includeElement.attribs.src} include.`); + logger.warn(`Missing 'name' or 'id' in variable for ${includeElement.attribs.src}'s include in ${ + includeElement.attribs[ATTRIB_CWF]}.\n`); return; } - if (!variables[variableName]) { - variables[variableName] = cheerio.html(child.children); + if (!includeChildVariables[variableName]) { + includeChildVariables[variableName] = cheerio.html(child.children); } }); + + return includeChildVariables; } @@ -401,14 +335,15 @@ class VariablePreprocessor { * See https://markbind.org/userGuide/reusingContents.html#specifying-variables-in-an-include for usage. * It is a subroutine for {@link renderIncludeFile} * @param includeElement include element to extract variables from - * @param parentVariables defined by parent pages, which have greater priority */ - static extractIncludeVariables(includeElement, parentVariables) { - const includedVariables = { ...parentVariables }; - VariablePreprocessor.extractIncludeInlineVariables(includeElement, includedVariables); - VariablePreprocessor.extractIncludeChildElementVariables(includeElement, includedVariables); + static extractIncludeVariables(includeElement) { + const includeInlineVariables = VariablePreprocessor.extractIncludeInlineVariables(includeElement); + const includeChildVariables = VariablePreprocessor.extractIncludeChildElementVariables(includeElement); - return includedVariables; + return { + ...includeChildVariables, + ...includeInlineVariables, + }; } @@ -419,38 +354,40 @@ class VariablePreprocessor { /** - * Renders an include file with the supplied context, returning the rendered + * Renders an file with the supplied context, returning the rendered * content and new context with respect to the child content. * @param filePath of the included file source * @param node of the include element - * @param context object containing the variables for the current context + * @param context object containing the parent (if any) variables for the current context, + * which have greater priority than the extracted include variables of the current context. * @param asIfAt where the included file should be rendered from */ - renderIncludeFile(filePath, node, context, asIfAt = filePath) { + renderIncludeFile(filePath, node, context, asIfAt) { + const fileContent = fs.readFileSync(filePath, 'utf8'); + // Extract included variables from the include element, merging with the parent context variables - const includeVariables = VariablePreprocessor.extractIncludeVariables(node, context.variables); + const includeVariables = VariablePreprocessor.extractIncludeVariables(node); - // Extract page variables from the **included** file - const fileContent = fs.readFileSync(filePath, 'utf8'); - const userDefinedVariables = this.getContentParentSiteVariables(asIfAt); - const pageVariables = this.extractPageVariables(asIfAt, fileContent, includeVariables); + // We pass in includeVariables as well to render variables used in page s + // see "Test Page Variable and Included Variable Integrations" under test_site/index.md for an example + const { + pageImportedVariables, + pageVariables, + } = this.extractPageVariables(asIfAt, fileContent, includeVariables); // Render the included content with all the variables - const content = njUtil.renderRaw(fileContent, { + const renderedContent = this.renderSiteVariables(asIfAt, fileContent, { + ...pageImportedVariables, ...pageVariables, ...includeVariables, - ...userDefinedVariables, + ...context.variables, }); - const includedSrces = this.extractInnerVariablesIfNotProcessed(content, asIfAt, asIfAt); - const renderedContent = this.renderInnerVariables(content, asIfAt, asIfAt); - const childContext = _.cloneDeep(context); childContext.cwf = asIfAt; childContext.variables = includeVariables; return { - includedSrces, renderedContent, childContext, }; @@ -458,38 +395,27 @@ class VariablePreprocessor { /** - * Extracts the content page's variables, then renders it. - * @param content to render + * Renders content belonging to a page with the appropriate variables. + * In increasing order of priority (overriding), + * 1. ed variables as extracted during {@link extractPageVariables} + * 2. locally declared s as extracted during {@link extractPageVariables} + * (see https://markbind.org/userGuide/reusingContents.html#specifying-variables-in-an-include) + * 3. site variables as defined in variables.md * @param contentFilePath of the content to render - * @param additionalVariables if any - */ - renderOuterVariables(content, contentFilePath, additionalVariables = {}) { - const pageVariables = this.extractPageVariables(contentFilePath, content, {}); - const userDefinedVariables = this.getContentParentSiteVariables(contentFilePath); - - return njUtil.renderRaw(content, { - ...pageVariables, - ...userDefinedVariables, - ...additionalVariables, - }); - } - - /** - * Extracts the content's page's inner variables, then renders it. * @param content to render - * @param contentFilePath of the content to render - * @param contentAsIfAtFilePath - * @param additionalVariables if any + * @param highestPriorityVariables to render with the highest priority if any. + * This is currently only used for the MAIN_CONTENT_BODY in layouts. */ - renderInnerVariables(content, contentFilePath, contentAsIfAtFilePath, additionalVariables = {}) { - const userDefinedVariables = this.getContentParentSiteVariables(contentFilePath); - const innerVariables = this.getImportedVariableMap(contentAsIfAtFilePath); - - return njUtil.renderRaw(content, { - ...userDefinedVariables, - ...additionalVariables, - ...innerVariables, - }); + renderPage(contentFilePath, content, highestPriorityVariables = {}) { + const { + pageImportedVariables, + pageVariables, + } = this.extractPageVariables(contentFilePath, content); + + return this.renderSiteVariables(contentFilePath, content, { + ...pageImportedVariables, + ...pageVariables, + }, highestPriorityVariables); } } diff --git a/test/functional/test_site/expected/index.html b/test/functional/test_site/expected/index.html index bd053212eb..a73f3f905e 100644 --- a/test/functional/test_site/expected/index.html +++ b/test/functional/test_site/expected/index.html @@ -196,7 +196,7 @@

Testing Site-NavHTML and Markdown +

Page Variable with HTML and Markdown

Nested Page Variable

@@ -229,6 +229,8 @@

Testing Site-NavHeading with multiple keywords

keyword 1 keyword 2

diff --git a/test/functional/test_site/expected/testImportVariables._include_.html b/test/functional/test_site/expected/testImportVariables._include_.html index dec4289734..005d1dabdf 100644 --- a/test/functional/test_site/expected/testImportVariables._include_.html +++ b/test/functional/test_site/expected/testImportVariables._include_.html @@ -6,7 +6,7 @@

Test import variables that itself imports other variables:

Trying to access a page variable:

There should be something red below:

-
This is a page variable from variablestoimport.md
+
This is a page variable from variablestoimport.md
Something should have appeared above in red.

Trying to access an imported variable via namespace:

There should be something blue below:

diff --git a/test/functional/test_site/expected/testImportVariables.html b/test/functional/test_site/expected/testImportVariables.html index c0e21d5e9c..e530aa08c9 100644 --- a/test/functional/test_site/expected/testImportVariables.html +++ b/test/functional/test_site/expected/testImportVariables.html @@ -34,7 +34,7 @@

Test import variables that itself imports other variables:

Trying to access a page variable:

There should be something red below:

-
This is a page variable from variablestoimport.md
+
This is a page variable from variablestoimport.md
Something should have appeared above in red.

Trying to access an imported variable via namespace:

There should be something blue below:

diff --git a/test/functional/test_site/index.md b/test/functional/test_site/index.md index d07daac64c..8e85d4a7d5 100644 --- a/test/functional/test_site/index.md +++ b/test/functional/test_site/index.md @@ -40,6 +40,7 @@ tags: ["tag-frontmatter-shown", "tag-included-file", "+tag-exp*", "-tag-exp-hidd **Page Variable with HTML and MD** Page Variable with HTML and **Markdown** + {{ page_variable_with_HTML_and_MD }} **Nested Page Variable** @@ -75,6 +76,12 @@ tags: ["tag-frontmatter-shown", "tag-included-file", "+tag-exp*", "-tag-exp-hidd Included Variable Overriding Page Variable +Variables for includes should not be recognised as page variables, hence, there should be no text between **this** + +{{ included_variable_overriding_page_variable }} + +and **this**. + # Heading with multiple keywords keyword 1 keyword 2