diff --git a/packages/reactivity/__tests__/collections/Map.spec.ts b/packages/reactivity/__tests__/collections/Map.spec.ts index c4e5476b..a52ef084 100644 --- a/packages/reactivity/__tests__/collections/Map.spec.ts +++ b/packages/reactivity/__tests__/collections/Map.spec.ts @@ -362,5 +362,34 @@ describe('reactivity/collections', () => { expect(map.has(key)).toBe(false) expect(map.get(key)).toBeUndefined() }) + + // #877 + it('should not trigger key iteration when setting existing keys', () => { + const map = reactive(new Map()) + const spy = jest.fn() + + effect(() => { + const keys = [] + for (const key of map.keys()) { + keys.push(key) + } + spy(keys) + }) + + expect(spy).toHaveBeenCalledTimes(1) + expect(spy.mock.calls[0][0]).toMatchObject([]) + + map.set('a', 0) + expect(spy).toHaveBeenCalledTimes(2) + expect(spy.mock.calls[1][0]).toMatchObject(['a']) + + map.set('b', 0) + expect(spy).toHaveBeenCalledTimes(3) + expect(spy.mock.calls[2][0]).toMatchObject(['a', 'b']) + + // keys didn't change, should not trigger + map.set('b', 1) + expect(spy).toHaveBeenCalledTimes(3) + }) }) }) diff --git a/packages/reactivity/src/collectionHandlers.ts b/packages/reactivity/src/collectionHandlers.ts index 07ad0144..e102ea8d 100644 --- a/packages/reactivity/src/collectionHandlers.ts +++ b/packages/reactivity/src/collectionHandlers.ts @@ -1,5 +1,5 @@ import { toRaw, reactive, readonly } from './reactive' -import { track, trigger, ITERATE_KEY } from './effect' +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' @@ -134,12 +134,16 @@ function createForEach(isReadonly: boolean) { function createIterableMethod(method: string | symbol, isReadonly: boolean) { return function(this: IterableCollections, ...args: unknown[]) { const target = toRaw(this) - const isPair = - method === 'entries' || - (method === Symbol.iterator && target instanceof Map) + const isMap = target instanceof Map + const isPair = method === 'entries' || (method === Symbol.iterator && isMap) + const isKeyOnly = method === 'keys' && isMap const innerIterator = getProto(target)[method].apply(target, args) const wrap = isReadonly ? toReadonly : toReactive - track(target, TrackOpTypes.ITERATE, ITERATE_KEY) + track( + target, + TrackOpTypes.ITERATE, + isKeyOnly ? MAP_KEY_ITERATE_KEY : ITERATE_KEY + ) // return a wrapped iterator which returns observed versions of the // values emitted from the real iterator return { diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts index abce4a1e..cc6d0a61 100644 --- a/packages/reactivity/src/effect.ts +++ b/packages/reactivity/src/effect.ts @@ -1,5 +1,5 @@ import { TrackOpTypes, TriggerOpTypes } from './operations' -import { EMPTY_OBJ, extend, isArray } from '@vue/shared' +import { EMPTY_OBJ, isArray } from '@vue/shared' // The main WeakMap that stores {target -> key -> dep} connections. // Conceptually, it's easier to think of a dependency as a Dep class @@ -43,7 +43,8 @@ export interface DebuggerEventExtraInfo { const effectStack: ReactiveEffect[] = [] export let activeEffect: ReactiveEffect | undefined -export const ITERATE_KEY = Symbol('iterate') +export const ITERATE_KEY = Symbol(__DEV__ ? 'iterate' : '') +export const MAP_KEY_ITERATE_KEY = Symbol(__DEV__ ? 'Map key iterate' : '') export function isEffect(fn: any): fn is ReactiveEffect { return fn && fn._isEffect === true @@ -174,97 +175,78 @@ export function trigger( // never been tracked return } + const effects = new Set() const computedRunners = new Set() + const add = (effectsToAdd: Set | undefined) => { + if (effectsToAdd !== void 0) { + effectsToAdd.forEach(effect => { + if (effect !== activeEffect || !shouldTrack) { + if (effect.options.computed) { + computedRunners.add(effect) + } else { + effects.add(effect) + } + } else { + // the effect mutated its own dependency during its execution. + // this can be caused by operations like foo.value++ + // do not trigger or we end in an infinite loop + } + }) + } + } + if (type === TriggerOpTypes.CLEAR) { // collection being cleared // trigger all effects for target - depsMap.forEach(dep => { - addRunners(effects, computedRunners, dep) - }) + depsMap.forEach(add) } else if (key === 'length' && isArray(target)) { depsMap.forEach((dep, key) => { if (key === 'length' || key >= (newValue as number)) { - addRunners(effects, computedRunners, dep) + add(dep) } }) } else { // schedule runs for SET | ADD | DELETE if (key !== void 0) { - addRunners(effects, computedRunners, depsMap.get(key)) + add(depsMap.get(key)) } // also run for iteration key on ADD | DELETE | Map.SET - if ( + const isAddOrDelete = type === TriggerOpTypes.ADD || - (type === TriggerOpTypes.DELETE && !isArray(target)) || + (type === TriggerOpTypes.DELETE && !isArray(target)) + if ( + isAddOrDelete || (type === TriggerOpTypes.SET && target instanceof Map) ) { - const iterationKey = isArray(target) ? 'length' : ITERATE_KEY - addRunners(effects, computedRunners, depsMap.get(iterationKey)) + add(depsMap.get(isArray(target) ? 'length' : ITERATE_KEY)) + } + if (isAddOrDelete && target instanceof Map) { + add(depsMap.get(MAP_KEY_ITERATE_KEY)) } } + const run = (effect: ReactiveEffect) => { - scheduleRun( - effect, - target, - type, - key, - __DEV__ - ? { - newValue, - oldValue, - oldTarget - } - : undefined - ) + if (__DEV__ && effect.options.onTrigger) { + effect.options.onTrigger({ + effect, + target, + key, + type, + newValue, + oldValue, + oldTarget + }) + } + if (effect.options.scheduler !== void 0) { + effect.options.scheduler(effect) + } else { + effect() + } } + // Important: computed effects must be run first so that computed getters // can be invalidated before any normal effects that depend on them are run. computedRunners.forEach(run) effects.forEach(run) } - -function addRunners( - effects: Set, - computedRunners: Set, - effectsToAdd: Set | undefined -) { - if (effectsToAdd !== void 0) { - effectsToAdd.forEach(effect => { - if (effect !== activeEffect || !shouldTrack) { - if (effect.options.computed) { - computedRunners.add(effect) - } else { - effects.add(effect) - } - } else { - // the effect mutated its own dependency during its execution. - // this can be caused by operations like foo.value++ - // do not trigger or we end in an infinite loop - } - }) - } -} - -function scheduleRun( - effect: ReactiveEffect, - target: object, - type: TriggerOpTypes, - key: unknown, - extraInfo?: DebuggerEventExtraInfo -) { - if (__DEV__ && effect.options.onTrigger) { - const event: DebuggerEvent = { - effect, - target, - key, - type - } - effect.options.onTrigger(extraInfo ? extend(event, extraInfo) : event) - } - if (effect.options.scheduler !== void 0) { - effect.options.scheduler(effect) - } else { - effect() - } -}