fix(reactivity): should not trigger map keys iteration when keys did not change

fix #877

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)
+    })
   })
 })

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 {

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,61 +175,10 @@ export function trigger(
     // never been tracked
     return
   }
+
   const effects = new Set<ReactiveEffect>()
   const computedRunners = new Set<ReactiveEffect>()
-  if (type === TriggerOpTypes.CLEAR) {
-    // collection being cleared
-    // trigger all effects for target
-    depsMap.forEach(dep => {
-      addRunners(effects, computedRunners, dep)
-    })
-  } else if (key === 'length' && isArray(target)) {
-    depsMap.forEach((dep, key) => {
-      if (key === 'length' || key >= (newValue as number)) {
-        addRunners(effects, computedRunners, dep)
-      }
-    })
-  } else {
-    // schedule runs for SET | ADD | DELETE
-    if (key !== void 0) {
-      addRunners(effects, computedRunners, depsMap.get(key))
-    }
-    // also run for iteration key on ADD | DELETE | Map.SET
-    if (
-      type === TriggerOpTypes.ADD ||
-      (type === TriggerOpTypes.DELETE && !isArray(target)) ||
-      (type === TriggerOpTypes.SET && target instanceof Map)
-    ) {
-      const iterationKey = isArray(target) ? 'length' : ITERATE_KEY
-      addRunners(effects, computedRunners, depsMap.get(iterationKey))
-    }
-  }
-  const run = (effect: ReactiveEffect) => {
-    scheduleRun(
-      effect,
-      target,
-      type,
-      key,
-      __DEV__
-        ? {
-            newValue,
-            oldValue,
-            oldTarget
-          }
-        : undefined
-    )
-  }
-  // 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<ReactiveEffect>,
-  computedRunners: Set<ReactiveEffect>,
-  effectsToAdd: Set<ReactiveEffect> | undefined
-) {
+  const add = (effectsToAdd: Set<ReactiveEffect> | undefined) => {
     if (effectsToAdd !== void 0) {
       effectsToAdd.forEach(effect => {
         if (effect !== activeEffect || !shouldTrack) {
@@ -244,27 +194,59 @@ function addRunners(
         }
       })
     }
-}
+  }
 
-function scheduleRun(
-  effect: ReactiveEffect,
-  target: object,
-  type: TriggerOpTypes,
-  key: unknown,
-  extraInfo?: DebuggerEventExtraInfo
-) {
+  if (type === TriggerOpTypes.CLEAR) {
+    // collection being cleared
+    // trigger all effects for target
+    depsMap.forEach(add)
+  } else if (key === 'length' && isArray(target)) {
+    depsMap.forEach((dep, key) => {
+      if (key === 'length' || key >= (newValue as number)) {
+        add(dep)
+      }
+    })
+  } else {
+    // schedule runs for SET | ADD | DELETE
+    if (key !== void 0) {
+      add(depsMap.get(key))
+    }
+    // also run for iteration key on ADD | DELETE | Map.SET
+    const isAddOrDelete =
+      type === TriggerOpTypes.ADD ||
+      (type === TriggerOpTypes.DELETE && !isArray(target))
+    if (
+      isAddOrDelete ||
+      (type === TriggerOpTypes.SET && target instanceof Map)
+    ) {
+      add(depsMap.get(isArray(target) ? 'length' : ITERATE_KEY))
+    }
+    if (isAddOrDelete && target instanceof Map) {
+      add(depsMap.get(MAP_KEY_ITERATE_KEY))
+    }
+  }
+
+  const run = (effect: ReactiveEffect) => {
     if (__DEV__ && effect.options.onTrigger) {
-    const event: DebuggerEvent = {
+      effect.options.onTrigger({
         effect,
         target,
         key,
-      type
-    }
-    effect.options.onTrigger(extraInfo ? extend(event, extraInfo) : event)
+        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)
 }