Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
thecodewarrior opened this issue Sep 9, 2024 · 6 comments · May be fixed by #12236
Open

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

thecodewarrior opened this issue Sep 9, 2024 · 6 comments · May be fixed by #12236

Comments

@thecodewarrior
Copy link

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
Copy link
Member

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

@thecodewarrior
Copy link
Author

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
Copy link
Author

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
Copy link
Member

@thecodewarrior PR welcome.

@edison1105
Copy link
Member

Summary

To make the behavior of watching an array consistent with Vue 2, should do the following.

export default {
  data() {
    return {
      arr: [], // initial value should be array
      watches: 0,
    };
  },
  watch: {
    arr: {
      handler() {
        this.watches++;
      },
      deep: 1 // instead of `true`
    },
  },
};

@thecodewarrior
Copy link
Author

Correct, however when explicitly specifying deep: 1 the initial value doesn't need to be an array. The fact that the compat doesn't work unless the initial value is an array is fixed in #12236.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants