Stable mutation of reactive arrays containing refs

[tesla3327]

What problem does this feature solve?

Any mutation in a reactive array that changes the position of a ref does not work as expected. Instead of moving the entire ref object only the values are moved.

For example, if we start with this array:

{
  1,
  2,
  // Representing a ref object
  { value: 3 }
}

Calling reverse on this object will swap the values, but leave the ref wrapper at index 2:

{
  3,
  2,
  // Representing a ref object
  { value: 1 }
}

The solution is to first use toRaw to get the underlying object. This doesn't seem like a great user experience, and it isn't obvious at all that this is the solution. Documenting it as a best practice could work, but I don't think it would be that difficult to do this automatically for the user.

If we add all (or most?) Array methods that cause a mutation to this base handler I think we can avoid this issue altogether. (Although it appears that toRaw might be a lot slower?)

I put together a live demo as well: https://codesandbox.io/s/recursing-wood-2ovm5

I ran into this issue after only a couple hours playing with Vue 3, so that makes me think this is a common enough use case to justify fixing.

What does the proposed API look like?

Currently: toRaw(list).reverse()

Proposed: list.reverse()

I would want to go through all Array methods and determine which would need to be included, as well as writing extensive tests for each. It's possible to just use the raw array for all Array methods, but I'm not sure that's all that helpful.

I've only spent a couple hours in this codebase so I'm not sure if there are other implications that a change like this could cause.

Before moving forward with this I wanted to confirm that this is a good direction to move in. I feel this is pretty in keeping with Vue's spirit of making things as simple as possible.

[yyx990803]

Curious what is the use case that led you to this? Why would you want to know if the underlying value is a ref or not?

[tesla3327]

I was playing around with the composition API, building a todo list where you can add items created by other sources (like useMousePosition).

const { x, y } = useMousePosition();
const list = ref([x, y, 'hello', 'world']);

Or in this case, where one item in the list is updated from a setInterval: https://codesandbox.io/s/quirky-dawn-wibro

[yyx990803]

I'm starting to think that Arrays should not automatically unwrap refs it contains. With ref unwrapping it becomes very complicated to ensure expected behavior for all the built-in Array methods. For example, the following could be ambiguous:

const n = ref(2)
const arr = [1, n, 3]
const mapped = arr.map(v => {
  // should v be the unwrapped value or the ref?
})

In addition, even if we special case these methods internally, it would still break when the user attempts to use external utility libs like lodash on a reactive array containing refs.

Currently, native collections like Map and Set also do not perform ref unwrapping.

I think in practice the cases where you actually want to mix refs in an array with normal values seem to be quite rare, so maybe ref unwrapping should be an object-only trait.

The upside is that all Array methods (internal or external) will work as expected; The downside is when you want to display the array you'll need too create a computed property for it:

const displayArray = computed(() => sourceArray.map(e => isRef(e) ? e.value : e))

[yyx990803]

/cc @jods4 would be interesting to hear your thoughts on this.

[jods4]

This is tricky stuff, there are no perfect answer for all cases.

I would set some design principles for a successful API:

1. have a simple, clear, coherent mental model;
2. be extensible: allow full control when required;
3. easily support common cases out of the box with little ceremony.

Magically unwrapping -- even with objects (which is gonna be more common than arrays IHMO) -- is a tricky proposal. Am I passing the value or the reactive reference around? It's gonna depend on the case, so no answer is always right. Which one is more common? How easy is it to access the non-default one?

Special casing built-in methods is a terrible idea and this I'm sure of. It strongly violates 1. Why am I getting refs when doing forEach but not for-of and what am I gonna get when I do for-of entries()?
Refs are a necessity: to pass reactive values around. They also quickly become confusing when half your state is regular variables and half is refs, and I speak from past experience. Using TS greatly helps catch errors but not everybody does. So don't add arbitrary rules like "mutating methods use refs (if any) and regular access returns values". It will only make matter worse: harder to comprehend, harder to control when you want to do things differently.

I've taken some time to think about it and here's what core apis I would do:

No magic, never auto-unwrap in my own code, i.e. both arrays and objects. Better perf and keeps the mental model predictable and simple (what you do is what you get).
Putting refs into reactive objects/arrays shouldn't be the most common scenario (the object is reactive already anyway!).
People should be encouraged to put all their own state into one reactive object instead of manipulating 5 refs.

Manipulating a value that might or might not be reactive (i.e. a ref) is a fairly common thing, esp. in generic (library) code. So isRef(e) ? e.value : e is fundamental and should be exposed as a core function (unwrap(e) ?).
Your example becomes: display = computed(() => array.map(unwrap)).

It is fairly common to return from setup a non-reactive object that contains some refs. So I would keep auto-unwrapping the setup() values inside templates (not deeply). I don't think the template is the place where you want to start messing around with your refs, put that code in your JS.

(Unsure about this one:) it might(?) be handy to have access to unwrap inside templates, in case some level of your model contains refs floating around.
Again, I would expect most refs to be at the root of the model, not hidden deeply into reactive objects.

That would be my core, you can offer more convenience functions if you'd like: for example an auto-unwrapping object/array, so that you can merge refs (maybe from mixins), into an easy to use state object.
It's easy to build your own and I know I'll be using some of my primitives, just a matter of preferences.

[yyx990803]

Arrays will no longer unwrap refs after 775a7c2

[F0rsaken]

I'm currently using [email protected] and refs unwraping in arrays is still happening and array items references are changed, even on push. I tested it on this example:

const testObj = { f: ref(0) };
const testArr = ref<any[]>([]);

testArr.push(testOj);
console.log(isRef(testObj.f)) // true
console.log(testArr.value[0] === testObj) // false
console.log(isRef(testArr.value[0].f)) // false

I don't expect this behaviour. I only want to watch for new items in array. If I'm doing something wrong, I would be very thankful for pointing it out!

[TKharaishvili]

100% agree with @jods4

I think even the setup function should not unwrap the returned object props. The fact that there's a difference between these two:

<template>
  <div>
    {{ name }} 
  </div>
  <div>
    {{ person.name.value }} <- value ??
  </div>
</template>

<script>
export default {
  setup() {
    return {
      name: ref(''),
      person: {
        name: ref('')
      }
    }
  }
}
</script>

really threw me off when I came across it. What I liked about Vue in the first place is that things are very intuitive. Having to memorize that one case works one way and the other differently is the opposite of that and seriously risks the success of the composition api imo.

Same goes for objects, please don't unwrap them automatically either. Types get unnecessarily complicated really fast too with these Unwrap types. One generic function like this:

const field = <T>(value: T) => {
  const v = ref(value); // type of v here is Ref<UnwrapRef<T>> what??
  
  return {
    v,
    touched: ref(false)
  }
};

and if I want to type the return type of this function (which I do) I don't want to have to type Ref<UnwrapRef<T>> I just want to type Ref<T>, I don't want to cast v to Ref<T> either just to shut the type checker up, that doesn't feel clean.

[TKharaishvili]

@yyx990803 This doesn't quite work with object arrays btw. Here's what I mean:

const item = {
  prop: ref('')
};

const array = ref([ item ]);

array.value.push(item); // <- the type checker complains