make sure a vm and its data are properly removed from Array when $des…

…troy is called directly
vuejs · Feb 7, 2014 · efcaad1 · efcaad1
1 parent e37b151
commit efcaad1
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 16 deletions.
diff --git a/README.md b/README.md
@@ -6,7 +6,7 @@
 
 ## Introduction
 
-Vue.js is a library for building interactive web interfaces. It provides the benefits of MVVM data binding with a simple and flexible API. You should try it out if you like:
+Vue.js is a library for building interactive web interfaces. It provides the benefits of MVVM data binding and a composable component system with a simple and flexible API. You should try it out if you like:
 
 - Extendable Data bindings
 - Plain JavaScript objects as models

diff --git a/src/compiler.js b/src/compiler.js
@@ -639,6 +639,10 @@ CompilerProto.parseDeps = function () {
  */
 CompilerProto.destroy = function () {
 
+    // avoid being called more than once
+    // this is irreversible!
+    if (this.destroyed) return
+
     var compiler = this,
         i, key, dir, instances, binding,
         vm          = compiler.vm,
@@ -697,6 +701,7 @@ CompilerProto.destroy = function () {
         vm.$remove()
     }
 
+    this.destroyed = true
     // emit destroy hook
     compiler.execHook('afterDestroy')
 

diff --git a/src/directives/repeat.js b/src/directives/repeat.js
@@ -107,7 +107,7 @@ module.exports = {
             var method = mutation.method
             mutationHandlers[method].call(self, mutation)
             if (method !== 'push' && method !== 'pop') {
-                self.updateIndexes()
+                self.updateIndex()
             }
             if (method === 'push' || method === 'unshift' || method === 'splice') {
                 self.changed()
@@ -169,32 +169,33 @@ module.exports = {
      */
     buildItem: function (data, index) {
 
-        var node    = this.el.cloneNode(true),
-            ctn     = this.container,
+        var el  = this.el.cloneNode(true),
+            ctn = this.container,
+            vms = this.vms,
+            col = this.collection,
             ref, item
 
         // append node into DOM first
         // so v-if can get access to parentNode
         if (data) {
-            ref = this.vms.length > index
-                ? this.vms[index].$el
+            ref = vms.length > index
+                ? vms[index].$el
                 : this.ref
             // make sure it works with v-if
             if (!ref.parentNode) ref = ref.vue_ref
             // process transition info before appending
-            node.vue_trans = utils.attr(node, 'transition', true)
-            transition(node, 1, function () {
-                ctn.insertBefore(node, ref)
+            el.vue_trans = utils.attr(el, 'transition', true)
+            transition(el, 1, function () {
+                ctn.insertBefore(el, ref)
             }, this.compiler)
         }
 
         item = new this.Ctor({
-            el: node,
+            el: el,
             data: data,
             compilerOptions: {
                 repeat: true,
                 repeatIndex: index,
-                repeatCollection: this.collection,
                 parentCompiler: this.compiler,
                 delegator: ctn
             }
@@ -205,14 +206,19 @@ module.exports = {
             // let's remove it...
             item.$destroy()
         } else {
-            this.vms.splice(index, 0, item)
+            vms.splice(index, 0, item)
+            // in case `$destroy` is called directly on a repeated vm
+            // make sure the vm's data is properly removed
+            item.$compiler.observer.on('hook:afterDestroy', function () {
+                col.remove(data)
+            })
         }
     },
 
     /**
      *  Update index of each item after a mutation
      */
-    updateIndexes: function () {
+    updateIndex: function () {
         var i = this.vms.length
         while (i--) {
             this.vms[i].$data.$index = i

diff --git a/test/functional/fixtures/repeated-items.html b/test/functional/fixtures/repeated-items.html
@@ -1,7 +1,7 @@
 <!DOCTYPE html>
 <html lang="en">
     <head>
-        <title>SEED repeated items</title>
+        <title>Vue.js repeated items</title>
         <meta charset="utf-8">
         <script src="../../../dist/vue.js"></script>
     </head>
@@ -20,7 +20,7 @@
             </p>
             <p>Total items: <span class="count" v-text="items.length"></span></p>
             <ul>
-                <li class="item" v-repeat="items">
+                <li class="item" v-repeat="items" v-on="click:this.$destroy()">
                     {{$index}} {{title}}
                 </li>
             </ul>

diff --git a/test/functional/specs/repeated-items.js b/test/functional/specs/repeated-items.js
@@ -1,4 +1,6 @@
-casper.test.begin('Repeated Items', 41, function (test) {
+/* global items */
+
+casper.test.begin('Repeated Items', 44, function (test) {
 
     casper
     .start('./fixtures/repeated-items.html')
@@ -81,6 +83,13 @@ casper.test.begin('Repeated Items', 41, function (test) {
         test.assertSelectorHasText('.item:nth-child(1)', '0 6')
         test.assertSelectorHasText('.item:nth-child(2)', '1 7')
     })
+    .thenClick('.item:nth-child(1)', function () {
+        test.assertSelectorHasText('.count', '1')
+        test.assertSelectorHasText('.item:nth-child(1)', '0 7')
+        test.assertEval(function () {
+            return items.length === 1 && items[0].title === '7'
+        })
+    })
     .run(function () {
         test.done()
     })