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): ensure that shallow and normal proxies are tracked seperately (close #2843) #2851

Merged
merged 2 commits into from
Mar 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 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,16 @@ describe('shallowReactive', () => {
expect(isReactive(props.n)).toBe(true)
})

// #2843
test('should allow shallow und normal reactive for same target', async () => {
const original = { foo: {} }
const shallowProxy = shallowReactive(original)
const reactiveProxy = reactive(original)
expect(shallowProxy).not.toBe(reactiveProxy)
expect(isReactive(shallowProxy.foo)).toBe(false)
expect(isReactive(reactiveProxy.foo)).toBe(true)
})

describe('collections', () => {
test('should be reactive', () => {
const shallowSet = shallowReactive(new Set())
Expand Down
12 changes: 11 additions & 1 deletion packages/reactivity/__tests__/shallowReadonly.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { isReactive, isReadonly, shallowReadonly } from '../src'
import { isReactive, isReadonly, readonly, shallowReadonly } from '../src'

describe('reactivity/shallowReadonly', () => {
test('should not make non-reactive properties reactive', () => {
Expand Down Expand Up @@ -27,6 +27,16 @@ describe('reactivity/shallowReadonly', () => {
).not.toHaveBeenWarned()
})

// #2843
test('should differentiate from normal readonly calls', async () => {
const original = { foo: {} }
const shallowProxy = shallowReadonly(original)
const reactiveProxy = readonly(original)
expect(shallowProxy).not.toBe(reactiveProxy)
expect(isReadonly(shallowProxy.foo)).toBe(false)
expect(isReadonly(reactiveProxy.foo)).toBe(true)
})

describe('collection/Map', () => {
;[Map, WeakMap].forEach(Collection => {
test('should make the map/weak-map readonly', () => {
Expand Down
14 changes: 12 additions & 2 deletions packages/reactivity/src/baseHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import {
ReactiveFlags,
Target,
readonlyMap,
reactiveMap
reactiveMap,
shallowReactiveMap,
shallowReadonlyMap
} from './reactive'
import { TrackOpTypes, TriggerOpTypes } from './operations'
import {
Expand Down Expand Up @@ -80,7 +82,15 @@ function createGetter(isReadonly = false, shallow = false) {
return isReadonly
} else if (
key === ReactiveFlags.RAW &&
receiver === (isReadonly ? readonlyMap : reactiveMap).get(target)
receiver ===
(isReadonly
? shallow
? shallowReadonlyMap
: readonlyMap
: shallow
? shallowReactiveMap
: reactiveMap
).get(target)
) {
return target
}
Expand Down
18 changes: 12 additions & 6 deletions packages/reactivity/src/reactive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ export interface Target {
}

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

const enum TargetType {
INVALID = 0,
Expand Down Expand Up @@ -92,7 +94,8 @@ export function reactive(target: object) {
target,
false,
mutableHandlers,
mutableCollectionHandlers
mutableCollectionHandlers,
reactiveMap
)
}

Expand All @@ -106,7 +109,8 @@ export function shallowReactive<T extends object>(target: T): T {
target,
false,
shallowReactiveHandlers,
shallowCollectionHandlers
shallowCollectionHandlers,
shallowReactiveMap
)
}

Expand Down Expand Up @@ -143,7 +147,8 @@ export function readonly<T extends object>(
target,
true,
readonlyHandlers,
readonlyCollectionHandlers
readonlyCollectionHandlers,
readonlyMap
)
}

Expand All @@ -160,15 +165,17 @@ export function shallowReadonly<T extends object>(
target,
true,
shallowReadonlyHandlers,
shallowReadonlyCollectionHandlers
shallowReadonlyCollectionHandlers,
shallowReadonlyMap
)
}

function createReactiveObject(
target: Target,
isReadonly: boolean,
baseHandlers: ProxyHandler<any>,
collectionHandlers: ProxyHandler<any>
collectionHandlers: ProxyHandler<any>,
proxyMap: WeakMap<Target, any>
) {
if (!isObject(target)) {
if (__DEV__) {
Expand All @@ -185,7 +192,6 @@ function createReactiveObject(
return target
}
// target already has corresponding Proxy
const proxyMap = isReadonly ? readonlyMap : reactiveMap
const existingProxy = proxyMap.get(target)
if (existingProxy) {
return existingProxy
Expand Down