Revert "Add imported variables (#751)" (#849)

PR #751 implements imported variables, by deferring the rendering of
Nunjucks variable to a later stage, so that the contents of the
imported variables can be correctly populated before rendering takes
place.

However, this also means that we cannot do the following:

    <import src="{{ file }}#var" />
    {{ var }}

Nunjucks rendering requires all variables to be resolved in one go.
Since Nunjucks rendering is delayed in #751, the src attribute would not
resolve correctly, so the logic for importing variables would fail and
var would not have the correct content.

However, if we do Nunjucks rendering at an earlier stage to resolve the
src attribute, we will have to resolve the var variable immediately as
well, as we have no way of selectively resolving certain variables at
different stages. This is because we do not have an algorithm to
determine which variables should be resolved and which shouldn't be
resolved (this said algorithm must also take into consider edge cases,
such as using Nunjucks' "set" instruction).

Therefore, the design of import variable needs to be reconsidered in
order to overcome this techincal obstacle.

Right now, the failure to resolve the src attribute properly, due to the
deferred Nunjuck rendering, is breaking website generation (for example,
MarkBind's "userGuide/formattingContents.html" is broken).

There doesn't seem to be a quick fix to overcome the technical obstacle
described above. The new design might take additional time, and authors
also have no quick ways to solve this issue. Let's revert the import
variable PR (#751) to prevent generated websites from breaking, and wait
for a new implementation of import variables to surface instead.

This reverts commit 4412153.

src/lib/markbind/src/parser.js

@@ -27,8 +27,6 @@ const ATTRIB_CWF = 'cwf';
 
 const BOILERPLATE_FOLDER_NAME = '_markbind/boilerplates';
 
-const VARIABLE_LOOKUP = {};
-
 /*
  * Utils
  */
@@ -144,47 +142,14 @@ function extractPageVariables(fileName, data, userDefinedVariables, includedVari
       return;
     }
     if (!pageVariables[variableName]) {
-      const variableValue
+      pageVariables[variableName]
         = nunjucks.renderString(md.renderInline(variableElement.html()),
                                 { ...pageVariables, ...userDefinedVariables, ...includedVariables });
-      pageVariables[variableName] = variableValue;
-      if (!VARIABLE_LOOKUP[fileName]) {
-        VARIABLE_LOOKUP[fileName] = {};
-      }
-      VARIABLE_LOOKUP[fileName][variableName] = variableValue;
     }
   });
   return pageVariables;
 }
 
-/**
- * Extract imported page variables from a page
- * @param context of the page
- */
-function extractImportedVariables(context) {
-  if (!context.importedVariables) {
-    return {};
-  }
-  const importedVariables = {};
-  Object.entries(context.importedVariables).forEach(([src, variables]) => {
-    variables.forEach((variableName) => {
-      const actualFilePath = utils.isUrl()
-        ? src
-        : path.resolve(path.dirname(context.cwf), decodeURIComponent(url.parse(src).path));
-      if (!VARIABLE_LOOKUP[actualFilePath] || !VARIABLE_LOOKUP[actualFilePath][variableName]) {
-        // eslint-disable-next-line no-console
-        console.warn(`Missing variable ${variableName} in ${src} referenced by ${context.cwf}\n`);
-        return;
-      }
-      const variableValue = VARIABLE_LOOKUP[actualFilePath][variableName];
-      if (!importedVariables[variableName]) {
-        importedVariables[variableName] = variableValue;
-      }
-    });
-  });
-  return importedVariables;
-}
-
 Parser.prototype.getDynamicIncludeSrc = function () {
   return _.clone(this.dynamicIncludeSrc);
 };
@@ -207,13 +172,13 @@ Parser.prototype._preprocess = function (node, context, config) {
   element.attribs = element.attribs || {};
   element.attribs[ATTRIB_CWF] = path.resolve(context.cwf);
 
-  const requiresSrc = ['include', 'import'].includes(element.name);
+  const requiresSrc = ['include'].includes(element.name);
   if (requiresSrc && _.isEmpty(element.attribs.src)) {
     const error = new Error(`Empty src attribute in ${element.name} in: ${element.attribs[ATTRIB_CWF]}`);
     this._onError(error);
     return createErrorNode(element, error);
   }
-  const shouldProcessSrc = ['include', 'panel', 'import'].includes(element.name);
+  const shouldProcessSrc = ['include', 'panel'].includes(element.name);
   const hasSrc = _.hasIn(element.attribs, 'src');
   let isUrl;
   let includeSrc;
@@ -246,8 +211,7 @@ Parser.prototype._preprocess = function (node, context, config) {
     }
   }
 
-  if (element.name === 'include' || element.name === 'import') {
-    const isImport = element.name === 'import';
+  if (element.name === 'include') {
     const isInline = _.hasIn(element.attribs, 'inline');
     const isDynamic = _.hasIn(element.attribs, 'dynamic');
     const isOptional = _.hasIn(element.attribs, 'optional');
@@ -299,17 +263,22 @@ Parser.prototype._preprocess = function (node, context, config) {
       element.name = 'markdown';
     }
 
-    const fileContent = self._fileCache[actualFilePath]; // cache the file contents to save some I/O
+    let fileContent = self._fileCache[actualFilePath]; // cache the file contents to save some I/O
     const { parent, relative } = calculateNewBaseUrls(filePath, config.rootPath, config.baseUrlMap);
     const userDefinedVariables = config.userDefinedVariablesMap[path.resolve(parent, relative)];
 
     // Extract included variables from the PARENT file
     const includeVariables = extractIncludeVariables(element, context.variables);
 
     // Extract page variables from the CHILD file
-    const pageVariables = extractPageVariables(actualFilePath, fileContent,
+    const pageVariables = extractPageVariables(element.attribs.src, fileContent,
                                                userDefinedVariables, includeVariables);
 
+    // Render inner file content
+    fileContent = nunjucks.renderString(fileContent,
+                                        { ...pageVariables, ...includeVariables, ...userDefinedVariables },
+                                        { path: actualFilePath });
+
     // Delete variable attributes in include
     Object.keys(element.attribs).forEach((attribute) => {
       if (attribute.startsWith('var-')) {
@@ -327,28 +296,8 @@ Parser.prototype._preprocess = function (node, context, config) {
       const segmentSrc = cheerio.parseHTML(fileContent, true);
       const $ = cheerio.load(segmentSrc);
       const hashContent = $(includeSrc.hash).html();
-
-      if (isImport) {
-        const variableContent = $(`variable[name=${includeSrc.hash.substring(1)}]`).html();
-        if (!variableContent) {
-          // eslint-disable-next-line no-console
-          console.warn(`Missing import variable ${includeSrc.pathname} in ${element.attribs[ATTRIB_CWF]}.`);
-          return createEmptyNode();
-        }
-        if (!context.importedVariables) {
-          // eslint-disable-next-line no-param-reassign
-          context.importedVariables = {};
-        }
-        if (!context.importedVariables[includeSrc.pathname]) {
-          // eslint-disable-next-line no-param-reassign
-          context.importedVariables[includeSrc.pathname] = [];
-        }
-        // eslint-disable-next-line no-param-reassign
-        context.importedVariables[includeSrc.pathname].push(includeSrc.hash.substring(1));
-        return createEmptyNode();
-      }
-
       let actualContent = (hashContent && isTrim) ? hashContent.trim() : hashContent;
+
       if (actualContent === null) {
         if (isOptional) {
           // set empty content for optional segment include that does not exist
@@ -382,12 +331,6 @@ Parser.prototype._preprocess = function (node, context, config) {
         true,
       );
     } else {
-      if (isImport) {
-        // eslint-disable-next-line no-console
-        console.warn(`Missing hash for import variable ${includeSrc.pathname}`
-                     + ` in ${element.attribs[ATTRIB_CWF]}.`);
-        return createEmptyNode();
-      }
       let actualContent = (fileContent && isTrim) ? fileContent.trim() : fileContent;
       if (isIncludeSrcMd) {
         if (context.mode === 'include') {
@@ -409,30 +352,15 @@ Parser.prototype._preprocess = function (node, context, config) {
     childContext.cwf = filePath;
     childContext.source = isIncludeSrcMd ? 'md' : 'html';
     childContext.callStack.push(context.cwf);
-    childContext.variables = { ...includeVariables };
-    childContext.importedVariables = {};
+    childContext.variables = includeVariables;
 
     if (element.children && element.children.length > 0) {
       if (childContext.callStack.length > CyclicReferenceError.MAX_RECURSIVE_DEPTH) {
         const error = new CyclicReferenceError(childContext.callStack);
         this._onError(error);
         return createErrorNode(element, error);
       }
-      element.children = element.children.map((e) => {
-        let processedEle = cheerio.html(self._preprocess(e, childContext, config));
-        const importedVariables = extractImportedVariables(childContext);
-        userDefinedVariables.hostBaseUrl = '{{hostBaseUrl}}';
-        processedEle
-          = nunjucks.renderString(processedEle,
-                                  {
-                                    ...pageVariables,
-                                    ...importedVariables,
-                                    ...includeVariables,
-                                    ...userDefinedVariables,
-                                  },
-                                  { path: actualFilePath });
-        return cheerio.parseHTML(processedEle)[0];
-      });
+      element.children = element.children.map(e => self._preprocess(e, childContext, config));
     }
   } else if ((element.name === 'panel') && hasSrc) {
     if (!isUrl && includeSrc.hash) {
@@ -603,17 +531,8 @@ Parser.prototype.includeFile = function (file, config) {
   context.cwf = config.cwf || file; // current working file
   context.mode = 'include';
   context.callStack = [];
-  let fileData = null;
 
   return new Promise((resolve, reject) => {
-    let actualFilePath = file;
-    if (!utils.fileExists(file)) {
-      const boilerplateFilePath = calculateBoilerplateFilePath(path.basename(file), file, config);
-      if (utils.fileExists(boilerplateFilePath)) {
-        actualFilePath = boilerplateFilePath;
-      }
-    }
-
     const handler = new htmlparser.DomHandler((error, dom) => {
       if (error) {
         reject(error);
@@ -630,29 +549,35 @@ Parser.prototype.includeFile = function (file, config) {
         }
         return processed;
       });
-      const { parent, relative } = calculateNewBaseUrls(file, config.rootPath, config.baseUrlMap);
-      const userDefinedVariables = config.userDefinedVariablesMap[path.resolve(parent, relative)];
-      const pageVariables = extractPageVariables(path.basename(file), fileData, userDefinedVariables, {});
-      const importedVariables = extractImportedVariables(context);
-      resolve(nunjucks.renderString(cheerio.html(nodes),
-                                    { ...pageVariables, ...importedVariables, ...userDefinedVariables },
-                                    { path: actualFilePath }));
+      resolve(cheerio.html(nodes));
     });
 
     const parser = new htmlparser.Parser(handler, {
       xmlMode: true,
       decodeEntities: true,
     });
 
+    let actualFilePath = file;
+    if (!utils.fileExists(file)) {
+      const boilerplateFilePath = calculateBoilerplateFilePath(path.basename(file), file, config);
+      if (utils.fileExists(boilerplateFilePath)) {
+        actualFilePath = boilerplateFilePath;
+      }
+    }
+
     // Read files
     fs.readFile(actualFilePath, 'utf-8', (err, data) => {
-      fileData = data;
       if (err) {
         reject(err);
         return;
       }
+      const { parent, relative } = calculateNewBaseUrls(file, config.rootPath, config.baseUrlMap);
+      const userDefinedVariables = config.userDefinedVariablesMap[path.resolve(parent, relative)];
+      const pageVariables = extractPageVariables(path.basename(file), data, userDefinedVariables, {});
+      const fileContent = nunjucks.renderString(data,
+                                                { ...pageVariables, ...userDefinedVariables },
+                                                { path: actualFilePath });
       const fileExt = utils.getExt(file);
-      const fileContent = data;
       if (utils.isMarkdownFileExt(fileExt)) {
         context.source = 'md';
         parser.parseComplete(fileContent.toString());

test/functional/test_site/expected/index.html

@@ -183,17 +183,6 @@ <h1 id="page-variable-referencing-included-variable">Page Variable Referencing I
             <div></div>
             Page Variable Referencing Included Variable
           </div>
-          <div>
-            <h1 id="imported-variable">Imported Variable<a class="fa fa-anchor" href="#imported-variable"></a></h1>
-            <div></div>
-            Imported Variable
-            <h1 id="imported-variable-referencing-global-variable">Imported Variable Referencing Global Variable<a class="fa fa-anchor" href="#imported-variable-referencing-global-variable"></a></h1>
-            <div></div>
-            Imported Variable Referencing Global Variable
-          </div>
-          <h1 id="imported-variable-on-top-level">Imported Variable on Top Level<a class="fa fa-anchor" href="#imported-variable-on-top-level"></a></h1>
-          <div></div>
-          Imported Variable on Top Level
           <h1 id="heading-with-multiple-keywords">Heading with multiple keywords<a class="fa fa-anchor" href="#heading-with-multiple-keywords"></a></h1>
           <p><span class="keyword">keyword 1</span>
             <span class="keyword">keyword 2</span></p>
@@ -486,9 +475,6 @@ <h6 class="always-index" id="level-6-header-outside-headingsearchindex-with-alwa
             <a class="nav-link py-1" href="#outer-page-variable-should-not-leak-into-inner-pages">Outer Page Variable Should Not Leak Into Inner Pages&#x200E;</a>
             <a class="nav-link py-1" href="#included-variable-overriding-page-variable">Included Variable Overriding Page Variable&#x200E;</a>
             <a class="nav-link py-1" href="#page-variable-referencing-included-variable">Page Variable Referencing Included Variable&#x200E;</a>
-            <a class="nav-link py-1" href="#imported-variable">Imported Variable&#x200E;</a>
-            <a class="nav-link py-1" href="#imported-variable-referencing-global-variable">Imported Variable Referencing Global Variable&#x200E;</a>
-            <a class="nav-link py-1" href="#imported-variable-on-top-level">Imported Variable on Top Level&#x200E;</a>
             <a class="nav-link py-1" href="#heading-with-multiple-keywords">Heading with multiple keywords&#x200E;</a>
             <a class="nav-link py-1" href="#heading-with-keyword-in-panel">Heading with keyword in panel&#x200E;</a>
             <a class="nav-link py-1" href="#panel-with-heading-with-keyword">Panel with heading with keyword&#x200E;</a>

test/functional/test_site/expected/siteData.json

@@ -53,9 +53,6 @@
         "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",
-        "imported-variable": "Imported Variable",
-        "imported-variable-referencing-global-variable": "Imported Variable Referencing Global Variable",
-        "imported-variable-on-top-level": "Imported Variable on Top Level",
         "heading-with-multiple-keywords": "Heading with multiple keywords | keyword 1, keyword 2",
         "heading-with-keyword-in-panel": "Heading with keyword in panel | panel keyword",
         "expanded-panel-without-heading-with-keyword": "Expanded panel without heading with keyword",

test/functional/test_site/index.md

@@ -55,12 +55,6 @@ tags: ["tag-frontmatter-shown", "tag-included-file", "+tag-exp*", "-tag-exp-hidd
   <variable name="included_variable_overriding_page_variable">Included Variable Overriding Page Variable</variable>
 </include>
 
-<include src="testImportedVariables.md" />
-
-# Imported Variable on Top Level
-<import src="testImportedVariablesToImport.md#imported_variable_on_top_level" />
-{{ imported_variable_on_top_level }}
-
 # Heading with multiple keywords
 <span class="keyword">keyword 1</span>
 <span class="keyword">keyword 2</span>