Add global flags to suppress unnecessary notification events. Fixes #…

…4262. * `Polymer.Settings.suppressTemplateNotifications `- disables `dom-change` and `rendered-item-count` events from `dom-if`, `dom-repeat`, and `don-bind`. Users can opt back into `dom-change` events by setting the `notify-dom-change` attribute (`notifyDomChange: true` property) to `dom-if`/`don-repeat` instances. * `Polymer.Settings.suppressBindingNotifications` - disables notify effects when propagating data downward via bindings. Generally these are never useful unless users are explicitly doing something like `<my-el foo="{{foo}} on-foo-changed="{{handleFoo}}">` or calling `addEventListener('foo-changed', ...)` on an element where `foo` is bound (we attempted to make this the default some time back but needed to revert it when we found via #3077 that users were indeed doing this). Users that avoid these patterns can enjoy the potentially significant benefit of suppressing unnecessary events during downward data flow by opting into this flag.
Polymer · Jan 20, 2017 · 83e14c4 · 83e14c4
1 parent 345e2c5
commit 83e14c4
Show file tree

Hide file tree

Showing 9 changed files with 109 additions and 29 deletions.
diff --git a/src/lib/template/dom-bind.html b/src/lib/template/dom-bind.html
@@ -168,7 +168,9 @@
         this._children = Polymer.TreeApi.arrayCopyChildNodes(this.root);
       }
       this._insertChildren();
-      this.fire('dom-change');
+      if (!Polymer.Settings.suppressTemplateNotifications) {
+        this.fire('dom-change');
+      }
     }
 
   });

diff --git a/src/lib/template/dom-if.html b/src/lib/template/dom-if.html
@@ -60,6 +60,15 @@
         type: Boolean,
         value: false,
         observer: '_queueRender'
+      },
+
+      /**
+       * When the global `Polymer.Settings.suppressDomChange` setting is used,
+       * setting `notifyDomChange: true` will enable firing `dom-change` events
+       * on this element.
+       */
+      notifyDomChange: {
+        type: Boolean
       }
 
     },
@@ -114,7 +123,9 @@
         this._showHideChildren();
       }
       if (this.if != this._lastIf) {
-        this.fire('dom-change');
+        if (!Polymer.Settings.suppressTemplateNotifications || this.notifyDomChange) {
+          this.fire('dom-change');
+        }
         this._lastIf = this.if;
       }
     },

diff --git a/src/lib/template/dom-repeat.html b/src/lib/template/dom-repeat.html
@@ -211,7 +211,7 @@
        */
       renderedItemCount: {
         type: Number,
-        notify: true,
+        notify: !Polymer.Settings.suppressTemplateNotifications,
         readOnly: true
       },
 
@@ -240,6 +240,15 @@
         value: 20
       },
 
+      /**
+       * When the global `Polymer.Settings.suppressDomChange` setting is used,
+       * setting `notifyDomChange: true` will enable firing `dom-change` events
+       * on this element.
+       */
+      notifyDomChange: {
+        type: Boolean
+      },
+
       _targetFrameTime: {
         type: Number,
         computed: '_computeFrameTime(targetFramerate)'
@@ -470,7 +479,9 @@
       // Set rendered item count
       this._setRenderedItemCount(this._instances.length);
       // Notify users
-      this.fire('dom-change');
+      if (!Polymer.Settings.suppressTemplateNotifications || this.notifyDomChange) {
+        this.fire('dom-change');
+      }
       // Check to see if we need to render more items
       this._tryRenderChunk();
     },

diff --git a/src/lib/template/templatizer.html b/src/lib/template/templatizer.html
@@ -254,6 +254,8 @@
               effect: {event:
                 Polymer.CaseMap.camelToDashCase(parentProp) + '-changed'}
             }];
+            proto._propertyEffects = proto._propertyEffects || {};
+            proto._propertyEffects[parentProp] = effects;
             Polymer.Bind._createAccessors(proto, parentProp, effects);
           }
         }
@@ -282,7 +284,7 @@
     _createHostPropEffector: function(prop) {
       var prefix = this._parentPropPrefix;
       return function(source, value) {
-        this.dataHost._templatized[prefix + prop] = value;
+        this.dataHost._templatized.__setProperty(prefix + prop, value);
       };
     },
 
