Subscribe on changes!

The set operation of Map data should not trigger the re-execution of the effect holding the size

avatar
Dec 11th 2022

Vue version

3.2.45

Link to minimal reproduction

https://codesandbox.io/s/nice-aryabhata-d1h6wp?file=/src/App.vue

Steps to reproduce

  1. There is proxyMap.size in the effect function
const map = new Map();
map.set("item1", 1);
map.set("item2", 2);
const proxyMap = reactive(map); 
effect(()=> {
   console.info("proxyMap.size", proxyMap.size);
});
  1. Perform proxyMap.set(key, newValue) operation in onMounted, key is an existing key
onMounted(()=> {
 setInterval(() => {
   proxyMap.set("item1", new Date().getTime());
 }, 2000);
});

What is expected?

Because proxyMap.set does not change the size of the Map, the proxyMap.set operation will not trigger the re-execution of the effect

What is actually happening?

effect will be re-executed

System Info

No response

Any additional comments?

In the Vue3 source code, I see that Map.size will track(toRaw(target), TrackOpTypes.ITERATE, ITERATE_KEY)

If the Map sets a new key, it will trigger(target, TriggerOpTypes.ADD, value, value) anddeps.push(depsMap.get(ITERATE_KEY)),I think reasonable, because it changes the Map.size

But the Map sets a existing key,it will trigger(target, TriggerOpTypes.SET, value, value) and deps.push(depsMap.get(ITERATE_KEY)),I think unreasonable, because it didn't changes the Map.size

avatar
Dec 12th 2022

This is not by accident. it's a compromise to get better performance when using an iterator.

Your proposed change would introduce a bug where a map.set() operation will no longer trigger effects that used i.e. map.values() or map.entries(). That's because for performance reasons, we use the original non-reactive target's iterator under the hood, avoiding n calls of the get() trap.

However, I'm currently unsure wether we could optimize something on the size trap's end.

avatar
Dec 12th 2022

@LinusBorg I'm sorry, I don't quite understand what get() trap means. I just want to express whether we should separate the size change from the value change, such as the following:

  1. Map.size -> track(toRaw(target), TrackOpTypes.ITERATE, ITERATE_KEY) ITERATE_KEY is only used for size changes

  2. Map.values()/Map.entries()/Map.forEach()

  • track(toRaw(target), TrackOpTypes.ITERATE, ITERATE_KEY) (ITERATE_KEY is only used for size changes, size changes will affect traversal)
  • track(toRaw(target), TrackOpTypes.ITERATE, new_ITERATE_KEY)(new_ITERATE_KEY is only used for value changes)
  1. Map.set(newKey, xxxx) -> trigger key = ITERATE_KEY
  2. Map.set(existingKey, xxxx) -> trigger key = new_ITERATE_KEY
avatar
Dec 12th 2022

I think we think in the same direction. I understood your initial description to argue for a simple removal of the ITERATE_KEY on set.

Not sure as of now if the PR that has alreay been submitted is the correct fix.

avatar
Dec 13th 2022

I was thinking that Map.size should only concern itself with the length of the key, and it should be similar to Map.keys.