Use stronger check for PropertyEffects clients. Fixes #5017

Polymer · Feb 8, 2018 · e6d558e · e6d558e
1 parent 0e564ae
commit e6d558e
Show file tree

Hide file tree

Showing 11 changed files with 140 additions and 30 deletions.
diff --git a/lib/mixins/property-effects.html b/lib/mixins/property-effects.html
@@ -567,6 +567,7 @@
     // e.g.: foo="{{obj.sub}}", path: 'obj.sub.prop', set 'foo.prop'=obj.sub.prop
     if (hasPaths && part.source && (path.length > part.source.length) &&
         (binding.kind == 'property') && !binding.isCompound &&
+        node.__isPropertyEffectsClient &&
         node.__dataHasAccessor && node.__dataHasAccessor[binding.target]) {
       let value = props[path];
       path = Polymer.Path.translate(part.source, binding.target, path);
@@ -603,7 +604,8 @@
     } else {
       // Property binding
       let prop = binding.target;
-      if (node.__dataHasAccessor && node.__dataHasAccessor[prop]) {
+      if (node.__isPropertyEffectsClient &&
+          node.__dataHasAccessor && node.__dataHasAccessor[prop]) {
         if (!node[TYPES.READ_ONLY] || !node[TYPES.READ_ONLY][prop]) {
           if (node._setPendingProperty(prop, value)) {
             inst._enqueueClient(node);
@@ -1131,6 +1133,9 @@
 
       constructor() {
         super();
+        /** @type {boolean} */
+        // Used to identify users of this mixin, ala instanceof
+        this.__isPropertyEffectsClient = true;
         /** @type {number} */
         // NOTE: used to track re-entrant calls to `_flushProperties`
         // path changes dirty check against `__dataTemp` only during one "turn"

diff --git a/test/unit/path-effects-elements.html b/test/unit/path-effects-elements.html
@@ -7,6 +7,32 @@
 Code distributed by Google as part of the polymer project is also
 subject to an additional IP rights grant found at http://polymer.github.io/PATENTS.txt
 -->
+
+<link rel="import" href="../../lib/mixins/properties-mixin.html">
+
+<script>
+  class XPropertiesElement extends Polymer.PropertiesMixin(HTMLElement) {
+    static get properties() {
+      return {
+        obj: Object
+      };
+    }
+    constructor() {
+      super();
+      this.resetObservers();
+    }
+    // Allow re-set objects to show in _propertiesChanged
+    _shouldPropertyChange(property, value, old) {
+      return (typeof value == 'object') ||
+        super._shouldPropertyChange(property, value, old);
+    }
+    resetObservers() {
+      this._propertiesChanged = sinon.spy();
+    }
+  }
+  customElements.define('x-pe', XPropertiesElement);
+</script>
+
 <script>
   Polymer({
     is: 'x-basic',
@@ -103,6 +129,7 @@
     <x-compose id="compose" obj="{{nested.obj}}"></x-compose>
     <x-forward id="forward" obj="{{nested.obj}}"></x-forward>
     <div id="boundChild" computed-from-paths="{{computeFromPaths(a, nested.b, nested.obj.c)}}" array-length="{{data.length}}"></div>
+    <x-pe id="pe" obj="[[nested.obj]]"></x-pe>
   </template>
   <script>
     Polymer({
@@ -161,6 +188,7 @@
         if (this.$) {
           this.$.compose.resetObservers();
           this.$.forward.resetObservers();
+          this.$.pe.resetObservers();
         }
       },
       computeFromPaths: function(a, b, c) {

diff --git a/test/unit/path-effects.html b/test/unit/path-effects.html
@@ -49,7 +49,7 @@
     document.body.removeChild(el);
   });
 
-  function verifyObserverOutput(nestingLevel, nested) {
+  async function verifyObserverOutput(nestingLevel, nested) {
     assert.equal(el.computed, '[' + nested.obj.value + ']');
     assert.equal(el.nestedChanged.callCount, nestingLevel == 0 ? 1 : 0);
     if (nestingLevel == 0) {
@@ -92,6 +92,7 @@
     assert.equal(el.$.compose.$.basic2.getAttribute('attrvalue'), '42');
     assert.equal(el.$.forward.$.compose.$.basic1.getAttribute('attrvalue'), '42');
     assert.equal(el.$.forward.$.compose.$.basic2.getAttribute('attrvalue'), '42');
+    assert.equal(el.$.pe.obj, nested.obj);
     switch (nestingLevel) {
       case 0:
         assert.equal(el.nestedSubpathChanged.firstCall.args[0].path, 'nested');
@@ -104,6 +105,9 @@
         assert.equal(el.$.forward.objSubpathChanged.firstCall.args[0].value, nested.obj);
         assert.equal(el.$.forward.$.compose.objSubpathChanged.firstCall.args[0].path, 'obj');
         assert.equal(el.$.forward.$.compose.objSubpathChanged.firstCall.args[0].value, nested.obj);
+        await Promise.resolve(); // x-pe uses async PropertiesChanged
+        assert.equal(el.$.pe._propertiesChanged.callCount, 1);
+        assert.equal(el.$.pe._propertiesChanged.firstCall.args[1].obj, nested.obj);
         break;
       case 1:
         assert.equal(el.nestedSubpathChanged.firstCall.args[0].path, 'nested.obj');
@@ -116,6 +120,9 @@
         assert.equal(el.$.forward.objSubpathChanged.firstCall.args[0].value, nested.obj);
         assert.equal(el.$.forward.$.compose.objSubpathChanged.firstCall.args[0].path, 'obj');
         assert.equal(el.$.forward.$.compose.objSubpathChanged.firstCall.args[0].value, nested.obj);
+        await Promise.resolve(); // x-pe uses async PropertiesChanged
+        assert.equal(el.$.pe._propertiesChanged.callCount, 1);
+        assert.equal(el.$.pe._propertiesChanged.firstCall.args[1].obj, nested.obj);
         break;
       case 2:
         assert.equal(el.nestedSubpathChanged.firstCall.args[0].path, 'nested.obj.value');
@@ -128,22 +135,27 @@
         assert.equal(el.$.forward.objSubpathChanged.firstCall.args[0].value, 42);
         assert.equal(el.$.forward.$.compose.objSubpathChanged.firstCall.args[0].path, 'obj.value');
         assert.equal(el.$.forward.$.compose.objSubpathChanged.firstCall.args[0].value, 42);
+        await Promise.resolve(); // x-pe uses async PropertiesChanged
+        // Note the PropertiesMixin element only sees a deep set to 'nested.obj.value'
+        // because it overrides _shouldPropertyChange to allow object re-sets through
+        assert.equal(el.$.pe._propertiesChanged.callCount, 1);
+        assert.equal(el.$.pe._propertiesChanged.firstCall.args[1].obj, nested.obj);
         break;
     }
   }
 
-  test('downward data flow', function() {
+  test('downward data flow', async function() {
     // Do the thing
     el.nested = {
       obj: {
         value: 42
       }
     };
     // Verify
-    verifyObserverOutput(0, el.nested);
+    await verifyObserverOutput(0, el.nested);
   });
 
-  test('notification from basic element property change', function() {
+  test('notification from basic element property change', async function() {
     // Setup
     el.nested = {
       obj: {
@@ -154,10 +166,10 @@
     // Do the thing
     el.$.basic.notifyingValue = 42;
     // Verify
-    verifyObserverOutput(2, el.nested);
+    await verifyObserverOutput(2, el.nested);
   });
 
-  test('notification from composed element property change', function() {
+  test('notification from composed element property change', async function() {
     // Setup
     el.nested = {
       obj: {
@@ -168,10 +180,10 @@
     // Do the thing
     el.$.compose.$.basic1.notifyingValue = 42;
     // Verify
-    verifyObserverOutput(2, el.nested);
+    await verifyObserverOutput(2, el.nested);
   });
 
-  test('notification from forward\'s composed element property change', function() {
+  test('notification from forward\'s composed element property change', async function() {
     // Setup
     el.nested = {
       obj: {
@@ -182,10 +194,10 @@
     // Do the thing
     el.$.forward.$.compose.$.basic1.notifyingValue = 42;
     // Verify
-    verifyObserverOutput(2, el.nested);
+    await verifyObserverOutput(2, el.nested);
   });
 
-  test('notification from set in top element', function() {
+  test('notification from set in top element', async function() {
     // Setup
     el.nested = {
       obj: {
@@ -196,10 +208,10 @@
     // Do the thing
     el.set('nested.obj.value', 42);
     // Verify
-    verifyObserverOutput(2, el.nested);
+    await verifyObserverOutput(2, el.nested);
   });
 
-  test('notification from set in composed element', function() {
+  test('notification from set in composed element', async function() {
     // Setup
     el.nested = {
       obj: {
@@ -210,10 +222,10 @@
     // Do the thing
     el.$.compose.set('obj.value', 42);
     // Verify
-    verifyObserverOutput(2, el.nested);
+    await verifyObserverOutput(2, el.nested);
   });
 
-  test('notification from set in forward element', function() {
+  test('notification from set in forward element', async function() {
     // Setup
     el.nested = {
       obj: {
@@ -224,10 +236,10 @@
     // Do the thing
     el.$.forward.set('obj.value', 42);
     // Verify
-    verifyObserverOutput(2, el.nested);
+    await verifyObserverOutput(2, el.nested);
   });
 
-  test('notification from set in forward\'s composed element', function() {
+  test('notification from set in forward\'s composed element', async function() {
     // Setup
     el.nested = {
       obj: {
@@ -238,10 +250,10 @@
     // Do the thing
     el.$.forward.$.compose.set('obj.value', 42);
     // Verify
-    verifyObserverOutput(2, el.nested);
+    await verifyObserverOutput(2, el.nested);
   });
 
-  test('notification from object change in compose element', function() {
+  test('notification from object change in compose element', async function() {
     // Setup
     el.nested = {
       obj: {
@@ -254,10 +266,10 @@
       value: 42
     };
     // Verify
-    verifyObserverOutput(1, el.nested);
+    await verifyObserverOutput(1, el.nested);
   });
 
-  test('notification from object change in forward element', function() {
+  test('notification from object change in forward element', async function() {
     // Setup
     el.nested = {
       obj: {
@@ -270,10 +282,10 @@
       value: 42
     };
     // Verify
-    verifyObserverOutput(1, el.nested);
+    await verifyObserverOutput(1, el.nested);
   });
 
-  test('notification from object change in forward\'s compose element', function() {
+  test('notification from object change in forward\'s compose element', async function() {
     // Setup
     el.nested = {
       obj: {
@@ -287,7 +299,7 @@
       value: 42
     };
     // Verify
-    verifyObserverOutput(1, el.nested);
+    await verifyObserverOutput(1, el.nested);
   });
 
   test('cached paths get invalidated by object sets', function() {

diff --git a/types/externs/closure-types.d.ts b/types/externs/closure-types.d.ts
@@ -0,0 +1,10 @@
+/**
+ * DO NOT EDIT
+ *
+ * This file was automatically generated by
+ *   https://github.com/Polymer/gen-typescript-declarations
+ *
+ * To modify these typings, edit the source file(s):
+ *   externs/closure-types.js
+ */
+
diff --git a/types/externs/polymer-externs.d.ts b/types/externs/polymer-externs.d.ts
@@ -0,0 +1,10 @@
+/**
+ * DO NOT EDIT
+ *
+ * This file was automatically generated by
+ *   https://github.com/Polymer/gen-typescript-declarations
+ *
+ * To modify these typings, edit the source file(s):
+ *   externs/polymer-externs.js
+ */
+
diff --git a/types/externs/polymer-internal-shared-types.d.ts b/types/externs/polymer-internal-shared-types.d.ts
@@ -0,0 +1,10 @@
+/**
+ * DO NOT EDIT
+ *
+ * This file was automatically generated by
+ *   https://github.com/Polymer/gen-typescript-declarations
+ *
+ * To modify these typings, edit the source file(s):
+ *   externs/polymer-internal-shared-types.js
+ */
+
diff --git a/types/externs/webcomponents-externs.d.ts b/types/externs/webcomponents-externs.d.ts
@@ -0,0 +1,10 @@
+/**
+ * DO NOT EDIT
+ *
+ * This file was automatically generated by
+ *   https://github.com/Polymer/gen-typescript-declarations
+ *
+ * To modify these typings, edit the source file(s):
+ *   externs/webcomponents-externs.js
+ */
+
diff --git a/types/gulpfile.d.ts b/types/gulpfile.d.ts
@@ -0,0 +1,18 @@
+/**
+ * DO NOT EDIT
+ *
+ * This file was automatically generated by
+ *   https://github.com/Polymer/gen-typescript-declarations
+ *
+ * To modify these typings, edit the source file(s):
+ *   gulpfile.js
+ */
+
+declare class BackfillStream {
+  _transform(file: any, enc: any, cb: any): any;
+  _flush(cb: any): any;
+}
+
+declare class AddClosureTypeImport {
+  _transform(file: any, enc: any, cb: any): any;
+}
diff --git a/types/lib/legacy/mutable-data-behavior.d.ts b/types/lib/legacy/mutable-data-behavior.d.ts
@@ -66,8 +66,6 @@ declare namespace Polymer {
     _shouldPropertyChange(property: string, value: any, old: any): boolean;
   }
 
-  const MutableDataBehavior: object;
-
   /**
    * Legacy element behavior to add the optional ability to skip strict
    * dirty-checking for objects and arrays (always consider them to be
@@ -130,6 +128,4 @@ declare namespace Polymer {
      */
     _shouldPropertyChange(property: string, value: any, old: any): boolean;
   }
-
-  const OptionalMutableDataBehavior: object;
 }
diff --git a/types/lib/legacy/templatizer-behavior.d.ts b/types/lib/legacy/templatizer-behavior.d.ts
@@ -111,6 +111,4 @@ declare namespace Polymer {
      */
     modelForElement(el: HTMLElement|null): TemplateInstanceBase|null;
   }
-
-  const Templatizer: object;
 }
diff --git a/types/util/minimalDocument.d.ts b/types/util/minimalDocument.d.ts
@@ -0,0 +1,13 @@
+/**
+ * DO NOT EDIT
+ *
+ * This file was automatically generated by
+ *   https://github.com/Polymer/gen-typescript-declarations
+ *
+ * To modify these typings, edit the source file(s):
+ *   util/minimalDocument.js
+ */
+
+declare class MinimalDocTransform {
+  _transform(file: any, enc: any, cb: any): any;
+}