feat(apiOptions): add warning for duplicated properties declared by options (#329)

packages/runtime-core/__tests__/apiOptions.spec.ts

@@ -575,5 +575,263 @@ describe('api: options', () => {
         'Computed property "foo" was assigned to but it has no setter.'
       ).toHaveBeenWarned()
     })
+
+    test('data property is already declared in props', () => {
+      const Comp = {
+        props: { foo: Number },
+        data: {
+          foo: 1
+        },
+        render() {}
+      }
+
+      const root = nodeOps.createElement('div')
+      render(h(Comp), root)
+      expect(
+        `Data property "foo" is already defined in Props.`
+      ).toHaveBeenWarned()
+    })
+
+    test('computed property is already declared in data', () => {
+      const Comp = {
+        data: {
+          foo: 1
+        },
+        computed: {
+          foo() {}
+        },
+        render() {}
+      }
+
+      const root = nodeOps.createElement('div')
+      render(h(Comp), root)
+      expect(
+        `Computed property "foo" is already defined in Data.`
+      ).toHaveBeenWarned()
+    })
+
+    test('computed property is already declared in props', () => {
+      const Comp = {
+        props: { foo: Number },
+        computed: {
+          foo() {}
+        },
+        render() {}
+      }
+
+      const root = nodeOps.createElement('div')
+      render(h(Comp), root)
+      expect(
+        `Computed property "foo" is already defined in Props.`
+      ).toHaveBeenWarned()
+    })
+
+    test('methods property is not a function', () => {
+      const Comp = {
+        methods: {
+          foo: 1
+        },
+        render() {}
+      }
+
+      const root = nodeOps.createElement('div')
+      render(h(Comp), root)
+      expect(
+        `Method "foo" has type "number" in the component definition. ` +
+          `Did you reference the function correctly?`
+      ).toHaveBeenWarned()
+    })
+
+    test('methods property is already declared in data', () => {
+      const Comp = {
+        data: {
+          foo: 2
+        },
+        methods: {
+          foo() {}
+        },
+        render() {}
+      }
+
+      const root = nodeOps.createElement('div')
+      render(h(Comp), root)
+      expect(
+        `Methods property "foo" is already defined in Data.`
+      ).toHaveBeenWarned()
+    })
+
+    test('methods property is already declared in props', () => {
+      const Comp = {
+        props: {
+          foo: Number
+        },
+        methods: {
+          foo() {}
+        },
+        render() {}
+      }
+
+      const root = nodeOps.createElement('div')
+      render(h(Comp), root)
+      expect(
+        `Methods property "foo" is already defined in Props.`
+      ).toHaveBeenWarned()
+    })
+
+    test('methods property is already declared in computed', () => {
+      const Comp = {
+        computed: {
+          foo: {
+            get() {},
+            set() {}
+          }
+        },
+        methods: {
+          foo() {}
+        },
+        render() {}
+      }
+
+      const root = nodeOps.createElement('div')
+      render(h(Comp), root)
+      expect(
+        `Methods property "foo" is already defined in Computed.`
+      ).toHaveBeenWarned()
+    })
+
+    test('inject property is already declared in data', () => {
+      const Comp = {
+        data() {
+          return {
+            a: 1
+          }
+        },
+        provide() {
+          return {
+            a: this.a
+          }
+        },
+        render() {
+          return [h(ChildA)]
+        }
+      } as any
+      const ChildA = {
+        data() {
+          return {
+            a: 1
+          }
+        },
+        inject: ['a'],
+        render() {
+          return this.a
+        }
+      } as any
+
+      const root = nodeOps.createElement('div')
+      render(h(Comp), root)
+      expect(
+        `Inject property "a" is already defined in Data.`
+      ).toHaveBeenWarned()
+    })
+
+    test('inject property is already declared in props', () => {
+      const Comp = {
+        data() {
+          return {
+            a: 1
+          }
+        },
+        provide() {
+          return {
+            a: this.a
+          }
+        },
+        render() {
+          return [h(ChildA)]
+        }
+      } as any
+      const ChildA = {
+        props: { a: Number },
+        inject: ['a'],
+        render() {
+          return this.a
+        }
+      } as any
+
+      const root = nodeOps.createElement('div')
+      render(h(Comp), root)
+      expect(
+        `Inject property "a" is already defined in Props.`
+      ).toHaveBeenWarned()
+    })
+
+    test('inject property is already declared in computed', () => {
+      const Comp = {
+        data() {
+          return {
+            a: 1
+          }
+        },
+        provide() {
+          return {
+            a: this.a
+          }
+        },
+        render() {
+          return [h(ChildA)]
+        }
+      } as any
+      const ChildA = {
+        computed: {
+          a: {
+            get() {},
+            set() {}
+          }
+        },
+        inject: ['a'],
+        render() {
+          return this.a
+        }
+      } as any
+
+      const root = nodeOps.createElement('div')
+      render(h(Comp), root)
+      expect(
+        `Inject property "a" is already defined in Computed.`
+      ).toHaveBeenWarned()
+    })
+
+    test('inject property is already declared in methods', () => {
+      const Comp = {
+        data() {
+          return {
+            a: 1
+          }
+        },
+        provide() {
+          return {
+            a: this.a
+          }
+        },
+        render() {
+          return [h(ChildA)]
+        }
+      } as any
+      const ChildA = {
+        methods: {
+          a: () => null
+        },
+        inject: ['a'],
+        render() {
+          return this.a
+        }
+      } as any
+
+      const root = nodeOps.createElement('div')
+      render(h(Comp), root)
+      expect(
+        `Inject property "a" is already defined in Methods.`
+      ).toHaveBeenWarned()
+    })
   })
 })

