fix(reactivity): unwrap ref types as props of arrays #1859
Conversation
|
I don't think this is fixing the the issue linked. It is about properties added to an array, not refs inside arrays and empty string keys |
|
| @@ -81,7 +81,10 @@ function createGetter(isReadonly = false, shallow = false) { | |||
|
|
|||
| if (isRef(res)) { | |||
| // ref unwrapping, only for Objects, not for Arrays. | |||
| return targetIsArray ? res : res.value | |||
| const shouldUnwrap = | |||
| !targetIsArray || isSymbol(key) || isNaN(key as any) || key === '' | |||
There was a problem hiding this comment.
why not just !targetIsArray || typeof key !== 'number'?
There was a problem hiding this comment.
the key is a string😭
There was a problem hiding this comment.
In fact, there are a lot of edge cases not taken into account in this kind of check. eg. '3e7' can be treated as a legal Number by JavaScript.
There was a problem hiding this comment.
3e7 is actually already normalized to "30000000" here, but this check does produce false positives for float strings like 1.234 which are not valid Array indices.
Technically we should only skip unwrap for integer keys. Maybe we can check '' + parseInt(key, 10) === key instead.
There was a problem hiding this comment.
yes! according to https://jsben.ch/MMQ6b, it has better accuracy and the time cost remains the same
| isSymbol(key) | ||
| ? builtInSymbols.has(key) | ||
| keyIsSymbol | ||
| ? builtInSymbols.has(key as symbol) |
There was a problem hiding this comment.
reusing the result of isSymbol(key) will disable type guard, so have to add as manually 😣
close #1846