diff --git a/packages/reactivity/__tests__/collections/Map.spec.ts b/packages/reactivity/__tests__/collections/Map.spec.ts index a52ef084563..a4c69e5b935 100644 --- a/packages/reactivity/__tests__/collections/Map.spec.ts +++ b/packages/reactivity/__tests__/collections/Map.spec.ts @@ -1,7 +1,10 @@ import { reactive, effect, toRaw, isReactive } from '../../src' +import { mockWarn } from '@vue/shared' describe('reactivity/collections', () => { describe('Map', () => { + mockWarn() + test('instanceof', () => { const original = new Map() const observed = reactive(original) @@ -363,6 +366,51 @@ describe('reactivity/collections', () => { expect(map.get(key)).toBeUndefined() }) + it('should track set of reactive keys in raw map', () => { + const raw = new Map() + const key = reactive({}) + raw.set(key, 1) + const map = reactive(raw) + + let dummy + effect(() => { + dummy = map.get(key) + }) + expect(dummy).toBe(1) + + map.set(key, 2) + expect(dummy).toBe(2) + }) + + it('should track deletion of reactive keys in raw map', () => { + const raw = new Map() + const key = reactive({}) + raw.set(key, 1) + const map = reactive(raw) + + let dummy + effect(() => { + dummy = map.has(key) + }) + expect(dummy).toBe(true) + + map.delete(key) + expect(dummy).toBe(false) + }) + + it('should warn when both raw and reactive versions of the same object is used as key', () => { + const raw = new Map() + const rawKey = {} + const key = reactive(rawKey) + raw.set(rawKey, 1) + raw.set(key, 1) + const map = reactive(raw) + map.set(key, 2) + expect( + `Reactive Map contains both the raw and reactive` + ).toHaveBeenWarned() + }) + // #877 it('should not trigger key iteration when setting existing keys', () => { const map = reactive(new Map()) diff --git a/packages/reactivity/__tests__/collections/Set.spec.ts b/packages/reactivity/__tests__/collections/Set.spec.ts index f0a627ee0d5..bf26e807dd5 100644 --- a/packages/reactivity/__tests__/collections/Set.spec.ts +++ b/packages/reactivity/__tests__/collections/Set.spec.ts @@ -1,7 +1,10 @@ import { reactive, effect, isReactive, toRaw } from '../../src' +import { mockWarn } from '@vue/shared' describe('reactivity/collections', () => { describe('Set', () => { + mockWarn() + it('instanceof', () => { const original = new Set() const observed = reactive(original) @@ -380,5 +383,34 @@ describe('reactivity/collections', () => { expect(set.delete(entry)).toBe(true) expect(set.has(entry)).toBe(false) }) + + it('should track deletion of reactive entries in raw set', () => { + const raw = new Set() + const entry = reactive({}) + raw.add(entry) + const set = reactive(raw) + + let dummy + effect(() => { + dummy = set.has(entry) + }) + expect(dummy).toBe(true) + + set.delete(entry) + expect(dummy).toBe(false) + }) + + it('should warn when set contains both raw and reactive versions of the same object', () => { + const raw = new Set() + const rawKey = {} + const key = reactive(rawKey) + raw.add(rawKey) + raw.add(key) + const set = reactive(raw) + set.delete(key) + expect( + `Reactive Set contains both the raw and reactive` + ).toHaveBeenWarned() + }) }) }) diff --git a/packages/reactivity/src/collectionHandlers.ts b/packages/reactivity/src/collectionHandlers.ts index e102ea8d077..ba9b9267320 100644 --- a/packages/reactivity/src/collectionHandlers.ts +++ b/packages/reactivity/src/collectionHandlers.ts @@ -2,7 +2,13 @@ import { toRaw, reactive, readonly } from './reactive' import { track, trigger, ITERATE_KEY, MAP_KEY_ITERATE_KEY } from './effect' import { TrackOpTypes, TriggerOpTypes } from './operations' import { LOCKED } from './lock' -import { isObject, capitalize, hasOwn, hasChanged } from '@vue/shared' +import { + isObject, + capitalize, + hasOwn, + hasChanged, + toRawType +} from '@vue/shared' export type CollectionTypes = IterableCollections | WeakCollections @@ -27,6 +33,9 @@ function get( ) { target = toRaw(target) const rawKey = toRaw(key) + if (key !== rawKey) { + track(target, TrackOpTypes.GET, key) + } track(target, TrackOpTypes.GET, rawKey) const { has, get } = getProto(target) if (has.call(target, key)) { @@ -39,6 +48,9 @@ function get( function has(this: CollectionTypes, key: unknown): boolean { const target = toRaw(this) const rawKey = toRaw(key) + if (key !== rawKey) { + track(target, TrackOpTypes.HAS, key) + } track(target, TrackOpTypes.HAS, rawKey) const has = getProto(target).has return has.call(target, key) || has.call(target, rawKey) @@ -64,12 +76,19 @@ function add(this: SetTypes, value: unknown) { function set(this: MapTypes, key: unknown, value: unknown) { value = toRaw(value) - key = toRaw(key) const target = toRaw(this) - const proto = getProto(target) - const hadKey = proto.has.call(target, key) - const oldValue = proto.get.call(target, key) - const result = proto.set.call(target, key, value) + const { has, get, set } = getProto(target) + + let hadKey = has.call(target, key) + if (!hadKey) { + key = toRaw(key) + hadKey = has.call(target, key) + } else if (__DEV__) { + checkIdentitiyKeys(target, has, key) + } + + const oldValue = get.call(target, key) + const result = set.call(target, key, value) if (!hadKey) { trigger(target, TriggerOpTypes.ADD, key, value) } else if (hasChanged(value, oldValue)) { @@ -85,7 +104,10 @@ function deleteEntry(this: CollectionTypes, key: unknown) { if (!hadKey) { key = toRaw(key) hadKey = has.call(target, key) + } else if (__DEV__) { + checkIdentitiyKeys(target, has, key) } + const oldValue = get ? get.call(target, key) : undefined // forward the operation before queueing reactions const result = del.call(target, key) @@ -251,3 +273,21 @@ export const mutableCollectionHandlers: ProxyHandler = { export const readonlyCollectionHandlers: ProxyHandler = { get: createInstrumentationGetter(readonlyInstrumentations) } + +function checkIdentitiyKeys( + target: CollectionTypes, + has: (key: unknown) => boolean, + key: unknown +) { + const rawKey = toRaw(key) + if (rawKey !== key && has.call(target, rawKey)) { + const type = toRawType(target) + console.warn( + `Reactive ${type} contains both the raw and reactive ` + + `versions of the same object${type === `Map` ? `as keys` : ``}, ` + + `which can lead to inconsistencies. ` + + `Avoid differentiating between the raw and reactive versions ` + + `of an object and only use the reactive version if possible.` + ) + } +}