Merge pull request #2394 from saul/2393-observe-non-configurable

Test whether a property is configurable before defining it as reactive

Author: yyx990803

src/config.js

@@ -35,14 +35,6 @@ const config = {
 
   warnExpressionErrors: true,
 
-  /**
-   * Whether or not to handle fully object properties which
-   * are already backed by getters and seters. Depending on
-   * use case and environment, this might introduce non-neglible
-   * performance penalties.
-   */
-  convertAllProperties: false,
-
   /**
    * Internal flag to indicate the delimiters have been
    * changed.

src/observer/index.js

@@ -1,4 +1,3 @@
-import config from '../config'
 import Dep from './dep'
 import { arrayMethods } from './array'
 import {
@@ -177,17 +176,15 @@ export function observe (value, vm) {
 export function defineReactive (obj, key, val) {
   var dep = new Dep()
 
-  // cater for pre-defined getter/setters
-  var getter, setter
-  if (config.convertAllProperties) {
-    var property = Object.getOwnPropertyDescriptor(obj, key)
-    if (property && property.configurable === false) {
-      return
-    }
-    getter = property && property.get
-    setter = property && property.set
+  var property = Object.getOwnPropertyDescriptor(obj, key)
+  if (property && property.configurable === false) {
+    return
   }
 
+  // cater for pre-defined getter/setters
+  var getter = property && property.get
+  var setter = property && property.set
+
   var childOb = observe(val)
   Object.defineProperty(obj, key, {
     enumerable: true,

test/unit/specs/directives/public/for/for_spec.js

@@ -1,13 +1,11 @@
 var _ = require('src/util')
 var Vue = require('src')
-var config = require('src/config')
 
 describe('v-for', function () {
   var el
   beforeEach(function () {
     el = document.createElement('div')
     spyWarns()
-    config.convertAllProperties = false
   })
 
   it('objects', function (done) {
@@ -21,18 +19,6 @@ describe('v-for', function () {
     assertMutations(vm, el, done)
   })
 
-  it('objects with convertAllProperties on', function (done) {
-    config.convertAllProperties = true
-    var vm = new Vue({
-      el: el,
-      data: {
-        items: [{a: 1}, {a: 2}]
-      },
-      template: '<div v-for="item in items">{{$index}} {{item.a}}</div>'
-    })
-    assertMutations(vm, el, done)
-  })
-
   it('primitives', function (done) {
     var vm = new Vue({
       el: el,

test/unit/specs/observer/observer_spec.js

@@ -4,7 +4,6 @@ var Observer = ob.Observer
 var observe = ob.observe
 var Dep = require('src/observer/dep')
 var _ = require('src/util')
-var config = require('src/config')
 
 describe('Observer', function () {
   beforeEach(function () {
@@ -59,9 +58,6 @@ describe('Observer', function () {
   })
 
   it('create on already observed object', function () {
-    var previousConvertAllProperties = config.convertAllProperties
-    config.convertAllProperties = true
-
     // on object
     var obj = {}
     var val = 0
@@ -97,14 +93,9 @@ describe('Observer', function () {
     // should call underlying setter
     obj.a = 10
     expect(val).toBe(10)
-
-    config.convertAllProperties = previousConvertAllProperties
   })
 
   it('create on property with only getter', function () {
-    var previousConvertAllProperties = config.convertAllProperties
-    config.convertAllProperties = true
-
     // on object
     var obj = {}
     Object.defineProperty(obj, 'a', {
@@ -134,14 +125,9 @@ describe('Observer', function () {
       obj.a = 101
     } catch (e) {}
     expect(obj.a).toBe(123)
-
-    config.convertAllProperties = previousConvertAllProperties
   })
 
   it('create on property with only setter', function () {
-    var previousConvertAllProperties = config.convertAllProperties
-    config.convertAllProperties = true
-
     // on object
     var obj = {}
     var val = 10
@@ -168,14 +154,9 @@ describe('Observer', function () {
     // writes should call the set function
     obj.a = 100
     expect(val).toBe(100)
-
-    config.convertAllProperties = previousConvertAllProperties
   })
 
   it('create on property which is marked not configurable', function () {
-    var previousConvertAllProperties = config.convertAllProperties
-    config.convertAllProperties = true
-
     // on object
     var obj = {}
     Object.defineProperty(obj, 'a', {
@@ -188,8 +169,6 @@ describe('Observer', function () {
     expect(ob instanceof Observer).toBe(true)
     expect(ob.value).toBe(obj)
     expect(obj.__ob__).toBe(ob)
-
-    config.convertAllProperties = previousConvertAllProperties
   })
 
   it('create on array', function () {
@@ -237,9 +216,6 @@ describe('Observer', function () {
   })
 
   it('observing object prop change on defined property', function () {
-    var previousConvertAllProperties = config.convertAllProperties
-    config.convertAllProperties = true
-
     var obj = { val: 2 }
     Object.defineProperty(obj, 'a', {
       configurable: true,
@@ -271,8 +247,6 @@ describe('Observer', function () {
     expect(obj.val).toBe(3) // make sure 'setter' was called
     obj.val = 5
     expect(obj.a).toBe(5) // make sure 'getter' was called
-
-    config.convertAllProperties = previousConvertAllProperties
   })
 
   it('observing set/delete', function () {