perf(reactivity): only trigger all effects on Array length mutation i…

…f new length is shorter than old length
vuejs · Feb 21, 2020 · 33622d6 · 33622d6
1 parent 9882788
commit 33622d6
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 55 deletions.
diff --git a/packages/reactivity/src/baseHandlers.ts b/packages/reactivity/src/baseHandlers.ts
@@ -91,20 +91,10 @@ function createSetter(isReadonly = false, shallow = false) {
     const result = Reflect.set(target, key, value, receiver)
     // don't trigger if target is something up in the prototype chain of original
     if (target === toRaw(receiver)) {
-      /* istanbul ignore else */
-      if (__DEV__) {
-        const extraInfo = { oldValue, newValue: value }
-        if (!hadKey) {
-          trigger(target, TriggerOpTypes.ADD, key, extraInfo)
-        } else if (hasChanged(value, oldValue)) {
-          trigger(target, TriggerOpTypes.SET, key, extraInfo)
-        }
-      } else {
-        if (!hadKey) {
-          trigger(target, TriggerOpTypes.ADD, key)
-        } else if (hasChanged(value, oldValue)) {
-          trigger(target, TriggerOpTypes.SET, key)
-        }
+      if (!hadKey) {
+        trigger(target, TriggerOpTypes.ADD, key, value)
+      } else if (hasChanged(value, oldValue)) {
+        trigger(target, TriggerOpTypes.SET, key, value, oldValue)
       }
     }
     return result
@@ -116,12 +106,7 @@ function deleteProperty(target: object, key: string | symbol): boolean {
   const oldValue = (target as any)[key]
   const result = Reflect.deleteProperty(target, key)
   if (result && hadKey) {
-    /* istanbul ignore else */
-    if (__DEV__) {
-      trigger(target, TriggerOpTypes.DELETE, key, { oldValue })
-    } else {
-      trigger(target, TriggerOpTypes.DELETE, key)
-    }
+    trigger(target, TriggerOpTypes.DELETE, key, undefined, oldValue)
   }
   return result
 }

diff --git a/packages/reactivity/src/collectionHandlers.ts b/packages/reactivity/src/collectionHandlers.ts
@@ -51,12 +51,7 @@ function add(this: SetTypes, value: unknown) {
   const hadKey = proto.has.call(target, value)
   const result = proto.add.call(target, value)
   if (!hadKey) {
-    /* istanbul ignore else */
-    if (__DEV__) {
-      trigger(target, TriggerOpTypes.ADD, value, { newValue: value })
-    } else {
-      trigger(target, TriggerOpTypes.ADD, value)
-    }
+    trigger(target, TriggerOpTypes.ADD, value, value)
   }
   return result
 }
@@ -69,20 +64,10 @@ function set(this: MapTypes, key: unknown, value: unknown) {
   const hadKey = proto.has.call(target, key)
   const oldValue = proto.get.call(target, key)
   const result = proto.set.call(target, key, value)
-  /* istanbul ignore else */
-  if (__DEV__) {
-    const extraInfo = { oldValue, newValue: value }
-    if (!hadKey) {
-      trigger(target, TriggerOpTypes.ADD, key, extraInfo)
-    } else if (hasChanged(value, oldValue)) {
-      trigger(target, TriggerOpTypes.SET, key, extraInfo)
-    }
-  } else {
-    if (!hadKey) {
-      trigger(target, TriggerOpTypes.ADD, key)
-    } else if (hasChanged(value, oldValue)) {
-      trigger(target, TriggerOpTypes.SET, key)
-    }
+  if (!hadKey) {
+    trigger(target, TriggerOpTypes.ADD, key, value)
+  } else if (hasChanged(value, oldValue)) {
+    trigger(target, TriggerOpTypes.SET, key, value, oldValue)
   }
   return result
 }
@@ -96,12 +81,7 @@ function deleteEntry(this: CollectionTypes, key: unknown) {
   // forward the operation before queueing reactions
   const result = proto.delete.call(target, key)
   if (hadKey) {
-    /* istanbul ignore else */
-    if (__DEV__) {
-      trigger(target, TriggerOpTypes.DELETE, key, { oldValue })
-    } else {
-      trigger(target, TriggerOpTypes.DELETE, key)
-    }
+    trigger(target, TriggerOpTypes.DELETE, key, undefined, oldValue)
   }
   return result
 }
@@ -117,12 +97,7 @@ function clear(this: IterableCollections) {
   // forward the operation before queueing reactions
   const result = getProto(target).clear.call(target)
   if (hadItems) {
-    /* istanbul ignore else */
-    if (__DEV__) {
-      trigger(target, TriggerOpTypes.CLEAR, void 0, { oldTarget })
-    } else {
-      trigger(target, TriggerOpTypes.CLEAR)
-    }
+    trigger(target, TriggerOpTypes.CLEAR, undefined, undefined, oldTarget)
   }
   return result
 }

diff --git a/packages/reactivity/src/effect.ts b/packages/reactivity/src/effect.ts
@@ -165,7 +165,9 @@ export function trigger(
   target: object,
   type: TriggerOpTypes,
   key?: unknown,
-  extraInfo?: DebuggerEventExtraInfo
+  newValue?: unknown,
+  oldValue?: unknown,
+  oldTarget?: Map<unknown, unknown> | Set<unknown>
 ) {
   const depsMap = targetMap.get(target)
   if (depsMap === void 0) {
@@ -174,7 +176,12 @@ export function trigger(
   }
   const effects = new Set<ReactiveEffect>()
   const computedRunners = new Set<ReactiveEffect>()
-  if (type === TriggerOpTypes.CLEAR || (key === 'length' && isArray(target))) {
+  if (
+    type === TriggerOpTypes.CLEAR ||
+    (key === 'length' &&
+      isArray(target) &&
+      (newValue as number) < (oldValue as number))
+  ) {
     // collection being cleared or Array length mutation
     // trigger all effects for target
     depsMap.forEach(dep => {
@@ -196,7 +203,19 @@ export function trigger(
     }
   }
   const run = (effect: ReactiveEffect) => {
-    scheduleRun(effect, target, type, key, extraInfo)
+    scheduleRun(
+      effect,
+      target,
+      type,
+      key,
+      __DEV__
+        ? {
+            newValue,
+            oldValue,
+            oldTarget
+          }
+        : undefined
+    )
   }
   // Important: computed effects must be run first so that computed getters
   // can be invalidated before any normal effects that depend on them are run.