Unexpected reactivity watching shallowReactive array

[ragnarlotus]

Vue version

3.3.12

Link to minimal reproduction

https://play.vuejs.org/#eNqNVE1zmzAQ/Ss7XMBTCk1yo8TTtM2hPSSdJJ0eQg4EFkwiSxpJ2O54+O9dSQYzHTfTE9Lu27dvP9A+uJIy2fQYZEGuK9VJAxpNL4GVvL0sAqOLYFnwbi2FMrAHhU0M96uSMbG9s2d96lxWpttgDNvSVKuYgrwBBmiUWENICcOCF7xipdbQCAH7ggOAVEKeZTP+XBvV8XYJl7NEURguPk748ww8iDBhSPbBMQuuDYjnlzMyc9zaJJGNmhznfzlGVyUYQ5Ir+DypLyB6tIyxC3+aB61R67JFTSFjsdHjAeGaEB1pY4gWcLn0JY+Biez1Kgp/WSwqoILaFhXWEMI7J/NraTBaWMYh9qE1osygKZnGmKw+WdNzr72XNUXcklzK5rt7lPD44SlxvU42JeuRVFv6hItttEiMuHf9dF1xzZxIy7omxhOEXv7UzkNknvqVogWii8G1ZJSGbgC5XObPvTFE+qliXfVKuzaXTFv3013B3iHSK9GzGm5uH8begFmhXzBUizz1ZMs8lf/kH9UT91VdW+KJ9z84HevqfPngoTrLU7o5a8/cl06sg837RijKFh1GG0PHa9wt6DNNuwgge8XfhHI+ErTfj04YhjxlnWdOHXWeznoXxPRT0tY1XZu8aMHpz3XTKIJKrGXHUN1KOxJKkh3mRD63xt+dzajeLoy3U6XV6wn7iyZVGR1+KNSoNlgEk8+UqkXj3df3N7ij8+Rci7pnhH7DeYdasN5q9LDPPa9J9gzn1H5zbw4t4oO+3hnkeizKCrXIweGLgF6TL2+UfpR7kVy4OFrOYPgDw/GuRw==

Steps to reproduce

Click on Update Obj1 (should NOT trigger the watcher) and you will see the message of the watcher having triggered.

What is expected?

That the watcher is not triggered since it is watching a shallowReactive array and has set { deep: false }, so only should trigger when applying changes to the array

What is actually happening?

It is triggeing also when props of objects in the array are changed

System Info

System:
    OS: Windows 10 10.0.19045
    CPU: (8) x64 Intel(R) Core(TM) i7-7700K CPU @ 4.20GHz
    Memory: 22.42 GB / 31.93 GB
  Binaries:
    Node: 20.7.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.19 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 10.1.0 - C:\Program Files\nodejs\npm.CMD
    pnpm: 8.8.0 - ~\AppData\Roaming\npm\pnpm.CMD
  Browsers:
    Chrome: 120.0.6099.129
    Edge: Chromium (120.0.2210.91)
    Internet Explorer: 11.0.19041.3636
  npmPackages:
    vue: ^3.0.0 => 3.3.12

Any additional comments?

#9900

In my case (open source vue project), is causing an extra overhead and lower performance since it is triggering and causing unwanted loop

[edison1105]

Maybe we shouldn't force deep here if user passes deep:false

core/packages/runtime-core/src/apiWatch.ts

Lines 211 to 214 in a8d0b1b

	} else if (isReactive(source)) {
	getter = () => source
	deep = true
	} else if (isArray(source)) {

[yyx990803]

@edison1105 I think we need two changes:

1. Do not set deep to true if the user option explicitly has deep: false
2. If the source is reactive and is shallow, we should use a shallow traverse (i.e. only traverse the root level).

[edison1105]

Is it possible to fix it based on this PR: https://github.com/vuejs/core/pull/9572/files. It does something similar.
/cc @Alfred-Skyblue

[Alfred-Skyblue]

@edison1105 I added the depth option in #9572 to control the depth of monitoring. When the monitored data source is shallowReactive, it only listens to the first layer.

[jods4]

@yyx990803
👍 for respecting { deep: false }, but RE: depth 1 for shallow reactives:

This PR is an (unannounced) breaking change in a patch (not even minor!) release!! Before 3.4.2 a shallow reactive was deeply observed, after upgrade to 3.4.2 it is only observed shallowly, leading to observable changes in behavior.

Worst, briefly looking at the source change I have the impression that deep observation is not even possible at all anymore!
At first glance, even passing { deep: true } will not work because of new condition isShallow(s) || deep === false ? 1 : undefined that can't be bypassed!!

For the philosophical discussion: I think it's not a good change as it ties the behavior of watch to unrelated implementation details of its source.

Shallow vs deep is not indicative of where reactive state is, it's more a DX preference for creating reactive state. For example, I default to shallow refs 90% of the time, but I still have nested reactive state. Just because I use a shallow reactive array for perf and predictability doesn't mean I don't put nested reactive state inside that array.

Whereas this somewhat made sense in @ragnarlotus example, because everything was located close together and he was in control of both the source and the watcher, it's not always this simple.
Consider for example a library author that creates a composable function, that internally uses a watch on a source that's passed from outside.

This can have nasty "changes at distance" effect, consider:
In file A, I change an array that was a deep reactive into a shallow reactive.
Doing so, I unknowingly create a bug in file B, where that array was imported and passed to watch, whose behavior has now changed.
This kind of cascade effects are very trappy. Most devs would not anticipate such consequences as they're highly unintuitive.

👉 please consider reverting the shallow rule, and merging something like @edison1105 's PR #9572 to let watch authors control the depth of observation instead.

[Alfred-Skyblue]

@jods4 You can still use deep: true to deeply watch shallowReactive, and it will trigger the following logic:

core/packages/runtime-core/src/apiWatch.ts

Lines 296 to 299 in 8f85b6d

	if (cb && deep) {
	const baseGetter = getter
	getter = () => traverse(baseGetter())
	}

playground

[yyx990803]

@jods4 deep: true is still respected if explicitly passed on a shallow reactive source. That said, it does occur to me that we are double-traversing in this case. I've updated the code to avoid the double traverse when deep: true is passed, and also added a test case for deep: true + shallowReactive source in 24d77c2

[yyx990803]

Reverted the shallow default behavior - see #9965 (comment)