Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(reactivity): avoid duplication proxy #3052

Closed
wants to merge 7 commits into from
28 changes: 21 additions & 7 deletions packages/reactivity/__tests__/readonly.spec.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import {
reactive,
readonly,
toRaw,
effect,
isProxy,
isReactive,
isReadonly,
markRaw,
effect,
reactive,
readonly,
ref,
shallowReadonly,
isProxy
toRaw
} from '../src'

/**
Expand Down Expand Up @@ -325,14 +325,20 @@ describe('reactivity/readonly', () => {
})
})

test('calling reactive on an readonly should return readonly', () => {
test('calling reactive on an readonly should return reactive', () => {
const a = readonly({})
const b = reactive(a)
expect(isReadonly(b)).toBe(true)
expect(isReactive(b)).toBe(true)
// should point to same original
expect(toRaw(a)).toBe(toRaw(b))
})

test('calling reactive on an nesting object, if the object property is readonly, get the value of the property will get the readonly version', () => {
const a = readonly({})
const b = reactive({ a: a })
expect(isReadonly(b.a)).toBe(true)
})

test('calling readonly on a reactive object should return readonly', () => {
const a = reactive({})
const b = readonly(a)
Expand Down Expand Up @@ -461,5 +467,13 @@ describe('reactivity/readonly', () => {
`Set operation on key "foo" failed: target is readonly.`
).not.toHaveBeenWarned()
})

test('should allow shallow und normal reactive for same target', () => {
const target = { foo: 1 }
const shallowProxy = shallowReadonly(target)
const normalProxy = readonly(target)

expect(normalProxy).not.toBe(shallowProxy)
})
})
})
11 changes: 10 additions & 1 deletion packages/reactivity/__tests__/shallowReactive.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { shallowReactive, isReactive, reactive } from '../src/reactive'
import { isReactive, reactive, shallowReactive } from '../src/reactive'

import { effect } from '../src/effect'

describe('shallowReactive', () => {
Expand All @@ -13,6 +14,14 @@ describe('shallowReactive', () => {
expect(isReactive(props.n)).toBe(true)
})

test('should allow shallow und normal reactive for same target', () => {
const target = { foo: 1 }
const shallowProxy = shallowReactive(target)
const normalProxy = reactive(target)

expect(normalProxy).not.toBe(shallowProxy)
})

describe('collections', () => {
test('should be reactive', () => {
const shallowSet = shallowReactive(new Set())
Expand Down
36 changes: 19 additions & 17 deletions packages/reactivity/src/baseHandlers.ts
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
import {
ITERATE_KEY,
pauseTracking,
resetTracking,
track,
trigger
} from './effect'
import {
ReactiveFlags,
Target,
reactive,
readonly,
toRaw,
ReactiveFlags,
Target,
readonlyMap,
reactiveMap
getProxyMap
} from './reactive'
import { TrackOpTypes, TriggerOpTypes } from './operations'
import {
track,
trigger,
ITERATE_KEY,
pauseTracking,
resetTracking
} from './effect'
import {
isObject,
hasOwn,
isSymbol,
extend,
hasChanged,
hasOwn,
isArray,
isIntegerKey,
extend
isObject,
isSymbol
} from '@vue/shared'

import { isRef } from './ref'

const builtInSymbols = new Set(
Expand Down Expand Up @@ -75,9 +75,11 @@ function createGetter(isReadonly = false, shallow = false) {
return !isReadonly
} else if (key === ReactiveFlags.IS_READONLY) {
return isReadonly
} else if (key === ReactiveFlags.IS_SHALLOW) {
return shallow
} else if (
key === ReactiveFlags.RAW &&
receiver === (isReadonly ? readonlyMap : reactiveMap).get(target)
receiver === getProxyMap(isReadonly, shallow).get(target)
) {
return target
}
Expand Down Expand Up @@ -116,7 +118,7 @@ function createGetter(isReadonly = false, shallow = false) {
// Convert returned value into a proxy as well. we do the isObject check
// here to avoid invalid value warning. Also need to lazy access readonly
// and reactive here to avoid circular dependency.
return isReadonly ? readonly(res) : reactive(res)
return isReadonly ? readonly(res, false) : reactive(res, false)
}

return res
Expand Down
4 changes: 2 additions & 2 deletions packages/reactivity/src/collectionHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ type MapTypes = Map<any, any> | WeakMap<any, any>
type SetTypes = Set<any> | WeakSet<any>

const toReactive = <T extends unknown>(value: T): T =>
isObject(value) ? reactive(value) : value
isObject(value) ? reactive(value, false) : value

const toReadonly = <T extends unknown>(value: T): T =>
isObject(value) ? readonly(value as Record<any, any>) : value
isObject(value) ? readonly(value as Record<any, any>, false) : value

const toShallow = <T extends unknown>(value: T): T => value

Expand Down
102 changes: 73 additions & 29 deletions packages/reactivity/src/reactive.ts
Original file line number Diff line number Diff line change
@@ -1,33 +1,47 @@
import { isObject, toRawType, def } from '@vue/shared'
import { Ref, UnwrapRef } from './ref'
import { def, isObject, toRawType } from '@vue/shared'
import {
mutableCollectionHandlers,
readonlyCollectionHandlers,
shallowCollectionHandlers
} from './collectionHandlers'
import {
mutableHandlers,
readonlyHandlers,
shallowReactiveHandlers,
shallowReadonlyHandlers
} from './baseHandlers'
import {
mutableCollectionHandlers,
readonlyCollectionHandlers,
shallowCollectionHandlers
} from './collectionHandlers'
import { UnwrapRef, Ref } from './ref'

export const enum ReactiveFlags {
SKIP = '__v_skip',
IS_REACTIVE = '__v_isReactive',
IS_READONLY = '__v_isReadonly',
IS_SHALLOW = '__v_isShallow',
RAW = '__v_raw'
}

export interface Target {
[ReactiveFlags.SKIP]?: boolean
[ReactiveFlags.IS_REACTIVE]?: boolean
[ReactiveFlags.IS_READONLY]?: boolean
[ReactiveFlags.IS_SHALLOW]?: boolean
[ReactiveFlags.RAW]?: any
}

export const reactiveMap = new WeakMap<Target, any>()
export const shallowReactiveMap = new WeakMap<Target, any>()
export const readonlyMap = new WeakMap<Target, any>()
export const shallowReadonlyMap = new WeakMap<Target, any>()

export function getProxyMap(isReadonly: boolean, shallow: boolean) {
return isReadonly
? shallow
? shallowReadonlyMap
: readonlyMap
: shallow
? shallowReactiveMap
: reactiveMap
}

const enum TargetType {
INVALID = 0,
Expand Down Expand Up @@ -82,16 +96,20 @@ type UnwrapNestedRefs<T> = T extends Ref ? T : UnwrapRef<T>
* ```
*/
export function reactive<T extends object>(target: T): UnwrapNestedRefs<T>
export function reactive(target: object) {
// if trying to observe a readonly proxy, return the readonly version.
if (target && (target as Target)[ReactiveFlags.IS_READONLY]) {
return target
}

export function reactive<T extends object>(
target: T,
force: boolean
): UnwrapNestedRefs<T>

export function reactive(target: object, force: boolean = true) {
return createReactiveObject(
target,
false,
false,
mutableHandlers,
mutableCollectionHandlers
mutableCollectionHandlers,
force
)
}

Expand All @@ -100,12 +118,17 @@ export function reactive(target: object) {
* level properties are reactive. It also does not auto-unwrap refs (even at the
* root level).
*/
export function shallowReactive<T extends object>(target: T): T {
export function shallowReactive<T extends object>(
target: T,
force: boolean = true
): T {
return createReactiveObject(
target,
false,
true,
shallowReactiveHandlers,
shallowCollectionHandlers
shallowCollectionHandlers,
force
)
}

Expand Down Expand Up @@ -136,13 +159,16 @@ export type DeepReadonly<T> = T extends Builtin
* made reactive, but `readonly` can be called on an already reactive object.
*/
export function readonly<T extends object>(
target: T
target: T,
force: boolean = true
): DeepReadonly<UnwrapNestedRefs<T>> {
return createReactiveObject(
target,
true,
false,
readonlyHandlers,
readonlyCollectionHandlers
readonlyCollectionHandlers,
force
)
}

Expand All @@ -153,42 +179,60 @@ export function readonly<T extends object>(
* This is used for creating the props proxy object for stateful components.
*/
export function shallowReadonly<T extends object>(
target: T
target: T,
force: boolean = true
): Readonly<{ [K in keyof T]: UnwrapNestedRefs<T[K]> }> {
return createReactiveObject(
target,
true,
true,
shallowReadonlyHandlers,
readonlyCollectionHandlers
readonlyCollectionHandlers,
force
)
}

function createReactiveObject(
target: Target,
isReadonly: boolean,
shallow: boolean,
baseHandlers: ProxyHandler<any>,
collectionHandlers: ProxyHandler<any>
collectionHandlers: ProxyHandler<any>,
force: boolean
) {
if (!isObject(target)) {
if (__DEV__) {
console.warn(`value cannot be made reactive: ${String(target)}`)
}
return target
}
// target is already a Proxy, return it.
// exception: calling readonly() on a reactive object
if (
target[ReactiveFlags.RAW] &&
!(isReadonly && target[ReactiveFlags.IS_REACTIVE])
) {
return target
}
const proxyMap = getProxyMap(isReadonly, shallow)
// target already has corresponding Proxy
const proxyMap = isReadonly ? readonlyMap : reactiveMap
const existingProxy = proxyMap.get(target)
if (existingProxy) {
return existingProxy
}
// target is already a Proxy, return it.
// exception: calling readonly() on a reactive object
if (target[ReactiveFlags.RAW]) {
if (
target[ReactiveFlags.IS_READONLY] === isReadonly &&
target[ReactiveFlags.IS_SHALLOW] === shallow
) {
return target
}
if (!isReadonly || !target[ReactiveFlags.IS_REACTIVE]) {
if (!force) {
return target
} else {
target = toRaw(target)
const existingProxy = proxyMap.get(target)
if (existingProxy) {
return existingProxy
}
}
}
}
// only a whitelist of value types can be observed.
const targetType = getTargetType(target)
if (targetType === TargetType.INVALID) {
Expand Down