From 6fecabe3e2c2c43a177ea025d926f386db420a72 Mon Sep 17 00:00:00 2001 From: Joiey Seeley Date: Sat, 5 May 2018 19:26:41 -0500 Subject: [PATCH 1/5] fix: In development mode, warns if user tries to Vue.set a property that already exists. In development mode, warns if user tries to Vue.set a property that already exists. Issue reported in #8129. Codepen demonstrating the issue available at https://codepen.io/chrisvfritz/pen/rvzgBR?editors=1010 fix #8129 --- src/core/observer/index.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/core/observer/index.js b/src/core/observer/index.js index 35469aaf16..ab730dfe3f 100644 --- a/src/core/observer/index.js +++ b/src/core/observer/index.js @@ -196,10 +196,15 @@ export function defineReactive ( * already exist. */ export function set (target: Array | Object, key: any, val: any): any { - if (process.env.NODE_ENV !== 'production' && - (isUndef(target) || isPrimitive(target)) - ) { - warn(`Cannot set reactive property on undefined, null, or primitive value: ${(target: any)}`) + if (process.env.NODE_ENV !== 'production') { + if (isUndef(target) || isPrimitive(target)) { + warn(`Cannot set reactive property on undefined, null, or primitive value: ${(target: any)}`) + } + if (Object.getOwnPropertyDescriptor(target, key) && + (typeof (Object.getOwnPropertyDescriptor(target, key).get) === 'undefined') && + !Array.isArray(target)) { + warn(`Cannot enable reactivity on a property that is already defined: ${(key: any)}`) + } } if (Array.isArray(target) && isValidArrayIndex(key)) { target.length = Math.max(target.length, key) From 3aea4dd51c5840622e9f4dc61774dbe4df8baa7e Mon Sep 17 00:00:00 2001 From: Joiey Seeley Date: Tue, 8 May 2018 09:43:58 -0500 Subject: [PATCH 2/5] test: Adding an observer spec for new feature to catch properties being set that already have been c fix #8129 --- test/unit/modules/observer/observer.spec.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/unit/modules/observer/observer.spec.js b/test/unit/modules/observer/observer.spec.js index 5f075bccc9..b7f59fedb0 100644 --- a/test/unit/modules/observer/observer.spec.js +++ b/test/unit/modules/observer/observer.spec.js @@ -316,6 +316,21 @@ describe('Observer', () => { }).then(done) }) + it('Cannot enable reactivity on a property that is already defined', done => { + const vm = new Vue({ + data: { + person: { + age: 32 + } + } + }).$mount() + vm.person.job = 'Programmer' + Vue.set(vm.person, 'job', 'Educator') + waitForUpdate(() => { + expect(`Cannot enable reactivity on a property that is already defined`).toHaveBeenWarned() + }).end(done) + }) + it('warning set/delete on Vue instance root $data', done => { const data = { a: 1 } const vm = new Vue({ From e568602bf6622370119daa1327958ad3391e1204 Mon Sep 17 00:00:00 2001 From: Joiey Seeley Date: Wed, 9 May 2018 11:52:45 -0500 Subject: [PATCH 3/5] fix: Adds a template to observer spec Vue instance to fix failing spec. fix #8129 --- test/unit/modules/observer/observer.spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/unit/modules/observer/observer.spec.js b/test/unit/modules/observer/observer.spec.js index b7f59fedb0..a479b497ac 100644 --- a/test/unit/modules/observer/observer.spec.js +++ b/test/unit/modules/observer/observer.spec.js @@ -318,6 +318,7 @@ describe('Observer', () => { it('Cannot enable reactivity on a property that is already defined', done => { const vm = new Vue({ + template: '
', data: { person: { age: 32 From 54e64df0cc46ab404deaa583707f22f2f3660728 Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Thu, 10 May 2018 18:09:44 +0200 Subject: [PATCH 4/5] Update observer.spec.js --- test/unit/modules/observer/observer.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/modules/observer/observer.spec.js b/test/unit/modules/observer/observer.spec.js index a479b497ac..444fb34c3e 100644 --- a/test/unit/modules/observer/observer.spec.js +++ b/test/unit/modules/observer/observer.spec.js @@ -329,7 +329,7 @@ describe('Observer', () => { Vue.set(vm.person, 'job', 'Educator') waitForUpdate(() => { expect(`Cannot enable reactivity on a property that is already defined`).toHaveBeenWarned() - }).end(done) + }).then(done) }) it('warning set/delete on Vue instance root $data', done => { From 178724d0759eb2f0fc454ec7cfa9f54e8f565317 Mon Sep 17 00:00:00 2001 From: Eduardo San Martin Morote Date: Mon, 21 May 2018 19:11:05 +0200 Subject: [PATCH 5/5] refactor: use else if instead of if --- src/core/observer/index.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/observer/index.js b/src/core/observer/index.js index ab730dfe3f..71d5ebd85d 100644 --- a/src/core/observer/index.js +++ b/src/core/observer/index.js @@ -199,8 +199,7 @@ export function set (target: Array | Object, key: any, val: any): any { if (process.env.NODE_ENV !== 'production') { if (isUndef(target) || isPrimitive(target)) { warn(`Cannot set reactive property on undefined, null, or primitive value: ${(target: any)}`) - } - if (Object.getOwnPropertyDescriptor(target, key) && + } else if (Object.getOwnPropertyDescriptor(target, key) && (typeof (Object.getOwnPropertyDescriptor(target, key).get) === 'undefined') && !Array.isArray(target)) { warn(`Cannot enable reactivity on a property that is already defined: ${(key: any)}`)