From 47f24430cb98faa004f495f04ef6ea7895ac0050 Mon Sep 17 00:00:00 2001 From: Evan You Date: Tue, 31 May 2016 13:31:29 -0400 Subject: [PATCH] also warn set/delete on instance root $data --- src/core/instance/lifecycle.js | 4 +- src/core/instance/state.js | 8 ++- src/core/observer/index.js | 59 ++++--------------- .../instance/methods-lifecycle.spec.js | 2 +- test/unit/modules/observer/observer.spec.js | 26 +++++++- 5 files changed, 42 insertions(+), 57 deletions(-) diff --git a/src/core/instance/lifecycle.js b/src/core/instance/lifecycle.js index 45c5921b..5a67edac 100644 --- a/src/core/instance/lifecycle.js +++ b/src/core/instance/lifecycle.js @@ -122,7 +122,7 @@ export function lifecycleMixin (Vue: Class) { } if (vm._watchers.length) { for (let i = 0; i < vm._watchers.length; i++) { - vm._watchers[i].update() + vm._watchers[i].update(true /* shallow */) } } } @@ -150,7 +150,7 @@ export function lifecycleMixin (Vue: Class) { // remove reference from data ob // frozen object may not have observer. if (vm._data.__ob__) { - vm._data.__ob__.removeVm(vm) + vm._data.__ob__.vmCount-- } // call the last hook... vm._isDestroyed = true diff --git a/src/core/instance/state.js b/src/core/instance/state.js index c6f99ad3..7d349ebe 100644 --- a/src/core/instance/state.js +++ b/src/core/instance/state.js @@ -62,7 +62,8 @@ function initData (vm: Component) { proxy(vm, keys[i]) } // observe data - observe(data, vm) + observe(data) + data.__ob__.vmCount++ } const computedSharedDefinition = { @@ -205,7 +206,8 @@ function setData (vm: Component, newData: Object) { proxy(vm, key) } } - oldData.__ob__.removeVm(vm) - observe(newData, vm) + oldData.__ob__.vmCount-- + observe(newData) + newData.__ob__.vmCount++ vm.$forceUpdate() } diff --git a/src/core/observer/index.js b/src/core/observer/index.js index cf1a5474..ecc69c86 100644 --- a/src/core/observer/index.js +++ b/src/core/observer/index.js @@ -5,7 +5,6 @@ import Dep from './dep' import { arrayMethods } from './array' import { def, - remove, isObject, isPlainObject, hasProto, @@ -35,12 +34,12 @@ export const observerState = { export class Observer { value: any; dep: Dep; - vms: ?Array; + vmCount: number; // number of vms that has this object as root $data constructor (value: any) { this.value = value this.dep = new Dep() - this.vms = null + this.vmCount = 0 def(value, '__ob__', this) if (Array.isArray(value)) { const augment = hasProto @@ -73,24 +72,6 @@ export class Observer { observe(items[i]) } } - - /** - * Add an owner vm, so that when $set/$delete mutations - * happen we can notify owner vms to proxy the keys and - * digest the watchers. This is only called when the object - * is observed as an instance's root $data. - */ - addVm (vm: Component) { - (this.vms || (this.vms = [])).push(vm) - } - - /** - * Remove an owner vm. This is called when the object is - * swapped out as an instance's $data object. - */ - removeVm (vm: Component) { - remove(this.vms, vm) - } } // helpers @@ -123,7 +104,7 @@ function copyAugment (target: Object, src: Object, keys: Array) { * returns the new observer if successfully observed, * or the existing observer if the value already has one. */ -export function observe (value: any, vm?: Component): Observer | void { +export function observe (value: any): Observer | void { if (!isObject(value)) { return } @@ -139,9 +120,6 @@ export function observe (value: any, vm?: Component): Observer | void { ) { ob = new Observer(value) } - if (ob && vm) { - ob.addVm(vm) - } return ob } @@ -210,28 +188,20 @@ export function set (obj: Array | Object, key: any, val: any) { obj[key] = val return } - if (obj._isVue) { + const ob = obj.__ob__ + if (obj._isVue || (ob && ob.vmCount)) { process.env.NODE_ENV !== 'production' && warn( - 'Do not add reactive properties to a Vue instance at runtime - ' + - 'delcare it upfront in the data option.' + 'Avoid adding reactive properties to a Vue instance or its root $data ' + + 'at runtime - delcare it upfront in the data option.' ) return } - const ob = obj.__ob__ if (!ob) { obj[key] = val return } defineReactive(ob.value, key, val) ob.dep.notify() - if (ob.vms) { - let i = ob.vms.length - while (i--) { - const vm = ob.vms[i] - proxy(vm, key) - vm.$forceUpdate() - } - } return val } @@ -239,9 +209,11 @@ export function set (obj: Array | Object, key: any, val: any) { * Delete a property and trigger change if necessary. */ export function del (obj: Object, key: string) { - if (obj._isVue) { + const ob = obj.__ob__ + if (obj._isVue || (ob && ob.vmCount)) { process.env.NODE_ENV !== 'production' && warn( - 'Do not delete properties on a Vue instance - just set it to null.' + 'Avoid deleting properties on a Vue instance or its root $data ' + + '- just set it to null.' ) return } @@ -249,19 +221,10 @@ export function del (obj: Object, key: string) { return } delete obj[key] - const ob = obj.__ob__ if (!ob) { return } ob.dep.notify() - if (ob.vms) { - let i = ob.vms.length - while (i--) { - const vm = ob.vms[i] - unproxy(vm, key) - vm.$forceUpdate() - } - } } export function proxy (vm: Component, key: string) { diff --git a/test/unit/features/instance/methods-lifecycle.spec.js b/test/unit/features/instance/methods-lifecycle.spec.js index 26cdc26a..07a687fb 100644 --- a/test/unit/features/instance/methods-lifecycle.spec.js +++ b/test/unit/features/instance/methods-lifecycle.spec.js @@ -60,7 +60,7 @@ describe('Instance methods lifecycle', () => { it('remove self from data observer', () => { const vm = new Vue({ data: { a: 1 }}) vm.$destroy() - expect(vm.$data.__ob__.vms.length).toBe(0) + expect(vm.$data.__ob__.vmCount).toBe(0) }) it('avoid duplicate calls', () => { diff --git a/test/unit/modules/observer/observer.spec.js b/test/unit/modules/observer/observer.spec.js index 186a352d..7d83d665 100644 --- a/test/unit/modules/observer/observer.spec.js +++ b/test/unit/modules/observer/observer.spec.js @@ -290,13 +290,33 @@ describe('Observer', () => { Vue.set(vm, 'a', 2) waitForUpdate(() => { expect(vm.$el.outerHTML).toBe('
2
') - expect('Do not add reactive properties to a Vue instance').not.toHaveBeenWarned() + expect('Avoid adding reactive properties to a Vue instance').not.toHaveBeenWarned() Vue.delete(vm, 'a') }).then(() => { - expect('Do not delete properties on a Vue instance').toHaveBeenWarned() + expect('Avoid deleting properties on a Vue instance').toHaveBeenWarned() expect(vm.$el.outerHTML).toBe('
2
') Vue.set(vm, 'b', 123) - expect('Do not add reactive properties to a Vue instance').toHaveBeenWarned() + expect('Avoid adding reactive properties to a Vue instance').toHaveBeenWarned() + }).then(done) + }) + + it('warning set/delete on Vue instance root $data', done => { + const data = { a: 1 } + const vm = new Vue({ + template: '
{{a}}
', + data + }).$mount() + expect(vm.$el.outerHTML).toBe('
1
') + Vue.set(data, 'a', 2) + waitForUpdate(() => { + expect(vm.$el.outerHTML).toBe('
2
') + expect('Avoid adding reactive properties to a Vue instance').not.toHaveBeenWarned() + Vue.delete(data, 'a') + }).then(() => { + expect('Avoid deleting properties on a Vue instance').toHaveBeenWarned() + expect(vm.$el.outerHTML).toBe('
2
') + Vue.set(data, 'b', 123) + expect('Avoid adding reactive properties to a Vue instance').toHaveBeenWarned() }).then(done) })