fix(reactivity): avoid length mutating array methods causing infinite…

… updates (#2138)

fix #2137

Co-authored-by: Evan You <[email protected]>

packages/reactivity/__tests__/effect.spec.ts

@@ -358,6 +358,29 @@ describe('reactivity/effect', () => {
     expect(counterSpy).toHaveBeenCalledTimes(2)
   })
 
+  it('should avoid infinite recursive loops when use Array.prototype.push/unshift/pop/shift', () => {
+    ;(['push', 'unshift'] as const).forEach(key => {
+      const arr = reactive<number[]>([])
+      const counterSpy1 = jest.fn(() => (arr[key] as any)(1))
+      const counterSpy2 = jest.fn(() => (arr[key] as any)(2))
+      effect(counterSpy1)
+      effect(counterSpy2)
+      expect(arr.length).toBe(2)
+      expect(counterSpy1).toHaveBeenCalledTimes(1)
+      expect(counterSpy2).toHaveBeenCalledTimes(1)
+    })
+    ;(['pop', 'shift'] as const).forEach(key => {
+      const arr = reactive<number[]>([1, 2, 3, 4])
+      const counterSpy1 = jest.fn(() => (arr[key] as any)())
+      const counterSpy2 = jest.fn(() => (arr[key] as any)())
+      effect(counterSpy1)
+      effect(counterSpy2)
+      expect(arr.length).toBe(2)
+      expect(counterSpy1).toHaveBeenCalledTimes(1)
+      expect(counterSpy2).toHaveBeenCalledTimes(1)
+    })
+  })
+
   it('should allow explicitly recursive raw function loops', () => {
     const counter = reactive({ num: 0 })
     const numSpy = jest.fn(() => {

packages/reactivity/src/baseHandlers.ts

@@ -8,7 +8,13 @@ import {
   reactiveMap
 } from './reactive'
 import { TrackOpTypes, TriggerOpTypes } from './operations'
-import { track, trigger, ITERATE_KEY } from './effect'
+import {
+  track,
+  trigger,
+  ITERATE_KEY,
+  pauseTracking,
+  enableTracking
+} from './effect'
 import {
   isObject,
   hasOwn,
@@ -32,22 +38,36 @@ const readonlyGet = /*#__PURE__*/ createGetter(true)
 const shallowReadonlyGet = /*#__PURE__*/ createGetter(true, true)
 
 const arrayInstrumentations: Record<string, Function> = {}
+// instrument identity-sensitive Array methods to account for possible reactive
+// values
 ;(['includes', 'indexOf', 'lastIndexOf'] as const).forEach(key => {
+  const method = Array.prototype[key] as any
   arrayInstrumentations[key] = function(this: unknown[], ...args: unknown[]) {
     const arr = toRaw(this)
     for (let i = 0, l = this.length; i < l; i++) {
       track(arr, TrackOpTypes.GET, i + '')
     }
     // we run the method using the original args first (which may be reactive)
-    const res = (arr[key] as any)(...args)
+    const res = method.apply(arr, args)
     if (res === -1 || res === false) {
       // if that didn't work, run it again using raw values.
-      return (arr[key] as any)(...args.map(toRaw))
+      return method.apply(arr, args.map(toRaw))
     } else {
       return res
     }
   }
 })
+// instrument length-altering mutation methods to avoid length being tracked
+// which leads to infinite loops in some cases (#2137)
+;(['push', 'pop', 'shift', 'unshift', 'splice'] as const).forEach(key => {
+  const method = Array.prototype[key] as any
+  arrayInstrumentations[key] = function(this: unknown[], ...args: unknown[]) {
+    pauseTracking()
+    const res = method.apply(this, args)
+    enableTracking()
+    return res
+  }
+})
 
 function createGetter(isReadonly = false, shallow = false) {
   return function get(target: Target, key: string | symbol, receiver: object) {