make v-repeat work with primitive values + minor directive refactor

yyx990803 · yyx990803 · commit 5ea44fbfe4a3 · 2014-02-10T14:11:59.000-05:00
diff --git a/src/compiler.js b/src/compiler.js
@@ -653,14 +653,18 @@ CompilerProto.destroy = function () {
 
     compiler.execHook('beforeDestroy')
 
+    // unobserve data
+    Observer.unobserve(compiler.data, '', compiler.observer)
+
     // unbind all direcitves
     i = directives.length
     while (i--) {
         dir = directives[i]
         // if this directive is an instance of an external binding
         // e.g. a directive that refers to a variable on the parent VM
         // we need to remove it from that binding's instances
-        if (!dir.isEmpty && dir.binding.compiler !== compiler) {
+        // * empty and literal bindings do not have binding.
+        if (dir.binding && dir.binding.compiler !== compiler) {
             instances = dir.binding.instances
             if (instances) instances.splice(instances.indexOf(dir), 1)
         }
@@ -673,13 +677,10 @@ CompilerProto.destroy = function () {
         exps[i].unbind()
     }
 
-    // unbind/unobserve all own bindings
+    // unbind all own bindings
     for (key in bindings) {
         binding = bindings[key]
         if (binding) {
-            if (binding.root) {
-                Observer.unobserve(binding.value, binding.key, compiler.observer)
-            }
             binding.unbind()
         }
     }
diff --git a/src/directive.js b/src/directive.js
@@ -152,16 +152,12 @@ DirProto.applyFilters = function (value) {
 
 /**
  *  Unbind diretive
- *  @ param {Boolean} update
- *    Sometimes we call unbind before an update (i.e. not destroy)
- *    just to teardown previous stuff, in that case we do not want
- *    to null everything.
  */
-DirProto.unbind = function (update) {
+DirProto.unbind = function () {
     // this can be called before the el is even assigned...
-    if (!this.el) return
-    if (this._unbind) this._unbind(update)
-    if (!update) this.vm = this.el = this.binding = this.compiler = null
+    if (!this.el || !this.vm) return
+    if (this._unbind) this._unbind()
+    this.vm = this.el = this.binding = this.compiler = null
 }
 
 // exposed methods ------------------------------------------------------------
diff --git a/src/directives/on.js b/src/directives/on.js
@@ -22,7 +22,7 @@ module.exports = {
     },
 
     update: function (handler) {
-        this.unbind(true)
+        this.reset()
         if (typeof handler !== 'function') {
             return utils.warn('Directive "on" expects a function value.')
         }
@@ -72,9 +72,13 @@ module.exports = {
         }
     },
 
-    unbind: function (update) {
+    reset: function () {
         this.el.removeEventListener(this.arg, this.handler)
         this.handler = null
-        if (!update) this.el.vue_viewmodel = null
+    },
+
+    unbind: function () {
+        this.reset()
+        this.el.vue_viewmodel = null
     }
 }
diff --git a/src/directives/repeat.js b/src/directives/repeat.js
@@ -118,7 +118,7 @@ module.exports = {
 
     update: function (collection, init) {
 
-        this.unbind(true)
+        this.reset()
         // attach an object to container to hold handlers
         this.container.vue_dHandlers = utils.hash()
         // if initiating with an empty collection, we need to
@@ -173,7 +173,7 @@ module.exports = {
             ctn = this.container,
             vms = this.vms,
             col = this.collection,
-            ref, item
+            ref, item, primitive
 
         // append node into DOM first
         // so v-if can get access to parentNode
@@ -188,6 +188,11 @@ module.exports = {
             transition(el, 1, function () {
                 ctn.insertBefore(el, ref)
             }, this.compiler)
+            // wrap primitive element in an object
+            if (utils.typeOf(data) !== 'Object') {
+                primitive = true
+                data = { value: data }
+            }
         }
 
         item = new this.Ctor({
@@ -207,6 +212,14 @@ module.exports = {
             item.$destroy()
         } else {
             vms.splice(index, 0, item)
+            // for primitive values, listen for value change
+            if (primitive) {
+                data.__observer__.on('set', function (key, val) {
+                    if (key === 'value') {
+                        col[item.$index] = val
+                    }
+                })
+            }
             // 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 () {
@@ -225,7 +238,7 @@ module.exports = {
         }
     },
 
-    unbind: function () {
+    reset: function () {
         if (this.childId) {
             delete this.vm.$[this.childId]
         }
@@ -242,5 +255,9 @@ module.exports = {
             ctn.removeEventListener(handlers[key].event, handlers[key])
         }
         ctn.vue_dHandlers = null
+    },
+
+    unbind: function () {
+        this.reset()
     }
 }
diff --git a/test/functional/fixtures/repeated-primitive.html b/test/functional/fixtures/repeated-primitive.html
@@ -0,0 +1,14 @@
+<div id="test">
+    <p v-repeat="numbers" v-on="click: value+=' modified'">{{value}}</p>
+</div>
+
+<script src="../../../dist/vue.js"></script>
+<script>
+    var numbers = [1, 2, 'text']
+    new Vue({
+        el: '#test',
+        data: {
+            numbers: numbers
+        }
+    })
+</script>
diff --git a/test/functional/specs/repeated-primitive.js b/test/functional/specs/repeated-primitive.js
@@ -0,0 +1,37 @@
+/* global numbers */
+
+casper.test.begin('Repeated Primitives', 8, function (test) {
+    
+    casper
+    .start('./fixtures/repeated-primitive.html')
+    .then(function () {
+
+        test.assertSelectorHasText('p:nth-child(1)', '1')
+        test.assertSelectorHasText('p:nth-child(2)', '2')
+        test.assertSelectorHasText('p:nth-child(3)', 'text')
+
+    })
+    // click everything to test event handlers (delegated)
+    .thenClick('p:nth-child(1)', function () {
+        test.assertSelectorHasText('p:nth-child(1)', '1 modified')
+    })
+    .thenClick('p:nth-child(2)', function () {
+        test.assertSelectorHasText('p:nth-child(2)', '2 modified')
+    })
+    .thenClick('p:nth-child(3)', function () {
+        test.assertSelectorHasText('p:nth-child(3)', 'text modified')
+    })
+    // more clicks
+    .thenClick('p:nth-child(1)', function () {
+        test.assertSelectorHasText('p:nth-child(1)', '1 modified modified')
+    })
+    .then(function () {
+        test.assertEvalEquals(function () {
+            return numbers
+        }, ['1 modified modified', '2 modified', 'text modified'])
+    })
+    .run(function () {
+        test.done()
+    })
+
+})
diff --git a/test/unit/specs/directive.js b/test/unit/specs/directive.js
@@ -289,16 +289,17 @@ describe('UNIT: Directive', function () {
             assert.strictEqual(unbound, false)
         })
 
-        it('should call _unbind() if it has an element', function () {
+        it('should call _unbind() and null everything if it has an element', function () {
             d.el = true
-            d.unbind(true)
+            d.unbind()
             assert.strictEqual(unbound, true)
-            assert.ok(d.el)
+            assert.ok(d.el === null && d.vm === null && d.binding === null && d.compiler === null)
         })
 
-        it('should null everything if it\'s called for VM destruction', function () {
+        it('should not execute if called more than once', function () {
+            unbound = false
             d.unbind()
-            assert.ok(d.el === null && d.vm === null && d.binding === null && d.compiler === null)
+            assert.notOk(unbound)
         })
 
     })
diff --git a/test/unit/specs/viewmodel.js b/test/unit/specs/viewmodel.js
@@ -370,7 +370,7 @@ describe('UNIT: ViewModel', function () {
             dirUnbindCalled = false,
             expUnbindCalled = false,
             bindingUnbindCalled = false,
-            unobserveCalled = 0,
+            unobserveCalled = false,
             elRemoved = false
 
         var dirMock = {
@@ -388,14 +388,6 @@ describe('UNIT: ViewModel', function () {
             test: {
                 root: true,
                 key: 'test',
-                value: {
-                    __observer__: {
-                        off: function () {
-                            unobserveCalled++
-                            return this
-                        }
-                    }
-                },
                 unbind: function () {
                     bindingUnbindCalled = true
                 }
@@ -411,12 +403,21 @@ describe('UNIT: ViewModel', function () {
                     afterDestroyCalled = true
                 }
             },
+            data: {
+                __observer__: {
+                    off: function () {
+                        unobserveCalled = true
+                        return this
+                    }
+                }
+            },
             observer: {
                 off: function () {
                     observerOffCalled = true
                 },
                 proxies: {
-                    'test.': {}
+                    'test.': {},
+                    '': {}
                 }
             },
             emitter: {
@@ -464,6 +465,10 @@ describe('UNIT: ViewModel', function () {
             assert.ok(emitterOffCalled)
         })
 
+        it('should unobserve the data', function () {
+            assert.ok(unobserveCalled)
+        })
+
         it('should unbind all directives', function () {
             assert.ok(dirUnbindCalled)
         })
@@ -478,7 +483,6 @@ describe('UNIT: ViewModel', function () {
 
         it('should unbind and unobserve own bindings', function () {
             assert.ok(bindingUnbindCalled)
-            assert.strictEqual(unobserveCalled, 3)
         })
 
         it('should remove self from parentCompiler', function () {

Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,7 @@ module.exports = {`
`22`	`22`	`},`
`23`	`23`
`24`	`24`	`update: function (handler) {`
`25`		`- this.unbind(true)`
	`25`	`+ this.reset()`
`26`	`26`	`if (typeof handler !== 'function') {`
`27`	`27`	`return utils.warn('Directive "on" expects a function value.')`
`28`	`28`	`}`
`@@ -72,9 +72,13 @@ module.exports = {`
`72`	`72`	`}`
`73`	`73`	`},`
`74`	`74`
`75`		`- unbind: function (update) {`
	`75`	`+ reset: function () {`
`76`	`76`	`this.el.removeEventListener(this.arg, this.handler)`
`77`	`77`	`this.handler = null`
`78`		`- if (!update) this.el.vue_viewmodel = null`
	`78`	`+ },`
	`79`	`+`
	`80`	`+ unbind: function () {`
	`81`	`+ this.reset()`
	`82`	`+ this.el.vue_viewmodel = null`
`79`	`83`	`}`
`80`	`84`	`}`