vuejs / core

🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
https://vuejs.org/
MIT License
47.36k stars 8.29k forks source link

Vue 2 migration build WATCH_ARRAY triggers on deep value modification #11872

Open thecodewarrior opened 1 month ago

thecodewarrior commented 1 month ago

Vue version

3.4.38 (compat)

Link to minimal reproduction

https://stackblitz.com/edit/vitejs-vite-cpcesj?file=src%2FApp.vue

Steps to reproduce

  1. Create a new project with the Vue 2 migration build
  2. Create an array with object elements (e.g. [{foo: 0}, {foo: 1}]) and add a watch on it
  3. Replace the array (arr = [...arr])
  4. Mutate the array (arr.push({foo: 2}))
  5. Modify the contents of one of the elements (arr[0].foo++)

What is expected?

The watch should be triggered when the array is replaced or mutated, but should not be triggered when the contents of the array elements are modified (Vue 2 repro)

What is actually happening?

The watch is triggered in all three cases, on replacement, mutation, and deep modification

System Info

No response

Any additional comments?

The compat is implemented here in createWatcher, and it's implemented by doing a deep traversal of array values. The Vue 2 behavior can be reproduced by instead calling array.slice(0, 0) there, which creates a reactive dependency on TrackOpTypes.ITERATE. The internal shallowReadArray function may also be more appropriate to call in internal code.

edison1105 commented 1 month ago

I’m not sure if this is considered a bug. It seems more like an undocumented breaking change.

thecodewarrior commented 1 month ago

It could arguably be considered an undocumented breaking change, however it's a very impactful breaking change which has a relatively easy fix, so I'd consider it less "bug" and more "oversight."

Unfortunately the migration path isn't so easy for a user updating past the migration build, so documenting it in the deprecation message might be cumbersome. (The best solutions I've come up with involve a computed property calling slice and returning either {} or new Proxy(arr, {}) so the object reference changes each time it's recomputed)

thecodewarrior commented 1 day ago

Now that we have configurable traversal depth in 3.5, we could actually implement this super easily. Just remove these lines (they shouldn't be necessary as the watch calls the getter once immediately), and edit this line to set the traverse depth to 1.

The migration doc page should also show depth: 1, then have a note that prior to 3.5 the closest you can get is depth: true.

If that all sounds good I can get to work on a PR tomorrow.

edison1105 commented 1 day ago

@thecodewarrior PR welcome.