Merge pull request #80 from Losant/master

Fix del for null/undefined props
mariocasciaro · Feb 27, 2017 · 4fc16a0 · 4fc16a0
2 parents 098d48b + 5df3264
commit 4fc16a0
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 9 deletions.
diff --git a/index.js b/index.js
@@ -83,8 +83,12 @@
       }, {});
     };
 
+    function hasShallowProperty(obj, prop) {
+      return (options.includeInheritedProps || (typeof prop === 'number' && Array.isArray(obj)) || hasOwnProperty(obj, prop))
+    }
+
     function getShallowProperty(obj, prop) {
-      if (options.includeInheritedProps || (typeof prop === 'number' && Array.isArray(obj)) || hasOwnProperty(obj, prop)) {
+      if (hasShallowProperty(obj, prop)) {
         return obj[prop];
       }
     }
@@ -186,7 +190,7 @@
         value.length = 0;
       } else if (isObject(value)) {
         for (i in value) {
-          if (hasOwnProperty(value, i)) {
+          if (hasShallowProperty(value, i)) {
             delete value[i];
           }
         }
@@ -261,9 +265,8 @@
       }
 
       var currentPath = getKey(path[0]);
-      var currentVal = getShallowProperty(obj, currentPath);
-      if(currentVal == null) {
-        return currentVal;
+      if (!hasShallowProperty(obj, currentPath)) {
+        return obj;
       }
 
       if(path.length === 1) {
@@ -273,9 +276,7 @@
           delete obj[currentPath];
         }
       } else {
-        if (obj[currentPath] !== void 0) {
-          return objectPath.del(obj[currentPath], path.slice(1));
-        }
+        return objectPath.del(obj[currentPath], path.slice(1));
       }
 
       return obj;

diff --git a/test.js b/test.js
@@ -385,6 +385,9 @@ describe('empty', function(){
     obj.path = true;
 
     expect(objectPath.empty(obj, 'inexistant')).to.equal(void 0);
+
+    expect(objectPath.empty(null, 'path')).to.equal(void 0);
+    expect(objectPath.empty(void 0, 'path')).to.equal(void 0);
   });
 
   it('should empty each path according to their types', function(){
@@ -407,7 +410,9 @@ describe('empty', function(){
           some:'property',
           sub: {
             'property': true
-          }
+          },
+          nullProp: null,
+          undefinedProp: void 0
         },
         instance: new Instance()
       };
@@ -421,6 +426,17 @@ describe('empty', function(){
     objectPath.empty(obj, 'object.sub');
     expect(obj.object.sub).to.deep.equal({});
 
+    objectPath.empty(obj, 'object.nullProp');
+    expect(obj.object.nullProp).to.equal(null);
+
+    objectPath.empty(obj, 'object.undefinedProp');
+    expect(obj.object.undefinedProp).to.equal(void 0);
+    expect(obj.object).to.have.property('undefinedProp');
+
+    objectPath.empty(obj, 'object.notAProp');
+    expect(obj.object.notAProp).to.equal(void 0);
+    expect(obj.object).to.not.have.property('notAProp');
+
     objectPath.empty(obj, 'instance.test');
     //instance.test is not own property, so it shouldn't be emptied
     expect(obj.instance.test).to.be.a('function');
@@ -452,6 +468,24 @@ describe('del', function(){
     expect(obj.b.d).to.deep.equal(['a']);
   });
 
+  it('should remove null and undefined props (but not explode on nested)', function(){
+    var obj = { nullProp: null, undefinedProp: void 0 };
+    expect(obj).to.have.property('nullProp');
+    expect(obj).to.have.property('undefinedProp');
+
+    objectPath.del(obj, 'nullProp.foo');
+    objectPath.del(obj, 'undefinedProp.bar');
+    expect(obj).to.have.property('nullProp');
+    expect(obj).to.have.property('undefinedProp');
+    expect(obj).to.deep.equal({ nullProp: null, undefinedProp: void 0 });
+
+    objectPath.del(obj, 'nullProp');
+    objectPath.del(obj, 'undefinedProp');
+    expect(obj).to.not.have.property('nullProp');
+    expect(obj).to.not.have.property('undefinedProp');
+    expect(obj).to.deep.equal({});
+  });
+
   it('should delete deep paths', function(){
     var obj = getTestObj();