Ensure that marshalArgs pulls wildcard info value from __data

It currently pulls the value from `changedProps` rather than __data, meaning it could provide stale data for re-entrant changes.
Polymer · Jan 31, 2019 · 4d99099 · 4d99099
1 parent 2e19ae0
commit 4d99099
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 26 deletions.
diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js
@@ -2140,37 +2140,33 @@ export const PropertyEffects = dedupingMixin(superClass => {
      */
     _marshalArgs(args, path, props) {
       const data = this.__data;
-      let values = [];
+      const values = [];
       for (let i=0, l=args.length; i<l; i++) {
-        let arg = args[i];
-        let name = arg.name;
-        let v;
-        if (arg.literal) {
-          v = arg.value;
-        } else {
-          if (arg.structured) {
-            v = get(data, name);
-            // when data is not stored e.g. `splices`
-            if (v === undefined) {
-              v = props[name];
+        let {name, structured, wildcard, value, literal} = args[i];
+        let argPath = name;
+        if (!literal) {
+          if (structured) {
+            // If the triggered path matches the wildcard, use that path
+            if (wildcard && (path === name || isDescendant(name, path))) {
+              argPath = path;
+            }
+            value = get(data, argPath);
+            // when data is not stored e.g. `splices`, get the changed value
+            if (value === undefined) {
+              value = props[argPath];
             }
           } else {
-            v = data[name];
+            value = data[name];
+          }
+          if (wildcard) {
+            value = {
+              path: argPath,
+              value,
+              base: argPath === path ? get(data, name) : value
+            };
           }
         }
-        if (arg.wildcard) {
-          // Only send the actual path changed info if the change that
-          // caused the observer to run matched the wildcard
-          let baseChanged = (name.indexOf(path + '.') === 0);
-          let matches = (path.indexOf(name) === 0 && !baseChanged);
-          values[i] = {
-            path: matches ? path : name,
-            value: matches ? props[path] : v,
-            base: v
-          };
-        } else {
-          values[i] = v;
-        }
+        values[i] = value;
       }
       return values;
     }

diff --git a/test/unit/path-effects-elements.js b/test/unit/path-effects-elements.js
@@ -245,6 +245,42 @@ Polymer({
     this.objChanged = sinon.spy();
   }
 });
+
+Polymer({
+
+  is: 'x-reentry-wildcard',
+
+  properties: {
+    array: {
+      type: Array,
+      computed: 'computeArray(trigger, size)'
+    },
+    size: {
+      type: Number
+    },
+    trigger: {
+      type: Boolean,
+      observer: 'triggerChanged',
+      value: true
+    }
+  },
+
+  observers: ['arrayChanged(array.*)'],
+
+  created() {
+    this.arrayChanged = sinon.spy();
+  },
+
+  computeArray(trigger, size) {
+    return new Array(size || 0);
+  },
+
+  triggerChanged() {
+    this.size = 1;
+  }
+
+});
+
 Polymer({
   is: 'x-path-client',
   observers: ['objChanged(obj.*)'],

diff --git a/test/unit/path-effects.html b/test/unit/path-effects.html
@@ -1099,6 +1099,17 @@
     assert.equal(host.objChanged.secondCall.args[0].value, 1);
   });
 
+  test('reentry after wildcard observer queued', function() {
+    let el = document.createElement('x-reentry-wildcard');
+    document.body.appendChild(el);
+    assert.equal(el.arrayChanged.callCount, 2);
+    assert.equal(el.arrayChanged.firstCall.args[0].base.length, 1);
+    assert.equal(el.arrayChanged.firstCall.args[0].value.length, 1);
+    assert.equal(el.arrayChanged.secondCall.args[0].base.length, 1);
+    assert.equal(el.arrayChanged.secondCall.args[0].value.length, 1);
+    document.body.removeChild(el);
+  });
+
   test('element without notify effects still notifies paths (1.x guarantee)', function() {
     let host = document.createElement('x-path-host');
     let listener = sinon.spy();