fix(baseHandlers): trigger will called two times when set the same proxy value

[liaoliao666]

import { reactive, toRaw, watchEffect } from 'vue'

const a = reactive({
  a: 1,
})

const b = reactive({
  b: 1,
})

const d = reactive([a, b])

watchEffect(() => {
  console.log('d', d[0]) // Will consoled two times
})

d[0] = a

[posva]

I would say watchEffect is different from watch which does observe the value: watchEffect tracks changes to d.

[liaoliao666]

I would say watchEffect is different from watch which does observe the value: watchEffect tracks changes to d.

i get ur mean, but why does set the same value will trigger the effect. im still confused

[LinusBorg]

@posva I think he has a point here, but not sure if the change actually is the right fix. The lack of a test to verify doesn't help here.

What's the issue?

hasChanged(value, oldValue) is meant to guard against triggering an effect if the new and old value being set are the same, and it should keep the watchEffect from running in this instance, as a === a.

But in the given example, the original array from which d was reactively constructed, contains a reactive proxy, and so now we end up effectively comparing the raw value to the proxy from the source array, because value has been set to the raw value before, but oldValue has not:

https://github.com/vuejs/vue-next/blob/e5a5a2cb8d9c38f5fca52a942361123ebd402fb7/packages/reactivity/src/baseHandlers.ts#L136-L138

I think this fix is valid but definitely needs a test case added to the suite and maybe some thinking about other edge cases.

[posva]

It's true this could be an optimization and the change makes sense

[LinusBorg]

@liaoliao666 can you add a test case?

[liaoliao666]

@liaoliao666 can you add a test case?

okay