packages/runtime-core/src/apiOptions.ts

@@ -179,6 +179,25 @@ export interface LegacyOptions<
   errorCaptured?: ErrorCapturedHook
 }
 
+const enum OptionTypes {
+  PROPS = 'Props',
+  DATA = 'Data',
+  COMPUTED = 'Computed',
+  METHODS = 'Methods',
+  INJECT = 'Inject'
+}
+
+function createDuplicateChecker() {
+  const cache = Object.create(null)
+  return (type: OptionTypes, key: string) => {
+    if (cache[key]) {
+      warn(`${type} property "${key}" is already defined in ${cache[key]}.`)
+    } else {
+      cache[key] = type
+    }
+  }
+}
+
 export function applyOptions(
   instance: ComponentInternalInstance,
   options: ComponentOptions,
@@ -194,6 +213,7 @@ export function applyOptions(
     mixins,
     extends: extendsOptions,
     // state
+    props: propsOptions,
     data: dataOptions,
     computed: computedOptions,
     methods,
@@ -218,6 +238,8 @@ export function applyOptions(
   } = options
 
   const globalMixins = instance.appContext.mixins
+  // call it only during dev
+  const checkDuplicateProperties = __DEV__ ? createDuplicateChecker() : null
   // applyOptions is called non-as-mixin once per instance
   if (!asMixin) {
     callSyncHook('beforeCreate', options, ctx, globalMixins)
@@ -233,12 +255,23 @@ export function applyOptions(
     applyMixins(instance, mixins)
   }
 
+  if (__DEV__ && propsOptions) {
+    for (const key in propsOptions) {
+      checkDuplicateProperties!(OptionTypes.PROPS, key)
+    }
+  }
+
   // state options
   if (dataOptions) {
     const data = isFunction(dataOptions) ? dataOptions.call(ctx) : dataOptions
     if (!isObject(data)) {
       __DEV__ && warn(`data() should return an object.`)
     } else if (instance.data === EMPTY_OBJ) {
+      if (__DEV__) {
+        for (const key in data) {
+          checkDuplicateProperties!(OptionTypes.DATA, key)
+        }
+      }
       instance.data = reactive(data)
     } else {
       // existing data: this is a mixin or extends.
@@ -249,6 +282,8 @@ export function applyOptions(
     for (const key in computedOptions) {
       const opt = (computedOptions as ComputedOptions)[key]
 
+      __DEV__ && checkDuplicateProperties!(OptionTypes.COMPUTED, key)
+
       if (isFunction(opt)) {
         renderContext[key] = computed(opt.bind(ctx))
       } else {
@@ -272,9 +307,19 @@ export function applyOptions(
       }
     }
   }
+
   if (methods) {
     for (const key in methods) {
-      renderContext[key] = (methods as MethodOptions)[key].bind(ctx)
+      const methodHandler = (methods as MethodOptions)[key]
+      if (isFunction(methodHandler)) {
+        __DEV__ && checkDuplicateProperties!(OptionTypes.METHODS, key)
+        renderContext[key] = methodHandler.bind(ctx)
+      } else if (__DEV__) {
+        warn(
+          `Method "${key}" has type "${typeof methodHandler}" in the component definition. ` +
+            `Did you reference the function correctly?`
+        )
+      }
     }
   }
   if (watchOptions) {
@@ -310,10 +355,12 @@ export function applyOptions(
     if (isArray(injectOptions)) {
       for (let i = 0; i < injectOptions.length; i++) {
         const key = injectOptions[i]
+        __DEV__ && checkDuplicateProperties!(OptionTypes.INJECT, key)
         renderContext[key] = inject(key)
       }
     } else {
       for (const key in injectOptions) {
+        __DEV__ && checkDuplicateProperties!(OptionTypes.INJECT, key)
         const opt = injectOptions[key]
         if (isObject(opt)) {
           renderContext[key] = inject(opt.from, opt.default)