From dd1b0040a7889dd93de2f9723698eb3b6d6bc800 Mon Sep 17 00:00:00 2001 From: xujiongbo Date: Mon, 26 Jun 2017 16:33:51 +0800 Subject: [PATCH 1/3] improve Vue.set/Vue.delete API --- src/core/observer/index.js | 5 +++-- src/shared/util.js | 10 ++++++++++ test/unit/features/global-api/set-delete.spec.js | 12 ++++++++++-- 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/core/observer/index.js b/src/core/observer/index.js index 68967e60a48..61ef2609e59 100644 --- a/src/core/observer/index.js +++ b/src/core/observer/index.js @@ -6,6 +6,7 @@ import { def, isObject, isPlainObject, + isNumeric, hasProto, hasOwn, warn, @@ -189,7 +190,7 @@ export function defineReactive ( * already exist. */ export function set (target: Array | Object, key: any, val: any): any { - if (Array.isArray(target) && (typeof key === 'number' || /^\d+$/.test(key))) { + if (Array.isArray(target) && isNumeric(key)) { target.length = Math.max(target.length, key) target.splice(key, 1, val) return val @@ -219,7 +220,7 @@ export function set (target: Array | Object, key: any, val: any): any { * Delete a property and trigger change if necessary. */ export function del (target: Array | Object, key: any) { - if (Array.isArray(target) && typeof key === 'number') { + if (Array.isArray(target) && isNumeric(key)) { target.splice(key, 1) return } diff --git a/src/shared/util.js b/src/shared/util.js index 8fbb1b812c8..9d795cf3141 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,15 @@ export function isRegExp (v: any): boolean { return _toString.call(v) === '[object RegExp]' } +/** + * Check if v is numeric + */ +export function isNumeric (v: any): boolean %checks { + return typeof v === 'number' || + isObject(v) && _toString.call(v) === '[object Number]' || + /^\d+$/.test(v) +} + /** * 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 fd27094522f..2d418292665 100644 --- a/test/unit/features/global-api/set-delete.spec.js +++ b/test/unit/features/global-api/set-delete.spec.js @@ -35,6 +35,13 @@ describe('Global API: set/delete', () => { Vue.set(vm.list, 1, 'd') waitForUpdate(() => { expect(vm.$el.innerHTML).toBe('
0-a
1-d
2-c
') + Vue.set(vm.list, '2', 'e') + }).then(() => { + expect(vm.$el.innerHTML).toBe('
0-a
1-d
2-e
') + /* eslint-disable no-new-wrappers */ + Vue.set(vm.list, new Number(1), 'f') + }).then(() => { + expect(vm.$el.innerHTML).toBe('
0-a
1-f
2-e
') }).then(done) }) @@ -88,10 +95,11 @@ describe('Global API: set/delete', () => { Vue.delete(vm.lists, 1) waitForUpdate(() => { expect(vm.$el.innerHTML).toBe('

A

C

') - Vue.delete(vm.lists, 1) + Vue.delete(vm.lists, '1') }).then(() => { expect(vm.$el.innerHTML).toBe('

A

') - 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) From 0cbe6f3e3f85fabf446144d2af1dc03a8e86c7d6 Mon Sep 17 00:00:00 2001 From: xujiongbo Date: Tue, 27 Jun 2017 15:24:32 +0800 Subject: [PATCH 2/3] isNumeric --> isValidArrayIndex --- src/core/observer/index.js | 6 +++--- src/shared/util.js | 9 ++++----- test/unit/features/global-api/set-delete.spec.js | 6 ++++++ 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/core/observer/index.js b/src/core/observer/index.js index 61ef2609e59..9b8f6eab460 100644 --- a/src/core/observer/index.js +++ b/src/core/observer/index.js @@ -6,7 +6,7 @@ import { def, isObject, isPlainObject, - isNumeric, + isValidArrayIndex, hasProto, hasOwn, warn, @@ -190,7 +190,7 @@ export function defineReactive ( * already exist. */ export function set (target: Array | Object, key: any, val: any): any { - if (Array.isArray(target) && isNumeric(key)) { + if (Array.isArray(target) && isValidArrayIndex(key)) { target.length = Math.max(target.length, key) target.splice(key, 1, val) return val @@ -220,7 +220,7 @@ export function set (target: Array | Object, key: any, val: any): any { * Delete a property and trigger change if necessary. */ export function del (target: Array | Object, key: any) { - if (Array.isArray(target) && isNumeric(key)) { + if (Array.isArray(target) && isValidArrayIndex(key)) { target.splice(key, 1) return } diff --git a/src/shared/util.js b/src/shared/util.js index 9d795cf3141..4ad915a3462 100644 --- a/src/shared/util.js +++ b/src/shared/util.js @@ -49,12 +49,11 @@ export function isRegExp (v: any): boolean { } /** - * Check if v is numeric + * Check if val is a valid array index. */ -export function isNumeric (v: any): boolean %checks { - return typeof v === 'number' || - isObject(v) && _toString.call(v) === '[object Number]' || - /^\d+$/.test(v) +export function isValidArrayIndex (val: any): boolean { + const n = parseFloat(val) + return n >= 0 && Math.floor(n) === n && isFinite(val) } /** diff --git a/test/unit/features/global-api/set-delete.spec.js b/test/unit/features/global-api/set-delete.spec.js index 2d418292665..7521717a960 100644 --- a/test/unit/features/global-api/set-delete.spec.js +++ b/test/unit/features/global-api/set-delete.spec.js @@ -40,6 +40,9 @@ describe('Global API: set/delete', () => { expect(vm.$el.innerHTML).toBe('
0-a
1-d
2-e
') /* eslint-disable no-new-wrappers */ Vue.set(vm.list, new Number(1), 'f') + }).then(() => { + expect(vm.$el.innerHTML).toBe('
0-a
1-f
2-e
') + Vue.set(vm.list, '3g', 'g') }).then(() => { expect(vm.$el.innerHTML).toBe('
0-a
1-f
2-e
') }).then(done) @@ -94,6 +97,9 @@ describe('Global API: set/delete', () => { expect(vm.$el.innerHTML).toBe('

A

B

C

') Vue.delete(vm.lists, 1) waitForUpdate(() => { + expect(vm.$el.innerHTML).toBe('

A

C

') + Vue.delete(vm.lists, NaN) + }).then(() => { expect(vm.$el.innerHTML).toBe('

A

C

') Vue.delete(vm.lists, '1') }).then(() => { From 51e91f17cb79356e62e7d236427c621b1ac69776 Mon Sep 17 00:00:00 2001 From: xujiongbo Date: Tue, 27 Jun 2017 15:35:45 +0800 Subject: [PATCH 3/3] add test cases --- test/unit/features/global-api/set-delete.spec.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/unit/features/global-api/set-delete.spec.js b/test/unit/features/global-api/set-delete.spec.js index 7521717a960..f487e4c76f9 100644 --- a/test/unit/features/global-api/set-delete.spec.js +++ b/test/unit/features/global-api/set-delete.spec.js @@ -99,6 +99,18 @@ describe('Global API: set/delete', () => { waitForUpdate(() => { expect(vm.$el.innerHTML).toBe('

A

C

') Vue.delete(vm.lists, NaN) + }).then(() => { + expect(vm.$el.innerHTML).toBe('

A

C

') + Vue.delete(vm.lists, -1) + }).then(() => { + expect(vm.$el.innerHTML).toBe('

A

C

') + Vue.delete(vm.lists, '1.3') + }).then(() => { + expect(vm.$el.innerHTML).toBe('

A

C

') + Vue.delete(vm.lists, true) + }).then(() => { + expect(vm.$el.innerHTML).toBe('

A

C

') + Vue.delete(vm.lists, {}) }).then(() => { expect(vm.$el.innerHTML).toBe('

A

C

') Vue.delete(vm.lists, '1')