Skip to content

Commit 3ff16df

Browse files
committed
v-for: update primitive values update strategy
1 parent f7dab27 commit 3ff16df

File tree

3 files changed

+113
-28
lines changed

3 files changed

+113
-28
lines changed

src/compiler/compile.js

+1
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,7 @@ function makeTerminalNodeLinkFn (el, dirName, value, options, def) {
546546
name: dirName,
547547
expression: parsed.expression,
548548
filters: parsed.filters,
549+
raw: value,
549550
// either an element directive, or if/for
550551
def: def || publicDirectives[dirName]
551552
}

src/directives/public/for.js

+28-22
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,6 @@ module.exports = {
109109
primitive = !isObject(value)
110110
frag = !init && this.getCachedFrag(value, i, key)
111111
if (frag) { // reusable fragment
112-
113-
if (process.env.NODE_ENV !== 'production' && frag.reused) {
114-
_.warn(
115-
'Duplicate objects found in v-for="' + this.expression + '": ' +
116-
JSON.stringify(value)
117-
)
118-
}
119-
120112
frag.reused = true
121113
// update $index
122114
frag.scope.$index = i
@@ -131,6 +123,7 @@ module.exports = {
131123
}
132124
} else { // new isntance
133125
frag = this.create(value, alias, i, key)
126+
frag.fresh = !init
134127
}
135128
frags[i] = frag
136129
if (init) {
@@ -180,7 +173,7 @@ module.exports = {
180173
// insert with updated stagger index.
181174
this.insert(frag, insertionIndex++, prevEl, inDoc)
182175
}
183-
frag.reused = false
176+
frag.reused = frag.fresh = false
184177
}
185178
},
186179

