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

Test whether a property is configurable before defining it as reactive

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 () {