From b0b9a83cc6c58b842b3e66a5b07b2ad96e4027b5 Mon Sep 17 00:00:00 2001
From: Matt Mo <matt@montoyatech.com>
Date: Fri, 21 Aug 2020 21:42:10 -0700
Subject: [PATCH] Added a rendering context which replaces the rendered flag on
 ignite elements so that properties won't return their value if we are
 rendering. This fixes an issue where lists would get messed up since they
 reconstruct whenever the list changes.

---
 src/ignite-element.js  | 20 +++++++++++++-----
 src/ignite-html.js     | 48 ++++++++++++++++++++++++++++++++++++++++++
 src/ignite-template.js | 20 ++++++++++++------
 3 files changed, 77 insertions(+), 11 deletions(-)

diff --git a/src/ignite-element.js b/src/ignite-element.js
index 3276512..43b48ab 100644
--- a/src/ignite-element.js
+++ b/src/ignite-element.js
@@ -1,6 +1,10 @@
 import { IgniteProperty } from './ignite-html.js';
 import { IgniteTemplate } from './ignite-template.js';
 
+/**
+ * The outline of a ignite element that extends the html element
+ * and can be used to create custom components.
+ */
 class IgniteElement extends HTMLElement {
     constructor() {
         super();
@@ -9,7 +13,6 @@ class IgniteElement extends HTMLElement {
         this.template = null;
         this.elements = [];
         this.createProperties();
-        this.rendered = false;
     }
 
     /**
@@ -42,7 +45,7 @@ class IgniteElement extends HTMLElement {
             ((propName) => {
                 Object.defineProperty(this, propName, {
                     get: function () {
-                        if (this.rendered) {
+                        if (IgniteRenderingContext.rendering == false) {
                             return this[`_${propName}`].value;
                         } else {
                             return this[`_${propName}`];
@@ -98,6 +101,9 @@ class IgniteElement extends HTMLElement {
         //Add any childNodes we have to the elements list within this
         this.childNodes.forEach((item) => this.elements.push(item));
 
+        //Enter a rendering context so properties don't expose their values until we are done.
+        IgniteRenderingContext.enter();
+
         //Make sure the render template is our template, if not, add it as a child.
         var renderTemplate = this.render();
         if (renderTemplate !== this.template && renderTemplate) {
@@ -107,10 +113,14 @@ class IgniteElement extends HTMLElement {
         }
 
         //Construct our template.
-        this.template.construct(this.parentElement);
+        try {
+            this.template.construct(this.parentElement);
+        } catch (error) {
+            console.error(error);
+        }
 
-        //Set rendered to true so that all future properties return their values.
-        this.rendered = true;
+        //Leave the rendering context.
+        IgniteRenderingContext.leave();
     }
 
     /**
diff --git a/src/ignite-html.js b/src/ignite-html.js
index 39dd564..bb69e61 100644
--- a/src/ignite-html.js
+++ b/src/ignite-html.js
@@ -1,3 +1,7 @@
+/**
+ * The outline of a ignite property which is a managed property that
+ * can be used to invoke call back functions when the value of the property changes.
+ */
 class IgniteProperty {
     constructor() {
         this.callbacks = [];
@@ -25,6 +29,18 @@ class IgniteProperty {
     }
 }
 
+/**
+ * Return the value of the property if we try to convert
+ * the property to a string.
+ */
+IgniteProperty.prototype.toString = function () {
+    return this.value.toString();
+}
+
+/**
+ * The outline of a ignite property callback that allows
+ * disconnecting of callbacks on demand when they are no longer needed.
+ */
 class IgnitePropertyCallback {
     constructor(property, callback) {
         this.callback = callback;
@@ -47,6 +63,38 @@ class IgnitePropertyCallback {
     }
 }
 
+/**
+ * The outline of a simple rendering context which allows us to 
+ * know if we are currently rendering anything ignite related. This works
+ * because Javascript is single threaded, if events could break the current execution
+ * this would fail. But it's safe since events cannot do that.
+ */
+class IgniteRenderingContext {
+    static enter() {
+        if (!IgniteRenderingContext.RenderCount) {
+            IgniteRenderingContext.RenderCount = 0;
+        }
+
+        IgniteRenderingContext.RenderCount++;
+    }
+
+    static leave() {
+        if (IgniteRenderingContext.RenderCount) {
+            IgniteRenderingContext.RenderCount--;
+        }
+    }
+
+    static get rendering() {
+        if (IgniteRenderingContext.RenderCount && IgniteRenderingContext.RenderCount > 0) {
+            return true;
+        }
+
+        return false;
+    }
+}
+
+window.IgniteRenderingContext = IgniteRenderingContext;
+
 export {
     IgniteProperty
 };
\ No newline at end of file
diff --git a/src/ignite-template.js b/src/ignite-template.js
index 8f65cd4..e8dd2f0 100644
--- a/src/ignite-template.js
+++ b/src/ignite-template.js
@@ -1,5 +1,4 @@
 import { IgniteProperty } from './ignite-html.js';
-import { IgniteElement } from './ignite-element.js';
 
 class IgniteTemplate {
     constructor(items) {
@@ -468,7 +467,7 @@ class html extends IgniteTemplate {
         this.elements.forEach((item) => item.remove());
         this.elements = [];
 
-        //Reconstruct them html
+        //Reconstruct the html
         this.construct(null, null);
     }
 }
@@ -479,10 +478,7 @@ class list extends IgniteTemplate {
         this.tagName = "shadow list";
 
         if (list instanceof IgniteProperty) {
-            this.callbacks.push(list.attach((oldValue, newValue) => {
-                this.list = newValue;
-                this.construct(null); //If the list changed, reconstruct this template.
-            }));
+            this.callbacks.push(list.attach((oldValue, newValue) => this.onListChanged(oldValue, newValue)));
 
             this.list = list.value;
         } else {
@@ -539,6 +535,18 @@ class list extends IgniteTemplate {
             }
         }
     }
+
+    onListChanged(oldValue, newValue) {
+        this.list = newValue;
+
+        IgniteRenderingContext.enter();
+
+        try {
+            this.construct(null); //The list changed, reconstruct this template.
+        } catch { }
+
+        IgniteRenderingContext.leave();
+    }
 }
 
 class slot extends IgniteTemplate {