Skip to content

Commit

Permalink
Fix #65
Browse files Browse the repository at this point in the history
A computed property can depend on properties on repeated items.
When new items are added to the Array, the dependency collection
has already been done so their properties are not collected by the
computed property on the parent. We need to re-parse the dependencies
in such situation, namely push, unshift, splice and when the entire
Array is swapped.

Also included casper test case by @daines.
  • Loading branch information
yyx990803 committed Feb 3, 2014
1 parent 3cb7027 commit a7dfb3f
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 14 deletions.
12 changes: 9 additions & 3 deletions src/compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,7 @@ function Compiler (vm, options) {
}

// extract dependencies for computed properties
if (compiler.computed.length) {
DepsParser.parse(compiler.computed)
}
compiler.parseDeps()

// done!
compiler.init = false
Expand Down Expand Up @@ -600,6 +598,14 @@ CompilerProto.hasKey = function (key) {
hasOwn.call(this.vm, baseKey)
}

/**
* Collect dependencies for computed properties
*/
CompilerProto.parseDeps = function () {
if (!this.computed.length) return
DepsParser.parse(this.computed)
}

/**
* Unbind and remove element
*/
Expand Down
4 changes: 3 additions & 1 deletion src/deps-parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ function catchDeps (binding) {
if (binding.isFn) return
utils.log('\n- ' + binding.key)
var got = utils.hash()
binding.deps = []
catcher.on('get', function (dep) {
var has = got[dep.key]
if (has && has.compiler === dep.compiler) return
Expand All @@ -36,7 +37,8 @@ module.exports = {
parse: function (bindings) {
utils.log('\nparsing dependencies...')
Observer.shouldGet = true
bindings.forEach(catchDeps)
var i = bindings.length
while (i--) { catchDeps(bindings[i]) }
Observer.shouldGet = false
utils.log('\ndone.')
}
Expand Down
41 changes: 31 additions & 10 deletions src/directives/repeat.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,38 +110,59 @@ module.exports = {
if (method !== 'push' && method !== 'pop') {
self.updateIndexes()
}
if (method === 'push' || method === 'unshift' || method === 'splice') {
self.changed()
}
}

},

update: function (collection) {
update: function (collection, init) {

this.unbind(true)
var self = this
self.unbind(true)
// attach an object to container to hold handlers
this.container.vue_dHandlers = utils.hash()
self.container.vue_dHandlers = utils.hash()
// if initiating with an empty collection, we need to
// force a compile so that we get all the bindings for
// dependency extraction.
if (!this.initiated && (!collection || !collection.length)) {
this.buildItem()
this.initiated = true
if (!self.initiated && (!collection || !collection.length)) {
self.buildItem()
self.initiated = true
}
collection = this.collection = collection || []
this.vms = []
collection = self.collection = collection || []
self.vms = []

// listen for collection mutation events
// the collection has been augmented during Binding.set()
if (!collection.__observer__) Observer.watchArray(collection, null, new Emitter())
collection.__observer__.on('mutate', this.mutationListener)
collection.__observer__.on('mutate', self.mutationListener)

// create child-vms and append to DOM
if (collection.length) {
for (var i = 0, l = collection.length; i < l; i++) {
this.buildItem(collection[i], i)
self.buildItem(collection[i], i)
}
if (!init) self.changed()
}
},

/**
* Notify parent compiler that new items
* have been added to the collection, it needs
* to re-calculate computed property dependencies.
* Batched to ensure it's called only once every event loop.
*/
changed: function () {
var self = this
if (self.queued) return
self.queued = true
setTimeout(function () {
self.compiler.parseDeps()
self.queued = false
}, 0)
},

/**
* Create a new child VM from a data object
* passing along compiler options indicating this
Expand Down
41 changes: 41 additions & 0 deletions test/functional/fixtures/computed-repeat.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<!DOCTYPE html>
<html lang="en">
<head>
<title>Repeated form elements</title>
<meta charset="utf-8">
<script src="../../../dist/vue.js"></script>
</head>
<body>
<form id="form">
<p v-repeat="items">
<input type="text" name="text{{$index}}" v-model="text">
</p>
<button v-on="click: add" id="add">Add</button>
<p id="texts">{{texts}}</p>
</form>
<script>
var app = new Vue({
el: '#form',
data: {
items: [
{ text: "a" },
{ text: "b" }
]
},
methods: {
add: function(e) {
this.items.push({ text: "c" })
e.preventDefault()
}
},
computed: {
texts: function () {
return this.items.map(function(item) {
return item.text
}).join(",")
}
}
})
</script>
</body>
</html>
29 changes: 29 additions & 0 deletions test/functional/specs/computed-repeat.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
casper.test.begin('Computed property depending on repeated items', 4, function (test) {

casper
.start('./fixtures/computed-repeat.html')
.then(function () {
test.assertSelectorHasText('#texts', 'a,b')
})
.thenClick('#add', function () {
test.assertSelectorHasText('#texts', 'a,b,c')
})
.then(function () {
this.fill('#form', {
"text0": 'd',
"text1": 'e',
"text2": 'f',
})
})
.then(function () {
this.sendKeys('input[name="text2"]', 'fff')
})
.then(function () {
test.assertField('text0', 'd')
test.assertSelectorHasText('#texts', 'd,e,ffff')
})
.run(function () {
test.done()
})

})

0 comments on commit a7dfb3f

Please sign in to comment.