fix(core): injections should not always reactive #(5913)

fix vuejs#5913

Author: Kingwl

src/core/instance/inject.js

@@ -1,8 +1,8 @@
 /* @flow */
 
 import { hasSymbol } from 'core/util/env'
-import { warn } from '../util/index'
-import { defineReactive } from '../observer/index'
+import { warn, defWithGetterSetter } from '../util/index'
+import { defineReactive, isObserver } from '../observer/index'
 import { hasOwn } from 'shared/util'
 
 export function initProvide (vm: Component) {
@@ -18,18 +18,28 @@ export function initInjections (vm: Component) {
   const result = resolveInject(vm.$options.inject, vm)
   if (result) {
     Object.keys(result).forEach(key => {
+      const value = result[key]
+      const warnSetter = () => {
+        warn(
+          `Avoid mutating an injected value directly since the changes will be ` +
+          `overwritten whenever the provided component re-renders. ` +
+          `injection being mutated: "${key}"`,
+          vm
+        )
+      }
       /* istanbul ignore else */
       if (process.env.NODE_ENV !== 'production') {
-        defineReactive(vm, key, result[key], () => {
-          warn(
-            `Avoid mutating an injected value directly since the changes will be ` +
-            `overwritten whenever the provided component re-renders. ` +
-            `injection being mutated: "${key}"`,
-            vm
-          )
-        })
+        if (isObserver(value)) {
+          defineReactive(vm, key, value, warnSetter)
+        } else {
+          defWithGetterSetter(vm, key, value, warnSetter)
+        }
       } else {
-        defineReactive(vm, key, result[key])
+        if (isObserver(value)) {
+          defineReactive(vm, key, value)
+        } else {
+          defWithGetterSetter(vm, key, value)
+        }
       }
     })
   }

src/core/observer/index.js

@@ -109,7 +109,7 @@ export function observe (value: any, asRootData: ?boolean): Observer | void {
     return
   }
   let ob: Observer | void
-  if (hasOwn(value, '__ob__') && value.__ob__ instanceof Observer) {
+  if (isObserver(value)) {
     ob = value.__ob__
   } else if (
     observerState.shouldConvert &&
@@ -255,3 +255,10 @@ function dependArray (value: Array<any>) {
     }
   }
 }
+
+export function isObserver (obj: any): boolean {
+  if (isObject(obj)) {
+    return hasOwn(obj, '__ob__') && obj.__ob__ instanceof Observer
+  }
+  return false
+}

src/core/util/lang.js

@@ -22,6 +22,26 @@ export function def (obj: Object, key: string, val: any, enumerable?: boolean) {
   })
 }
 
+/**
+ * Define a property with getter and setter.
+ */
+export function defWithGetterSetter (obj: Object, key: string, val: any, customSetter?: Function) {
+  Object.defineProperty(obj, key, {
+    enumerable: true,
+    configurable: true,
+    get: function getter () {
+      return val
+    },
+    set: function setter (newVal) {
+      /* eslint-enable no-self-compare */
+      if (process.env.NODE_ENV !== 'production' && customSetter) {
+        customSetter()
+      }
+      val = newVal
+    }
+  })
+}
+
 /**
  * Parse simple path.
  */

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

@@ -1,5 +1,6 @@
 import Vue from 'vue'
 import { isNative } from 'core/util/env'
+import { isObserver } from 'core/observer'
 
 describe('Options provide/inject', () => {
   let injected
@@ -186,40 +187,78 @@ describe('Options provide/inject', () => {
     })
   }
 
-  // Github issue #5223
-  it('should work with reactive array', done => {
+  it('should work when the provide change', done => {
     const vm = new Vue({
       template: `<div><child></child></div>`,
       data () {
         return {
-          foo: []
+          foo: 0,
+          bar: {
+            val: 0
+          },
+          baz: []
         }
       },
       provide () {
         return {
-          foo: this.foo
+          foo: this.foo,
+          bar: this.bar,
+          baz: this.baz
         }
       },
       components: {
         child: {
-          inject: ['foo'],
-          template: `<span>{{foo.length}}</span>`
+          inject: ['foo', 'bar', 'baz'],
+          template: `<span>{{foo}},{{bar.val}},{{baz.length}}</span>`
         }
       }
     }).$mount()
 
-    expect(vm.$el.innerHTML).toEqual(`<span>0</span>`)
-    vm.foo.push(vm.foo.length)
+    expect(vm.$el.innerHTML).toEqual(`<span>0,0,0</span>`)
+    vm.foo = 1 // primitive should no modified
+    vm.bar.val = 1 // reactive should modified
+    vm.baz.push(0) // reactive array should modified
     vm.$nextTick(() => {
-      expect(vm.$el.innerHTML).toEqual(`<span>1</span>`)
-      vm.foo.pop()
-      vm.$nextTick(() => {
-        expect(vm.$el.innerHTML).toEqual(`<span>0</span>`)
-        done()
-      })
+      expect(vm.$el.innerHTML).toEqual(`<span>0,1,1</span>`)
+      done()
     })
   })
 
+  // Github issue #5913
+  it('should keep the reactive with provide', () => {
+    const vm = new Vue({
+      template: `<div><child ref='child'></child></div>`,
+      data () {
+        return {
+          foo: {},
+          $foo: {},
+          foo1: []
+        }
+      },
+      provide () {
+        return {
+          foo: this.foo,
+          $foo: this.$foo,
+          foo1: this.foo1,
+          bar: {},
+          baz: []
+        }
+      },
+      components: {
+        child: {
+          inject: ['foo', '$foo', 'foo1', 'bar', 'baz'],
+          template: `<span/>`
+        }
+      }
+    }).$mount()
+    const child = vm.$refs.child
+    expect(isObserver(child.foo)).toBe(true)
+    expect(isObserver(child.$foo)).toBe(false)
+    expect(isObserver(child.foo1)).toBe(true)
+    expect(isObserver(child.bar)).toBe(false)
+    expect(isObserver(child.baz)).toBe(false)
+  })
+
   it('should extend properly', () => {
     const parent = Vue.extend({
       template: `<span/>`,
@@ -250,24 +289,31 @@ describe('Options provide/inject', () => {
   })
 
   it('should warn when injections has been modified', () => {
-    const key = 'foo'
+    const makeWarnText = key =>
+      `Avoid mutating an injected value directly since the changes will be ` +
+      `overwritten whenever the provided component re-renders. ` +
+      `injection being mutated: "${key}"`
+
     const vm = new Vue({
       provide: {
-        foo: 1
+        foo: 1,
+        bar: {
+          val: 1
+        }
       }
     })
 
     const child = new Vue({
       parent: vm,
-      inject: ['foo']
+      inject: ['foo', 'bar']
     })
 
     expect(child.foo).toBe(1)
+    expect(child.bar.val).toBe(1)
     child.foo = 2
-    expect(
-      `Avoid mutating an injected value directly since the changes will be ` +
-      `overwritten whenever the provided component re-renders. ` +
-      `injection being mutated: "${key}"`).toHaveBeenWarned()
+    expect(makeWarnText('foo')).toHaveBeenWarned()
+    child.bar = { val: 2 }
+    expect(makeWarnText('bar')).toHaveBeenWarned()
   })
 
   it('should warn when injections cannot be found', () => {