Ensure List resize cleans up referenced nodes.

Fixes immutable-js#540, and includes referenced tests
donabrams · Jul 14, 2015 · c7c4ddc · c7c4ddc
1 parent 12ad749
commit c7c4ddc
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 19 deletions.
diff --git a/__tests__/List.ts b/__tests__/List.ts
@@ -525,6 +525,26 @@ describe('List', () => {
     expect(v3.get(1800)).toBe(undefined);
   });
 
+  it('discards truncated elements when using slice', () => {
+    var list = [1,2,3,4,5,6];
+    var v1 = Immutable.fromJS(list);
+    var v2 = v1.slice(0,3);
+    var v3 = v2.setSize(6);
+
+    expect(v2.toArray()).toEqual([1,2,3]);
+    expect(v3.toArray()).toEqual([1,2,3,undefined,undefined,undefined]);
+  });
+
+  it('discards truncated elements when using setSize', () => {
+    var list = [1,2,3,4,5,6];
+    var v1 = Immutable.fromJS(list);
+    var v2 = v1.setSize(3);
+    var v3 = v2.setSize(6);
+
+    expect(v2.toArray()).toEqual([1,2,3]);
+    expect(v3.toArray()).toEqual([1,2,3,undefined,undefined,undefined]);
+  });
+
   it('can be efficiently sliced', () => {
     var v1 = Immutable.Range(0,2000).toList();
     var v2 = v1.slice(100,-100).toList();

diff --git a/dist/immutable.js b/dist/immutable.js
@@ -2828,29 +2828,25 @@
     };
 
     VNode.prototype.removeAfter = function(ownerID, level, index) {
-      if (index === level ? 1 << level : 0 || this.array.length === 0) {
+      if (index === (level ? 1 << level : 0) || this.array.length === 0) {
         return this;
       }
       var sizeIndex = ((index - 1) >>> level) & MASK;
       if (sizeIndex >= this.array.length) {
         return this;
       }
-      var removingLast = sizeIndex === this.array.length - 1;
+
       var newChild;
       if (level > 0) {
         var oldChild = this.array[sizeIndex];
         newChild = oldChild && oldChild.removeAfter(ownerID, level - SHIFT, index);
-        if (newChild === oldChild && removingLast) {
+        if (newChild === oldChild && sizeIndex === this.array.length - 1) {
           return this;
         }
       }
-      if (removingLast && !newChild) {
-        return this;
-      }
+
       var editable = editableVNode(this, ownerID);
-      if (!removingLast) {
-        editable.array.pop();
-      }
+      editable.array.splice(sizeIndex + 1);
       if (newChild) {
         editable.array[sizeIndex] = newChild;
       }

diff --git a/dist/immutable.min.js b/dist/immutable.min.js
diff --git a/src/List.js b/src/List.js
@@ -258,29 +258,25 @@ class VNode {
   }
 
   removeAfter(ownerID, level, index) {
-    if (index === level ? 1 << level : 0 || this.array.length === 0) {
+    if (index === (level ? 1 << level : 0) || this.array.length === 0) {
       return this;
     }
     var sizeIndex = ((index - 1) >>> level) & MASK;
     if (sizeIndex >= this.array.length) {
       return this;
     }
-    var removingLast = sizeIndex === this.array.length - 1;
+
     var newChild;
     if (level > 0) {
       var oldChild = this.array[sizeIndex];
       newChild = oldChild && oldChild.removeAfter(ownerID, level - SHIFT, index);
-      if (newChild === oldChild && removingLast) {
+      if (newChild === oldChild && sizeIndex === this.array.length - 1) {
         return this;
       }
     }
-    if (removingLast && !newChild) {
-      return this;
-    }
+
     var editable = editableVNode(this, ownerID);
-    if (!removingLast) {
-      editable.array.pop();
-    }
+    editable.array.splice(sizeIndex + 1);
     if (newChild) {
       editable.array[sizeIndex] = newChild;
     }