refactor: improve option type check warnings

Author: yyx990803

src/core/instance/state.js

@@ -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 = {}
@@ -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()
@@ -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') {
@@ -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)) {

src/core/util/debug.js

@@ -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]

src/core/util/options.js

@@ -14,6 +14,7 @@ import {
   extend,
   hasOwn,
   camelize,
+  toRawType,
   capitalize,
   isBuiltInTag,
   isPlainObject
@@ -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) {
@@ -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)
@@ -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)
@@ -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 = {}
@@ -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)) {
@@ -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
+    )
   }
 }
 
@@ -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.
@@ -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) {

src/core/util/props.js

@@ -5,6 +5,7 @@ import { observe, observerState } from '../observer/index'
 import {
   hasOwn,
   isObject,
+  toRawType,
   hyphenate,
   capitalize,
   isPlainObject
@@ -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

src/shared/util.js

@@ -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.

test/helpers/test-object-option.js

@@ -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()
   })
 }

test/unit/features/options/components.spec.js

@@ -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>',

test/unit/features/options/computed.spec.js

@@ -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>',
@@ -49,8 +51,6 @@ describe('Options computed', () => {
     }).then(done)
   })
 
-  testObjectOption('computed')
-
   it('warn with setter and no getter', () => {
     const vm = new Vue({
       template: `

test/unit/features/options/extends.spec.js

@@ -1,4 +1,5 @@
 import Vue from 'vue'
+import { nativeWatch } from 'core/util/env'
 
 describe('Options extends', () => {
   it('should work on objects', () => {
@@ -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)
-  })
+  }
 })

test/unit/features/options/inject.spec.js

@@ -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'],