@@ -359,24 +352,21 @@ module.exports = {
359352
? idKey === '$index'
360353
? index
361354
: value[idKey]
362-
: (key || index)
355+
: (key || value)
363356
if (!cache[id]) {
364357
cache[id] = frag
365-
} else if (!primitive && idKey !== '$index') {
366-
process.env.NODE_ENV !== 'production' && _.warn(
367-
'Duplicate objects with the same track-by key in v-for: ' + id
368-
)
358+
} else if (idKey !== '$index') {
359+
process.env.NODE_ENV !== 'production' &&
360+
this.warnDuplicate(value)
369361
}
370362
} else {
371363
id = this.id
372364
if (value.hasOwnProperty(id)) {
373365
if (value[id] === null) {
374366
value[id] = frag
375367
} else {
376-
process.env.NODE_ENV !== 'production' && _.warn(
377-
'Duplicate objects found in v-for="' + this.expression + '": ' +
378-
JSON.stringify(value)
379-
)
368+
process.env.NODE_ENV !== 'production' &&
369+
this.warnDuplicate(value)
380370
}
381371
} else {
382372
_.define(value, id, frag)
@@ -397,16 +387,22 @@ module.exports = {
397387
getCachedFrag: function (value, index, key) {
398388
var idKey = this.idKey
399389
var primitive = !isObject(value)
390+
var frag
400391
if (key || idKey || primitive) {
401392
var id = idKey
402393
? idKey === '$index'
403394
? index
404395
: value[idKey]
405-
: (key || index)
406-
return this.cache[id]
396+
: (key || value)
397+
frag = this.cache[id]
407398
} else {
408-
return value[this.id]
399+
frag = value[this.id]
400+
}
401+
if (frag && (frag.reused || frag.fresh)) {
402+
process.env.NODE_ENV !== 'production' &&
403+
this.warnDuplicate(value)
409404
}
405+
return frag
410406
},
411407

412408
/**
@@ -429,7 +425,7 @@ module.exports = {
429425
? idKey === '$index'
430426
? index
431427
: value[idKey]
432-
: (key || index)
428+
: (key || value)
433429
this.cache[id] = null
434430
} else {
435431
value[this.id] = null
@@ -579,3 +575,13 @@ function range (n) {
579575
}
580576
return ret
581577
}
578+
579+
if (process.env.NODE_ENV !== 'production') {
580+
module.exports.warnDuplicate = function (value) {
581+
_.warn(
582+
'Duplicate value found in v-for="' + this.descriptor.raw + '": ' +
583+
JSON.stringify(value) + '. Use track-by="$index" if ' +
584+
'you are expecting duplicate values.'
585+
)
586+
}
587+
}

test/unit/specs/directives/public/for/for_spec.js

+84-6
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ if (_.inBrowser) {
2525
var vm = new Vue({
2626
el: el,
2727
data: {
28-
items: [2, 1, 2]
28+
items: [1, 2, 3]
2929
},
3030
template: '<div v-for="item in items">{{$index}} {{item}}</div>'
3131
})
@@ -169,7 +169,7 @@ if (_.inBrowser) {
169169
var vm = new Vue({
170170
el: el,
171171
data: {
172-
items: [2, 1, 2]
172+
items: [1, 2, 3]
173173
},
174174
template: '<test v-for="item in items" :index="$index" :value="item"></test>',
175175
components: {
@@ -536,6 +536,17 @@ if (_.inBrowser) {
536536
}
537537
})
538538

539+
it('primitive values track by $index', function (done) {
540+
var vm = new Vue({
541+
el: el,
542+
data: {
543+
items: [1, 2, 3]
544+
},
545+
template: '<div v-for="item in items" track-by="$index">{{$index}} {{item}}</div>'
546+
})
547+
assertPrimitiveMutationsWithDuplicates(vm, el, done)
548+
})
549+
539550
it('warn missing alias', function () {
540551
new Vue({
541552
el: el,
@@ -553,7 +564,7 @@ if (_.inBrowser) {
553564
items: [obj, obj]
554565
}
555566
})
556-
expect(hasWarned(_, 'Duplicate objects')).toBe(true)
567+
expect(hasWarned(_, 'Duplicate value')).toBe(true)
557568
})
558569

559570
it('warn duplicate objects on diff', function (done) {
@@ -568,7 +579,7 @@ if (_.inBrowser) {
568579
expect(_.warn).not.toHaveBeenCalled()
569580
vm.items.push(obj)
570581
_.nextTick(function () {
571-
expect(hasWarned(_, 'Duplicate objects')).toBe(true)
582+
expect(hasWarned(_, 'Duplicate value')).toBe(true)
572583
done()
573584
})
574585
})
@@ -581,7 +592,7 @@ if (_.inBrowser) {
581592
items: [{id: 1}, {id: 1}]
582593
}
583594
})
584-
expect(hasWarned(_, 'Duplicate objects with the same track-by key')).toBe(true)
595+
expect(hasWarned(_, 'Duplicate value')).toBe(true)
585596
})
586597

587598
it('repeat number', function () {
@@ -873,7 +884,74 @@ function assertPrimitiveMutations (vm, el, done) {
873884
assertMarkup()
874885
go(
875886
function () {
876-
// check duplicate
887+
vm.items.push(4)
888+
},
889+
assertMarkup
890+
)
891+
.then(
892+
function () {
893+
vm.items.shift()
894+
},
895+
assertMarkup
896+
)
897+
.then(
898+
function () {
899+
vm.items.reverse()
900+
},
901+
assertMarkup
902+
)
903+
.then(
904+
function () {
905+
vm.items.pop()
906+
},
907+
assertMarkup
908+
)
909+
.then(
910+
function () {
911+
vm.items.unshift(1)
912+
},
913+
assertMarkup
914+
)
915+
.then(
916+
function () {
917+
vm.items.sort(function (a, b) {
918+
return a > b ? 1 : -1
919+
})
920+
},
921+
assertMarkup
922+
)
923+
.then(
924+
function () {
925+
vm.items.splice(1, 1, 5)
926+
},
927+
assertMarkup
928+
)
929+
// test swapping the array
930+
.then(
931+
function () {
932+
vm.items = [1, 2, 3]
933+
},
934+
assertMarkup
935+
)
936+
.run(done)
937+
938+
function assertMarkup () {
939+
var markup = vm.items.map(function (item, i) {
940+
return '<div>' + i + ' ' + item + '</div>'
941+
}).join('')
942+
expect(el.innerHTML).toBe(markup)
943+
}
944+
}
945+
946+
/**
947+
* Assert mutation and markup correctness for v-for on
948+
* an Array of primitive values when using track-by="$index"
949+
*/
950+
951+
function assertPrimitiveMutationsWithDuplicates (vm, el, done) {
952+
assertMarkup()
953+
go(
954+
function () {
877955
vm.items.push(2, 2, 3)
878956
},
879957
assertMarkup

0 commit comments

Comments
 (0)