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.
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.
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?
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
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))
/cc @jods4 would be interesting to hear your thoughts on this.
This is tricky stuff, there are no perfect answer for all cases.
I would set some design principles for a successful API:
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.
Arrays will no longer unwrap refs after 775a7c2
Most helpful comment
This is tricky stuff, there are no perfect answer for all cases.
I would set some design principles for a successful API:
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
forEachbut notfor-ofand what am I gonna get when I dofor-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 : eis 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
setupa non-reactive object that contains some refs. So I would keep auto-unwrapping thesetup()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
unwrapinside 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.