8000 Merge branch 'getset' into dev · cdxfish/vue@437409b · GitHub
[go: up one dir, main page]

Skip to co 6608 ntent

Commit 437409b

Browse files
committed
Merge branch 'getset' into dev
2 parents 50a79e4 + fd19698 commit 437409b

File tree

3 files changed

+223
-9
lines changed

3 files changed

+223
-9
lines changed

src/config.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,14 @@ module.exports = {
3030

3131
warnExpressionErrors: true,
3232

33+
/**
34+
* Whether or not to handle fully object properties which
35+
* are already backed by getters and seters. Depending on
36+
* use case and environment, this might introduce non-neglible
37+
* performance penalties.
38+
*/
39+
convertAllProperties: false,
40+
3341
/**
3442
* Internal flag to indicate the delimiters have been
3543
* changed.

src/observer/index.js

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
var _ = require('../util')
2+
var config = require('../config')
23
var Dep = require('./dep')
34
var arrayMethods = require('./array')
45
var arrayKeys = Object.getOwnPropertyNames(arrayMethods)
@@ -172,11 +173,37 @@ function copyAugment (target, src, keys) {
172173

173174
function defineReactive (obj, key, val) {
174175
var dep = new Dep()
175-
var childOb = Observer.create(val)
176-
Object.defineProperty(obj, key, {
176+
var hasGetter = true
177+
var hasSetter = true
178+
179+
var target = {
180+
val: val
181+
}
182+
183+
if (config.convertAllProperties) {
184+
var property = Object.getOwnPropertyDescriptor(obj, key)
185+
if (property && property.configurable === false) {
186+
return
187+
}
188+
if (property && (property.get || property.set)) {
189+
hasGetter = property.get !== undefined
190+
hasSetter = property.set !== undefined
191+
Object.defineProperty(target, 'val', {
192+
get: property.get && _.bind(property.get, obj),
193+
set: property.set && _.bind(property.set, obj)
194+
})
195+
}
196+
}
197+
198+
var childOb = Observer.create(target.val)
199+
var propertyDefinition = {
177200
enumerable: true,
178-
configurable: true,
179-
get: function metaGetter () {
201+
configurable: true
202+
}
203+
204+
if (hasGetter) {
205+
propertyDefinition.get = function metaGetter () {
206+
var val = target.val
180207
if (Dep.target) {
181208
dep.depend()
182209
if (childOb) {
@@ -190,14 +217,19 @@ function defineReactive (obj, key, val) {
190217
}
191218
}
192219
return val
193-
},
194-
set: function metaSetter (newVal) {
195-
if (newVal === val) return
196-
val = newVal
220+
}
221+
}
222+
223+
if (hasSetter) {
224+
propertyDefinition.set = function metaSetter (newVal) {
225+
if (newVal === target.val) return
226+
target.val = newVal
197227
childOb = Observer.create(newVal)
198228
dep.notify()
199229
}
200-
})
230+
}
231+
232+
Object.defineProperty(obj, key, propertyDefinition)
201233
}
202234

203235
// Attach to the util object so it can be used elsewhere.

test/unit/specs/observer/observer_spec.js

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
var Observer = require('../../../../src/observer')
22
var Dep = require('../../../../src/observer/dep')
33
var _ = require('../../../../src/util')
4+
var config = require('../../../../src/config')
45

56
describe('Observer', function () {
67

@@ -55,6 +56,140 @@ describe('Observer', function () {
5556
expect(ob2).toBe(ob)
5657
})
5758

59+
it('create on already observed object', function () {
60+
var previousConvertAllProperties = config.convertAllProperties
61+
config.convertAllProperties = true
62+
63+
// on object
64+
var obj = {}
65+
var val = 0
66+
var getCount = 0
67+
Object.defineProperty(obj, 'a', {
68+
configurable: true,
69+
enumerable: true,
70+
get: function () {
71+
getCount++
72+
return val
73+
},
74+
set: function (v) {
75+
val = v
76+
}
77+
})
78+
79+
var ob = Observer.create(obj)
80+
expect(ob instanceof Observer).toBe(true)
81+
expect(ob.value).toBe(obj)
82+
expect(obj.__ob__).toBe(ob)
83+
84+
getCount = 0
85+
// Each read of 'a' should result in only one get underlying get call
86+
obj.a
87+
expect(getCount).toBe(1)
88+
obj.a
89+
expect(getCount).toBe(2)
90+
91+
// should return existing ob on already observed objects
92+
var ob2 = Observer.create(obj)
93+
expect(ob2).toBe(ob)
94+
95+
// should call underlying setter
96+
obj.a = 10
97+
expect(val).toBe(10)
98+
99+
config.convertAllProperties = previousConvertAllProperties
100+
})
101+
102+
it('create on property with only getter', function () {
103+
var previousConvertAllProperties = config.convertAllProperties
104+
config.convertAllProperties = true
105+
106+
// on object
107+
var obj = {}
108+
Object.defineProperty(obj, 'a', {
109+
configurable: true,
110+
enumerable: true,
111+
get: function () {
112+
return 123
113+
}
114+
})
115+
116+
var ob = Observer.create(obj)
117+
expect(ob instanceof Observer).toBe(true)
118+
expect(ob.value).toBe(obj)
119+
expect(obj.__ob__).toBe(ob)
120+
121+
// should be able to read
122+
expect(obj.a).toBe(123)
123+
124+
// should return existing ob on already observed objects
125+
var ob2 = Observer.create(obj)
126+
expect(ob2).toBe(ob)
127+
128+
// since there is no setter, you shouldn't be able to write to it
129+
// PhantomJS throws when a property with no setter is set
130+
// but other real browsers don't
131+
try {
132+
obj.a = 101
133+
} catch (e) {}
134+
expect(obj.a).toBe(123)
135+
136+
config.convertAllProperties = previousConvertAllProperties
137+
})
E377
138+
139+
it('create on property with only setter', function () {
140+
var previousConvertAllProperties = config.convertAllProperties
141+
config.convertAllProperties = true
142+
143+
// on object
144+
var obj = {}
145+
var val = 10
146+
Object.defineProperty(obj, 'a', { // eslint-disable-line accessor-pairs
147+
configurable: true,
148+
enumerable: true,
149+
set: function (v) {
150+
val = v
151+
}
152+
})
153+
154+
var ob = Observer.create(obj)
155+
expect(ob instanceof Observer).toBe(true)
156+
expect(ob.value).toBe(obj)
157+
expect(obj.__ob__).toBe(ob)
158+
159+
// reads should return undefined
160+
expect(obj.a).toBe(undefined)
161+
162+
// should return existing ob on already observed objects
163+
var ob2 = Observer.create(obj)
164+
expect(ob2).toBe(ob)
165+
166+
// writes should call the set function
167+
obj.a = 100
168+
expect(val).toBe(100)
169+
170+
config.convertAllProperties = previousConvertAllProperties
171+
})
172+
173+
it('create on property which is marked not configurable', function () {
174+
var previousConvertAllProperties = config.convertAllProperties
175+
config.convertAllProperties = true
176+
177+
// on object
178+
var obj = {}
179+
Object.defineProperty(obj, 'a', {
180+
configurable: false,
181+
enumerable: true,
182+
val: 10
183+
})
184+
185+
var ob = Observer.create(obj)
186+
expect(ob instanceof Observer).toBe(true)
187+
expect(ob.value).toBe(obj)
188+
expect(obj.__ob__).toBe(ob)
189+
190+
config.convertAllProperties = previousConvertAllProperties
191+
})
192+
58193
it('create on array', function () {
59194
// on object
60195
var arr = [{}, {}]
@@ -99,6 +234,45 @@ describe('Observer', function () {
99234
expect(watcher.update.calls.count()).toBe(3)
100235
})
101236

237+
it('observing object prop change on defined property', function () {
238+
var previousConvertAllProperties = config.convertAllProperties
239+
config.convertAllProperties = true
240+
241+
var obj = { val: 2 }
242+
Object.defineProperty(obj, 'a', {
243+
configurable: true,
244+
enumerable: true,
245+
get: function () {
246+
return this.val
247+
},
248+
set: function (v) {
249+
this.val = v
250+
return this.val
251+
}
252+
})
253+
254+
Observer.create(obj)
255+
// mock a watcher!
256+
var watcher = {
257+
deps: [],
258+
addDep: function (dep) {
259+
this.deps.push(dep)
260+
dep.addSub(this)
261+
},
262+
update: jasmine.createSpy()
263+
}
264+
// collect dep
265+
Dep.target = watcher
266+
expect(obj.a).toBe(2) // Make sure 'this' is preserved
267+
Dep.target = null
268+
obj.a = 3
269+
expect(obj.val).toBe(3) // make sure 'setter' was called
270+
obj.val = 5
271+
expect(obj.a).toBe(5) // make sure 'getter' was called
272+
273+
config.convertAllProperties = previousConvertAllProperties
274+
})
275+
102276
it('observing set/delete', function () {
103277
var obj = { a: 1 }
104278
var ob = Observer.create(obj)

0 commit comments

Comments
 (0)
0