From 711fc743b20cc0635b9aa6f6b1825cfb4d158412 Mon Sep 17 00:00:00 2001 From: Ze Yu Date: Sun, 6 Dec 2020 10:52:15 +0800 Subject: [PATCH 1/3] Remove old variable and import features --- packages/core/src/Page/index.js | 27 +-- packages/core/src/Site/index.js | 19 +- packages/core/src/html/NodePreprocessor.js | 3 - .../core/src/variables/VariableProcessor.js | 188 +----------------- 4 files changed, 20 insertions(+), 217 deletions(-) diff --git a/packages/core/src/Page/index.js b/packages/core/src/Page/index.js index a7288c05da..917dacb7cd 100644 --- a/packages/core/src/Page/index.js +++ b/packages/core/src/Page/index.js @@ -534,18 +534,20 @@ class Page { // Set expressive layout file as an includedFile this.includedFiles.add(layoutPagePath); + const { variableProcessor } = this.pageConfig; + return fs.readFile(layoutPagePath, 'utf8') /* Render {{ MAIN_CONTENT_BODY }} and {% raw/endraw %} back to itself first, - which is then dealt with in the call below to {@link renderSiteVariables}. + which is then dealt with in the call below to {@link renderWithSiteVariables}. */ - .then(result => this.pageConfig.variableProcessor.renderPage(layoutPagePath, result, pageSources, { + .then(result => variableProcessor.renderWithSiteVariables(layoutPagePath, result, pageSources, {}, { [LAYOUT_PAGE_BODY_VARIABLE]: `{{${LAYOUT_PAGE_BODY_VARIABLE}}}`, }, true)) // Include file with the cwf set to the layout page path .then(result => nodePreprocessor.includeFile(layoutPagePath, result)) // Note: The {% raw/endraw %}s previously kept are removed here. - .then(result => this.pageConfig.variableProcessor.renderSiteVariables( + .then(result => this.pageConfig.variableProcessor.renderWithSiteVariables( this.pageConfig.rootPath, result, pageSources, { [LAYOUT_PAGE_BODY_VARIABLE]: pageData, })); @@ -565,8 +567,8 @@ class Page { const headerContent = fs.readFileSync(this.header, 'utf8'); this.includedFiles.add(this.header); - const renderedHeader = this.pageConfig.variableProcessor.renderSiteVariables(this.pageConfig.sourcePath, - headerContent, pageSources); + const renderedHeader = this.pageConfig.variableProcessor.renderWithSiteVariables( + this.pageConfig.sourcePath, headerContent, pageSources); return `
${renderedHeader}
\n${pageData}`; } @@ -584,8 +586,8 @@ class Page { // Set footer file as an includedFile this.includedFiles.add(this.footer); - const renderedFooter = this.pageConfig.variableProcessor.renderSiteVariables(this.pageConfig.sourcePath, - footerContent, pageSources); + const renderedFooter = this.pageConfig.variableProcessor.renderWithSiteVariables( + this.pageConfig.sourcePath, footerContent, pageSources); return `
${renderedFooter}
\n${pageData}`; } @@ -608,7 +610,7 @@ class Page { } this.includedFiles.add(this.siteNav); - const siteNavMappedData = this.pageConfig.variableProcessor.renderSiteVariables( + const siteNavMappedData = this.pageConfig.variableProcessor.renderWithSiteVariables( this.pageConfig.sourcePath, siteNavContent, pageSources); // Check navigation elements @@ -796,7 +798,7 @@ class Page { // Set head file as an includedFile this.includedFiles.add(headFilePath); - const headFileMappedData = this.pageConfig.variableProcessor.renderSiteVariables( + const headFileMappedData = this.pageConfig.variableProcessor.renderWithSiteVariables( this.pageConfig.sourcePath, headFileContent, pageSources).trim(); // Split top and bottom contents const $ = cheerio.load(headFileMappedData); @@ -875,8 +877,8 @@ class Page { const nodeProcessor = new NodeProcessor(fileConfig); return fs.readFile(this.pageConfig.sourcePath, 'utf-8') - .then(result => this.pageConfig.variableProcessor.renderPage(this.pageConfig.sourcePath, - result, pageSources)) + .then(result => this.pageConfig.variableProcessor.renderWithSiteVariables(this.pageConfig.sourcePath, + result, pageSources)) .then(result => nodePreprocessor.includeFile(this.pageConfig.sourcePath, result)) .then((result) => { this.collectFrontMatter(result); @@ -1188,7 +1190,8 @@ class Page { const nodeProcessor = new NodeProcessor(fileConfig); return fs.readFile(dependency.to, 'utf-8') - .then(result => this.pageConfig.variableProcessor.renderPage(dependency.to, result, pageSources)) + .then(result => this.pageConfig.variableProcessor.renderWithSiteVariables(dependency.to, result, + pageSources)) .then(result => nodePreprocessor.includeFile(dependency.to, result, file)) .then(result => Page.removeFrontMatter(result)) .then(result => this.collectPluginSources(result)) diff --git a/packages/core/src/Site/index.js b/packages/core/src/Site/index.js index 82f255d77c..fb842826c3 100644 --- a/packages/core/src/Site/index.js +++ b/packages/core/src/Site/index.js @@ -557,7 +557,6 @@ class Site { this.baseUrlMap.forEach((base) => { const userDefinedVariablesPath = path.resolve(base, USER_VARIABLES_PATH); - const userDefinedVariablesDir = path.dirname(userDefinedVariablesPath); let content; try { content = fs.readFileSync(userDefinedVariablesPath, 'utf8'); @@ -580,22 +579,8 @@ class Site { const $ = cheerio.load(content, { decodeEntities: false }); $('variable,span').each((index, element) => { const name = $(element).attr('name') || $(element).attr('id'); - const variableSource = $(element).attr('from'); - - if (variableSource !== undefined) { - try { - const variableFilePath = path.resolve(userDefinedVariablesDir, variableSource); - const jsonData = fs.readFileSync(variableFilePath); - const varData = JSON.parse(jsonData); - Object.entries(varData).forEach(([varName, varValue]) => { - this.variableProcessor.renderAndAddUserDefinedVariable(base, varName, varValue); - }); - } catch (err) { - logger.warn(`Error ${err.message}`); - } - } else { - this.variableProcessor.renderAndAddUserDefinedVariable(base, name, $(element).html()); - } + + this.variableProcessor.renderAndAddUserDefinedVariable(base, name, $(element).html()); }); }); } diff --git a/packages/core/src/html/NodePreprocessor.js b/packages/core/src/html/NodePreprocessor.js index 721cb7e201..7338e0aab6 100644 --- a/packages/core/src/html/NodePreprocessor.js +++ b/packages/core/src/html/NodePreprocessor.js @@ -343,9 +343,6 @@ class NodePreprocessor { switch (element.name) { case 'panel': return this._preProcessPanel(element, context); - case 'variable': - case 'import': - return utils.createEmptyNode(); case 'include': return this._preProcessInclude(element, context); case 'body': diff --git a/packages/core/src/variables/VariableProcessor.js b/packages/core/src/variables/VariableProcessor.js index ae21871cef..65f7e951b6 100644 --- a/packages/core/src/variables/VariableProcessor.js +++ b/packages/core/src/variables/VariableProcessor.js @@ -1,6 +1,5 @@ const cheerio = require('cheerio'); const fs = require('fs'); -const path = require('path'); const _ = {}; _.clone = require('lodash/clone'); @@ -154,8 +153,8 @@ class VariableProcessor { * Currently only used for layouts. @param keepPercentRaw whether to reoutput {% raw/endraw %} tags, also used only for layouts. */ - renderSiteVariables(contentFilePath, content, pageSources, lowerPriorityVariables = {}, - higherPriorityVariables = {}, keepPercentRaw = false) { + renderWithSiteVariables(contentFilePath, content, pageSources, lowerPriorityVariables = {}, + higherPriorityVariables = {}, keepPercentRaw = false) { const userDefinedVariables = this.getParentSiteVariables(contentFilePath); const parentSitePath = urlUtils.getParentSiteAbsolutePath(contentFilePath, this.rootPath, this.baseUrlMap); @@ -172,151 +171,6 @@ class VariableProcessor { * Page level variable storage methods */ - /** - * 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 - */ - 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); - - 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; - } - - pageVariables[variableName] = renderVariable(filePath, elemHtml); - } - } - - /** - * 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 {PageSources} pageSources instance to add sources from rendering imported variables - * @param filePath that the tag is from - * @param renderFrom callback to render the 'from' attribute with - */ - addImportVariables(pageImportedVariables, elem, pageSources, 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; - } - - // recursively extract the imported page's variables first - const importedFileContent = fs.readFileSync(resolvedFilePath); - pageSources.staticIncludeSrc.push({ to: resolvedFilePath }); - const { - pageVariables: importedFilePageVariables, - } = this.extractPageVariables(resolvedFilePath, importedFileContent, pageSources); - - 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]; - }); - } - - /** - * Extract page variables from a page. - * These include all locally declared s and variables ed from other pages. - * @param filePath for error printing - * @param data to extract variables from - * @param {PageSources} pageSources to add sources found when rendering extracted variables to - * @param includeVariables from the parent include, if any, used during {@link renderIncludeFile} - */ - extractPageVariables(filePath, data, pageSources, includeVariables = {}) { - const pageVariables = {}; - const pageImportedVariables = {}; - - const $ = cheerio.load(data); - - /* - 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, - ...includeVariables, - }; - - return this.renderSiteVariables(contentFilePath, content, pageSources, previousVariables); - }; - - // NOTE: Selecting both at once is important to respect variable/import declaration order - $('variable, import[from]').not('include > variable').each((index, elem) => { - logger.warn(' and tags used in pages will be deprecated in v3.0.\n' - + 'Use nunjucks\' {% set %} and {% import %} functionalities instead.\n' - + `filePath: ${filePath}`); - - if (elem.name === 'variable') { - VariableProcessor.addVariable(pageVariables, elem, $(elem).html(), filePath, renderVariable); - } else { - /* - 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, pageSources, filePath, renderVariable); - } - }); - - return { pageImportedVariables, pageVariables }; - } - /** * Extracts variables specified as in include elements. * @param includeElement to extract inline variables from @@ -399,18 +253,8 @@ class VariableProcessor { // Extract included variables from the include element, merging with the parent context variables const includeVariables = VariableProcessor.extractIncludeVariables(node); - - // 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, pageSources, includeVariables); - // Render the included content with all the variables - const renderedContent = this.renderSiteVariables(asIfAt, fileContent, pageSources, { - ...pageImportedVariables, - ...pageVariables, + const renderedContent = this.renderWithSiteVariables(asIfAt, fileContent, pageSources, { ...includeVariables, ...context.variables, }); @@ -424,32 +268,6 @@ class VariableProcessor { childContext, }; } - - /** - * 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 content to render - * @param {PageSources} pageSources to add dependencies found during nunjucks rendering to - * @param highestPriorityVariables to render with the highest priority if any. - * This is currently only used for the MAIN_CONTENT_BODY in layouts. - * @param keepPercentRaw whether to reoutput {% raw/endraw %} tags, also used only for layouts. - */ - renderPage(contentFilePath, content, pageSources, highestPriorityVariables = {}, keepPercentRaw = false) { - const { - pageImportedVariables, - pageVariables, - } = this.extractPageVariables(contentFilePath, content, pageSources); - - return this.renderSiteVariables(contentFilePath, content, pageSources, { - ...pageImportedVariables, - ...pageVariables, - }, highestPriorityVariables, keepPercentRaw); - } } module.exports = VariableProcessor; From 2ddf00eca8300f390a3fe2650e2d6a2177b85c69 Mon Sep 17 00:00:00 2001 From: Ze Yu Date: Sat, 5 Dec 2020 20:39:11 +0800 Subject: [PATCH 2/3] Remove old variable and import feature tests --- .../functional/test_site/expected/index.html | 50 ++--------- .../test_site/expected/jsonPageVariable.json | 3 - .../test_site/expected/siteData.json | 15 ---- ..._.html => testReuseSubsite._include_.html} | 2 - .../test_site/expected/testDates.html | 1 - .../testImportVariables._include_.html | 14 --- .../expected/testImportVariables.html | 85 ------------------- .../testPanelsWithImportedVariables.html | 79 ----------------- .../expected/testVariableContainsInclude.html | 1 - .../cli/test/functional/test_site/index.md | 73 ++++------------ .../test_site/jsonPageVariable.json | 3 - .../test_site/moreVariablesToImport.md | 8 -- .../test/functional/test_site/panelSrcs.md | 10 --- .../cli/test/functional/test_site/site.json | 8 -- .../test_site/sub_site/_markbind/variables.md | 1 + .../{testReuse.md => testReuseSubsite.md} | 3 - .../test/functional/test_site/testDates.md | 2 +- .../test_site/testImportVariables.md | 11 --- .../test_site/testIncludeVariables.md | 4 +- .../test_site/testPageVariablesInInclude.md | 19 ----- .../testPanelsWithImportedVariables.md | 11 --- .../test_site/testVariableContainsInclude.md | 4 +- .../functional/test_site/variablesToImport.md | 26 ------ .../expected/index.html | 13 ++- 24 files changed, 38 insertions(+), 408 deletions(-) delete mode 100644 packages/cli/test/functional/test_site/expected/jsonPageVariable.json rename packages/cli/test/functional/test_site/expected/sub_site/{testReuse._include_.html => testReuseSubsite._include_.html} (97%) delete mode 100644 packages/cli/test/functional/test_site/expected/testImportVariables._include_.html delete mode 100644 packages/cli/test/functional/test_site/expected/testImportVariables.html delete mode 100644 packages/cli/test/functional/test_site/expected/testPanelsWithImportedVariables.html delete mode 100644 packages/cli/test/functional/test_site/jsonPageVariable.json delete mode 100644 packages/cli/test/functional/test_site/moreVariablesToImport.md delete mode 100644 packages/cli/test/functional/test_site/panelSrcs.md create mode 100644 packages/cli/test/functional/test_site/sub_site/_markbind/variables.md rename packages/cli/test/functional/test_site/sub_site/{testReuse.md => testReuseSubsite.md} (95%) delete mode 100644 packages/cli/test/functional/test_site/testImportVariables.md delete mode 100644 packages/cli/test/functional/test_site/testPanelsWithImportedVariables.md delete mode 100644 packages/cli/test/functional/test_site/variablesToImport.md diff --git a/packages/cli/test/functional/test_site/expected/index.html b/packages/cli/test/functional/test_site/expected/index.html index c171b2122a..e04978b424 100644 --- a/packages/cli/test/functional/test_site/expected/index.html +++ b/packages/cli/test/functional/test_site/expected/index.html @@ -180,53 +180,18 @@

Test

arrayVarItem1

arrayVarItem2

nestedVarValue

-

Json Variable

-
front back
-

Json Variable can be referenced Referencing jsonVar1: Json Variable can be referenced

Variables that reference another variable

This variable can be referenced.

References can be several levels deep.

-

Page Variable

-
-
-

Page Variable Json Variable

-

Page Variable with HTML and MD

-
-

Page Variable with HTML and Markdown

-

Nested Page Variable

-
- -
-
- Nested Page Variable -

Page Variable with Global Variable

-
- Page Variable with Global Variable -

Page Variable referencing Page Variable

-
- Page Variable referencing Page Variable -

Global Variable overriding Page Variable

-
- Global Variable Overriding Page Variable +

Global Variables can be referenced in {% set %}

+

Page Variable with Global Variable

+

Global Variables should override {% set %}

+

Global Variable Overriding Page Variable

Test Page Variable and Included Variable Integrations

-
-

Explicitly Included Page Variable

-

Explicitly Included Page Variable

-

Inner Page Variable Should Not Be Overridden by Outer Page Variable

-
- Inner Page Variable Should Not Be Overridden by Outer Page Variable

Outer Page Variable Should Not Leak Into Inner Pages

Outer Page Variable Should Not Leak Into Inner Pages

-

Included Variable Overriding Page Variable

-
- Included Variable Overriding Page Variable -

Page Variable Referencing Included Variable

-
- Page Variable Referencing Included Variable
-

Variables for includes should not be recognised as page variables, hence, there should be no text between this

-

and this.

Heading with multiple keywords

keyword 1 keyword 2

@@ -404,8 +369,6 @@

Feature list

This is a page from another Markbind site. The purpose of this page is to ensure that reuse works as expected. All the following images should display correctly.

-

Some variables:

-

IMG tags: @@ -430,7 +393,6 @@

Feature list -

Include segment from another Markbind site

@@ -556,6 +518,8 @@

Feature list

Test missing variable with default

Missing Variable

+

Variables for includes should not be recognised as page variables, hence, there should be no text between this

+

and this.

Included variables should not leak into other files

Should be blank:

@@ -607,7 +571,7 @@

-