From eea0920f14d0ea63d1b94c648eeb36ac7dfb4b05 Mon Sep 17 00:00:00 2001
From: AchillesJ <xujiongbo@gmail.com>
Date: Sat, 1 Jul 2017 11:22:25 +0800
Subject: [PATCH] fix: improve Vue.set/Vue.delete API to support multi type of
 array index (#5973)

related #5884
---
 src/core/observer/index.js                    |  5 ++--
 src/shared/util.js                            |  9 ++++++
 .../features/global-api/set-delete.spec.js    | 30 +++++++++++++++++--
 3 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/src/core/observer/index.js b/src/core/observer/index.js
index 68967e60a4..9b8f6eab46 100644
--- a/src/core/observer/index.js
+++ b/src/core/observer/index.js
@@ -6,6 +6,7 @@ import {
   def,
   isObject,
   isPlainObject,
+  isValidArrayIndex,
   hasProto,
   hasOwn,
   warn,
@@ -189,7 +190,7 @@ export function defineReactive (
  * already exist.
  */
 export function set (target: Array<any> | Object, key: any, val: any): any {
-  if (Array.isArray(target) && (typeof key === 'number' || /^\d+$/.test(key))) {
+  if (Array.isArray(target) && isValidArrayIndex(key)) {
     target.length = Math.max(target.length, key)
     target.splice(key, 1, val)
     return val
@@ -219,7 +220,7 @@ export function set (target: Array<any> | Object, key: any, val: any): any {
  * Delete a property and trigger change if necessary.
  */
 export function del (target: Array<any> | Object, key: any) {
-  if (Array.isArray(target) && typeof key === 'number') {
+  if (Array.isArray(target) && isValidArrayIndex(key)) {
     target.splice(key, 1)
     return
   }
diff --git a/src/shared/util.js b/src/shared/util.js
index 8fbb1b812c..4ad915a346 100644
--- a/src/shared/util.js
+++ b/src/shared/util.js
@@ -17,6 +17,7 @@ export function isTrue (v: any): boolean %checks {
 export function isFalse (v: any): boolean %checks {
   return v === false
 }
+
 /**
  * Check if value is primitive
  */
@@ -47,6 +48,14 @@ export function isRegExp (v: any): boolean {
   return _toString.call(v) === '[object RegExp]'
 }
 
+/**
+ * Check if val is a valid array index.
+ */
+export function isValidArrayIndex (val: any): boolean {
+  const n = parseFloat(val)
+  return n >= 0 && Math.floor(n) === n && isFinite(val)
+}
+
 /**
  * Convert a value to a string that is actually rendered.
  */
diff --git a/test/unit/features/global-api/set-delete.spec.js b/test/unit/features/global-api/set-delete.spec.js
index f9070a7908..d758422d03 100644
--- a/test/unit/features/global-api/set-delete.spec.js
+++ b/test/unit/features/global-api/set-delete.spec.js
@@ -35,6 +35,16 @@ describe('Global API: set/delete', () => {
       Vue.set(vm.list, 1, 'd')
       waitForUpdate(() => {
         expect(vm.$el.innerHTML).toBe('<div>0-a</div><div>1-d</div><div>2-c</div>')
+        Vue.set(vm.list, '2', 'e')
+      }).then(() => {
+        expect(vm.$el.innerHTML).toBe('<div>0-a</div><div>1-d</div><div>2-e</div>')
+        /* eslint-disable no-new-wrappers */
+        Vue.set(vm.list, new Number(1), 'f')
+      }).then(() => {
+        expect(vm.$el.innerHTML).toBe('<div>0-a</div><div>1-f</div><div>2-e</div>')
+        Vue.set(vm.list, '3g', 'g')
+      }).then(() => {
+        expect(vm.$el.innerHTML).toBe('<div>0-a</div><div>1-f</div><div>2-e</div>')
       }).then(done)
     })
 
@@ -106,10 +116,26 @@ describe('Global API: set/delete', () => {
       Vue.delete(vm.lists, 1)
       waitForUpdate(() => {
         expect(vm.$el.innerHTML).toBe('<p>A</p><p>C</p>')
-        Vue.delete(vm.lists, 1)
+        Vue.delete(vm.lists, NaN)
+      }).then(() => {
+        expect(vm.$el.innerHTML).toBe('<p>A</p><p>C</p>')
+        Vue.delete(vm.lists, -1)
+      }).then(() => {
+        expect(vm.$el.innerHTML).toBe('<p>A</p><p>C</p>')
+        Vue.delete(vm.lists, '1.3')
+      }).then(() => {
+        expect(vm.$el.innerHTML).toBe('<p>A</p><p>C</p>')
+        Vue.delete(vm.lists, true)
+      }).then(() => {
+        expect(vm.$el.innerHTML).toBe('<p>A</p><p>C</p>')
+        Vue.delete(vm.lists, {})
+      }).then(() => {
+        expect(vm.$el.innerHTML).toBe('<p>A</p><p>C</p>')
+        Vue.delete(vm.lists, '1')
       }).then(() => {
         expect(vm.$el.innerHTML).toBe('<p>A</p>')
-        Vue.delete(vm.lists, 0)
+        /* eslint-disable no-new-wrappers */
+        Vue.delete(vm.lists, new Number(0))
       }).then(() => {
         expect(vm.$el.innerHTML).toBe('')
       }).then(done)