fix(compiler): refactor template resolution (#220)

fixes #220, fixes #216 BREAKING CHANGE: - The templateLoader and scriptResolver are now 2 separate functions that can be set on the compiler - The Runtime.template() has an extra argument 'id' that specifies the group (script) the template is defined.
adobe · Jul 29, 2020 · b64729b · b64729b
1 parent 3dc23b7
commit b64729b
Show file tree

Hide file tree

Showing 19 changed files with 265 additions and 155 deletions.
diff --git a/src/compiler/Compiler.js b/src/compiler/Compiler.js
@@ -12,22 +12,21 @@
 
 /* eslint-disable no-await-in-loop */
 
-// built-in modules
 const path = require('path');
-// declared dependencies
+const crypto = require('crypto');
 const fse = require('fs-extra');
 const { SourceMapGenerator } = require('source-map');
-// local modules
 const TemplateParser = require('../parser/html/TemplateParser');
 const ThrowingErrorListener = require('../parser/htl/ThrowingErrorListener');
 const JSCodeGenVisitor = require('./JSCodeGenVisitor');
 const ExpressionFormatter = require('./ExpressionFormatter');
-const TemplateReference = require('../parser/commands/TemplateReference');
+const FileReference = require('../parser/commands/FileReference');
 const VariableBinding = require('../parser/commands/VariableBinding');
 const RuntimeCall = require('../parser/htl/nodes/RuntimeCall');
 const Identifier = require('../parser/htl/nodes/Identifier');
 const FunctionBlock = require('../parser/commands/FunctionBlock');
 const TemplateLoader = require('./TemplateLoader.js');
+const ScriptResolver = require('./ScriptResolver.js');
 
 const DEFAULT_TEMPLATE = 'JSCodeTemplate.js';
 const RUNTIME_TEMPLATE = 'JSRuntimeTemplate.js';
@@ -62,7 +61,8 @@ module.exports = class Compiler {
     this._sourceFile = null;
     this._sourceOffset = 0;
     this._moduleImportGenerator = Compiler.defaultModuleGenerator;
-    this._templateLoader = TemplateLoader('.');
+    this._templateLoader = TemplateLoader();
+    this._scriptResolver = ScriptResolver('.');
   }
 
   /**
@@ -79,7 +79,7 @@ module.exports = class Compiler {
    */
   withDirectory(dir) {
     this._dir = dir;
-    this._templateLoader = TemplateLoader(dir);
+    this._scriptResolver = ScriptResolver(dir);
     return this;
   }
 
@@ -163,13 +163,22 @@ module.exports = class Compiler {
 
   /**
    * Sets the function that loads the templates.
-   * @param {function} fn the async function taking the baseDir and file name.
+   * @param {function} fn the async function taking the file path.
    */
   withTemplateLoader(fn) {
     this._templateLoader = fn;
     return this;
   }
 
+  /**
+   * Sets the function that resolves a script.
+   * @param {function} fn the async function taking the baseDir and file name.
+   */
+  withScriptResolver(fn) {
+    this._scriptResolver = fn;
+    return this;
+  }
+
   /**
    * Compiles the specified source file and saves the result, overwriting the
    * file name.
@@ -250,56 +259,62 @@ module.exports = class Compiler {
    * @param {String} source the HTL template code
    * @param {String} baseDir the base directory to resolve file references
    * @param {object} mods object with module mappings from use classes
+   * @param {object} templates Object with resolved templates
    * @returns {object} The result object with a `commands` stream and `templates`.
    */
-  async _parse(source, baseDir, mods) {
+  async _parse(source, baseDir, mods, templates = {}) {
     const commands = new TemplateParser()
       .withErrorListener(ThrowingErrorListener.INSTANCE)
       .withDefaultMarkupContext(this._defaultMarkupContext)
       .parse(source);
 
-    const templates = [];
-
     // find any templates references and use classes and process them
     for (let i = 0; i < commands.length; i += 1) {
       const c = commands[i];
-      if (c instanceof TemplateReference) {
+      if (c instanceof FileReference) {
         if (c.isTemplate()) {
-          const {
-            data: templateSource,
-            path: templatePath,
-          } = await this._templateLoader(baseDir, c.filename);
-
-          const res = await this._parse(templateSource, path.dirname(templatePath), mods);
-
-          // add recursive templates, if any.
-          templates.push(...res.templates);
-
-          // add this templates
-          const template = {
-            file: templatePath,
-            commands: [],
-          };
-          templates.push(template);
-          // prefix all templates with the variable name of the use class and discard commands
-          // outside functions.
-          let inside = false;
-          res.commands.forEach((cmd) => {
-            if (cmd instanceof FunctionBlock.Start) {
-              inside = true;
-              // eslint-disable-next-line no-underscore-dangle,no-param-reassign
-              cmd._expression = `${c.name}.${cmd._expression}`;
-            } else if (cmd instanceof FunctionBlock.End) {
-              inside = false;
-            } else if (!inside) {
-              return;
-            }
-            template.commands.push(cmd);
-          });
-
-          // remove the template reference from the stream
-          commands.splice(i, 1);
-          i -= 1;
+          const templatePath = await this._scriptResolver(baseDir, c.filename);
+          let template = templates[templatePath];
+          if (!template) {
+            // hash the template path for a somewhat stable id
+            const id = crypto.createHash('md5').update(templatePath, 'utf-8').digest().toString('hex');
+
+            // load template
+            const {
+              data: templateSource,
+            } = await this._templateLoader(templatePath);
+
+            // register template
+            template = {
+              id,
+              file: templatePath,
+              commands: [],
+            };
+            // eslint-disable-next-line no-param-reassign
+            templates[templatePath] = template;
+
+            // parse the template
+            const res = await this._parse(
+              templateSource, path.dirname(templatePath), mods, templates,
+            );
+
+            // extract the template functions and discard commands outside the functions
+            let inside = false;
+            res.commands.forEach((cmd) => {
+              if (cmd instanceof FunctionBlock.Start) {
+                inside = true;
+                // eslint-disable-next-line no-param-reassign
+                cmd.id = template.id;
+              } else if (cmd instanceof FunctionBlock.End) {
+                inside = false;
+              } else if (!inside) {
+                return;
+              }
+              template.commands.push(cmd);
+            });
+          }
+          // remember the file id on the use statement
+          c.id = template.id;
         } else {
           let file = c.filename;
           if (file.startsWith('./') || file.startsWith('../')) {
@@ -345,7 +360,7 @@ module.exports = class Compiler {
     });
 
     const {
-      code, templateCode, mappings, templateMappings,
+      code, templateCode, mappings, templateMappings, globalTemplateNames,
     } = new JSCodeGenVisitor()
       .withIndent('  ')
       .withSourceMap(this._sourceMap)
@@ -366,6 +381,14 @@ module.exports = class Compiler {
       }
       imports += `  ${exp}\n`;
     });
+    // add global template names
+    globalTemplateNames.forEach((ident) => {
+      if (this._runtimeGlobals.indexOf(ident) >= 0) {
+        imports += `  ${ident} = $.template('global').${ident};\n`;
+      } else {
+        imports += `  let ${ident} = $.template('global').${ident};\n`;
+      }
+    });
 
     let template = this._codeTemplate;
     if (!template) {

diff --git a/src/compiler/DomHandler.js b/src/compiler/DomHandler.js
@@ -17,6 +17,7 @@ module.exports = class DomHandler {
   constructor(generator) {
     this._gen = generator;
     this._out = generator.out.bind(generator);
+    this._globalTemplates = {};
   }
 
   beginDocument() {
@@ -72,15 +73,23 @@ module.exports = class DomHandler {
     }
   }
 
+  bindFunction(cmd) {
+    this._out(`const ${cmd.name} = $.template('${cmd.id}');`);
+  }
+
   functionStart(cmd) {
     const exp = ExpressionFormatter.escapeVariable(ExpressionFormatter.format(cmd.expression));
-    const functionName = `_template_${exp.replace(/\./g, '_')}`;
-    this._out(`$.template('${exp}', function* ${functionName}(args) { `);
+    const id = cmd.id || 'global';
+    const functionName = `_template_${id}_${exp.replace(/\./g, '_')}`;
+    this._out(`$.template('${id}', '${exp}', function* ${functionName}(args) { `);
     this._gen.indent();
     cmd.args.forEach((arg) => {
       this._out(`const ${ExpressionFormatter.escapeVariable(arg)} = args[1]['${arg}'] || '';`);
     });
     this._out('let $t, $n = args[0];');
+    if (!cmd.id) {
+      this._globalTemplates[exp] = true;
+    }
   }
 
   functionEnd() {
@@ -91,6 +100,6 @@ module.exports = class DomHandler {
   functionCall(cmd) {
     const funcName = ExpressionFormatter.format(cmd.functionName);
     const args = ExpressionFormatter.format(cmd.args);
-    this._out(`yield $.call($.template().${funcName}, [$n, ${args}]);`);
+    this._out(`yield $.call(${funcName}, [$n, ${args}]);`);
   }
 };
diff --git a/src/compiler/JSCodeGenVisitor.js b/src/compiler/JSCodeGenVisitor.js
@@ -12,6 +12,7 @@
 
 const OutText = require('../parser/commands/OutText');
 const VariableBinding = require('../parser/commands/VariableBinding');
+const FileReference = require('../parser/commands/FileReference');
 const FunctionBlock = require('../parser/commands/FunctionBlock');
 const FunctionCall = require('../parser/commands/FunctionCall');
 const Conditional = require('../parser/commands/Conditional');
@@ -127,7 +128,7 @@ module.exports = class JSCodeGenVisitor {
 
     // then process the templates
     this._sourceOffset = 0;
-    templates.forEach((t) => {
+    Object.values(templates).forEach((t) => {
       this._sourceFile = t.file;
       t.commands.forEach((c) => {
         c.accept(this);
@@ -157,6 +158,8 @@ module.exports = class JSCodeGenVisitor {
       mappings: this._main.map,
       templateCode,
       templateMappings,
+      // eslint-disable-next-line no-underscore-dangle
+      globalTemplateNames: Object.keys(this._dom._globalTemplates),
     };
   }
 
@@ -212,6 +215,8 @@ module.exports = class JSCodeGenVisitor {
       this.out(`const ${ExpressionFormatter.escapeVariable(cmd.variableName)} = ${exp};`);
     } else if (cmd instanceof VariableBinding.End) {
       // nop
+    } else if (cmd instanceof FileReference) {
+      this._dom.bindFunction(cmd);
     } else if (cmd instanceof FunctionBlock.Start) {
       this.pushBlock(cmd.expression);
       this._dom.functionStart(cmd);

diff --git a/src/compiler/ScriptResolver.js b/src/compiler/ScriptResolver.js
@@ -0,0 +1,57 @@
+/*
+ * Copyright 2020 Adobe. All rights reserved.
+ * This file is licensed to you under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License. You may obtain a copy
+ * of the License at http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under
+ * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
+ * OF ANY KIND, either express or implied. See the License for the specific language
+ * governing permissions and limitations under the License.
+ */
+const path = require('path');
+const fse = require('fs-extra');
+
+/**
+ * Creates a script resolver.
+ * @param {string[]} roots Root directories for resolution.
+ */
+module.exports = function createResolver(roots) {
+  if (!Array.isArray(roots)) {
+    // eslint-disable-next-line no-param-reassign
+    roots = [roots];
+  }
+
+  /**
+   * Resolves the script against the given roots.
+   * @param {string} baseDir additional root
+   * @param {string} uri script uri
+   * @returns {Promise<string>}
+   */
+  async function resolve(baseDir, uri) {
+    let bases = [baseDir, ...roots];
+
+    // if uri starts with '.' or '..', only consider specified bases.
+    // otherwise also consider apps and libs.
+    if (!uri.startsWith('./') && !uri.startsWith('../')) {
+      bases = bases.reduce((prev, root) => {
+        prev.push(root);
+        prev.push(path.resolve(root, 'apps'));
+        prev.push(path.resolve(root, 'libs'));
+        return prev;
+      }, []);
+    }
+
+    // eslint-disable-next-line no-restricted-syntax
+    for (const base of bases) {
+      const scriptPath = path.resolve(base, uri);
+      // eslint-disable-next-line no-await-in-loop
+      if (await fse.pathExists(scriptPath)) {
+        return scriptPath;
+      }
+    }
+    throw Error(`Unable to resolve script: ${uri}. Search Path: ${bases}`);
+  }
+
+  return resolve;
+};
diff --git a/src/compiler/TemplateLoader.js b/src/compiler/TemplateLoader.js
@@ -9,61 +9,20 @@
  * OF ANY KIND, either express or implied. See the License for the specific language
  * governing permissions and limitations under the License.
  */
-const path = require('path');
 const fse = require('fs-extra');
 
 /**
  * Creates an template loader.
- * @param {string[]} roots Root directories for resolution.
  */
-module.exports = function createLoader(roots) {
-  if (!Array.isArray(roots)) {
-    // eslint-disable-next-line no-param-reassign
-    roots = [roots];
-  }
-
-  /**
-   * Resolves the template against the given roots.
-   * @param {string} baseDir additional root
-   * @param {string} uri template uri
-   * @returns {Promise<string>}
-   */
-  async function resolve(baseDir, uri) {
-    let bases = [baseDir, ...roots];
-
-    // if uri starts with '.' or '..', only consider specified bases.
-    // otherwise also consider apps and libs.
-    if (!uri.startsWith('./') && !uri.startsWith('../')) {
-      bases = bases.reduce((prev, root) => {
-        prev.push(root);
-        prev.push(path.resolve(root, 'apps'));
-        prev.push(path.resolve(root, 'libs'));
-        return prev;
-      }, []);
-    }
-
-    // eslint-disable-next-line no-restricted-syntax
-    for (const base of bases) {
-      const templatePath = path.resolve(base, uri);
-      // eslint-disable-next-line no-await-in-loop
-      if (await fse.pathExists(templatePath)) {
-        return templatePath;
-      }
-    }
-    throw Error(`Unable to resolve template: ${uri}. Search Path: ${bases}`);
-  }
-
+module.exports = function createLoader() {
   /**
-   * Load the template.
-   * @param {string} baseDir additional root
-   * @param {string} uri template uri
-   * @returns {Promise<>} the template source and resolved path
+   * @param {string} filePath Template path
+   * @returns {Promise<>} the template source and  path
    */
-  async function load(baseDir, uri) {
-    const templatePath = await resolve(baseDir, uri);
+  async function load(filePath) {
     return {
-      data: await fse.readFile(templatePath, 'utf-8'),
-      path: templatePath,
+      data: await fse.readFile(filePath, 'utf-8'),
+      path: filePath,
     };
   }