From f9820e02d92b8aa4c823cc56eba097e5516f0085 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C4=B0smail=20Ar=C4=B1l=C4=B1k?= <arilik.ismail@gmail.com>
Date: Sun, 14 Jan 2018 19:52:30 +0300
Subject: [PATCH 1/4] fix(core): Give warning for set/delete methods on null,
 undefined, String, Number and Boolean values\nFixes #7381

---
 src/core/observer/index.js                  | 26 ++++++
 test/unit/modules/observer/observer.spec.js | 90 +++++++++++++++++++++
 2 files changed, 116 insertions(+)

diff --git a/src/core/observer/index.js b/src/core/observer/index.js
index 86a21f9e4f2..a5406b2371c 100644
--- a/src/core/observer/index.js
+++ b/src/core/observer/index.js
@@ -191,6 +191,19 @@ export function defineReactive (
  * already exist.
  */
 export function set (target: Array<any> | Object, key: any, val: any): any {
+  if (target === null || target === undefined) {
+    process.env.NODE_ENV !== 'production' && warn(
+      'Cannot add reactive property to a null/undefined value'
+    )
+    return undefined
+  }
+  if (target.constructor === String || target.constructor === Number ||
+      target.constructor === Boolean) {
+    process.env.NODE_ENV !== 'production' && warn(
+      'Cannot add reactive property to a ' + target.constructor.name + ' object'
+    )
+    return undefined
+  }
   if (Array.isArray(target) && isValidArrayIndex(key)) {
     target.length = Math.max(target.length, key)
     target.splice(key, 1, val)
@@ -221,6 +234,19 @@ 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 (target === null || target === undefined) {
+    process.env.NODE_ENV !== 'production' && warn(
+      'Cannot delete reactive property from a null/undefined value'
+    )
+    return undefined
+  }
+  if (target.constructor === String || target.constructor === Number ||
+      target.constructor === Boolean) {
+    process.env.NODE_ENV !== 'production' && warn(
+      'Cannot delete reactive property from a ' + target.constructor.name + ' object'
+    )
+    return undefined
+  }
   if (Array.isArray(target) && isValidArrayIndex(key)) {
     target.splice(key, 1)
     return
diff --git a/test/unit/modules/observer/observer.spec.js b/test/unit/modules/observer/observer.spec.js
index 0cda76960b1..5729f7d829b 100644
--- a/test/unit/modules/observer/observer.spec.js
+++ b/test/unit/modules/observer/observer.spec.js
@@ -336,6 +336,96 @@ describe('Observer', () => {
     }).then(done)
   })
 
+  it('warning set/delete on null value', done => {
+    const data = { a: null }
+    const vm = new Vue({
+      template: '<div>{{a}}</div>',
+      data
+    }).$mount()
+    expect(vm.$el.outerHTML).toBe('<div></div>')
+    expect(Vue.set(data.a, 'b', 2)).toBe(undefined)
+    waitForUpdate(() => {
+      expect(vm.$el.outerHTML).toBe('<div></div>')
+      expect('Cannot add reactive property to a null/undefined value').toHaveBeenWarned()
+      Vue.delete(data.a, 'b')
+    }).then(() => {
+      expect(vm.$el.outerHTML).toBe('<div></div>')
+      expect('Cannot delete reactive property from a null/undefined value').toHaveBeenWarned()
+    }).then(done)
+  })
+
+  it('warning set/delete on undefined value', done => {
+    const data = { a: undefined }
+    const vm = new Vue({
+      template: '<div>{{a}}</div>',
+      data
+    }).$mount()
+    expect(vm.$el.outerHTML).toBe('<div></div>')
+    expect(Vue.set(data.a, 'b', 2)).toBe(undefined)
+    waitForUpdate(() => {
+      expect(vm.$el.outerHTML).toBe('<div></div>')
+      expect('Cannot add reactive property to a null/undefined value').toHaveBeenWarned()
+      Vue.delete(data.a, 'b')
+    }).then(() => {
+      expect(vm.$el.outerHTML).toBe('<div></div>')
+      expect('Cannot delete reactive property from a null/undefined value').toHaveBeenWarned()
+    }).then(done)
+  })
+
+  it('warning set/delete on String instance', done => {
+    const data = { a: 'foo' }
+    const vm = new Vue({
+      template: '<div>{{a}}</div>',
+      data
+    }).$mount()
+    expect(vm.$el.outerHTML).toBe('<div>foo</div>')
+    expect(Vue.set(data.a, 'b', 'bar')).toBe(undefined)
+    waitForUpdate(() => {
+      expect(vm.$el.outerHTML).toBe('<div>foo</div>')
+      expect('Cannot add reactive property to a String object').toHaveBeenWarned()
+      Vue.delete(data.a, 'b')
+    }).then(() => {
+      expect(vm.$el.outerHTML).toBe('<div>foo</div>')
+      expect('Cannot delete reactive property from a String object').toHaveBeenWarned()
+    }).then(done)
+  })
+
+  it('warning set/delete on Number instance', done => {
+    const data = { a: 1 }
+    const vm = new Vue({
+      template: '<div>{{a}}</div>',
+      data
+    }).$mount()
+    expect(vm.$el.outerHTML).toBe('<div>1</div>')
+    expect(Vue.set(data.a, 'b', 2)).toBe(undefined)
+    waitForUpdate(() => {
+      expect(vm.$el.outerHTML).toBe('<div>1</div>')
+      expect('Cannot add reactive property to a Number object').toHaveBeenWarned()
+      Vue.delete(data.a, 'b')
+    }).then(() => {
+      expect(vm.$el.outerHTML).toBe('<div>1</div>')
+      expect('Cannot delete reactive property from a Number object').toHaveBeenWarned()
+    }).then(done)
+  })
+
+  it('warning set/delete on Boolean instance', done => {
+    const data = { a: true }
+    const vm = new Vue({
+      template: '<div>{{a}}</div>',
+      data
+    }).$mount()
+    expect(vm.$el.outerHTML).toBe('<div>true</div>')
+    expect(Vue.set(data.a, 'b', false)).toBe(undefined)
+    waitForUpdate(() => {
+      expect(vm.$el.outerHTML).toBe('<div>true</div>')
+      expect('Cannot add reactive property to a Boolean object').toHaveBeenWarned()
+      Vue.delete(data.a, 'b')
+    }).then(() => {
+      expect(vm.$el.outerHTML).toBe('<div>true</div>')
+      expect('Cannot delete reactive property from a Boolean object').toHaveBeenWarned()
+    }).then(done)
+  })
+
   it('observing array mutation', () => {
     const arr = []
     const ob = observe(arr)

From 07ef0a10ce47c224f8381fb4b69dd0431a606411 Mon Sep 17 00:00:00 2001
From: Evan You <yyx990803@gmail.com>
Date: Thu, 8 Mar 2018 10:18:13 -0500
Subject: [PATCH 2/4] simplify

---
 src/core/observer/index.js | 34 ++++++++++------------------------
 1 file changed, 10 insertions(+), 24 deletions(-)

diff --git a/src/core/observer/index.js b/src/core/observer/index.js
index a5406b2371c..7d377f9c583 100644
--- a/src/core/observer/index.js
+++ b/src/core/observer/index.js
@@ -191,18 +191,11 @@ export function defineReactive (
  * already exist.
  */
 export function set (target: Array<any> | Object, key: any, val: any): any {
-  if (target === null || target === undefined) {
-    process.env.NODE_ENV !== 'production' && warn(
-      'Cannot add reactive property to a null/undefined value'
-    )
-    return undefined
-  }
-  if (target.constructor === String || target.constructor === Number ||
-      target.constructor === Boolean) {
-    process.env.NODE_ENV !== 'production' && warn(
-      'Cannot add reactive property to a ' + target.constructor.name + ' object'
-    )
-    return undefined
+  if (process.env.NODE_ENV !== 'production' && (
+    target == null ||
+    typeof target !== 'object'
+  )) {
+    warn(`Cannot set reactive property on non-object/array value: ${target}`)
   }
   if (Array.isArray(target) && isValidArrayIndex(key)) {
     target.length = Math.max(target.length, key)
@@ -234,18 +227,11 @@ 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 (target === null || target === undefined) {
-    process.env.NODE_ENV !== 'production' && warn(
-      'Cannot delete reactive property from a null/undefined value'
-    )
-    return undefined
-  }
-  if (target.constructor === String || target.constructor === Number ||
-      target.constructor === Boolean) {
-    process.env.NODE_ENV !== 'production' && warn(
-      'Cannot delete reactive property from a ' + target.constructor.name + ' object'
-    )
-    return undefined
+  if (process.env.NODE_ENV !== 'production' && (
+    target == null ||
+    typeof target !== 'object'
+  )) {
+    warn(`Cannot delete reactive property on non-object/array value: ${target}`)
   }
   if (Array.isArray(target) && isValidArrayIndex(key)) {
     target.splice(key, 1)

From ea2ab15af418739b1a91f61ee4d5a8dd75f7f87f Mon Sep 17 00:00:00 2001
From: Evan You <yyx990803@gmail.com>
Date: Thu, 8 Mar 2018 10:23:34 -0500
Subject: [PATCH 3/4] Update observer.spec.js

---
 test/unit/modules/observer/observer.spec.js | 102 +++-----------------
 1 file changed, 12 insertions(+), 90 deletions(-)

diff --git a/test/unit/modules/observer/observer.spec.js b/test/unit/modules/observer/observer.spec.js
index 5729f7d829b..f1ce6f243ec 100644
--- a/test/unit/modules/observer/observer.spec.js
+++ b/test/unit/modules/observer/observer.spec.js
@@ -336,96 +336,6 @@ describe('Observer', () => {
     }).then(done)
   })
 
-  it('warning set/delete on null value', done => {
-    const data = { a: null }
-    const vm = new Vue({
-      template: '<div>{{a}}</div>',
-      data
-    }).$mount()
-    expect(vm.$el.outerHTML).toBe('<div></div>')
-    expect(Vue.set(data.a, 'b', 2)).toBe(undefined)
-    waitForUpdate(() => {
-      expect(vm.$el.outerHTML).toBe('<div></div>')
-      expect('Cannot add reactive property to a null/undefined value').toHaveBeenWarned()
-      Vue.delete(data.a, 'b')
-    }).then(() => {
-      expect(vm.$el.outerHTML).toBe('<div></div>')
-      expect('Cannot delete reactive property from a null/undefined value').toHaveBeenWarned()
-    }).then(done)
-  })
-
-  it('warning set/delete on undefined value', done => {
-    const data = { a: undefined }
-    const vm = new Vue({
-      template: '<div>{{a}}</div>',
-      data
-    }).$mount()
-    expect(vm.$el.outerHTML).toBe('<div></div>')
-    expect(Vue.set(data.a, 'b', 2)).toBe(undefined)
-    waitForUpdate(() => {
-      expect(vm.$el.outerHTML).toBe('<div></div>')
-      expect('Cannot add reactive property to a null/undefined value').toHaveBeenWarned()
-      Vue.delete(data.a, 'b')
-    }).then(() => {
-      expect(vm.$el.outerHTML).toBe('<div></div>')
-      expect('Cannot delete reactive property from a null/undefined value').toHaveBeenWarned()
-    }).then(done)
-  })
-
-  it('warning set/delete on String instance', done => {
-    const data = { a: 'foo' }
-    const vm = new Vue({
-      template: '<div>{{a}}</div>',
-      data
-    }).$mount()
-    expect(vm.$el.outerHTML).toBe('<div>foo</div>')
-    expect(Vue.set(data.a, 'b', 'bar')).toBe(undefined)
-    waitForUpdate(() => {
-      expect(vm.$el.outerHTML).toBe('<div>foo</div>')
-      expect('Cannot add reactive property to a String object').toHaveBeenWarned()
-      Vue.delete(data.a, 'b')
-    }).then(() => {
-      expect(vm.$el.outerHTML).toBe('<div>foo</div>')
-      expect('Cannot delete reactive property from a String object').toHaveBeenWarned()
-    }).then(done)
-  })
-
-  it('warning set/delete on Number instance', done => {
-    const data = { a: 1 }
-    const vm = new Vue({
-      template: '<div>{{a}}</div>',
-      data
-    }).$mount()
-    expect(vm.$el.outerHTML).toBe('<div>1</div>')
-    expect(Vue.set(data.a, 'b', 2)).toBe(undefined)
-    waitForUpdate(() => {
-      expect(vm.$el.outerHTML).toBe('<div>1</div>')
-      expect('Cannot add reactive property to a Number object').toHaveBeenWarned()
-      Vue.delete(data.a, 'b')
-    }).then(() => {
-      expect(vm.$el.outerHTML).toBe('<div>1</div>')
-      expect('Cannot delete reactive property from a Number object').toHaveBeenWarned()
-    }).then(done)
-  })
-
-  it('warning set/delete on Boolean instance', done => {
-    const data = { a: true }
-    const vm = new Vue({
-      template: '<div>{{a}}</div>',
-      data
-    }).$mount()
-    expect(vm.$el.outerHTML).toBe('<div>true</div>')
-    expect(Vue.set(data.a, 'b', false)).toBe(undefined)
-    waitForUpdate(() => {
-      expect(vm.$el.outerHTML).toBe('<div>true</div>')
-      expect('Cannot add reactive property to a Boolean object').toHaveBeenWarned()
-      Vue.delete(data.a, 'b')
-    }).then(() => {
-      expect(vm.$el.outerHTML).toBe('<div>true</div>')
-      expect('Cannot delete reactive property from a Boolean object').toHaveBeenWarned()
-    }).then(done)
-  })
-
   it('observing array mutation', () => {
     const arr = []
     const ob = observe(arr)
@@ -445,4 +355,16 @@ describe('Observer', () => {
       expect(obj.__ob__ instanceof Observer).toBe(true)
     })
   })
+  
+  it('warn set/delete on non valid values', () => {
+    try {
+      setProp(null, 'foo', 1)
+    } catch (e) {}
+    expect(`Cannot set reactive property on non-object/array value`).toHaveBeenWarned()
+
+    try {
+      delProp(null, 'foo')
+    } catch (e) {}
+    expect(`Cannot delete reactive property on non-object/array value`).toHaveBeenWarned()
+  })
 })

From 3d71dcf2a59ce3f74dcf4546ed2d734be202675d Mon Sep 17 00:00:00 2001
From: Evan You <yyx990803@gmail.com>
Date: Thu, 8 Mar 2018 10:24:03 -0500
Subject: [PATCH 4/4] Update index.js

---
 src/core/observer/index.js | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/core/observer/index.js b/src/core/observer/index.js
index 7d377f9c583..7a876034d06 100644
--- a/src/core/observer/index.js
+++ b/src/core/observer/index.js
@@ -191,10 +191,10 @@ export function defineReactive (
  * already exist.
  */
 export function set (target: Array<any> | Object, key: any, val: any): any {
-  if (process.env.NODE_ENV !== 'production' && (
-    target == null ||
-    typeof target !== 'object'
-  )) {
+  if (process.env.NODE_ENV !== 'production' &&
+    !Array.isArray(target) &&
+    !isObject(target)
+  ) {
     warn(`Cannot set reactive property on non-object/array value: ${target}`)
   }
   if (Array.isArray(target) && isValidArrayIndex(key)) {
@@ -227,10 +227,10 @@ 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 (process.env.NODE_ENV !== 'production' && (
-    target == null ||
-    typeof target !== 'object'
-  )) {
+  if (process.env.NODE_ENV !== 'production' &&
+    !Array.isArray(target) &&
+    !isObject(target)
+  ) {
     warn(`Cannot delete reactive property on non-object/array value: ${target}`)
   }
   if (Array.isArray(target) && isValidArrayIndex(key)) {