@@ -305,10 +307,16 @@
       }
       for (var i=0, n; (i<n$.length) && (n=n$[i]); i++) {
         var val = template[n];
-        var pd = Object.getOwnPropertyDescriptor(proto, n);
-        Object.defineProperty(template, n, pd);
-        if (val !== undefined) {
-          template._propertySetter(n, val);
+        if (val && n == '_propertyEffects') {
+          // Merge property effects in
+          var pe = Polymer.Base.mixin({}, val);
+          template._propertyEffects = Polymer.Base.mixin(pe, proto._propertyEffects);
+        } else {
+          var pd = Object.getOwnPropertyDescriptor(proto, n);
+          Object.defineProperty(template, n, pd);
+          if (val !== undefined) {
+            template._propertySetter(n, val);
+          }   
         }
       }
     },

diff --git a/src/standard/effectBuilder.html b/src/standard/effectBuilder.html
@@ -332,10 +332,11 @@
         if (pinfo && pinfo.readOnly) {
           return;
         }
-        // Ideally we would call setProperty using fromAbove: true to avoid
-        // spinning the wheel needlessly, but we found that users were listening
-        // for change events outside of bindings
-        this.__setProperty(property, value, false, node);
+        // Downward data-flow via bindings uses `fromAbove: true` if the
+        // global `suppressBindingNotifications` opt-in flag is set as a
+        // perf optimization to avoid needless event dispatch cost
+        this.__setProperty(property, value,
+          Polymer.Settings.suppressBindingNotifications, node);
       }
     },
 

diff --git a/test/runner.html b/test/runner.html
@@ -42,6 +42,7 @@
       'unit/polymer-dom-observeNodes.html?dom=shadow',
       'unit/debounce.html',
       'unit/bind.html',
+      'unit/bind.html?suppressBindingNotifications&suppressTemplateNotifications',
       'unit/bind.html?dom=shadow',
       'unit/notify-path.html',
       'unit/path.html',
@@ -83,7 +84,9 @@
       'unit/templatizer.html',
       'unit/template-preserve-content.html',
       'unit/dom-repeat.html',
+      'unit/dom-repeat.html?suppressBindingNotifications&suppressTemplateNotifications',
       'unit/dom-if.html',
+      'unit/dom-if.html?suppressBindingNotifications&suppressTemplateNotifications',
       'unit/dom-template.html',
       'unit/dom-bind.html',
       'unit/dom-bind-yield.html',

diff --git a/test/unit/bind.html b/test/unit/bind.html
@@ -505,14 +505,16 @@
     assert.equal(el.boundreadonlyvalue, 46, 'property bound to read-only property should change from change to bound value');
   });
 
-  test('listener for value-changed fires when value changed from host', function() {
-    var listener = sinon.spy();
-    el.$.basic1.addEventListener('notifyingvalue-changed', listener);
-    el.boundnotifyingvalue = 678;
-    assert.equal(el.$.basic1.notifyingvalue, 678);
-    assert.isTrue(listener.calledOnce);
-    assert.equal(listener.getCalls()[0].args[0].detail.value, 678);
-  });
+  if (!Polymer.Settings.suppressBindingNotifications) {
+    test('listener for value-changed fires when value changed from host', function() {
+      var listener = sinon.spy();
+      el.$.basic1.addEventListener('notifyingvalue-changed', listener);
+      el.boundnotifyingvalue = 678;
+      assert.equal(el.$.basic1.notifyingvalue, 678);
+      assert.isTrue(listener.calledOnce);
+      assert.equal(listener.getCalls()[0].args[0].detail.value, 678);
+    });    
+  }
 
   test('negated binding update negates value for parent', function() {
     assert.equal(el.negatedValue, false);

diff --git a/test/unit/dom-if.html b/test/unit/dom-if.html
@@ -337,7 +337,8 @@
         var stamped = Polymer.dom(unconfigured1.root).querySelectorAll('*:not(template):not(span)');
         assert.equal(stamped.length, 3, 'total stamped count incorrect');
         stamped[0].prop = 'outer';
-        assert.equal(unconfigured1.domUpdateHandlerCount, 1);
+        assert.equal(unconfigured1.domUpdateHandlerCount, 
+          Polymer.Settings.suppressTemplateNotifications ? 0 : 1);
       });
 
       test('if=false, restamp=false: everything hidden', function() {
@@ -351,7 +352,8 @@
         stamped.forEach(function(n) {
           assert.equal(getComputedStyle(n).display, 'none', 'node was not hidden but should have been');
         });
-        assert.equal(unconfigured1.domUpdateHandlerCount, 1);
+        assert.equal(unconfigured1.domUpdateHandlerCount, 
+          Polymer.Settings.suppressTemplateNotifications ? 0 : 1);
       });
 
       test('if=true, restamp=true, everything rendered and visible', function() {
@@ -366,7 +368,8 @@
         stamped.forEach(function(n) {
           assert.equal(getComputedStyle(n).display, 'inline', 'node was hidden but should not have been');
         });
-        assert.equal(unconfigured1.domUpdateHandlerCount, 1);
+        assert.equal(unconfigured1.domUpdateHandlerCount, 
+          Polymer.Settings.suppressTemplateNotifications ? 0 : 1);
       });
 
       test('if=false, restamp=true, everything gone', function() {
@@ -377,9 +380,11 @@
         CustomElements.takeRecords();
         CustomElements.takeRecords();
         var stamped = Polymer.dom(unconfigured1.root).querySelectorAll('*:not(template)');
-        assert.equal(unconfigured1.domUpdateHandlerCount, 1);
+        assert.equal(unconfigured1.domUpdateHandlerCount, 
+          Polymer.Settings.suppressTemplateNotifications ? 0 : 1);
         assert.equal(stamped.length, 0, 'total stamped count incorrect');
-        assert.equal(unconfigured1.domUpdateHandlerCount, 1);
+        assert.equal(unconfigured1.domUpdateHandlerCount, 
+          Polymer.Settings.suppressTemplateNotifications ? 0 : 1);
       });
 
       // repeat, just to get everything rendered again...
