perf: shrink instrumentations requires toRaw parameter #9051
Conversation
|
@chenfan0 is attempting to deploy a commit to the vuejs Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Size ReportBundles
Usages
|
| @@ -66,7 +66,7 @@ function createArrayInstrumentations() { | |||
| const res = arr[key](...args) | |||
| if (res === -1 || res === false) { | |||
| // if that didn't work, run it again using raw values. | |||
| return arr[key](...args.map(toRaw)) | |||
| return arr[key](toRaw(args[0]), ...args.slice(1)) | |||
There was a problem hiding this comment.
I don't think this is necessary, because the parameters passed in by the user are uncontrollable. The parameters passed in by the user may all be responsive objects. Only use toRaw to process the first parameter. If the subsequent parameters are responsive objects, Incur access tracking overhead
There was a problem hiding this comment.
I don't think this is necessary, because the parameters passed in by the user are uncontrollable. The parameters passed in by the user may all be responsive objects. Only use toRaw to process the first parameter. If the subsequent parameters are responsive objects, Incur access tracking overhead
The purpose here is to use the original value to re-check whether it exists, so only the first parameter needs to be toRaw, and other parameters do not require additional processing. At the same time, the modification here will not cause additional overhead, because before executing this code, it will execute const res = arr[key](...args)
There was a problem hiding this comment.
I think it would be great if we could add or point out a test case to clarify it further.
There was a problem hiding this comment.
I think it would be great if we could add or point out a test case to clarify it further.
// reactiveArray.spec.ts
test('Array identity methods should work with raw values', () => {
const raw = {}
const arr = reactive([{}, {}])
arr.push(raw)
expect(arr.indexOf(raw)).toBe(2)
expect(arr.indexOf(raw, 3)).toBe(-1)
// should work also for the observed version
const observed = arr[2]
expect(arr.indexOf(observed)).toBe(2)
expect(arr.indexOf(observed, 3)).toBe(-1)
})
if the code is
arr.indexOf(observed, 3, anotherObserved, anotherObserved, anotherObserved, anotherObserved, anotherObserved)
we just need to use toRaw for the first param, and we dont need to use toRaw for anotherObserved.
|
toRaw will return the original object only if the parameter is an object created by |
You are right. The type passed in the second parameter of these methods is number. If reactive is passed in, then it is an unreasonable approach. |
|
The code here has changed since #9511 was merged. Therefore, this PR is no longer needed. |
When using the array functions such as
indexOfincludeslastIndexOfto search, we only need to performtoRawon the first parameter (searchElement), and do not need to performtoRawon other parameters.