Skip to content

Commit

Permalink
refactor(reactivity): readonly collections should not track
Browse files Browse the repository at this point in the history
  • Loading branch information
yyx990803 committed Aug 6, 2020
1 parent ed43810 commit 50adc01
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 23 deletions.
36 changes: 36 additions & 0 deletions packages/reactivity/__tests__/readonly.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,42 @@ describe('reactivity/readonly', () => {
expect(dummy).toBe(2)
})

test('readonly collection should not track', () => {
const map = new Map()
map.set('foo', 1)

const reMap = reactive(map)
const roMap = readonly(map)

let dummy
effect(() => {
dummy = roMap.get('foo')
})
expect(dummy).toBe(1)
reMap.set('foo', 2)
expect(roMap.get('foo')).toBe(2)
// should not trigger
expect(dummy).toBe(1)
})

test('readonly should track and trigger if wrapping reactive original (collection)', () => {
const a = reactive(new Map())
const b = readonly(a)
// should return true since it's wrapping a reactive source
expect(isReactive(b)).toBe(true)

a.set('foo', 1)

let dummy
effect(() => {
dummy = b.get('foo')
})
expect(dummy).toBe(1)
a.set('foo', 2)
expect(b.get('foo')).toBe(2)
expect(dummy).toBe(2)
})

test('wrapping already wrapped value should return same Proxy', () => {
const original = { foo: 1 }
const wrapped = readonly(original)
Expand Down
50 changes: 27 additions & 23 deletions packages/reactivity/src/collectionHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,40 +30,42 @@ const getProto = <T extends CollectionTypes>(v: T): any =>
function get(
target: MapTypes,
key: unknown,
wrap: typeof toReactive | typeof toReadonly | typeof toShallow
isReadonly = false,
isShallow = false
) {
// #1772: readonly(reactive(Map)) should return readonly + reactive version
// of the value
target = (target as any)[ReactiveFlags.RAW]
const rawTarget = toRaw(target)
const rawKey = toRaw(key)
if (key !== rawKey) {
track(rawTarget, TrackOpTypes.GET, key)
!isReadonly && track(rawTarget, TrackOpTypes.GET, key)
}
track(rawTarget, TrackOpTypes.GET, rawKey)
!isReadonly && track(rawTarget, TrackOpTypes.GET, rawKey)
const { has } = getProto(rawTarget)
const wrap = isReadonly ? toReadonly : isShallow ? toShallow : toReactive
if (has.call(rawTarget, key)) {
return wrap(target.get(key))
} else if (has.call(rawTarget, rawKey)) {
return wrap(target.get(rawKey))
}
}

function has(this: CollectionTypes, key: unknown): boolean {
const target = toRaw(this)
function has(this: CollectionTypes, key: unknown, isReadonly = false): boolean {
const target = (this as any)[ReactiveFlags.RAW]
const rawTarget = toRaw(target)
const rawKey = toRaw(key)
if (key !== rawKey) {
track(target, TrackOpTypes.HAS, key)
!isReadonly && track(rawTarget, TrackOpTypes.HAS, key)
}
track(target, TrackOpTypes.HAS, rawKey)
const has = getProto(target).has
return has.call(target, key) || has.call(target, rawKey)
!isReadonly && track(rawTarget, TrackOpTypes.HAS, rawKey)
return target.has(key) || target.has(rawKey)
}

function size(target: IterableCollections) {
target = toRaw(target)
track(target, TrackOpTypes.ITERATE, ITERATE_KEY)
return Reflect.get(getProto(target), 'size', target)
function size(target: IterableCollections, isReadonly = false) {
target = (target as any)[ReactiveFlags.RAW]
!isReadonly && track(toRaw(target), TrackOpTypes.ITERATE, ITERATE_KEY)
return Reflect.get(target, 'size', target)
}

function add(this: SetTypes, value: unknown) {
Expand Down Expand Up @@ -137,15 +139,15 @@ function clear(this: IterableCollections) {
return result
}

function createForEach(isReadonly: boolean, shallow: boolean) {
function createForEach(isReadonly: boolean, isShallow: boolean) {
return function forEach(
this: IterableCollections,
callback: Function,
thisArg?: unknown
) {
const observed = this
const target = toRaw(observed)
const wrap = isReadonly ? toReadonly : shallow ? toShallow : toReactive
const wrap = isReadonly ? toReadonly : isShallow ? toShallow : toReactive
!isReadonly && track(target, TrackOpTypes.ITERATE, ITERATE_KEY)
// important: create sure the callback is
// 1. invoked with the reactive map as `this` and 3rd arg
Expand Down Expand Up @@ -173,19 +175,19 @@ interface IterationResult {
function createIterableMethod(
method: string | symbol,
isReadonly: boolean,
shallow: boolean
isShallow: boolean
) {
return function(
this: IterableCollections,
...args: unknown[]
): Iterable & Iterator {
const target = (this as any)[ReactiveFlags.RAW]
const rawTarget = toRaw(this)
const rawTarget = toRaw(target)
const isMap = rawTarget instanceof Map
const isPair = method === 'entries' || (method === Symbol.iterator && isMap)
const isKeyOnly = method === 'keys' && isMap
const innerIterator = target[method](...args)
const wrap = isReadonly ? toReadonly : shallow ? toShallow : toReactive
const wrap = isReadonly ? toReadonly : isShallow ? toShallow : toReactive
!isReadonly &&
track(
rawTarget,
Expand Down Expand Up @@ -228,7 +230,7 @@ function createReadonlyMethod(type: TriggerOpTypes): Function {

const mutableInstrumentations: Record<string, Function> = {
get(this: MapTypes, key: unknown) {
return get(this, key, toReactive)
return get(this, key)
},
get size() {
return size((this as unknown) as IterableCollections)
Expand All @@ -243,7 +245,7 @@ const mutableInstrumentations: Record<string, Function> = {

const shallowInstrumentations: Record<string, Function> = {
get(this: MapTypes, key: unknown) {
return get(this, key, toShallow)
return get(this, key, false, true)
},
get size() {
return size((this as unknown) as IterableCollections)
Expand All @@ -258,12 +260,14 @@ const shallowInstrumentations: Record<string, Function> = {

const readonlyInstrumentations: Record<string, Function> = {
get(this: MapTypes, key: unknown) {
return get(this, key, toReadonly)
return get(this, key, true)
},
get size() {
return size((this as unknown) as IterableCollections)
return size((this as unknown) as IterableCollections, true)
},
has(this: MapTypes, key: unknown) {
return has.call(this, key, true)
},
has,
add: createReadonlyMethod(TriggerOpTypes.ADD),
set: createReadonlyMethod(TriggerOpTypes.SET),
delete: createReadonlyMethod(TriggerOpTypes.DELETE),
Expand Down

0 comments on commit 50adc01

Please sign in to comment.