@@ -393,7 +398,8 @@
         var stamped = Polymer.dom(unconfigured1.root).querySelectorAll('*:not(template):not(span)');
         assert.equal(stamped.length, 3, 'total stamped count incorrect');
         stamped[0].prop = 'outer';
-        assert.equal(unconfigured1.domUpdateHandlerCount, 1);
+        assert.equal(unconfigured1.domUpdateHandlerCount, 
+          Polymer.Settings.suppressTemplateNotifications ? 0 : 1);
       });
 
       test('parent scope binding', function() {
@@ -689,6 +695,22 @@
 
     });
 
+    suite('notification suppression', function() {
+
+      test('re-enable dom-change', function() {
+        if (Polymer.Settings.suppressTemplateNotifications) {
+          test('enable dom-change on instance', function() {
+            unconfigured1.$['if-1'].setAttribute('notify-dom-change', '');
+            unconfigured1.domUpdateHandlerCount = 0;
+            unconfigured.shouldStamp = false;
+            setTimeout(function() {
+              assert.equal(unconfigured1.domUpdateHandlerCount, 1);
+            });
+          });        
+        }
+      });
+
+    });
 
   </script>
 

diff --git a/test/unit/dom-repeat.html b/test/unit/dom-repeat.html
@@ -830,7 +830,8 @@ <h4>x-repeat-chunked</h4>
           assert.equal(stamped2[39].itemaProp, 'new-1');
           assert.equal(stamped2[40].itemaProp, 'new-2');
           assert.equal(stamped2[41], undefined);
-          assert.equal(unconfigured1.domUpdateHandlerCount, 1);
+          assert.equal(unconfigured1.domUpdateHandlerCount, 
+            Polymer.Settings.suppressTemplateNotifications ? 0 : 1);
           done();
         });
       });
@@ -853,7 +854,8 @@ <h4>x-repeat-chunked</h4>
           assert.equal(stamped2[38].indexb, 2);
           assert.equal(stamped2[38].indexc, 2);
           assert.equal(stamped2[39], undefined);
-          assert.equal(unconfigured1.domUpdateHandlerCount, 1);
+          assert.equal(unconfigured1.domUpdateHandlerCount, 
+            Polymer.Settings.suppressTemplateNotifications ? 0 : 1);
           done();
         });
       });
@@ -3250,6 +3252,7 @@ <h4>x-repeat-chunked</h4>
           });
         });
       });
+
     });
 
     suite('repeater API', function() {
@@ -3867,6 +3870,23 @@ <h4>x-repeat-chunked</h4>
 
       });
 
+      suite('notification suppression', function() {
+
+        test('re-enable dom-change', function() {
+          if (Polymer.Settings.suppressTemplateNotifications) {
+            test('enable dom-change on instance', function() {
+              unconfigured1.$.repeater.setAttribute('notify-dom-change', '');
+              unconfigured1.domUpdateHandlerCount = 0;
+              unconfigured.items = unconfigured.items.slice();
+              setTimeout(function() {
+                assert.equal(unconfigured1.domUpdateHandlerCount, 1);
+              });
+            });        
+          }
+        });
+
+      });
+
     }
 
   </script>