Skip to content

Commit

Permalink
fix(v-model): update model correctly when input event is bound
Browse files Browse the repository at this point in the history 
Use the "input" event instead of "change" when listening for
changes to the selection state since the "input" event will
always fire before the "change" event.

This avoids an issue where a user binds to the "input" event
with "v-bind" and causes the component to update and set its
model value back to the value of the select before it has
received the new selection value.

Fixes vuejs#6690
  • Loading branch information
@chriscasola
chriscasola committed Oct 5, 2017
1 parent 1780b1f commit bbd13af
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 18 deletions.
2 changes: 1 addition & 1 deletion src/platforms/web/compiler/directives/model.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ function genSelect (
const assignment = '$event.target.multiple ? $$selectedVal : $$selectedVal[0]'
let code = `var $$selectedVal = ${selectedVal};`
code = `${code} ${genAssignmentCode(value, assignment)}`
addHandler(el, 'change', code, null, true)
addHandler(el, 'input', code, null, true)
}

function genDefaultModel (
Expand Down
4 changes: 2 additions & 2 deletions src/platforms/web/runtime/directives/model.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,13 @@ export default {
const prevOptions = el._vOptions
const curOptions = el._vOptions = [].map.call(el.options, getValue)
if (curOptions.some((o, i) => !looseEqual(o, prevOptions[i]))) {
// trigger change event if
// trigger input event if
// no matching option found for at least one value
const needReset = el.multiple
? binding.value.some(v => hasNoMatchingOption(v, curOptions))
: binding.value !== binding.oldValue && hasNoMatchingOption(binding.value, curOptions)
if (needReset) {
trigger(el, 'change')
trigger(el, 'input')
}
}
}
Expand Down
57 changes: 42 additions & 15 deletions test/unit/features/directives/model-select.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ describe('Directive v-model select', () => {
expect(vm.$el.value).toBe('c')
expect(vm.$el.childNodes[2].selected).toBe(true)
updateSelect(vm.$el, 'a')
triggerEvent(vm.$el, 'change')
triggerEvent(vm.$el, 'input')
expect(vm.test).toBe('a')
}).then(done)
})
Expand All @@ -82,11 +82,11 @@ describe('Directive v-model select', () => {
expect(vm.$el.childNodes[2].selected).toBe(true)

updateSelect(vm.$el, '1')
triggerEvent(vm.$el, 'change')
triggerEvent(vm.$el, 'input')
expect(vm.test).toBe('1')

updateSelect(vm.$el, '2')
triggerEvent(vm.$el, 'change')
triggerEvent(vm.$el, 'input')
expect(vm.test).toBe(2)
}).then(done)
})
Expand All @@ -110,11 +110,11 @@ describe('Directive v-model select', () => {
expect(vm.$el.childNodes[2].selected).toBe(true)

updateSelect(vm.$el, '1')
triggerEvent(vm.$el, 'change')
triggerEvent(vm.$el, 'input')
expect(vm.test).toBe('1')

updateSelect(vm.$el, { a: 2 })
triggerEvent(vm.$el, 'change')
triggerEvent(vm.$el, 'input')
expect(vm.test).toEqual({ a: 2 })
}).then(done)
})
Expand All @@ -138,11 +138,11 @@ describe('Directive v-model select', () => {
expect(vm.$el.childNodes[2].selected).toBe(true)

updateSelect(vm.$el, '1')
triggerEvent(vm.$el, 'change')
triggerEvent(vm.$el, 'input')
expect(vm.test).toBe('1')

updateSelect(vm.$el, [{ a: 2 }])
triggerEvent(vm.$el, 'change')
triggerEvent(vm.$el, 'input')
expect(vm.test).toEqual([{ a: 2 }])
}).then(done)
})
Expand All @@ -167,7 +167,7 @@ describe('Directive v-model select', () => {
expect(vm.$el.value).toBe('c')
expect(vm.$el.childNodes[2].selected).toBe(true)
updateSelect(vm.$el, 'a')
triggerEvent(vm.$el, 'change')
triggerEvent(vm.$el, 'input')
expect(vm.test).toBe('a')
// update v-for opts
vm.opts = ['d', 'a']
Expand Down Expand Up @@ -196,7 +196,7 @@ describe('Directive v-model select', () => {
expect(vm.$el.value).toBe('3')
expect(vm.$el.childNodes[2].selected).toBe(true)
updateSelect(vm.$el, 1)
triggerEvent(vm.$el, 'change')
triggerEvent(vm.$el, 'input')
expect(vm.test).toBe(1)
// update v-for opts
vm.opts = [0, 1]
Expand Down Expand Up @@ -256,7 +256,7 @@ describe('Directive v-model select', () => {
expect(opts[2].selected).toBe(true)
opts[0].selected = false
opts[1].selected = true
triggerEvent(vm.$el, 'change')
triggerEvent(vm.$el, 'input')
expect(vm.test).toEqual(['b', 'c'])
}).then(done)
})
Expand All @@ -283,14 +283,14 @@ describe('Directive v-model select', () => {
expect(opts[2].selected).toBe(true)
opts[0].selected = false
opts[1].selected = true
triggerEvent(vm.$el, 'change')
triggerEvent(vm.$el, 'input')
expect(vm.test).toEqual(['b', 'c'])
// update v-for opts
vm.opts = ['c', 'd']
}).then(() => {
expect(opts[0].selected).toBe(true)
expect(opts[1].selected).toBe(false)
expect(vm.test).toEqual(['c']) // should remove 'd' which no longer has a matching option
expect(vm.test).toEqual(['c']) // should remove 'b' which no longer has a matching option
}).then(done)
})
}
Expand All @@ -313,7 +313,7 @@ describe('Directive v-model select', () => {
}).$mount()
document.body.appendChild(vm.$el)
vm.$el.options[1].selected = true
triggerEvent(vm.$el, 'change')
triggerEvent(vm.$el, 'input')
waitForUpdate(() => {
expect(spy).toHaveBeenCalled()
expect(vm.selections).toEqual(['1', '2'])
Expand Down Expand Up @@ -382,7 +382,7 @@ describe('Directive v-model select', () => {
var selects = vm.$el.getElementsByTagName('select')
var select0 = selects[0]
select0.options[0].selected = true
triggerEvent(select0, 'change')
triggerEvent(select0, 'input')
waitForUpdate(() => {
expect(spy).toHaveBeenCalled()
expect(vm.selections).toEqual(['foo', ''])
Expand All @@ -403,7 +403,7 @@ describe('Directive v-model select', () => {
}).$mount()
document.body.appendChild(vm.$el)
updateSelect(vm.$el, '1')
triggerEvent(vm.$el, 'change')
triggerEvent(vm.$el, 'input')
expect(vm.test).toBe(1)
})

Expand Down Expand Up @@ -546,4 +546,31 @@ describe('Directive v-model select', () => {
expect(spy).not.toHaveBeenCalled()
}).then(done)
})

// #6690
it('should work on an element with an input binding', done => {
const vm = new Vue({
data: {
test1: '',
inputEvaluated: ''
},
template:
`<select v-model="test1" v-on:input="inputEvaluated = 'blarg'" v-bind:name="inputEvaluated">` +
'<option value="">test1 Please Select</option>' +
'<option value="alpha">Alpha</option>' +
'<option value="beta">Beta</option>' +
'</select>'
}).$mount()
document.body.appendChild(vm.$el)

vm.$el.children[1].selected = true

triggerEvent(vm.$el, 'input')

waitForUpdate(() => {
triggerEvent(vm.$el, 'change')
}).then(() => {
expect(vm.$el.value).toBe('alpha')
}).then(done)
})
})

0 comments on commit bbd13af

Please sign in to comment.