Merge pull request #88 from valtech-nyc/feature/update-patch-as-obser…

…vable

Add blacklist to patchAsObservable

src/render/patchAsObservable.js

@@ -7,12 +7,20 @@ const { createNode, NodeCache, memory, decodeEntities, escape } = Internals;
 const { protectVTree, unprotectVTree } = memory;
 
 const blockText = new Set(['script', 'noscript', 'style', 'code', 'template']);
+const blacklist = new Set();
 
 const removeAttribute = (domNode, name) => {
     domNode.removeAttribute(name);
 
-    if (name in domNode) {
-        domNode[name] = undefined;
+    // Runtime checking if the property can be set.
+    const blacklistName = domNode.nodeName + '-' + name;
+
+    if (!blacklist.has(blacklistName)) {
+        try {
+            domNode[name] = undefined;
+        } catch (unhandledException) {
+            blacklist.add(blacklistName);
+        }
     }
 };
 
@@ -36,18 +44,26 @@ export default function patchAsObservable(patches) {
             // Events must be lowercased otherwise they will not be set correctly.
             const name = attribute.indexOf('on') === 0 ? attribute.toLowerCase() : attribute;
 
+            // Runtime checking if the property can be set.
+            const blacklistName = vTree.nodeName + '-' + name;
+
             let effect$ = Kefir.never();
 
             // Normal attribute value.
             if (!isObject && !isFunction && name) {
                 effect$ = Kefir.stream(emitter => {
                     const noValue = newValue === null || newValue === undefined;
 
-                    // Allow the user to find the real value in the DOM Node as a
-                    // property.
-                    try {
-                        domNode[name] = newValue;
-                    } catch (unhandledException) {} // eslint-disable-line no-empty
+                    // If the property has not been blacklisted then use try/catch to try
+                    // and set it.
+                    if (!blacklist.has(blacklistName)) {
+                        try {
+                            domNode[name] = newValue;
+                        } catch (unhandledException) {
+                            blacklist.add(blacklistName);
+                        }
+                    }
+
 
                     // Set the actual attribute, this will ensure attributes like
                     // `autofocus` aren't reset by the property call above.
@@ -68,6 +84,15 @@ export default function patchAsObservable(patches) {
                 });
             } else if (typeof newValue !== 'string') {
                 effect$ = Kefir.stream(emitter => {
+                    // Since this is a property value it gets set directly on the node.
+                    if (!blacklist.has(blacklistName)) {
+                        try {
+                            domNode[name] = newValue;
+                        } catch (unhandledException) {
+                            blacklist.add(blacklistName);
+                        }
+                    }
+
                     // We remove and re-add the attribute to trigger a change in a web
                     // component or mutation observer. Although you could use a setter or
                     // proxy, this is more natural.
@@ -78,10 +103,18 @@ export default function patchAsObservable(patches) {
                     // Necessary to track the attribute/prop existence.
                     domNode.setAttribute(name, '');
 
-                    // Since this is a property value it gets set directly on the node.
-                    try {
-                        domNode[name] = newValue;
-                    } catch (unhandledException) {} // eslint-disable-line no-empty
+                    // FIXME This is really unfortunate, but after trigger a change via
+                    // attr, we need to reset the actual value in the instance for things
+                    // like event handlers. In the future it would be great to limit this
+                    // to actual attr -> prop keys. Custom attributes do not suffer from
+                    // this problem as they are not translated.
+                    if (!blacklist.has(blacklistName)) {
+                        try {
+                            domNode[name] = newValue;
+                        } catch (unhandledException) {
+                            blacklist.add(blacklistName);
+                        }
+                    }
 
                     emitter.end();
                 });