more reliably handle prop reactivity (fix #2569)

Author: yyx990803

src/compiler/compile-props.js

@@ -1,6 +1,5 @@
 import config from '../config'
 import { parseDirective } from '../parsers/directive'
-import { isSimplePath } from '../parsers/expression'
 import { defineReactive } from '../observer/index'
 import propDef from '../directives/internal/prop'
 import {
@@ -226,15 +225,7 @@ export function initProp (vm, prop, value) {
     value = getPropDefaultValue(vm, prop.options)
   }
   if (assertProp(prop, value)) {
-    var doNotObserve =
-      // if the passed down prop was already converted, then
-      // subsequent sets should also be converted, because the user
-      // may mutate the prop binding in the child component (#2549)
-      !(value && value.__ob__) &&
-      // otherwise we can skip observation for props that are either
-      // literal or points to a simple path (non-derived values)
-      (!prop.dynamic || isSimplePath(prop.raw))
-    defineReactive(vm, key, value, doNotObserve)
+    defineReactive(vm, key, value)
   }
 }
 

src/directives/internal/prop.js

@@ -5,28 +5,37 @@
 
 import Watcher from '../../watcher'
 import config from '../../config'
+import { isSimplePath } from '../../parsers/expression'
+import { withoutConversion } from '../../observer/index'
 import { assertProp, initProp, coerceProp } from '../../compiler/compile-props'
 
 const bindingModes = config._propBindingModes
 
 export default {
 
   bind () {
-    var child = this.vm
-    var parent = child._context
+    const child = this.vm
+    const parent = child._context
     // passed in from compiler directly
-    var prop = this.descriptor.prop
-    var childKey = prop.path
-    var parentKey = prop.parentPath
-    var twoWay = prop.mode === bindingModes.TWO_WAY
+    const prop = this.descriptor.prop
+    const childKey = prop.path
+    const parentKey = prop.parentPath
+    const twoWay = prop.mode === bindingModes.TWO_WAY
+    const isSimple = isSimplePath(parentKey)
 
-    var parentWatcher = this.parentWatcher = new Watcher(
+    const parentWatcher = this.parentWatcher = new Watcher(
       parent,
       parentKey,
       function (val) {
         val = coerceProp(prop, val)
         if (assertProp(prop, val)) {
-          child[childKey] = val
+          if (isSimple) {
+            withoutConversion(() => {
+              child[childKey] = val
+            })
+          } else {
+            child[childKey] = val
+          }
         }
       }, {
         twoWay: twoWay,
@@ -38,7 +47,14 @@ export default {
     )
 
     // set the child initial value.
-    initProp(child, prop, parentWatcher.value)
+    const value = parentWatcher.value
+    if (isSimple && value !== undefined) {
+      withoutConversion(() => {
+        initProp(child, prop, value)
+      })
+    } else {
+      initProp(child, prop, value)
+    }
 
     // setup two-way binding
     if (twoWay) {

src/directives/public/for.js

@@ -1,5 +1,6 @@
 import FragmentFactory from '../../fragment/factory'
 import { FOR } from '../priorities'
+import { withoutConversion } from '../../observer/index'
 import {
   isObject,
   warn,
@@ -143,7 +144,9 @@ const vFor = {
         // update data for track-by, object repeat &
         // primitive values.
         if (trackByKey || convertedFromObject || primitive) {
-          frag.scope[alias] = value
+          withoutConversion(() => {
+            frag.scope[alias] = value
+          })
         }
       } else { // new isntance
         frag = this.create(value, alias, i, key)
@@ -238,7 +241,11 @@ const vFor = {
     // for two-way binding on alias
     scope.$forContext = this
     // define scope properties
-    defineReactive(scope, alias, value, true /* do not observe */)
+    // important: define the scope alias without forced conversion
+    // so that frozen data structures remain non-reactive.
+    withoutConversion(() => {
+      defineReactive(scope, alias, value)
+    })
     defineReactive(scope, '$index', index)
     if (key) {
       defineReactive(scope, '$key', key)

src/observer/index.js

@@ -2,7 +2,6 @@ import Dep from './dep'
 import { arrayMethods } from './array'
 import {
   def,
-  isObject,
   isArray,
   isPlainObject,
   hasProto,
@@ -11,6 +10,23 @@ import {
 
 const arrayKeys = Object.getOwnPropertyNames(arrayMethods)
 
+/**
+ * By default, when a reactive property is set, the new value is
+ * also converted to become reactive. However in certain cases, e.g.
+ * v-for scope alias and props, we don't want to force conversion
+ * because the value may be a nested value under a frozen data structure.
+ *
+ * So whenever we want to set a reactive property without forcing
+ * conversion on the new value, we wrap that call inside this function.
+ */
+
+let shouldConvert = true
+export function withoutConversion (fn) {
+  shouldConvert = false
+  fn()
+  shouldConvert = true
+}
+
 /**
  * Observer class that are attached to each observed
  * object. Once attached, the observer converts target
@@ -154,6 +170,7 @@ export function observe (value, vm) {
   ) {
     ob = value.__ob__
   } else if (
+    shouldConvert &&
     (isArray(value) || isPlainObject(value)) &&
     Object.isExtensible(value) &&
     !value._isVue
@@ -172,10 +189,9 @@ export function observe (value, vm) {
  * @param {Object} obj
  * @param {String} key
  * @param {*} val
- * @param {Boolean} doNotObserve
  */
 
-export function defineReactive (obj, key, val, doNotObserve) {
+export function defineReactive (obj, key, val) {
   var dep = new Dep()
 
   var property = Object.getOwnPropertyDescriptor(obj, key)
@@ -187,13 +203,7 @@ export function defineReactive (obj, key, val, doNotObserve) {
   var getter = property && property.get
   var setter = property && property.set
 
-  // if doNotObserve is true, only use the child value observer
-  // if it already exists, and do not attempt to create it.
-  // this allows freezing a large object from the root and
-  // avoid unnecessary observation inside v-for fragments.
-  var childOb = doNotObserve
-    ? isObject(val) && val.__ob__
-    : observe(val)
+  var childOb = observe(val)
   Object.defineProperty(obj, key, {
     enumerable: true,
     configurable: true,
@@ -223,9 +233,7 @@ export function defineReactive (obj, key, val, doNotObserve) {
       } else {
         val = newVal
       }
-      childOb = doNotObserve
-        ? isObject(newVal) && newVal.__ob__
-        : observe(newVal)
+      childOb = observe(newVal)
       dep.notify()
     }
   })

test/unit/specs/directives/internal/prop_spec.js

@@ -678,11 +678,13 @@ describe('prop', function () {
 
   it('non reactive values passed down as prop should not be converted', function (done) {
     var a = Object.freeze({
-      msg: 'hello'
+      nested: {
+        msg: 'hello'
+      }
     })
     var parent = new Vue({
       el: el,
-      template: '<comp :a="a"></comp>',
+      template: '<comp :a="a.nested"></comp>',
       data: {
         a: a
       },
@@ -695,7 +697,11 @@ describe('prop', function () {
     var child = parent.$children[0]
     expect(child.a.msg).toBe('hello')
     expect(child.a.__ob__).toBeUndefined() // should not be converted
-    parent.a = Object.freeze({ msg: 'yo' })
+    parent.a = Object.freeze({
+      nested: {
+        msg: 'yo'
+      }
+    })
     Vue.nextTick(function () {
       expect(child.a.msg).toBe('yo')
       expect(child.a.__ob__).toBeUndefined()
@@ -723,7 +729,7 @@ describe('prop', function () {
   })
 
   // #2549
-  it('mutating child prop binding should be reactive if parent value was reactive', function (done) {
+  it('mutating child prop binding should be reactive', function (done) {
     var vm = new Vue({
       el: el,
       template: '<comp :list="list"></comp>',
@@ -747,4 +753,32 @@ describe('prop', function () {
       done()
     })
   })
+
+  it('prop default value should be reactive', function (done) {
+    var vm = new Vue({
+      el: el,
+      template: '<comp :list="list"></comp>',
+      data: {
+        list: undefined
+      },
+      components: {
+        comp: {
+          props: {
+            list: {
+              default: function () {
+                return [2, 3, 4]
+              }
+            }
+          },
+          template: '<div v-for="i in list">{{ i }}</div>'
+        }
+      }
+    })
+    expect(vm.$el.textContent).toBe('234')
+    vm.$children[0].list.push(5)
+    Vue.nextTick(function () {
+      expect(vm.$el.textContent).toBe('2345')
+      done()
+    })
+  })
 })