fix(reactivity): trigger the ref that is created by toRef from a reactive array #12431
Conversation
@vue/compiler-core
@vue/compiler-sfc
@vue/compiler-dom
@vue/compiler-ssr
@vue/runtime-core
@vue/reactivity
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
Size ReportBundles
Usages
|
edison1105
added
scope: reactivity
🔨 p3-minor-bug
|
According to the reproduction in #12427, the issue likely lies within |
But for |
edison1105
changed the title
triggerRef fails when target is an array
edison1105
added
ready to merge
There was a problem hiding this comment.
This fix looks reasonable to me, but I'd like to propose an alternative.
I think the underlying reason why this is happening is that JavaScript treats all property keys as either strings and symbols. Even for arrays, the numbers used as keys are treated as strings. e.g.:
Object.keys([7, 8, 9]) // returns ['0', '1', '2']When we go through a Proxy, the key passed to the trap will either be a string or a symbol. Any other value is coerced to a string before it's passed. e.g.:
const p = new Proxy([], {
get(target, key) {
console.log(key, typeof key)
return target[key]
}
})
p[0]This example is using a Proxy around an array, and p[0] triggers the trap with a string key of "0".
But the coercion doesn't just apply to numbers, it applies to any value that isn't a string or symbol. For example, if you tried to use an array as a property key:
p[[0]]This would actually work, as [0] is coerced to the string "0".
So the reactive Proxy wrappers used by Vue are passed key values that are either symbols or strings, nothing else. This carries across throughout dependency tracking, leading to strings in the dependency maps, even for arrays.
Collections (e.g. Set, Map, etc.) are different, as they can have other types of key, but as they aren't relevant to toRef I'm going to ignore them.
For the most part, Vue doesn't need to worry about ensuring the keys are coerced correctly, because that's already handled by JavaScript. It's only an issue when Vue gets passed a property name directly. An example of that is hasOwnProperty, see:
There we need to coerce non-symbols to strings.
So I think the toRef problem is essentially the same underlying issue as #10455. We need to ensure we coerce non-symbol keys to strings. While the primary use case is for arrays, it also ensures that we don't hit any other weird edge cases, like the example I gave earlier where I passed an array as a key.
The fix I'd propose is something like this:
class ObjectRefImpl<T extends object, K extends keyof T> {
public readonly [ReactiveFlags.IS_REF] = true
public _value: T[K] = undefined!
private readonly _key: K
constructor(
private readonly _object: T,
key: K,
private readonly _defaultValue?: T[K],
) {
this._key = isSymbol(key) ? key : String(key) as K
}The important part is isSymbol(key) ? key : String(key).
There may be a neater way to write that (I'm not entirely happy with the types), but I think this is a more appropriate place to handle the coercion. We could handle it further upstream, but this seemed more natural to me.
I tested this locally and it seemed to work. I think it's a more accurate fix and it also adds fewer bytes to the build.
fix #12427