From f53e52392bc62175465a5d4041986e822d2479f6 Mon Sep 17 00:00:00 2001 From: Robert Pemberton Date: Tue, 14 Nov 2017 12:49:50 +0000 Subject: [PATCH 1/4] test($compile): warn if v-model and :value used on same text input #7048 --- src/compiler/parser/index.js | 9 +++++++++ .../features/directives/model-text.spec.js | 20 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/src/compiler/parser/index.js b/src/compiler/parser/index.js index 1a28e86de2e..806c0d5e25f 100644 --- a/src/compiler/parser/index.js +++ b/src/compiler/parser/index.js @@ -598,6 +598,15 @@ function makeAttrsMap (attrs: Array): Object { } map[attrs[i].name] = attrs[i].value } + if ( + process.env.NODE_ENV !== 'production' && + map['v-model'] && + (map['v-bind:value'] || map[':value']) && + (!map['type'] || map['type'] === 'text') + ) { + var vBind = map['v-bind:value'] ? 'v-bind:value' : ':value' + warn('v-model and ' + vBind + ' used on the same text input') + } return map } diff --git a/test/unit/features/directives/model-text.spec.js b/test/unit/features/directives/model-text.spec.js index 7e291356c2f..7793383d9c1 100644 --- a/test/unit/features/directives/model-text.spec.js +++ b/test/unit/features/directives/model-text.spec.js @@ -250,6 +250,26 @@ describe('Directive v-model text', () => { expect('You are binding v-model directly to a v-for iteration alias').toHaveBeenWarned() }) + it('warn if both v-model and v-bind:value are used on the same text input', () => { + new Vue({ + data: { + test: 'foo' + }, + template: '' + }).$mount() + expect('v-model and v-bind:value used on the same text input').toHaveBeenWarned() + }) + + it('warn if both v-model and :value are used on the same text input', () => { + new Vue({ + data: { + test: 'foo' + }, + template: '' + }).$mount() + expect('v-model and :value used on the same text input').toHaveBeenWarned() + }) + if (!isAndroid) { it('does not trigger extra input events with single compositionend', () => { const spy = jasmine.createSpy() From 38981c1125573912c753d22f92f9c6a800cc7a2b Mon Sep 17 00:00:00 2001 From: Robert Pemberton Date: Tue, 14 Nov 2017 14:25:32 +0000 Subject: [PATCH 2/4] test($compile): make v-model and v-bind:value warning apply to all but exceptions #7048 --- src/compiler/parser/index.js | 15 ++++++++------- test/unit/features/directives/model-text.spec.js | 8 ++++---- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/compiler/parser/index.js b/src/compiler/parser/index.js index 806c0d5e25f..a82098906a4 100644 --- a/src/compiler/parser/index.js +++ b/src/compiler/parser/index.js @@ -51,7 +51,7 @@ export function createASTElement ( type: 1, tag, attrsList: attrs, - attrsMap: makeAttrsMap(attrs), + attrsMap: makeAttrsMap(attrs, tag), parent, children: [] } @@ -587,7 +587,7 @@ function parseModifiers (name: string): Object | void { } } -function makeAttrsMap (attrs: Array): Object { +function makeAttrsMap (attrs: Array, tag: string): Object { const map = {} for (let i = 0, l = attrs.length; i < l; i++) { if ( @@ -600,12 +600,13 @@ function makeAttrsMap (attrs: Array): Object { } if ( process.env.NODE_ENV !== 'production' && - map['v-model'] && - (map['v-bind:value'] || map[':value']) && - (!map['type'] || map['type'] === 'text') + map['v-model'] && (map['v-bind:value'] || map[':value']) && + map['type'] !== 'checkbox' && + map['type'] !== 'radio' && + tag !== 'select' ) { - var vBind = map['v-bind:value'] ? 'v-bind:value' : ':value' - warn('v-model and ' + vBind + ' used on the same text input') + var vBindValue = map['v-bind:value'] ? 'v-bind:value' : ':value' + warn(vBindValue + ' conflicts with v-model on the same element because the latter already expands to a value binding internally') } return map } diff --git a/test/unit/features/directives/model-text.spec.js b/test/unit/features/directives/model-text.spec.js index 7793383d9c1..9227b5ed1a2 100644 --- a/test/unit/features/directives/model-text.spec.js +++ b/test/unit/features/directives/model-text.spec.js @@ -250,24 +250,24 @@ describe('Directive v-model text', () => { expect('You are binding v-model directly to a v-for iteration alias').toHaveBeenWarned() }) - it('warn if both v-model and v-bind:value are used on the same text input', () => { + it('warn if both v-model and v-bind:value are used on the same element', () => { new Vue({ data: { test: 'foo' }, template: '' }).$mount() - expect('v-model and v-bind:value used on the same text input').toHaveBeenWarned() + expect('v-bind:value conflicts with v-model on the same element because the latter already expands to a value binding internally').toHaveBeenWarned() }) - it('warn if both v-model and :value are used on the same text input', () => { + it('warn if both v-model and :value are used on the same element', () => { new Vue({ data: { test: 'foo' }, template: '' }).$mount() - expect('v-model and :value used on the same text input').toHaveBeenWarned() + expect(':value conflicts with v-model on the same element because the latter already expands to a value binding internally').toHaveBeenWarned() }) if (!isAndroid) { From 2cab21e31a711f778e696d1e72f89b670d230c0b Mon Sep 17 00:00:00 2001 From: Robert Pemberton Date: Thu, 16 Nov 2017 15:24:41 +0000 Subject: [PATCH 3/4] test($compile): move v-model/:value conflict warner to model.js #7048 --- src/compiler/parser/index.js | 14 ++------------ src/platforms/web/compiler/directives/model.js | 15 +++++++++++++++ test/unit/features/directives/model-text.spec.js | 4 ++-- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/compiler/parser/index.js b/src/compiler/parser/index.js index a82098906a4..1a28e86de2e 100644 --- a/src/compiler/parser/index.js +++ b/src/compiler/parser/index.js @@ -51,7 +51,7 @@ export function createASTElement ( type: 1, tag, attrsList: attrs, - attrsMap: makeAttrsMap(attrs, tag), + attrsMap: makeAttrsMap(attrs), parent, children: [] } @@ -587,7 +587,7 @@ function parseModifiers (name: string): Object | void { } } -function makeAttrsMap (attrs: Array, tag: string): Object { +function makeAttrsMap (attrs: Array): Object { const map = {} for (let i = 0, l = attrs.length; i < l; i++) { if ( @@ -598,16 +598,6 @@ function makeAttrsMap (attrs: Array, tag: string): Object { } map[attrs[i].name] = attrs[i].value } - if ( - process.env.NODE_ENV !== 'production' && - map['v-model'] && (map['v-bind:value'] || map[':value']) && - map['type'] !== 'checkbox' && - map['type'] !== 'radio' && - tag !== 'select' - ) { - var vBindValue = map['v-bind:value'] ? 'v-bind:value' : ':value' - warn(vBindValue + ' conflicts with v-model on the same element because the latter already expands to a value binding internally') - } return map } diff --git a/src/platforms/web/compiler/directives/model.js b/src/platforms/web/compiler/directives/model.js index 98d4259cf98..3ae0e75b27a 100644 --- a/src/platforms/web/compiler/directives/model.js +++ b/src/platforms/web/compiler/directives/model.js @@ -21,6 +21,7 @@ export default function model ( const modifiers = dir.modifiers const tag = el.tag const type = el.attrsMap.type + const attrsMap = el.attrsMap if (process.env.NODE_ENV !== 'production') { // inputs with type="file" are read only and setting the input's @@ -31,6 +32,20 @@ export default function model ( `File inputs are read only. Use a v-on:change listener instead.` ) } + + // warn if v-bind:value conflicts with v-model + if ( + (attrsMap['v-bind:value'] || attrsMap[':value']) && + type !== 'checkbox' && + type !== 'radio' && + tag !== 'select' + ) { + const vBindValue = attrsMap['v-bind:value'] ? 'v-bind:value' : ':value' + warn( + `${vBindValue} conflicts with v-model on the same element ` + + 'because the latter already expands to a value binding internally' + ) + } } if (el.component) { diff --git a/test/unit/features/directives/model-text.spec.js b/test/unit/features/directives/model-text.spec.js index 9227b5ed1a2..bc77969ba85 100644 --- a/test/unit/features/directives/model-text.spec.js +++ b/test/unit/features/directives/model-text.spec.js @@ -250,7 +250,7 @@ describe('Directive v-model text', () => { expect('You are binding v-model directly to a v-for iteration alias').toHaveBeenWarned() }) - it('warn if both v-model and v-bind:value are used on the same element', () => { + it('warn if v-model and v-bind:value conflict', () => { new Vue({ data: { test: 'foo' @@ -260,7 +260,7 @@ describe('Directive v-model text', () => { expect('v-bind:value conflicts with v-model on the same element because the latter already expands to a value binding internally').toHaveBeenWarned() }) - it('warn if both v-model and :value are used on the same element', () => { + it('warn if v-model and :value conflict', () => { new Vue({ data: { test: 'foo' From caf15c287a6a5776c66482bd2bda80e3fe1e0f86 Mon Sep 17 00:00:00 2001 From: Robert Pemberton Date: Thu, 16 Nov 2017 15:45:31 +0000 Subject: [PATCH 4/4] style: split long warning messages onto new lines --- test/unit/features/directives/model-text.spec.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/unit/features/directives/model-text.spec.js b/test/unit/features/directives/model-text.spec.js index bc77969ba85..2c186cc16f4 100644 --- a/test/unit/features/directives/model-text.spec.js +++ b/test/unit/features/directives/model-text.spec.js @@ -257,7 +257,10 @@ describe('Directive v-model text', () => { }, template: '' }).$mount() - expect('v-bind:value conflicts with v-model on the same element because the latter already expands to a value binding internally').toHaveBeenWarned() + expect( + 'v-bind:value conflicts with v-model on the same element because the latter already ' + + 'expands to a value binding internally' + ).toHaveBeenWarned() }) it('warn if v-model and :value conflict', () => { @@ -267,7 +270,10 @@ describe('Directive v-model text', () => { }, template: '' }).$mount() - expect(':value conflicts with v-model on the same element because the latter already expands to a value binding internally').toHaveBeenWarned() + expect( + ':value conflicts with v-model on the same element because the latter already ' + + 'expands to a value binding internally' + ).toHaveBeenWarned() }) if (!isAndroid) {