From d362fb825e40367a4b5761610a634cfd0ab55064 Mon Sep 17 00:00:00 2001 From: Ze Yu Date: Wed, 20 May 2020 23:00:13 +0800 Subject: [PATCH] Reorganise variable processing (part 2) --- src/Site.js | 4 +- src/lib/markbind/src/constants.js | 5 - src/lib/markbind/src/parser.js | 13 +- .../preprocessors/componentPreprocessor.js | 15 +- .../src/preprocessors/variablePreprocessor.js | 377 +++++++----------- test/functional/test_site/expected/index.html | 2 +- .../testImportVariables._include_.html | 2 +- .../expected/testImportVariables.html | 2 +- test/functional/test_site/index.md | 1 + 9 files changed, 172 insertions(+), 249 deletions(-) diff --git a/src/Site.js b/src/Site.js index a6b12cb9db..9869a5a3d7 100644 --- a/src/Site.js +++ b/src/Site.js @@ -688,7 +688,7 @@ class Site { const filePathArray = Array.isArray(filePaths) ? filePaths : [filePaths]; const uniquePaths = _.uniq(filePathArray); logger.info('Rebuilding affected source files'); - this.variablePreprocessor.resetVariables(); + this.variablePreprocessor.resetPageVariables(); return new Promise((resolve, reject) => { this.regenerateAffectedPages(uniquePaths) .then(() => fs.removeAsync(this.tempPath)) @@ -706,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`)); - this.variablePreprocessor.resetVariables(); + this.variablePreprocessor.resetPageVariables(); /* 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 36f63839c0..a6d014ba7c 100644 --- a/src/lib/markbind/src/parser.js +++ b/src/lib/markbind/src/parser.js @@ -245,21 +245,16 @@ 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); + this.variablePreprocessor.extractPageVariables(file, content); + 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 25b851f489..e643e085a4 100644 --- a/src/lib/markbind/src/preprocessors/componentPreprocessor.js +++ b/src/lib/markbind/src/preprocessors/componentPreprocessor.js @@ -256,9 +256,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); @@ -329,6 +327,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 @@ -355,8 +363,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 1fc6a6fed4..99480f0f04 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'); @@ -49,23 +47,18 @@ class VariablePreprocessor { */ /** - * Map of file paths to another object containing its variable names matched to values + * Map of file paths to another object containing its variable names matched to variables + * declared locally within the page. (...) * @type {Object>} */ - this.fileVariableMap = {}; + this.pageVariableMap = {}; /** - * Map of file paths to another object containing its - * variable aliases matched to the respective imported file + * Map of file paths to another object containing its variable names matched to variables + * declared via imports. () * @type {Object>} */ - this.fileVariableAliasesMap = {}; - - /** - * Set of files that were already processed - * @type {Set} - */ - this.processedFiles = new Set(); + this.pageImportedVariableMap = {}; } @@ -137,219 +130,159 @@ class VariablePreprocessor { */ - resetVariables() { - this.fileVariableMap = {}; - this.fileVariableAliasesMap = {}; - this.processedFiles.clear(); + resetPageVariables() { + this.pageVariableMap = {}; + this.pageImportedVariableMap = {}; } /** - * 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 pageVariableMap}. + * + * @param elem "dom node" as parsed by htmlparser2 + * @param elemHtml as outputted by $(elem).html() + * @param filePath that the tag is from + * @param includeVariables which is used during {@link renderIncludeFile} only, used for rendering + * variables additionally using s inside s. + * (see https://markbind.org/userGuide/reusingContents.html#specifying-variables-in-an-include) */ - 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; - }); - }); + addVariable(elem, elemHtml, filePath, includeVariables) { + 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); - return innerVariables; - } + try { + const jsonData = JSON.parse(rawData); + Object.entries(jsonData).forEach(([name, value]) => { + this.pageVariableMap[filePath][name] = this.renderPage(filePath, value, includeVariables); + }); + } 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; + } - /** - * 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; + this.pageVariableMap[filePath][variableName] = this.renderPage(filePath, elemHtml, includeVariables); } - 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 + * Subroutine for {@link extractPageVariables}. + * Processes an element with a 'from' attribute. + * {@link extractPageVariables} is recursively called if the imported file has never had its + * page variables extracted before. + * 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. + * + * @param elem "dom node" of the element as parsed by htmlparser2 + * @param filePath that the tag is from */ - 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; - } + addImportVariables(elem, filePath) { + // render the 'from' file path for the edge case that a variable is used inside it + const importedFilePath = this.renderPage(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; + } - /** - * 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); + if (!this.pageVariableMap[resolvedFilePath]) { + // recursively extract that page's variables first, if not done before + this.extractPageVariables(resolvedFilePath, fs.readFileSync(resolvedFilePath)); + } + const importedFileVariableMap = this.pageVariableMap[resolvedFilePath]; - const importedVariables = VariablePreprocessor.extractPseudoImportedVariables($); - const pageVariables = {}; - const userDefinedVariables = this.getContentParentSiteVariables(fileName); + const alias = elem.attribs.as; + if (alias) { + // import everything under the alias if there is one + this.pageImportedVariableMap[filePath][alias] = importedFileVariableMap; + } - const setPageVariableIfNotSetBefore = (variableName, rawVariableValue) => { - if (pageVariables[variableName]) { + // additionally, import the inline variables without an alias + Object.keys(elem.attribs).forEach((name) => { + if (name === 'from' || name === 'as') { 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())); + const isExistingAttribute = !!importedFileVariableMap[name]; + if (!isExistingAttribute) { + logger.warn(`Invalid inline attribute ${name} imported in ${filePath} from ${resolvedFilePath}\n`); + return; } - }); - - 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, + this.pageImportedVariableMap[filePath][name] = importedFileVariableMap[name]; }); - - 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 + * Extract page variables from a page and store it in {@link pageVariableMap}. + * These include all locally declared s + * and variables ed from other pages. + * @param filePath for error printing + * @param data to extract variables from + * @param includeVariables from the parent include, if any */ - extractInnerVariablesIfNotProcessed(content, contentFilePath, filePathToExtract) { - if (!this.processedFiles.has(filePathToExtract)) { - this.processedFiles.add(filePathToExtract); - return this.extractInnerVariables(content, contentFilePath); + extractPageVariables(filePath, data, includeVariables) { + if (this.pageVariableMap[filePath]) { + // extracted previously before already, don't do it again. + return; } - return []; + this.pageVariableMap[filePath] = {}; + this.pageImportedVariableMap[filePath] = {}; + + const $ = cheerio.load(data); + + // NOTE: Selecting both at once is important to respect variable/import declaration order + $('variable, import[from]').each((index, elem) => { + if (elem.name === 'variable') { + this.addVariable(elem, $(elem).html(), filePath, includeVariables); + } else { + this.addImportVariables(elem, filePath); + } + }); } /** * 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) { + 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. * @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') { @@ -357,15 +290,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; } @@ -377,11 +311,14 @@ class VariablePreprocessor { * @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); + const includeInlineVariables = VariablePreprocessor.extractIncludeInlineVariables(includeElement); + const includeChildVariables = VariablePreprocessor.extractIncludeChildElementVariables(includeElement); - return includedVariables; + return { + ...includeChildVariables, + ...includeInlineVariables, + ...parentVariables, + }; } @@ -399,31 +336,25 @@ class VariablePreprocessor { * @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) { + renderIncludeFile(filePath, node, context, asIfAt) { + // Extract page variables from the **included** file if not done previously + 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); - // 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 + 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 renderedContent = this.renderPage(asIfAt, fileContent, includeVariables); const childContext = _.cloneDeep(context); childContext.cwf = asIfAt; childContext.variables = includeVariables; return { - includedSrces, renderedContent, childContext, }; @@ -431,37 +362,29 @@ 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} + * 3. otherVariables1, which is currently only used for s inside s + * (see https://markbind.org/userGuide/reusingContents.html#specifying-variables-in-an-include) + * 4. site variables as defined in variables.md + * 5. otherVariables2, which is currently only used for the MAIN_CONTENT_BODY variable of + * expressive layouts * @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 otherVariables1 if any + * @param otherVariables2 if any */ - renderInnerVariables(content, contentFilePath, contentAsIfAtFilePath, additionalVariables = {}) { + renderPage(contentFilePath, content, otherVariables1 = {}, otherVariables2 = {}) { const userDefinedVariables = this.getContentParentSiteVariables(contentFilePath); - const innerVariables = this.getImportedVariableMap(contentAsIfAtFilePath); return njUtil.renderRaw(content, { + ...this.pageImportedVariableMap[contentFilePath], + ...this.pageVariableMap[contentFilePath], + ...otherVariables1, ...userDefinedVariables, - ...additionalVariables, - ...innerVariables, + ...otherVariables2, }); } } diff --git a/test/functional/test_site/expected/index.html b/test/functional/test_site/expected/index.html index 6002d95d8e..bec830c1b0 100644 --- a/test/functional/test_site/expected/index.html +++ b/test/functional/test_site/expected/index.html @@ -161,7 +161,7 @@

Testing Site-NavHTML and Markdown +

Page Variable with HTML and Markdown

Nested Page Variable

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 4e3f0e583f..fc107ebcf3 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**