Skip to content

Commit

Permalink
fix(reactivity): ensure extended method arguments are not lost
Browse files Browse the repository at this point in the history
  • Loading branch information
edison1105 authored and yyx990803 committed Aug 10, 2024
1 parent b15869a commit 2442add
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 16 deletions.
90 changes: 84 additions & 6 deletions packages/reactivity/__tests__/reactiveArray.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -625,19 +625,97 @@ describe('reactivity/reactive/Array', () => {

test('extend methods', () => {
class Collection extends Array {
find(id: any) {
return super.find(obj => obj.id === id)
// @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 val = { id: 'foo', value: 'bar' }
state.things.push(val)

expect(state.things.find('foo')).toBe(val)
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)
})
})
})
21 changes: 11 additions & 10 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 @@ -236,12 +236,13 @@ 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, arrayProto.slice.call(arguments, 2))
return methodFn.apply(arr, args)
}

let needsWrap = false
Expand Down

0 comments on commit 2442add

Please sign in to comment.