Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vue doesn't respect existing setters on Modelclasses #6845

Closed
kaipaysen opened this issue Oct 18, 2017 · 8 comments
Closed

Vue doesn't respect existing setters on Modelclasses #6845

kaipaysen opened this issue Oct 18, 2017 · 8 comments

Comments

@kaipaysen
Copy link

kaipaysen commented Oct 18, 2017

Version

2.5.0

Reproduction link

https://codesandbox.io/s/vjojnn5w7l

Steps to reproduce

  1. Run the attached Sample
  2. enter a Value for _bar
  3. see that bar is changed as well
  4. enter a value for bar
  5. see that bar is now detached from _bar

What is expected?

The existing setter "bar" should be used.

What is actually happening?

A new property "bar" is created.

@phanan
Copy link
Member

phanan commented Oct 18, 2017

data object must be plain.

@kaipaysen
Copy link
Author

Well, seems I've missed that bit of the documentation. Good point phanan.
Would be great to have Vue throw an errormessage in devtools when using a non plain data object.

@kaipaysen
Copy link
Author

kaipaysen commented Oct 19, 2017

IMHO it's a bad idea to restrict data to plain objects. Working with components and props validation I'm using Modelclasses as type parameter like this:

props: { state: { type: ValidationState, required: false, default: function() { return new ValidationState(); } } }

This wouldn't work without typed models.

@code-elf
Copy link

code-elf commented Oct 21, 2017

Just to confirm: Does the addition of the regression label mean this is on track to be fixed? I'm asking whether I should revert to vue@2.4.x for now and wait for a fix, or attempt to restructure code.
Specifically, I have a class that's being used as a reactive object, and it's using a custom setter on a property for side effects. This used to work up to vue@2.4.x.

@kaipaysen
Copy link
Author

Might be related to #5932.

@Kingwl
Copy link
Member

Kingwl commented Oct 26, 2017

I prefer to revert this feature
Because the properties of the class declare is located on the prototype of its instance
If we want to check its prototype, we may check the entire prototype chain
The side effect is too big for the benefits it brings

erweixin pushed a commit to erweixin/vue that referenced this issue Dec 15, 2017
lovelope pushed a commit to lovelope/vue that referenced this issue Feb 1, 2018
@drdevelop
Copy link

in加! in 原型判断与hasOwnProperty判断属性的区别是啥

@wangzongxu
Copy link
Contributor

class Model {
	constructor() {
		this.foo = '123'
		this._bar = null
	}
	get bar() {
		return this._bar;
	}
	set bar(newvalue) {
		this._bar = newvalue;
	}
}

new Vue({
	data: function() {
		return {
			data: new Model() 
		}
	}
  })

上面data字段初始化时是Model类的实例:

{
    foo: '123',
    _bar: null,
    __proto__: {
        get bar: function() {
            return this._bar
        },
        set bar: function(val) {
            return this._bar = val
        },
    }
}

注意bar并不是实例的私有属性;

由于v-model使用的Vue.set方法;
如果使用hasOwnProperty判断的话,这个判断不成立;

会走到下边代码:

  defineReactive(ob.value, key, val)
  ob.dep.notify()

这段代码相当于为实例重新定义了一个私有属性bar,这是不符合预期的,因为用户是把bar写在原型上了;

所以要把判断条件hasOwnProperty改为in判断:

  if (key in target && !(key in Object.prototype)) {
    target[key] = val
    return val
  }

上边条件成立;
相当于对Model原型上的bar赋值,这样跑起来就正常了

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants