Skip to content

Commit

Permalink
fix(reactivity): ensure extended method arguments are not lost (#11574)
Browse files Browse the repository at this point in the history
close #11570
  • Loading branch information
edison1105 committed Aug 10, 2024
1 parent 63b7c01 commit 4085def
Show file tree
Hide file tree
Showing 2 changed files with 113 additions and 11 deletions.
95 changes: 95 additions & 0 deletions packages/reactivity/__tests__/reactiveArray.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -622,5 +622,100 @@ describe('reactivity/reactive/Array', () => {
const firstItem = Array.from(deep.values())[0]
expect(isReactive(firstItem)).toBe(true)
})

test('extend methods', () => {
class Collection extends Array {
// @ts-expect-error
every(foo: any, bar: any, baz: any) {
expect(foo).toBe('foo')
expect(bar).toBe('bar')
expect(baz).toBe('baz')
return super.every(obj => obj.id === foo)
}

// @ts-expect-error
filter(foo: any, bar: any, baz: any) {
expect(foo).toBe('foo')
expect(bar).toBe('bar')
expect(baz).toBe('baz')
return super.filter(obj => obj.id === foo)
}

// @ts-expect-error
find(foo: any, bar: any, baz: any) {
expect(foo).toBe('foo')
expect(bar).toBe('bar')
expect(baz).toBe('baz')
return super.find(obj => obj.id === foo)
}

// @ts-expect-error
findIndex(foo: any, bar: any, baz: any) {
expect(foo).toBe('foo')
expect(bar).toBe('bar')
expect(baz).toBe('baz')
return super.findIndex(obj => obj.id === bar)
}

findLast(foo: any, bar: any, baz: any) {
expect(foo).toBe('foo')
expect(bar).toBe('bar')
expect(baz).toBe('baz')
// @ts-expect-error our code is limited to es2016 but user code is not
return super.findLast(obj => obj.id === bar)
}

findLastIndex(foo: any, bar: any, baz: any) {
expect(foo).toBe('foo')
expect(bar).toBe('bar')
expect(baz).toBe('baz')
return super.findIndex(obj => obj.id === bar)
}

// @ts-expect-error
forEach(foo: any, bar: any, baz: any) {
expect(foo).toBe('foo')
expect(bar).toBe('bar')
expect(baz).toBe('baz')
}

// @ts-expect-error
map(foo: any, bar: any, baz: any) {
expect(foo).toBe('foo')
expect(bar).toBe('bar')
expect(baz).toBe('baz')
return super.map(obj => obj.value)
}

// @ts-expect-error
some(foo: any, bar: any, baz: any) {
expect(foo).toBe('foo')
expect(bar).toBe('bar')
expect(baz).toBe('baz')
return super.some(obj => obj.id === baz)
}
}

const state = reactive({
things: new Collection(),
})

const foo = { id: 'foo', value: '1' }
const bar = { id: 'bar', value: '2' }
const baz = { id: 'baz', value: '3' }
state.things.push(foo)
state.things.push(bar)
state.things.push(baz)

expect(state.things.every('foo', 'bar', 'baz')).toBe(false)
expect(state.things.filter('foo', 'bar', 'baz')).toEqual([foo])
expect(state.things.find('foo', 'bar', 'baz')).toBe(foo)
expect(state.things.findIndex('foo', 'bar', 'baz')).toBe(1)
expect(state.things.findLast('foo', 'bar', 'baz')).toBe(bar)
expect(state.things.findLastIndex('foo', 'bar', 'baz')).toBe(1)
expect(state.things.forEach('foo', 'bar', 'baz')).toBeUndefined()
expect(state.things.map('foo', 'bar', 'baz')).toEqual(['1', '2', '3'])
expect(state.things.some('foo', 'bar', 'baz')).toBe(true)
})
})
})
29 changes: 18 additions & 11 deletions packages/reactivity/src/arrayInstrumentations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,42 +47,42 @@ export const arrayInstrumentations: Record<string | symbol, Function> = <any>{
fn: (item: unknown, index: number, array: unknown[]) => unknown,
thisArg?: unknown,
) {
return apply(this, 'every', fn, thisArg)
return apply(this, 'every', fn, thisArg, undefined, arguments)
},

