Skip to content

Commit f0155ba

Browse files
solkimicrebsolkimicreb
committed
fix: add observable wrapping for collection get like operations
1 parent da52934 commit f0155ba

File tree

4 files changed

+164
-23
lines changed

4 files changed

+164
-23
lines changed

‎src/builtIns/collections.js‎

Lines changed: 55 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,54 @@
1+
import { observable } from '../observable'
12
import {
23
registerRunningReactionForOperation,
3-
queueReactionsForOperation
4+
queueReactionsForOperation,
5+
hasRunningReaction
46
} from '../reactionRunner'
5-
import { proxyToRaw } from '../internals'
7+
import { proxyToRaw, rawToProxy } from '../internals'
68

79
const hasOwnProperty = Object.prototype.hasOwnProperty
810

11+
function findObservable(obj) {
12+
const observableObj = rawToProxy.get(obj)
13+
if (hasRunningReaction() && typeof obj === 'object' && obj !== null) {
14+
if (observableObj) {
15+
return observableObj
16+
}
17+
return observable(obj)
18+
}
19+
return observableObj || obj
20+
}
21+
22+
function patchIterator(iterator, isEntries) {
23+
const originalNext = iterator.next
24+
iterator.next = () => {
25+
let { done, value } = originalNext.call(iterator)
26+
if (!done) {
27+
if (isEntries) {
28+
value[1] = findObservable(value[1])
29+
} else {
30+
value = findObservable(value)
31+
}
32+
}
33+
return { done, value }
34+
}
35+
return iterator
36+
}
37+
938
const instrumentations = {
10-
has (key) {
39+
has(key) {
1140
const target = proxyToRaw.get(this)
1241
const proto = Reflect.getPrototypeOf(this)
1342
registerRunningReactionForOperation({ target, key, type: 'has' })
1443
return proto.has.apply(target, arguments)
1544
},
16-
get (key) {
45+
get(key) {
1746
const target = proxyToRaw.get(this)
1847
const proto = Reflect.getPrototypeOf(this)
1948
registerRunningReactionForOperation({ target, key, type: 'get' })
20-
return proto.get.apply(target, arguments)
49+
return findObservable(proto.get.apply(target, arguments))
2150
},
22-
add (key) {
51+
add(key) {
2352
const target = proxyToRaw.get(this)
2453
const proto = Reflect.getPrototypeOf(this)
2554
const hadKey = proto.has.call(target, key)
@@ -30,7 +59,7 @@ const instrumentations = {
3059
}
3160
return result
3261
},
33-
set (key, value) {
62+
set(key, value) {
3463
const target = proxyToRaw.get(this)
3564
const proto = Reflect.getPrototypeOf(this)
3665
const hadKey = proto.has.call(target, key)
@@ -44,7 +73,7 @@ const instrumentations = {
4473
}
4574
return result
4675
},
47-
delete (key) {
76+
delete(key) {
4877
const target = proxyToRaw.get(this)
4978
const proto = Reflect.getPrototypeOf(this)
5079
const hadKey = proto.has.call(target, key)
@@ -56,7 +85,7 @@ const instrumentations = {
5685
}
5786
return result
5887
},
59-
clear () {
88+
clear() {
6089
const target = proxyToRaw.get(this)
6190
const proto = Reflect.getPrototypeOf(this)
6291
const hadItems = target.size !== 0
@@ -68,37 +97,43 @@ const instrumentations = {
6897
}
6998
return result
7099
},
71-
forEach () {
100+
forEach(cb, ...args) {
72101
const target = proxyToRaw.get(this)
73102
const proto = Reflect.getPrototypeOf(this)
74103
registerRunningReactionForOperation({ target, type: 'iterate' })
75-
return proto.forEach.apply(target, arguments)
104+
// swap out the raw values with their observable pairs
105+
// before passing them to the callback
106+
const wrappedCb = (value, ...rest) => cb(findObservable(value), ...rest)
107+
return proto.forEach.call(target, wrappedCb, ...args)
76108
},
77-
keys () {
109+
keys() {
78110
const target = proxyToRaw.get(this)
79111
const proto = Reflect.getPrototypeOf(this)
80112
registerRunningReactionForOperation({ target, type: 'iterate' })
81113
return proto.keys.apply(target, arguments)
82114
},
83-
values () {
115+
values() {
84116
const target = proxyToRaw.get(this)
85117
const proto = Reflect.getPrototypeOf(this)
86118
registerRunningReactionForOperation({ target, type: 'iterate' })
87-
return proto.values.apply(target, arguments)
119+
const iterator = proto.values.apply(target, arguments)
120+
return patchIterator(iterator, false)
88121
},
89-
entries () {
122+
entries() {
90123
const target = proxyToRaw.get(this)
91124
const proto = Reflect.getPrototypeOf(this)
92125
registerRunningReactionForOperation({ target, type: 'iterate' })
93-
return proto.entries.apply(target, arguments)
126+
const iterator = proto.entries.apply(target, arguments)
127+
return patchIterator(iterator, true)
94128
},
95-
[Symbol.iterator] () {
129+
[Symbol.iterator]() {
96130
const target = proxyToRaw.get(this)
97131
const proto = Reflect.getPrototypeOf(this)
98132
registerRunningReactionForOperation({ target, type: 'iterate' })
99-
return proto[Symbol.iterator].apply(target, arguments)
133+
const iterator = proto[Symbol.iterator].apply(target, arguments)
134+
return patchIterator(iterator, target instanceof Map)
100135
},
101-
get size () {
136+
get size() {
102137
const target = proxyToRaw.get(this)
103138
const proto = Reflect.getPrototypeOf(this)
104139
registerRunningReactionForOperation({ target, type: 'iterate' })
@@ -107,7 +142,7 @@ const instrumentations = {
107142
}
108143

109144
export default {
110-
get (target, key, receiver) {
145+
get(target, key, receiver) {
111146
// instrument methods and property accessors to be reactive
112147
target = hasOwnProperty.call(instrumentations, key)
113148
? instrumentations

‎tests/builtIns/Map.test.js‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect } from 'chai'
2-
import { observable, observe, raw } from '@nx-js/observer-util'
2+
import { observable, isObservable, observe, raw } from '@nx-js/observer-util'
33
import { spy } from '../utils'
44

55
describe('Map', () => {
@@ -287,4 +287,56 @@ describe('Map', () => {
287287
expect(dummy).to.equal(1)
288288
expect(mapSpy.callCount).to.equal(2)
289289
})
290+
291+
it('should wrap object values with observables when requested from a reaction', () => {
292+
const map = observable(new Map())
293+
map.set('key', {})
294+
map.set('key2', {})
295+
296+
expect(isObservable(map.get('key'))).to.be.false
297+
expect(isObservable(map.get('key2'))).to.be.false
298+
observe(() => expect(isObservable(map.get('key'))).to.be.true)
299+
expect(isObservable(map.get('key'))).to.be.true
300+
expect(isObservable(map.get('key2'))).to.be.false
301+
})
302+
303+
it('should wrap object values with observables when iterated from a reaction', () => {
304+
const map = observable(new Map())
305+
map.set('key', {})
306+
307+
map.forEach(value => expect(isObservable(value)).to.be.false)
308+
for (let [key, value] of map) {
309+
expect(isObservable(value)).to.be.false
310+
}
311+
for (let [key, value] of map.entries()) {
312+
expect(isObservable(value)).to.be.false
313+
}
314+
for (let value of map.values()) {
315+
expect(isObservable(value)).to.be.false
316+
}
317+
318+
observe(() => {
319+
map.forEach(value => expect(isObservable(value)).to.be.true)
320+
for (let [key, value] of map) {
321+
expect(isObservable(value)).to.be.true
322+
}
323+
for (let [key, value] of map.entries()) {
324+
expect(isObservable(value)).to.be.true
325+
}
326+
for (let value of map.values()) {
327+
expect(isObservable(value)).to.be.true
328+
}
329+
})
330+
331+
map.forEach(value => expect(isObservable(value)).to.be.true)
332+
for (let [key, value] of map) {
333+
expect(isObservable(value)).to.be.true
334+
}
335+
for (let [key, value] of map.entries()) {
336+
expect(isObservable(value)).to.be.true
337+
}
338+
for (let value of map.values()) {
339+
expect(isObservable(value)).to.be.true
340+
}
341+
})
290342
})

‎tests/builtIns/Set.test.js‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect } from 'chai'
2-
import { observable, observe, raw } from '@nx-js/observer-util'
2+
import { observable, isObservable, observe, raw } from '@nx-js/observer-util'
33
import { spy } from '../utils'
44

55
describe('Set', () => {
@@ -275,4 +275,44 @@ describe('Set', () => {
275275
expect(dummy).to.equal(true)
276276
expect(setSpy.callCount).to.equal(2)
277277
})
278+
279+
it('should wrap object values with observables when iterated from a reaction', () => {
280+
const set = observable(new Set())
281+
set.add({})
282+
283+
set.forEach(value => expect(isObservable(value)).to.be.false)
284+
for (let value of set) {
285+
expect(isObservable(value)).to.be.false
286+
}
287+
for (let [_, value] of set.entries()) {
288+
expect(isObservable(value)).to.be.false
289+
}
290+
for (let value of set.values()) {
291+
expect(isObservable(value)).to.be.false
292+
}
293+
294+
observe(() => {
295+
set.forEach(value => expect(isObservable(value)).to.be.true)
296+
for (let value of set) {
297+
expect(isObservable(value)).to.be.true
298+
}
299+
for (let [_, value] of set.entries()) {
300+
expect(isObservable(value)).to.be.true
301+
}
302+
for (let value of set.values()) {
303+
expect(isObservable(value)).to.be.true
304+
}
305+
})
306+
307+
set.forEach(value => expect(isObservable(value)).to.be.true)
308+
for (let value of set) {
309+
expect(isObservable(value)).to.be.true
310+
}
311+
for (let [_, value] of set.entries()) {
312+
expect(isObservable(value)).to.be.true
313+
}
314+
for (let value of set.values()) {
315+
expect(isObservable(value)).to.be.true
316+
}
317+
})
278318
})

‎tests/builtIns/WeakMap.test.js‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { expect } from 'chai'
2-
import { observable, observe, raw } from '@nx-js/observer-util'
2+
import { observable, isObservable, observe, raw } from '@nx-js/observer-util'
33
import { spy } from '../utils'
44

55
describe('WeakMap', () => {
@@ -82,4 +82,18 @@ describe('WeakMap', () => {
8282
raw(map).delete(key)
8383
expect(dummy).to.equal(undefined)
8484
})
85+
86+
it('should wrap object values with observables when requested from a reaction', () => {
87+
const key = {}
88+
const key2 = {}
89+
const map = observable(new Map())
90+
map.set(key, {})
91+
map.set(key2, {})
92+
93+
expect(isObservable(map.get(key))).to.be.false
94+
expect(isObservable(map.get(key2))).to.be.false
95+
observe(() => expect(isObservable(map.get(key))).to.be.true)
96+
expect(isObservable(map.get(key))).to.be.true
97+
expect(isObservable(map.get(key2))).to.be.false
98+
})
8599
})

0 commit comments

Comments
 (0)