Updates based on review.

Polymer · Feb 4, 2019 · 98304fb · 98304fb
1 parent 2480b25
commit 98304fb
Show file tree

Hide file tree

Showing 3 changed files with 183 additions and 32 deletions.
diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js
@@ -964,17 +964,13 @@ function parseArg(rawArg) {
   return a;
 }
 
-function getArgValue(data, props, structured, path) {
-  if (structured) {
-    let value = get(data, path);
-    // when data is not stored e.g. `splices`, get the changed value
-    if (value === undefined) {
-      value = props[path];
-    }
-    return value;
-  } else {
-    return data[path];
+function getArgValue(data, props, path) {
+  let value = get(data, path);
+  // when data is not stored e.g. `splices`, get the changed value
+  if (value === undefined) {
+    value = props[path];
   }
+  return value;
 }
 
 // data api
@@ -992,8 +988,7 @@ function getArgValue(data, props, structured, path) {
  * @private
  */
 function notifySplices(inst, array, path, splices) {
-  let splicesPath = path + '.splices';
-  inst.notifyPath(splicesPath, { indexSplices: splices });
+  inst.notifyPath(path + '.splices', { indexSplices: splices });
   inst.notifyPath(path + '.length', array.length);
 }
 
@@ -2155,14 +2150,16 @@ export const PropertyEffects = dedupingMixin(superClass => {
       for (let i=0, l=args.length; i<l; i++) {
         let {name, structured, wildcard, value, literal} = args[i];
         if (!literal) {
-          value = getArgValue(data, props, structured, name);
           if (wildcard) {
             const matches = isDescendant(name, path);
+            const pathValue = getArgValue(data, props, matches ? path : name);
             value = {
               path: matches ? path : name,
-              value: matches ? getArgValue(data, props, structured, path) : value,
-              base: value
+              value: pathValue,
+              base: matches ? get(data, name) : pathValue
             };
+          } else {
+            value = structured ? getArgValue(data, props, name) : data[name];
           }
         }
         values[i] = value;

diff --git a/test/unit/path-effects-elements.js b/test/unit/path-effects-elements.js
@@ -254,15 +254,50 @@ Polymer({
 
 Polymer({
 
-  is: 'x-reentry-wildcard',
+  is: 'x-reentry-true',
 
   properties: {
-    array: {
-      type: Array,
-      computed: 'computeArray(trigger, size)'
+    prop: {
+      type: Boolean,
+      computed: 'computeProp(trigger, value)'
+    },
+    value: {
+      type: Boolean
+    },
+    trigger: {
+      type: Boolean,
+      observer: 'triggerChanged',
+      value: true
+    }
+  },
+
+  observers: ['propChanged(prop.*)'],
+
+  created() {
+    this.propChanged = sinon.spy();
+  },
+
+  computeProp(trigger, value) {
+    return Boolean(value);
+  },
+
+  triggerChanged() {
+    this.value = true;
+  }
+
+});
+
+Polymer({
+
+  is: 'x-reentry-undefined',
+
+  properties: {
+    prop: {
+      type: Boolean,
+      computed: 'computeProp(trigger, value)'
     },
-    size: {
-      type: Number
+    value: {
+      type: Boolean
     },
     trigger: {
       type: Boolean,
@@ -271,18 +306,93 @@ Polymer({
     }
   },
 
-  observers: ['arrayChanged(array.*)'],
+  observers: ['propChanged(prop.*)'],
 
   created() {
-    this.arrayChanged = sinon.spy();
+    this.propChanged = sinon.spy();
   },
 
-  computeArray(trigger, size) {
-    return new Array(size || 0);
+  computeProp(trigger, value) {
+    return value ? undefined : true;
   },
 
   triggerChanged() {
-    this.size = 1;
+    this.value = true;
+  }
+
+});
+
+Polymer({
+
+  is: 'x-reentry-undefined-path',
+
+  properties: {
+    prop: {
+      type: Object
+    },
+    value: {
+      type: Boolean
+    },
+    trigger: {
+      type: Boolean,
+      value: true
+    }
+  },
+
+  observers: [
+    'computeProp(trigger, value)',
+    'triggerChanged(trigger)',
+    'propChanged(prop.sub.*)'
+  ],
+
+  created() {
+    this.propChanged = sinon.spy(function() {
+      this.debug = true;
+    });
+  },
+
+  computeProp(trigger, value) {
+    if (value) {
+      this.set('prop.sub', undefined);
+    } else {
+      this.prop = {sub: true};
+    }
+  },
+
+  triggerChanged() {
+    this.value = true;
+  }
+
+});
+
+Polymer({
+
+  is: 'x-reentry-splices',
+
+  properties: {
+    array: {
+      type: Array,
+      value() { return []; }
+    }
+  },
+
+  observers: [
+    'changeArray(array.splices.*)',
+    'arrayChanged(array.splices.*)'
+  ],
+
+  created() {
+    this.arrayChanged = sinon.spy(function() {
+      this.debug = true;
+    });
+  },
+
+  changeArray() {
+    if (this.array.length === 0) {
+      this.push('array', 'one');
+    } else if (this.array.length === 1) {
+      this.push('array', 'two')
+    }
   }
 
 });

diff --git a/test/unit/path-effects.html b/test/unit/path-effects.html
@@ -1206,13 +1206,57 @@
   });
 
   test('reentry after wildcard observer queued', function() {
-    let el = document.createElement('x-reentry-wildcard');
+    let el = document.createElement('x-reentry-true');
     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);
+    assert.equal(el.propChanged.callCount, 2);
+    assert.equal(el.propChanged.firstCall.args[0].base, true);
+    assert.equal(el.propChanged.firstCall.args[0].value, true);
+    assert.equal(el.propChanged.secondCall.args[0].base, true);
+    assert.equal(el.propChanged.secondCall.args[0].value, true);
+    document.body.removeChild(el);
+  });
+
+  // TODO(kschaaf): address code in `getArgValue` that looks in changedProps
+  // when `undefined` for splices, which breaks this test (latent issue)
+  test.skip('reentry after prop goes back to undefined', function() {
+    let el = document.createElement('x-reentry-undefined');
+    document.body.appendChild(el);
+    assert.equal(el.propChanged.callCount, 2);
+    assert.equal(el.propChanged.firstCall.args[0].base, undefined);
+    assert.equal(el.propChanged.firstCall.args[0].value, undefined);
+    assert.equal(el.propChanged.secondCall.args[0].base, undefined);
+    assert.equal(el.propChanged.secondCall.args[0].value, undefined);
+    document.body.removeChild(el);
+  });
+
+  // TODO(kschaaf): address code in `getArgValue` that looks in changedProps
+  // when `undefined` for splices, which breaks this test (latent issue)
+  test.skip('reentry after path goes back to undefined', function() {
+    let el = document.createElement('x-reentry-undefined-path');
+    document.body.appendChild(el);
+    assert.equal(el.propChanged.callCount, 2);
+    assert.equal(el.propChanged.firstCall.args[0].base, undefined);
+    assert.equal(el.propChanged.firstCall.args[0].value, undefined);
+    assert.equal(el.propChanged.secondCall.args[0].base, undefined);
+    assert.equal(el.propChanged.secondCall.args[0].value, undefined);
+    document.body.removeChild(el);
+  });
+
+  test('reentry from array splices', function() {
+    let el = document.createElement('x-reentry-splices');
+    document.body.appendChild(el);
+    assert.equal(el.arrayChanged.callCount, 3);
+    assert.deepEqual(el.arrayChanged.getCalls()[0].args[0].value, {
+      indexSplices: [{ 
+        index: 1, addedCount: 1, removed: [], object: el.array, type: 'splice'
+      }]
+    });
+    assert.deepEqual(el.arrayChanged.getCalls()[1].args[0].value, {
+      indexSplices: [{ 
+        index: 0, addedCount: 1, removed: [], object: el.array, type: 'splice'
+      }]
+    });
+    assert.deepEqual(el.arrayChanged.getCalls()[2].args[0].value, undefined);
     document.body.removeChild(el);
   });