filter(
fn: (item: unknown, index: number, array: unknown[]) => unknown,
thisArg?: unknown,
) {
return apply(this, 'filter', fn, thisArg, v => v.map(toReactive))
return apply(this, 'filter', fn, thisArg, v => v.map(toReactive), arguments)
},

find(
fn: (item: unknown, index: number, array: unknown[]) => boolean,
thisArg?: unknown,
) {
return apply(this, 'find', fn, thisArg, toReactive)
return apply(this, 'find', fn, thisArg, toReactive, arguments)
},

findIndex(
fn: (item: unknown, index: number, array: unknown[]) => boolean,
thisArg?: unknown,
) {
return apply(this, 'findIndex', fn, thisArg)
return apply(this, 'findIndex', fn, thisArg, undefined, arguments)
},

findLast(
fn: (item: unknown, index: number, array: unknown[]) => boolean,
thisArg?: unknown,
) {
return apply(this, 'findLast', fn, thisArg, toReactive)
return apply(this, 'findLast', fn, thisArg, toReactive, arguments)
},

findLastIndex(
fn: (item: unknown, index: number, array: unknown[]) => boolean,
thisArg?: unknown,
) {
return apply(this, 'findLastIndex', fn, thisArg)
return apply(this, 'findLastIndex', fn, thisArg, undefined, arguments)
},

// flat, flatMap could benefit from ARRAY_ITERATE but are not straight-forward to implement
Expand All @@ -91,7 +91,7 @@ export const arrayInstrumentations: Record<string | symbol, Function> = <any>{
fn: (item: unknown, index: number, array: unknown[]) => unknown,
thisArg?: unknown,
) {
return apply(this, 'forEach', fn, thisArg)
return apply(this, 'forEach', fn, thisArg, undefined, arguments)
},

includes(...args: unknown[]) {
Expand All @@ -116,7 +116,7 @@ export const arrayInstrumentations: Record<string | symbol, Function> = <any>{
fn: (item: unknown, index: number, array: unknown[]) => unknown,
thisArg?: unknown,
) {
return apply(this, 'map', fn, thisArg)
return apply(this, 'map', fn, thisArg, undefined, arguments)
},

pop() {
Expand Down Expand Up @@ -161,7 +161,7 @@ export const arrayInstrumentations: Record<string | symbol, Function> = <any>{
fn: (item: unknown, index: number, array: unknown[]) => unknown,
thisArg?: unknown,
) {
return apply(this, 'some', fn, thisArg)
return apply(this, 'some', fn, thisArg, undefined, arguments)
},

splice(...args: unknown[]) {
Expand Down Expand Up @@ -227,6 +227,7 @@ function iterator(
// higher than that
type ArrayMethods = keyof Array<any> | 'findLast' | 'findLastIndex'

const arrayProto = Array.prototype
// instrument functions that read (potentially) all items
// to take ARRAY_ITERATE dependency
function apply(
Expand All @@ -235,8 +236,15 @@ function apply(
fn: (item: unknown, index: number, array: unknown[]) => unknown,
thisArg?: unknown,
wrappedRetFn?: (result: any) => unknown,
args?: IArguments,
) {
const arr = shallowReadArray(self)
let methodFn
// @ts-expect-error our code is limited to es2016 but user code is not
if ((methodFn = arr[method]) !== arrayProto[method]) {
return methodFn.apply(arr, args)
}

let needsWrap = false
let wrappedFn = fn
if (arr !== self) {
Expand All @@ -251,8 +259,7 @@ function apply(
}
}
}
// @ts-expect-error our code is limited to es2016 but user code is not
const result = arr[method](wrappedFn, thisArg)
const result = methodFn.call(arr, wrappedFn, thisArg)
return needsWrap && wrappedRetFn ? wrappedRetFn(result) : result
}

Expand Down

0 comments on commit 4085def

Please sign in to comment.