Skip to content

Commit

Permalink
refactor: improve option type check warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
yyx990803 committed Oct 12, 2017
1 parent adff008 commit b7105ae
Show file tree
Hide file tree
Showing 12 changed files with 115 additions and 79 deletions.
13 changes: 0 additions & 13 deletions src/core/instance/state.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,6 @@ export function initState (vm: Component) {
}
}

function checkOptionType (vm: Component, name: string) {
const option = vm.$options[name]
if (!isPlainObject(option)) {
warn(
`component option "${name}" should be an object.`,
vm
)
}
}

function initProps (vm: Component, propsOptions: Object) {
const propsData = vm.$options.propsData || {}
const props = vm._props = {}
Expand Down Expand Up @@ -171,7 +161,6 @@ function getData (data: Function, vm: Component): any {
const computedWatcherOptions = { lazy: true }

function initComputed (vm: Component, computed: Object) {
process.env.NODE_ENV !== 'production' && checkOptionType(vm, 'computed')
const watchers = vm._computedWatchers = Object.create(null)
// computed properties are just getters during SSR
const isSSR = isServerRendering()
Expand Down Expand Up @@ -260,7 +249,6 @@ function createComputedGetter (key) {
}

function initMethods (vm: Component, methods: Object) {
process.env.NODE_ENV !== 'production' && checkOptionType(vm, 'methods')
const props = vm.$options.props
for (const key in methods) {
if (process.env.NODE_ENV !== 'production') {
Expand Down Expand Up @@ -289,7 +277,6 @@ function initMethods (vm: Component, methods: Object) {
}

function initWatch (vm: Component, watch: Object) {
process.env.NODE_ENV !== 'production' && checkOptionType(vm, 'watch')
for (const key in watch) {
const handler = watch[key]
if (Array.isArray(handler)) {
Expand Down
16 changes: 7 additions & 9 deletions src/core/util/debug.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,13 @@ if (process.env.NODE_ENV !== 'production') {
if (vm.$root === vm) {
return '<Root>'
}
let name = typeof vm === 'string'
? vm
: typeof vm === 'function' && vm.options
? vm.options.name
: vm._isVue
? vm.$options.name || vm.$options._componentTag
: vm.name

const file = vm._isVue && vm.$options.__file
const options = typeof vm === 'function' && vm.cid != null
? vm.options
: vm._isVue
? vm.$options || vm.constructor.options
: vm || {}
let name = options.name || options._componentTag
const file = options.__file
if (!name && file) {
const match = file.match(/([^/\\]+)\.vue$/)
name = match && match[1]
Expand Down
67 changes: 57 additions & 10 deletions src/core/util/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
extend,
hasOwn,
camelize,
toRawType,
capitalize,
isBuiltInTag,
isPlainObject
Expand Down Expand Up @@ -155,11 +156,19 @@ LIFECYCLE_HOOKS.forEach(hook => {
* a three-way merge between constructor options, instance
* options and parent options.
*/
function mergeAssets (parentVal: ?Object, childVal: ?Object): Object {
function mergeAssets (
parentVal: ?Object,
childVal: ?Object,
vm?: Component,
key: string
): Object {
const res = Object.create(parentVal || null)
return childVal
? extend(res, childVal)
: res
if (childVal) {
process.env.NODE_ENV !== 'production' && assertObjectType(key, childVal, vm)
return extend(res, childVal)
} else {
return res
}
}

ASSET_TYPES.forEach(function (type) {
Expand All @@ -172,12 +181,20 @@ ASSET_TYPES.forEach(function (type) {
* Watchers hashes should not overwrite one
* another, so we merge them as arrays.
*/
strats.watch = function (parentVal: ?Object, childVal: ?Object): ?Object {
strats.watch = function (
parentVal: ?Object,
childVal: ?Object,
vm?: Component,
key: string
): ?Object {
// work around Firefox's Object.prototype.watch...
if (parentVal === nativeWatch) parentVal = undefined
if (childVal === nativeWatch) childVal = undefined
/* istanbul ignore if */
if (!childVal) return Object.create(parentVal || null)
if (process.env.NODE_ENV !== 'production') {
assertObjectType(key, childVal, vm)
}
if (!parentVal) return childVal
const ret = {}
extend(ret, parentVal)
Expand All @@ -200,7 +217,15 @@ strats.watch = function (parentVal: ?Object, childVal: ?Object): ?Object {
strats.props =
strats.methods =
strats.inject =
strats.computed = function (parentVal: ?Object, childVal: ?Object): ?Object {
strats.computed = function (
parentVal: ?Object,
childVal: ?Object,
vm?: Component,
key: string
): ?Object {
if (childVal && process.env.NODE_ENV !== 'production') {
assertObjectType(key, childVal, vm)
}
if (!parentVal) return childVal
const ret = Object.create(null)
extend(ret, parentVal)
Expand Down Expand Up @@ -237,7 +262,7 @@ function checkComponents (options: Object) {
* Ensure all props option syntax are normalized into the
* Object-based format.
*/
function normalizeProps (options: Object) {
function normalizeProps (options: Object, vm: ?Component) {
const props = options.props
if (!props) return
const res = {}
Expand All @@ -261,14 +286,20 @@ function normalizeProps (options: Object) {
? val
: { type: val }
}
} else if (process.env.NODE_ENV !== 'production' && props) {
warn(
`Invalid value for option "props": expected an Array or an Object, ` +
`but got ${toRawType(props)}.`,
vm
)
}
options.props = res
}

/**
* Normalize all injections into Object-based format
*/
function normalizeInject (options: Object) {
function normalizeInject (options: Object, vm: ?Component) {
const inject = options.inject
const normalized = options.inject = {}
if (Array.isArray(inject)) {
Expand All @@ -282,6 +313,12 @@ function normalizeInject (options: Object) {
? extend({ from: key }, val)
: { from: val }
}
} else if (process.env.NODE_ENV !== 'production' && inject) {
warn(
`Invalid value for option "inject": expected an Array or an Object, ` +
`but got ${toRawType(inject)}.`,
vm
)
}
}

Expand All @@ -300,6 +337,16 @@ function normalizeDirectives (options: Object) {
}
}

function assertObjectType (name: string, value: any, vm: ?Component) {
if (!isPlainObject(value)) {
warn(
`Invalid value for option "${name}": expected an Object, ` +
`but got ${toRawType(value)}.`,
vm
)
}
}

/**
* Merge two option objects into a new one.
* Core utility used in both instantiation and inheritance.
Expand All @@ -317,8 +364,8 @@ export function mergeOptions (
child = child.options
}

normalizeProps(child)
normalizeInject(child)
normalizeProps(child, vm)
normalizeInject(child, vm)
normalizeDirectives(child)
const extendsFrom = child.extends
if (extendsFrom) {
Expand Down
7 changes: 4 additions & 3 deletions src/core/util/props.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { observe, observerState } from '../observer/index'
import {
hasOwn,
isObject,
toRawType,
hyphenate,
capitalize,
isPlainObject
Expand Down Expand Up @@ -118,9 +119,9 @@ function assertProp (
}
if (!valid) {
warn(
'Invalid prop: type check failed for prop "' + name + '".' +
' Expected ' + expectedTypes.map(capitalize).join(', ') +
', got ' + Object.prototype.toString.call(value).slice(8, -1) + '.',
`Invalid prop: type check failed for prop "${name}".` +
` Expected ${expectedTypes.map(capitalize).join(', ')}` +
`, got ${toRawType(value)}.`,
vm
)
return
Expand Down
7 changes: 7 additions & 0 deletions src/shared/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,15 @@ export function isObject (obj: mixed): boolean %checks {
return obj !== null && typeof obj === 'object'
}

/**
* Get the raw type string of a value e.g. [object Object]
*/
const _toString = Object.prototype.toString

export function toRawType (value: any): string {
return _toString.call(value).slice(8, -1)
}

/**
* Strict object type check. Only returns true
* for plain JavaScript objects.
Expand Down
8 changes: 4 additions & 4 deletions test/helpers/test-object-option.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import Vue from 'vue'

export default function testObjectOption (name) {
it('should warn non object value', () => {
it(`Options ${name}: should warn non object value`, () => {
const options = {}
options[name] = () => {}
new Vue(options)
expect(`component option "${name}" should be an object`).toHaveBeenWarned()
expect(`Invalid value for option "${name}"`).toHaveBeenWarned()
})

it('should not warn valid object value', () => {
it(`Options ${name}: should not warn valid object value`, () => {
const options = {}
options[name] = {}
new Vue(options)
expect(`component option "${name}" should be an object`).not.toHaveBeenWarned()
expect(`Invalid value for option "${name}"`).not.toHaveBeenWarned()
})
}
3 changes: 3 additions & 0 deletions test/unit/features/options/components.spec.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import Vue from 'vue'
import { UA } from 'core/util/env'
import testObjectOption from '../../../helpers/test-object-option'

describe('Options components', () => {
testObjectOption('components')

it('should accept plain object', () => {
const vm = new Vue({
template: '<test></test>',
Expand Down
4 changes: 2 additions & 2 deletions test/unit/features/options/computed.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import Vue from 'vue'
import testObjectOption from '../../../helpers/test-object-option'

describe('Options computed', () => {
testObjectOption('computed')

it('basic usage', done => {
const vm = new Vue({
template: '<div>{{ b }}</div>',
Expand Down Expand Up @@ -49,8 +51,6 @@ describe('Options computed', () => {
}).then(done)
})

testObjectOption('computed')

it('warn with setter and no getter', () => {
const vm = new Vue({
template: `
Expand Down
59 changes: 23 additions & 36 deletions test/unit/features/options/extends.spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Vue from 'vue'
import { nativeWatch } from 'core/util/env'

describe('Options extends', () => {
it('should work on objects', () => {
Expand Down Expand Up @@ -47,42 +48,28 @@ describe('Options extends', () => {
expect(vm.c).toBe(3)
})

it('should work with global mixins + Object.prototype.watch', done => {
let fakeWatch = false
if (!Object.prototype.watch) {
fakeWatch = true
// eslint-disable-next-line no-extend-native
Object.defineProperty(Object.prototype, 'watch', {
writable: true,
configurable: true,
enumerable: false,
value: () => {}
})
}

Vue.mixin({})
if (nativeWatch) {
it('should work with global mixins + Object.prototype.watch', done => {
Vue.mixin({})

const spy = jasmine.createSpy('watch')
const A = Vue.extend({
data: function () {
return { a: 1 }
},
watch: {
a: spy
},
created: function () {
this.a = 2
}
})
new Vue({
extends: A
const spy = jasmine.createSpy('watch')
const A = Vue.extend({
data: function () {
return { a: 1 }
},
watch: {
a: spy
},
created: function () {
this.a = 2
}
})
new Vue({
extends: A
})
waitForUpdate(() => {
expect(spy).toHaveBeenCalledWith(2, 1)
}).then(done)
})
waitForUpdate(() => {
expect(spy).toHaveBeenCalledWith(2, 1)

if (fakeWatch) {
delete Object.prototype.watch
}
}).then(done)
})
}
})
3 changes: 3 additions & 0 deletions test/unit/features/options/inject.spec.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import Vue from 'vue'
import { Observer } from 'core/observer/index'
import { isNative, isObject, hasOwn } from 'core/util/index'
import testObjectOption from '../../../helpers/test-object-option'

describe('Options provide/inject', () => {
testObjectOption('inject')

let injected
const injectedComp = {
inject: ['foo', 'bar'],
Expand Down
4 changes: 2 additions & 2 deletions test/unit/features/options/methods.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import Vue from 'vue'
import testObjectOption from '../../../helpers/test-object-option'

describe('Options methods', () => {
testObjectOption('methods')

it('should have correct context', () => {
const vm = new Vue({
data: {
Expand All @@ -17,8 +19,6 @@ describe('Options methods', () => {
expect(vm.a).toBe(2)
})

testObjectOption('methods')

it('should warn undefined methods', () => {
new Vue({
methods: {
Expand Down
Loading

0 comments on commit b7105ae

Please sign in to comment.