v-for: call detached hook for instances in removed fragments (fix vuejs#1440)

yyx990803 · yyx990803 · commit d58c7e40b5a2 · 2015-10-17T22:58:42.000-04:00
diff --git a/src/directives/for.js b/src/directives/for.js
@@ -154,7 +154,6 @@ module.exports = {
       frag = oldFrags[i]
       if (!frag.reused) {
         this.deleteCachedFrag(frag)
-        frag.destroy()
         this.remove(frag, removalIndex++, totalRemoved, inDoc)
       }
     }
@@ -321,11 +320,11 @@ module.exports = {
     if (inDoc && staggerAmount) {
       var op = frag.staggerCb = _.cancellable(function () {
         frag.staggerCb = null
-        frag.remove()
+        frag.remove(true)
       })
       setTimeout(op, staggerAmount)
     } else {
-      frag.remove()
+      frag.remove(true)
     }
   },
 
diff --git a/src/directives/if.js b/src/directives/if.js
@@ -40,8 +40,7 @@ module.exports = {
 
   insert: function () {
     if (this.elseFrag) {
-      this.elseFrag.remove()
-      this.elseFrag.destroy()
+      this.elseFrag.remove(true)
       this.elseFrag = null
     }
     this.frag = this.factory.create(this._host, this._scope, this._frag)
@@ -50,8 +49,7 @@ module.exports = {
 
   remove: function () {
     if (this.frag) {
-      this.frag.remove()
-      this.frag.destroy()
+      this.frag.remove(true)
       this.frag = null
     }
     if (this.elseFactory) {
diff --git a/src/fragment/fragment.js b/src/fragment/fragment.js
@@ -83,33 +83,36 @@ function singleBefore (target, trans) {
 
 /**
  * Remove fragment, single node version
+ *
+ * @param {Boolean} [destroy]
  */
 
-function singleRemove () {
+function singleRemove (destroy) {
   var shouldCallRemove = _.inDoc(this.node)
-  transition.remove(this.node, this.vm)
-  this.inserted = false
-  if (shouldCallRemove) {
-    this.callHook(detach)
-  }
+  var self = this
+  transition.remove(this.node, this.vm, function () {
+    self.inserted = false
+    if (shouldCallRemove) {
+      self.callHook(detach)
+    }
+    if (destroy) {
+      self.destroy()
+    }
+  })
 }
 
 /**
  * Insert fragment before target, multi-nodes version
  *
  * @param {Node} target
- * @param {Boolean} trans
  */
 
-function multiBefore (target, trans) {
+function multiBefore (target) {
   _.before(this.node, target)
   var nodes = this.nodes
   var vm = this.vm
-  var method = trans !== false
-    ? transition.before
-    : _.before
   for (var i = 0, l = nodes.length; i < l; i++) {
-    method(nodes[i], target, vm)
+    _.before(nodes[i], target, vm)
   }
   _.before(this.end, target)
   this.inserted = true
@@ -120,9 +123,11 @@ function multiBefore (target, trans) {
 
 /**
  * Remove fragment, multi-nodes version
+ *
+ * @param {Boolean} [destroy]
  */
 
-function multiRemove () {
+function multiRemove (destroy) {
   var shouldCallRemove = _.inDoc(this.node)
   var parent = this.node.parentNode
   var node = this.node.nextSibling
@@ -132,7 +137,7 @@ function multiRemove () {
   while (node !== this.end) {
     nodes.push(node)
     next = node.nextSibling
-    transition.remove(node, vm)
+    parent.removeChild(node, vm)
     node = next
   }
   parent.removeChild(this.node)
@@ -141,6 +146,9 @@ function multiRemove () {
   if (shouldCallRemove) {
     this.callHook(detach)
   }
+  if (destroy) {
+    this.destroy()
+  }
 }
 
 /**
diff --git a/test/unit/specs/directives/for/for_spec.js b/test/unit/specs/directives/for/for_spec.js
@@ -774,6 +774,43 @@ if (_.inBrowser) {
       })
     })
 
+    it('call attach/detach for contained components', function (done) {
+      document.body.appendChild(el)
+      var attachSpy = jasmine.createSpy('attach')
+      var detachSpy = jasmine.createSpy('detach')
+      var vm = new Vue({
+        el: el,
+        template: '<test v-for="item in items"></test>',
+        data: {
+          items: [1, 2]
+        },
+        components: {
+          test: {
+            attached: attachSpy,
+            detached: detachSpy
+          }
+        }
+      })
+      expect(attachSpy.calls.count()).toBe(2)
+      expect(detachSpy.calls.count()).toBe(0)
+      vm.items.push(3)
+      _.nextTick(function () {
+        expect(attachSpy.calls.count()).toBe(3)
+        expect(detachSpy.calls.count()).toBe(0)
+        vm.items.pop()
+        _.nextTick(function () {
+          expect(attachSpy.calls.count()).toBe(3)
+          expect(detachSpy.calls.count()).toBe(1)
+          vm.items = []
+          _.nextTick(function () {
+            expect(attachSpy.calls.count()).toBe(3)
+            expect(detachSpy.calls.count()).toBe(3)
+            done()
+          })
+        })
+      })
+    })
+
   })
 }
 
