Refactor to make code more readable, add tests, remove dead code.

Polymer · Feb 1, 2019 · c78f679 · c78f679
1 parent 4d99099
commit c78f679
Show file tree

Hide file tree

Showing 3 changed files with 131 additions and 20 deletions.
diff --git a/lib/mixins/property-effects.js b/lib/mixins/property-effects.js
@@ -964,6 +964,19 @@ 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];
+  }
+}
+
 // data api
 
 /**
@@ -982,8 +995,6 @@ function notifySplices(inst, array, path, splices) {
   let splicesPath = path + '.splices';
   inst.notifyPath(splicesPath, { indexSplices: splices });
   inst.notifyPath(path + '.length', array.length);
-  // Null here to allow potentially large splice records to be GC'ed.
-  inst.__data[splicesPath] = {indexSplices: null};
 }
 
 /**
@@ -2143,26 +2154,14 @@ export const PropertyEffects = dedupingMixin(superClass => {
       const values = [];
       for (let i=0, l=args.length; i<l; i++) {
         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 {
-            value = data[name];
-          }
+          value = getArgValue(data, props, structured, name);
           if (wildcard) {
+            const matches = isDescendant(name, path);
             value = {
-              path: argPath,
-              value,
-              base: argPath === path ? get(data, name) : value
+              path: matches ? path : name,
+              value: matches ? getArgValue(data, props, structured, path) : value,
+              base: value
             };
           }
         }

diff --git a/test/unit/path-effects-elements.js b/test/unit/path-effects-elements.js
@@ -162,6 +162,9 @@ Polymer({
     'aChanged(a.*)',
     'xChanged(x.*)',
     'yChanged(y.*)',
+    'abChanged(a.b.*)',
+    'abcChanged(a.b.c.*)',
+    'abcxChanged(a.b.c.*, x)'
   ],
 
   created: function() {
@@ -190,6 +193,9 @@ Polymer({
       this.$.forward.resetObservers();
       this.$.pe.resetObservers();
     }
+    this.abChanged = sinon.spy();
+    this.abcChanged = sinon.spy();
+    this.abcxChanged = sinon.spy();
   },
 
   computeFromPaths: function(a, b, c) {
@@ -309,4 +315,4 @@ Polymer({
   created: function() {
     this.objChanged = sinon.spy();
   }
-});
+});
diff --git a/test/unit/path-effects.html b/test/unit/path-effects.html
@@ -341,6 +341,112 @@
     assert.equal(el.$.compose.$.basic1.othervalue, 98);
   });
 
+  suite('ancestors and descendants', function() {
+
+    test('change base', function() {
+      let c = {}, b = {c}, a = {b};
+      el.a = a;
+      assert.equal(el.aChanged.callCount, 1);
+      assert.deepEqual(el.aChanged.firstCall.args[0], {path: 'a', base: a, value: a});
+      assert.equal(el.abChanged.callCount, 1);
+      assert.deepEqual(el.abChanged.firstCall.args[0], {path: 'a.b', base: b, value: b});
+      assert.equal(el.abcChanged.callCount, 1);
+      assert.deepEqual(el.abcChanged.firstCall.args[0], {path: 'a.b.c', base: c, value: c});
+      assert.equal(el.abcxChanged.callCount, 1);
+      assert.deepEqual(el.abcxChanged.firstCall.args[0], {path: 'a.b.c', base: c, value: c});
+    });
+
+    test('change subpath level 1', function() {
+      let c = {}, b = {c}, a = {b};
+      el.a = a;
+      el.resetObservers();
+      c = {},
+      b = {c};
+      el.set('a.b', b);
+      assert.equal(el.aChanged.callCount, 1);
+      assert.deepEqual(el.aChanged.firstCall.args[0], {path: 'a.b', base: a, value: b});
+      assert.equal(el.abChanged.callCount, 1);
+      assert.deepEqual(el.abChanged.firstCall.args[0], {path: 'a.b', base: b, value: b});
+      assert.equal(el.abcChanged.callCount, 1);
+      assert.deepEqual(el.abcChanged.firstCall.args[0], {path: 'a.b.c', base: c, value: c});
+      assert.equal(el.abcxChanged.callCount, 1);
+      assert.deepEqual(el.abcxChanged.firstCall.args[0], {path: 'a.b.c', base: c, value: c});
+    });
+
+    test('change subpath level 2', function() {
+      let c = {}, b = {c}, a = {b};
+      el.a = a;
+      el.resetObservers();
+      c = {},
+      el.set('a.b.c', c);
+      assert.equal(el.aChanged.callCount, 1);
+      assert.deepEqual(el.aChanged.firstCall.args[0], {path: 'a.b.c', base: a, value: c});
+      assert.equal(el.abChanged.callCount, 1);
+      assert.deepEqual(el.abChanged.firstCall.args[0], {path: 'a.b.c', base: b, value: c});
+      assert.equal(el.abcChanged.callCount, 1);
+      assert.deepEqual(el.abcChanged.firstCall.args[0], {path: 'a.b.c', base: c, value: c});
+      assert.equal(el.abcxChanged.callCount, 1);
+      assert.deepEqual(el.abcxChanged.firstCall.args[0], {path: 'a.b.c', base: c, value: c});
+    });
+
+    test('change subpath of base only', function() {
+      let c = {}, b = {c}, a = {b};
+      el.a = a;
+      el.resetObservers();
+      let d = {};
+      el.set('a.d', d);
+      assert.equal(el.aChanged.callCount, 1);
+      assert.deepEqual(el.aChanged.firstCall.args[0], {path: 'a.d', base: a, value: d});
+      assert.equal(el.abChanged.callCount, 0);
+      assert.equal(el.abcChanged.callCount, 0);
+      assert.equal(el.abcxChanged.callCount, 0);
+    });
+
+    test('change subpath of level 1 only', function() {
+      let c = {}, b = {c}, a = {b};
+      el.a = a;
+      el.resetObservers();
+      let e = {};
+      el.set('a.b.e', e);
+      assert.equal(el.aChanged.callCount, 1);
+      assert.deepEqual(el.aChanged.firstCall.args[0], {path: 'a.b.e', base: a, value: e});
+      assert.equal(el.abChanged.callCount, 1);
+      assert.deepEqual(el.abChanged.firstCall.args[0], {path: 'a.b.e', base: b, value: e});
+      assert.equal(el.abcChanged.callCount, 0);
+      assert.equal(el.abcxChanged.callCount, 0);
+    });
+
+    test('change subpath of level 2', function() {
+      let c = {}, b = {c}, a = {b};
+      el.a = a;
+      el.resetObservers();
+      let f = {};
+      el.set('a.b.c.f', f);
+      assert.equal(el.aChanged.callCount, 1);
+      assert.deepEqual(el.aChanged.firstCall.args[0], {path: 'a.b.c.f', base: a, value: f});
+      assert.equal(el.abChanged.callCount, 1);
+      assert.deepEqual(el.abChanged.firstCall.args[0], {path: 'a.b.c.f', base: b, value: f});
+      assert.equal(el.abcChanged.callCount, 1);
+      assert.deepEqual(el.abcChanged.firstCall.args[0], {path: 'a.b.c.f', base: c, value: f});
+      assert.equal(el.abcxChanged.callCount, 1);
+      assert.deepEqual(el.abcxChanged.firstCall.args[0], {path: 'a.b.c.f', base: c, value: f});
+    });
+
+    test('change non-wildcard argument', function() {
+      let c = {}, b = {c}, a = {b};
+      el.a = a;
+      el.resetObservers();
+      let x = el.x = {};
+      assert.equal(el.aChanged.callCount, 0);
+      assert.equal(el.abChanged.callCount, 0);
+      assert.equal(el.abcChanged.callCount, 0);
+      assert.equal(el.abcxChanged.callCount, 1);
+      assert.deepEqual(el.abcxChanged.firstCall.args[0], {path: 'a.b.c', base: c, value: c});
+      assert.equal(el.abcxChanged.firstCall.args[1], x);
+    });
+
+  });
+
 });
 
 suite('path effects', function() {