From e0dcb7bccdb15c5040de86488e248555bbcb4cdb Mon Sep 17 00:00:00 2001 From: linzhe Date: Sat, 31 Aug 2024 11:55:09 +0800 Subject: [PATCH 1/2] fix(reactivity): ensure array extension methods execute correctly --- .../reactivity/src/arrayInstrumentations.ts | 34 +++++++------------ packages/reactivity/src/baseHandlers.ts | 7 +++- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/packages/reactivity/src/arrayInstrumentations.ts b/packages/reactivity/src/arrayInstrumentations.ts index 6cf7602eca5..69f08d6c5d1 100644 --- a/packages/reactivity/src/arrayInstrumentations.ts +++ b/packages/reactivity/src/arrayInstrumentations.ts @@ -47,42 +47,42 @@ export const arrayInstrumentations: Record = { fn: (item: unknown, index: number, array: unknown[]) => unknown, thisArg?: unknown, ) { - return apply(this, 'every', fn, thisArg, undefined, arguments) + return apply(this, 'every', fn, thisArg) }, filter( fn: (item: unknown, index: number, array: unknown[]) => unknown, thisArg?: unknown, ) { - return apply(this, 'filter', fn, thisArg, v => v.map(toReactive), arguments) + return apply(this, 'filter', fn, thisArg, v => v.map(toReactive)) }, find( fn: (item: unknown, index: number, array: unknown[]) => boolean, thisArg?: unknown, ) { - return apply(this, 'find', fn, thisArg, toReactive, arguments) + return apply(this, 'find', fn, thisArg, toReactive) }, findIndex( fn: (item: unknown, index: number, array: unknown[]) => boolean, thisArg?: unknown, ) { - return apply(this, 'findIndex', fn, thisArg, undefined, arguments) + return apply(this, 'findIndex', fn, thisArg) }, findLast( fn: (item: unknown, index: number, array: unknown[]) => boolean, thisArg?: unknown, ) { - return apply(this, 'findLast', fn, thisArg, toReactive, arguments) + return apply(this, 'findLast', fn, thisArg, toReactive) }, findLastIndex( fn: (item: unknown, index: number, array: unknown[]) => boolean, thisArg?: unknown, ) { - return apply(this, 'findLastIndex', fn, thisArg, undefined, arguments) + return apply(this, 'findLastIndex', fn, thisArg) }, // flat, flatMap could benefit from ARRAY_ITERATE but are not straight-forward to implement @@ -91,7 +91,7 @@ export const arrayInstrumentations: Record = { fn: (item: unknown, index: number, array: unknown[]) => unknown, thisArg?: unknown, ) { - return apply(this, 'forEach', fn, thisArg, undefined, arguments) + return apply(this, 'forEach', fn, thisArg) }, includes(...args: unknown[]) { @@ -116,7 +116,7 @@ export const arrayInstrumentations: Record = { fn: (item: unknown, index: number, array: unknown[]) => unknown, thisArg?: unknown, ) { - return apply(this, 'map', fn, thisArg, undefined, arguments) + return apply(this, 'map', fn, thisArg) }, pop() { @@ -161,7 +161,7 @@ export const arrayInstrumentations: Record = { fn: (item: unknown, index: number, array: unknown[]) => unknown, thisArg?: unknown, ) { - return apply(this, 'some', fn, thisArg, undefined, arguments) + return apply(this, 'some', fn, thisArg) }, splice(...args: unknown[]) { @@ -227,7 +227,6 @@ function iterator( // higher than that type ArrayMethods = keyof Array | 'findLast' | 'findLastIndex' -const arrayProto = Array.prototype // instrument functions that read (potentially) all items // to take ARRAY_ITERATE dependency function apply( @@ -236,20 +235,12 @@ function apply( fn: (item: unknown, index: number, array: unknown[]) => unknown, thisArg?: unknown, wrappedRetFn?: (result: any) => unknown, - args?: IArguments, ) { const arr = shallowReadArray(self) - const needsWrap = arr !== self && !isShallow(self) - // @ts-expect-error our code is limited to es2016 but user code is not - const methodFn = arr[method] - // @ts-expect-error - if (methodFn !== arrayProto[method]) { - const result = methodFn.apply(arr, args) - return needsWrap ? toReactive(result) : result - } - + let needsWrap = false let wrappedFn = fn if (arr !== self) { + needsWrap = !isShallow(self) if (needsWrap) { wrappedFn = function (this: unknown, item, index) { return fn.call(this, toReactive(item), index, self) @@ -260,7 +251,8 @@ function apply( } } } - const result = methodFn.call(arr, wrappedFn, thisArg) + // @ts-expect-error our code is limited to es2016 but user code is not + const result = arr[method](wrappedFn, thisArg) return needsWrap && wrappedRetFn ? wrappedRetFn(result) : result } diff --git a/packages/reactivity/src/baseHandlers.ts b/packages/reactivity/src/baseHandlers.ts index 5916c0b5d81..41c6936428c 100644 --- a/packages/reactivity/src/baseHandlers.ts +++ b/packages/reactivity/src/baseHandlers.ts @@ -86,7 +86,12 @@ class BaseReactiveHandler implements ProxyHandler { if (!isReadonly) { let fn: Function | undefined - if (targetIsArray && (fn = arrayInstrumentations[key])) { + if ( + targetIsArray && + // @ts-expect-error our code is limited to es2016 but user code is not + (target as any[])[key] === Array.prototype[key] && + (fn = arrayInstrumentations[key]) + ) { return fn } if (key === 'hasOwnProperty') { From d3bca06845c97ceec759b4a9072e6fa652a406aa Mon Sep 17 00:00:00 2001 From: linzhe Date: Sat, 31 Aug 2024 11:58:59 +0800 Subject: [PATCH 2/2] chore: update test --- .../__tests__/reactiveArray.spec.ts | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/reactivity/__tests__/reactiveArray.spec.ts b/packages/reactivity/__tests__/reactiveArray.spec.ts index fabafc21e25..74357cb7295 100644 --- a/packages/reactivity/__tests__/reactiveArray.spec.ts +++ b/packages/reactivity/__tests__/reactiveArray.spec.ts @@ -725,5 +725,26 @@ describe('reactivity/reactive/Array', () => { expect(state.things.map('foo', 'bar', 'baz')).toEqual(['1', '2', '3']) expect(state.things.some('foo', 'bar', 'baz')).toBe(true) }) + + test('computed + extend methods', () => { + class Collection extends Array { + find(matcher: any) { + return super.find(matcher) + } + } + + const state = reactive({ + // @ts-expect-error + things: new Collection({ foo: '' }), + }) + + const bar = computed(() => { + return state.things.find((obj: any) => obj.foo === 'bar') + }) + bar.value + state.things[0].foo = 'bar' + + expect(bar.value).toEqual({ foo: 'bar' }) + }) }) })