Refactor template rendering (#1583)

* Update _errorToJson to _serializeError

* Remove async

* Refactor render

* Simplify _getModifiedData

* Rename data => commonData

* Rename templates => template for consistency

* Improve errors check

* Update tests

7 files changed, 53 insertions, 48 deletions

diff --git a/ext/js/data/anki-note-builder.js b/ext/js/data/anki-note-builder.js
index dfd92493..38f1eb16 100644
--- a/ext/js/data/anki-note-builder.js
+++ b/ext/js/data/anki-note-builder.js
@@ -30,7 +30,7 @@ class AnkiNoteBuilder {
         definition,
         mode,
         context,
-        templates,
+        template,
         deckName,
         modelName,
         fields,
@@ -51,10 +51,10 @@ class AnkiNoteBuilder {
             duplicateScopeCheckChildren = true;
         }
 
-        const data = this._createData(definition, mode, context, resultOutputMode, glossaryLayoutMode, compactTags, injectedMedia);
+        const commonData = this._createData(definition, mode, context, resultOutputMode, glossaryLayoutMode, compactTags, injectedMedia);
         const formattedFieldValuePromises = [];
         for (const [, fieldValue] of fields) {
-            const formattedFieldValuePromise = this._formatField(fieldValue, data, templates, errors);
+            const formattedFieldValuePromise = this._formatField(fieldValue, commonData, template, errors);
             formattedFieldValuePromises.push(formattedFieldValuePromise);
         }
 
@@ -110,12 +110,12 @@ class AnkiNoteBuilder {
         };
     }
 
-    async _formatField(field, data, templates, errors=null) {
+    async _formatField(field, commonData, template, errors=null) {
         return await this._stringReplaceAsync(field, this._markerPattern, async (g0, marker) => {
             try {
-                return await this._renderTemplate(templates, data, marker);
+                return await this._renderTemplate(template, marker, commonData);
             } catch (e) {
-                if (errors) {
+                if (Array.isArray(errors)) {
                     const error = new Error(`Template render error for {${marker}}`);
                     error.data = {error: e};
                     errors.push(error);
@@ -140,7 +140,7 @@ class AnkiNoteBuilder {
         return (await Promise.all(parts)).join('');
     }
 
-    async _renderTemplate(template, data, marker) {
-        return await this._templateRenderer.render(template, {data, marker}, 'ankiNote');
+    async _renderTemplate(template, marker, commonData) {
+        return await this._templateRenderer.render(template, {marker, commonData}, 'ankiNote');
     }
 }
diff --git a/ext/js/display/display.js b/ext/js/display/display.js
index 1e5c569d..ce4459fe 100644
--- a/ext/js/display/display.js
+++ b/ext/js/display/display.js
@@ -1463,7 +1463,7 @@ class Display extends EventDispatcher {
 
     async _createNote(definition, mode, context, injectMedia, errors) {
         const options = this._options;
-        const templates = this._ankiFieldTemplates;
+        const template = this._ankiFieldTemplates;
         const {
             general: {resultOutputMode, glossaryLayoutMode, compactTags},
             anki: ankiOptions
@@ -1488,7 +1488,7 @@ class Display extends EventDispatcher {
             definition,
             mode,
             context,
-            templates,
+            template,
             deckName,
             modelName,
             fields,
diff --git a/ext/js/pages/settings/anki-templates-controller.js b/ext/js/pages/settings/anki-templates-controller.js
index 710946be..90e88f57 100644
--- a/ext/js/pages/settings/anki-templates-controller.js
+++ b/ext/js/pages/settings/anki-templates-controller.js
@@ -176,14 +176,14 @@ class AnkiTemplatesController {
                     sentence: {text: definition.rawSource, offset: 0},
                     documentTitle: document.title
                 };
-                let templates = options.anki.fieldTemplates;
-                if (typeof templates !== 'string') { templates = this._defaultFieldTemplates; }
+                let template = options.anki.fieldTemplates;
+                if (typeof template !== 'string') { template = this._defaultFieldTemplates; }
                 const {general: {resultOutputMode, glossaryLayoutMode, compactTags}} = options;
                 const note = await this._ankiNoteBuilder.createNote({
                     definition,
                     mode,
                     context,
-                    templates,
+                    template,
                     deckName: '',
                     modelName: '',
                     fields: [
diff --git a/ext/js/templates/template-renderer-frame-api.js b/ext/js/templates/template-renderer-frame-api.js
index 6eebc199..104e357b 100644
--- a/ext/js/templates/template-renderer-frame-api.js
+++ b/ext/js/templates/template-renderer-frame-api.js
@@ -19,8 +19,8 @@ class TemplateRendererFrameApi {
     constructor(templateRenderer) {
         this._templateRenderer = templateRenderer;
         this._windowMessageHandlers = new Map([
-            ['render', {async: true, handler: this._onRender.bind(this)}],
-            ['getModifiedData', {async: true, handler: this._onGetModifiedData.bind(this)}]
+            ['render', {async: false, handler: this._onRender.bind(this)}],
+            ['getModifiedData', {async: false, handler: this._onGetModifiedData.bind(this)}]
         ]);
     }
 
@@ -47,25 +47,25 @@ class TemplateRendererFrameApi {
             }
             response = {result};
         } catch (error) {
-            response = {error: this._errorToJson(error)};
+            response = {error: this._serializeError(error)};
         }
 
         if (typeof id === 'undefined') { return; }
         source.postMessage({action: `${action}.response`, params: response, id}, '*');
     }
 
-    async _onRender({template, data, type}) {
-        return await this._templateRenderer.render(template, data, type);
+    _onRender({template, data, type}) {
+        return this._templateRenderer.render(template, data, type);
     }
 
-    async _onGetModifiedData({data, type}) {
-        const result = await this._templateRenderer.getModifiedData(data, type);
+    _onGetModifiedData({data, type}) {
+        const result = this._templateRenderer.getModifiedData(data, type);
         return this._clone(result);
     }
 
-    _errorToJson(error) {
+    _serializeError(error) {
         try {
-            if (error !== null && typeof error === 'object') {
+            if (typeof error === 'object' && error !== null) {
                 return {
                     name: error.name,
                     message: error.message,
diff --git a/ext/js/templates/template-renderer-frame-main.js b/ext/js/templates/template-renderer-frame-main.js
index c915d6b0..40ecf308 100644
--- a/ext/js/templates/template-renderer-frame-main.js
+++ b/ext/js/templates/template-renderer-frame-main.js
@@ -27,7 +27,7 @@
     const templateRenderer = new TemplateRenderer(japaneseUtil);
     const ankiNoteDataCreator = new AnkiNoteDataCreator(japaneseUtil);
     templateRenderer.registerDataType('ankiNote', {
-        modifier: ({data, marker}) => ankiNoteDataCreator.create(marker, data)
+        modifier: ({marker, commonData}) => ankiNoteDataCreator.create(marker, commonData)
     });
     const templateRendererFrameApi = new TemplateRendererFrameApi(templateRenderer);
     templateRendererFrameApi.prepare();
diff --git a/ext/js/templates/template-renderer.js b/ext/js/templates/template-renderer.js
index a7a4a842..5441528c 100644
--- a/ext/js/templates/template-renderer.js
+++ b/ext/js/templates/template-renderer.js
@@ -33,10 +33,21 @@ class TemplateRenderer {
         this._dataTypes.set(name, {modifier});
     }
 
-    async render(template, data, type) {
+    render(template, data, type) {
+        const instance = this._getTemplateInstance(template);
+        data = this._getModifiedData(data, type);
+        return this._renderTemplate(instance, data);
+    }
+
+    getModifiedData(data, type) {
+        return this._getModifiedData(data, type);
+    }
+
+    // Private
+
+    _getTemplateInstance(template) {
         if (!this._helpersRegistered) {
             this._registerHelpers();
-            this._helpersRegistered = true;
         }
 
         const cache = this._cache;
@@ -47,8 +58,11 @@ class TemplateRenderer {
             cache.set(template, instance);
         }
 
+        return instance;
+    }
+
+    _renderTemplate(instance, data) {
         try {
-            data = this._getModifiedData(data, type);
             this._stateStack = [new Map()];
             return instance(data).trim();
         } finally {
@@ -56,27 +70,16 @@ class TemplateRenderer {
         }
     }
 
-    async getModifiedData(data, type) {
-        return this._getModifiedData(data, type);
-    }
-
-    // Private
-
-    _getModifier(type) {
+    _getModifiedData(data, type) {
         if (typeof type === 'string') {
             const typeInfo = this._dataTypes.get(type);
             if (typeof typeInfo !== 'undefined') {
-                return typeInfo.modifier;
+                const {modifier} = typeInfo;
+                if (typeof modifier === 'function') {
+                    data = modifier(data);
+                }
             }
         }
-        return null;
-    }
-
-    _getModifiedData(data, type) {
-        const modifier = this._getModifier(type);
-        if (typeof modifier === 'function') {
-            data = modifier(data);
-        }
         return data;
     }
 
@@ -122,6 +125,8 @@ class TemplateRenderer {
         for (const [name, helper] of helpers) {
             this._registerHelper(name, helper);
         }
+
+        this._helpersRegistered = true;
     }
 
     _registerHelper(name, helper) {
diff --git a/test/test-anki-note-builder.js b/test/test-anki-note-builder.js
index aa2cf572..b1fffbcd 100644
--- a/test/test-anki-note-builder.js
+++ b/test/test-anki-note-builder.js
@@ -55,7 +55,7 @@ async function createVM() {
             const japaneseUtil = new JapaneseUtil(null);
             this._templateRenderer = new TemplateRenderer(japaneseUtil);
             this._templateRenderer.registerDataType('ankiNote', {
-                modifier: ({data, marker}) => ankiNoteDataCreator.create(marker, data)
+                modifier: ({marker, commonData}) => ankiNoteDataCreator.create(marker, commonData)
             });
         }
 
@@ -122,7 +122,7 @@ function getFieldMarkers(type) {
     }
 }
 
-async function getRenderResults(dictionaryEntries, type, mode, templates, AnkiNoteBuilder, write) {
+async function getRenderResults(dictionaryEntries, type, mode, template, AnkiNoteBuilder, write) {
     const markers = getFieldMarkers(type);
     const fields = [];
     for (const marker of markers) {
@@ -158,7 +158,7 @@ async function getRenderResults(dictionaryEntries, type, mode, templates, AnkiNo
             definition: dictionaryEntry,
             mode: null,
             context,
-            templates,
+            template,
             deckName: 'deckName',
             modelName: 'modelName',
             fields,
@@ -193,7 +193,7 @@ async function main() {
     const expectedResults1 = JSON.parse(fs.readFileSync(testResults1FilePath, {encoding: 'utf8'}));
     const actualResults1 = [];
 
-    const templates = fs.readFileSync(path.join(__dirname, '..', 'ext', 'data/templates/default-anki-field-templates.handlebars'), {encoding: 'utf8'});
+    const template = fs.readFileSync(path.join(__dirname, '..', 'ext', 'data/templates/default-anki-field-templates.handlebars'), {encoding: 'utf8'});
 
     for (let i = 0, ii = tests.length; i < ii; ++i) {
         const test = tests[i];
@@ -204,7 +204,7 @@ async function main() {
                     const {name, mode, text} = test;
                     const options = vm.buildOptions(optionsPresets, test.options);
                     const {dictionaryEntries} = clone(await vm.translator.findTerms(mode, text, options));
-                    const results = mode !== 'simple' ? clone(await getRenderResults(dictionaryEntries, 'terms', mode, templates, AnkiNoteBuilder, write)) : null;
+                    const results = mode !== 'simple' ? clone(await getRenderResults(dictionaryEntries, 'terms', mode, template, AnkiNoteBuilder, write)) : null;
                     actualResults1.push({name, results});
                     if (!write) {
                         assert.deepStrictEqual(results, expected1.results);
@@ -216,7 +216,7 @@ async function main() {
                     const {name, text} = test;
                     const options = vm.buildOptions(optionsPresets, test.options);
                     const dictionaryEntries = clone(await vm.translator.findKanji(text, options));
-                    const results = clone(await getRenderResults(dictionaryEntries, 'kanji', null, templates, AnkiNoteBuilder, write));
+                    const results = clone(await getRenderResults(dictionaryEntries, 'kanji', null, template, AnkiNoteBuilder, write));
                     actualResults1.push({name, results});
                     if (!write) {
                         assert.deepStrictEqual(results, expected1.results);