Skip to content

Commit 9a1eeda

Browse files
committed
warn against using built-in element as component name
1 parent 5e96940 commit 9a1eeda

File tree

6 files changed

+72
-51
lines changed

6 files changed

+72
-51
lines changed

src/util/misc.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ function formatValue (val) {
8080
* @return {String|undefined}
8181
*/
8282

83-
var commonTagRE = /^(div|p|span|img|a|br|ul|ol|li|h1|h2|h3|h4|h5|code|pre)$/
84-
var tableElementsRE = /^caption|colgroup|thead|tfoot|tbody|tr|td|th$/
83+
exports.commonTagRE = /^(div|p|span|img|a|br|ul|ol|li|h1|h2|h3|h4|h5|code|pre)$/
84+
exports.tableElementsRE = /^caption|colgroup|thead|tfoot|tbody|tr|td|th$/
8585

8686
exports.checkComponent = function (el, options) {
8787
var tag = el.tagName.toLowerCase()
@@ -91,12 +91,12 @@ exports.checkComponent = function (el, options) {
9191
el.removeAttribute('is')
9292
return exp
9393
} else if (
94-
!commonTagRE.test(tag) &&
94+
!exports.commonTagRE.test(tag) &&
9595
_.resolveAsset(options, 'components', tag)
9696
) {
9797
return tag
9898
} else if (
99-
tableElementsRE.test(tag) &&
99+
exports.tableElementsRE.test(tag) &&
100100
(tag = _.attr(el, 'component'))
101101
) {
102102
return tag

src/util/options.js

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,12 @@ function guardComponents (components) {
214214
if (components) {
215215
var def
216216
for (var key in components) {
217+
if (_.commonTagRE.test(key)) {
218+
_.warn(
219+
'Do not use built-in HTML elements as component ' +
220+
'name: ' + key
221+
)
222+
}
217223
def = components[key]
218224
if (_.isPlainObject(def)) {
219225
def.name = key
@@ -275,4 +281,4 @@ exports.resolveAsset = function resolve (options, type, id) {
275281
asset = options[type][id]
276282
}
277283
return asset
278-
}
284+
}

test/unit/specs/async_component_spec.js

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -42,18 +42,18 @@ describe('Async components', function () {
4242
el: el,
4343
template: '<component is="{{view}}"></component>',
4444
data: {
45-
view: 'a'
45+
view: 'view-a'
4646
},
4747
components: {
48-
a: function (resolve) {
48+
'view-a': function (resolve) {
4949
setTimeout(function () {
5050
resolve({
5151
template: 'A'
5252
})
5353
step1()
5454
}, 0)
5555
},
56-
b: function (resolve) {
56+
'view-b': function (resolve) {
5757
setTimeout(function () {
5858
resolve({
5959
template: 'B'
@@ -69,11 +69,11 @@ describe('Async components', function () {
6969
expect(aCalled).toBe(false)
7070
aCalled = true
7171
expect(el.textContent).toBe('A')
72-
vm.view = 'b'
72+
vm.view = 'view-b'
7373
}
7474
function step2 () {
7575
expect(el.textContent).toBe('B')
76-
vm.view = 'a'
76+
vm.view = 'view-a'
7777
_.nextTick(function () {
7878
expect(el.textContent).toBe('A')
7979
done()
@@ -86,18 +86,18 @@ describe('Async components', function () {
8686
el: el,
8787
template: '<component is="{{view}}"></component>',
8888
data: {
89-
view: 'a'
89+
view: 'view-a'
9090
},
9191
components: {
92-
a: function (resolve) {
92+
'view-a': function (resolve) {
9393
setTimeout(function () {
9494
resolve({
9595
template: 'A'
9696
})
9797
step1()
9898
}, 100)
9999
},
100-
b: function (resolve) {
100+
'view-b': function (resolve) {
101101
setTimeout(function () {
102102
resolve({
103103
template: 'B'
@@ -108,7 +108,7 @@ describe('Async components', function () {
108108
}
109109
})
110110
expect(el.textContent).toBe('')
111-
vm.view = 'b'
111+
vm.view = 'view-b'
112112
function step1 () {
113113
// called after A resolves, but A should have been
114114
// invalidated so not cotrId should be set
@@ -126,7 +126,7 @@ describe('Async components', function () {
126126
el: el,
127127
template: '<test></test>',
128128
data: {
129-
view: 'a'
129+
view: 'view-a'
130130
},
131131
components: {
132132
test: function (resolve) {
@@ -309,4 +309,4 @@ describe('Async components', function () {
309309
})
310310
})
311311

312-
})
312+
})

test/unit/specs/directives/component_spec.js

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -106,17 +106,17 @@ if (_.inBrowser) {
106106
el: el,
107107
template: '<component is="{{view}}" v-attr="view:view"></component>',
108108
data: {
109-
view: 'a'
109+
view: 'view-a'
110110
},
111111
components: {
112-
a: {
112+
'view-a': {
113113
template: '<div>AAA</div>',
114114
replace: true,
115115
data: function () {
116116
return { view: 'a' }
117117
}
118118
},
119-
b: {
119+
'view-b': {
120120
template: '<div>BBB</div>',
121121
replace: true,
122122
data: function () {
@@ -125,10 +125,10 @@ if (_.inBrowser) {
125125
}
126126
}
127127
})
128-
expect(el.innerHTML).toBe('<div view="a">AAA</div>')
129-
vm.view = 'b'
128+
expect(el.innerHTML).toBe('<div view="view-a">AAA</div>')
129+
vm.view = 'view-b'
130130
_.nextTick(function () {
131-
expect(el.innerHTML).toBe('<div view="b">BBB</div>')
131+
expect(el.innerHTML).toBe('<div view="view-b">BBB</div>')
132132
vm.view = ''
133133
_.nextTick(function () {
134134
expect(el.innerHTML).toBe('')
@@ -144,15 +144,15 @@ if (_.inBrowser) {
144144
el: el,
145145
template: '<component is="{{view}}" keep-alive></component>',
146146
data: {
147-
view: 'a'
147+
view: 'view-a'
148148
},
149149
components: {
150-
a: {
150+
'view-a': {
151151
created: spyA,
152152
template: '<div>AAA</div>',
153153
replace: true
154154
},
155-
b: {
155+
'view-b': {
156156
created: spyB,
157157
template: '<div>BBB</div>',
158158
replace: true
@@ -162,17 +162,17 @@ if (_.inBrowser) {
162162
expect(el.innerHTML).toBe('<div>AAA</div>')
163163
expect(spyA.calls.count()).toBe(1)
164164
expect(spyB.calls.count()).toBe(0)
165-
vm.view = 'b'
165+
vm.view = 'view-b'
166166
_.nextTick(function () {
167167
expect(el.innerHTML).toBe('<div>BBB</div>')
168168
expect(spyA.calls.count()).toBe(1)
169169
expect(spyB.calls.count()).toBe(1)
170-
vm.view = 'a'
170+
vm.view = 'view-a'
171171
_.nextTick(function () {
172172
expect(el.innerHTML).toBe('<div>AAA</div>')
173173
expect(spyA.calls.count()).toBe(1)
174174
expect(spyB.calls.count()).toBe(1)
175-
vm.view = 'b'
175+
vm.view = 'view-b'
176176
_.nextTick(function () {
177177
expect(el.innerHTML).toBe('<div>BBB</div>')
178178
expect(spyA.calls.count()).toBe(1)
@@ -267,20 +267,20 @@ if (_.inBrowser) {
267267
var vm = new Vue({
268268
el: el,
269269
data: {
270-
view: 'a'
270+
view: 'view-a'
271271
},
272272
template: '<component is="{{view}}" wait-for="ok"></component>',
273273
components: {
274-
a: {
274+
'view-a': {
275275
template: 'AAA'
276276
},
277-
b: {
277+
'view-b': {
278278
template: 'BBB'
279279
}
280280
}
281281
})
282282
vm._children[0].$emit('ok')
283-
vm.view = 'b'
283+
vm.view = 'view-b'
284284
_.nextTick(function () {
285285
expect(el.textContent).toBe('AAA')
286286
// old vm is already removed, this is the new vm
@@ -297,12 +297,12 @@ if (_.inBrowser) {
297297
var vm = new Vue({
298298
el: el,
299299
data: {
300-
view: 'a'
300+
view: 'view-a'
301301
},
302302
template: '<component is="{{view}}" v-transition="test" transition-mode="in-out"></component>',
303303
components: {
304-
a: { template: 'AAA' },
305-
b: { template: 'BBB' }
304+
'view-a': { template: 'AAA' },
305+
'view-b': { template: 'BBB' }
306306
},
307307
transitions: {
308308
test: {
@@ -318,7 +318,7 @@ if (_.inBrowser) {
318318
}
319319
})
320320
expect(el.textContent).toBe('AAA')
321-
vm.view = 'b'
321+
vm.view = 'view-b'
322322
_.nextTick(function () {
323323
expect(spy1).toHaveBeenCalled()
324324
expect(spy2).not.toHaveBeenCalled()
@@ -341,12 +341,12 @@ if (_.inBrowser) {
341341
var vm = new Vue({
342342
el: el,
343343
data: {
344-
view: 'a'
344+
view: 'view-a'
345345
},
346346
template: '<component is="{{view}}" v-transition="test" transition-mode="out-in"></component>',
347347
components: {
348-
a: { template: 'AAA' },
349-
b: { template: 'BBB' }
348+
'view-a': { template: 'AAA' },
349+
'view-b': { template: 'BBB' }
350350
},
351351
transitions: {
352352
test: {
@@ -362,7 +362,7 @@ if (_.inBrowser) {
362362
}
363363
})
364364
expect(el.textContent).toBe('AAA')
365-
vm.view = 'b'
365+
vm.view = 'view-b'
366366
_.nextTick(function () {
367367
expect(spy1).toHaveBeenCalled()
368368
expect(spy2).not.toHaveBeenCalled()
@@ -409,4 +409,4 @@ if (_.inBrowser) {
409409
})
410410

411411
})
412-
}
412+
}

test/unit/specs/directives/if_spec.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -110,14 +110,14 @@ if (_.inBrowser) {
110110
el: el,
111111
data: {
112112
ok: false,
113-
view: 'a'
113+
view: 'view-a'
114114
},
115115
template: '<component is="{{view}}" v-if="ok"></component>',
116116
components: {
117-
a: {
117+
'view-a': {
118118
template: 'AAA'
119119
},
120-
b: {
120+
'view-b': {
121121
template: 'BBB'
122122
}
123123
}
@@ -130,7 +130,7 @@ if (_.inBrowser) {
130130
expect(el.innerHTML).toBe('<component>AAA</component>')
131131
expect(vm._children.length).toBe(1)
132132
// switch view when if=true
133-
vm.view = 'b'
133+
vm.view = 'view-b'
134134
_.nextTick(function () {
135135
expect(el.innerHTML).toBe('<component>BBB</component>')
136136
expect(vm._children.length).toBe(1)
@@ -140,7 +140,7 @@ if (_.inBrowser) {
140140
expect(el.innerHTML).toBe('')
141141
expect(vm._children.length).toBe(0)
142142
// toggle if and switch view at the same time
143-
vm.view = 'a'
143+
vm.view = 'view-a'
144144
vm.ok = true
145145
_.nextTick(function () {
146146
expect(el.innerHTML).toBe('<component>AAA</component>')
@@ -328,4 +328,4 @@ if (_.inBrowser) {
328328
})
329329

330330
})
331-
}
331+
}

test/unit/specs/util/options_spec.js

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@ var Vue = require('../../../../src/vue')
33
var merge = _.mergeOptions
44

55
describe('Util - Option merging', function () {
6+
7+
beforeEach(function () {
8+
spyOn(_, 'warn')
9+
})
610

711
it('default strat', function () {
812
// child undefined
@@ -110,16 +114,27 @@ describe('Util - Option merging', function () {
110114
})
111115

112116
it('guard components', function () {
117+
var res = merge({
118+
components: null
119+
}, {
120+
components: {
121+
test: { template: 'hi' }
122+
}
123+
})
124+
expect(typeof res.components.test).toBe('function')
125+
expect(res.components.test.options.name).toBe('test')
126+
expect(res.components.test.super).toBe(Vue)
127+
})
128+
129+
it('guard components warn built-in elements', function () {
113130
var res = merge({
114131
components: null
115132
}, {
116133
components: {
117134
a: { template: 'hi' }
118135
}
119136
})
120-
expect(typeof res.components.a).toBe('function')
121-
expect(res.components.a.options.name).toBe('a')
122-
expect(res.components.a.super).toBe(Vue)
137+
expect(hasWarned(_, 'Do not use built-in HTML elements')).toBe(true)
123138
})
124139

125140
it('should ignore non-function el & data in class merge', function () {
@@ -254,4 +269,4 @@ describe('Util - Option merging', function () {
254269
expect(res.created[3]).toBe(f4)
255270
})
256271

257-
})
272+
})

0 commit comments

Comments
 (0)