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

Objects with an array as a prototype are not reactive #4049

Closed
ghost opened this issue Oct 27, 2016 · 11 comments
Closed

Objects with an array as a prototype are not reactive #4049

ghost opened this issue Oct 27, 2016 · 11 comments

Comments

@ghost
Copy link

ghost commented Oct 27, 2016

Vue.js version

2.0.3

Reproduction Link

https://jsfiddle.net/yMv7y/1860/

Steps to reproduce

  1. Press the buttons.

What is Expected?

Both counters increment.

What is actually happening?

Very simply, Vue reactivity works for objects that are [] but not objects that have [] as a prototype. This is happening in Vue 2.0.

FancyArray.prototype = [];

function FancyArray() { }

new Vue({
  data: {
    normalArray: [],
    objectWithArrayPrototype: new FancyArray()
  },
  watch: {
    normalArray: function(a) {
      console.log("normal array triggered", a);
    },
    objectWithArrayPrototype: function(a) { // THIS WATCH IS NEVER TRIGGERED
      console.log("object with array prototype triggered", a)
    }
  },
  created: function() {
    this.normalArray.push("hey!"); // -> prints "normal array triggered, hey!"
    this.objectWithArrayPrototype.push("hello again!"); // -> prints nothing
  }
}).$mount('#app');

While this problem is easily circumvented by assigning a property in the object to be an array, it would be nice to be able to use objects that have an array as a prototype.

@ghost ghost changed the title Objects with an array as a prototype are not reactive Objects with an array as a prototype are not reactive (fails to trigger a watch, renders in the template) Oct 27, 2016
@ghost ghost changed the title Objects with an array as a prototype are not reactive (fails to trigger a watch, renders in the template) Objects with an array as a prototype are not reactive Oct 27, 2016
@ghost
Copy link
Author

ghost commented Oct 27, 2016

I looked through the source code and I see now why this is happening. Basically, the code uses Array.isArray(value), which is false for objects that are not an array. However this is easily solved by evaluating Array.isArray(value.__proto__) as well. It's a tough call.

if (Array.isArray(value)) {
const augment = hasProto
? protoAugment
: copyAugment
augment(value, arrayMethods, arrayKeys)
this.observeArray(value)
} else {
this.walk(value)
}

@HerringtonDarkholme
Copy link
Member

HerringtonDarkholme commented Oct 28, 2016

Please read the doc. http://vuejs.org/api/#data

The object must be plain: native objects such as browser API objects and prototype properties are ignored.

I just think data is recursively watched so every nested object should also be plain object.

@phanan
Copy link
Member

phanan commented Oct 28, 2016

@lgdexter Array.isArray(value.__proto__) works, though the whole picture might be a bit more complex than this. Pinging @yyx990803 for decision.
@HerringtonDarkholme I believe this is a different issue. The object itself is till plain in this case.

@ghost
Copy link
Author

ghost commented Oct 28, 2016

@HerringtonDarkholme has a point. This isn't a purely plain object; it's using an array as a prototype. What a shame. Might be nice to add this in as a feature in the future.

Let me know what you guys decide @phanan @yyx990803

@phanan
Copy link
Member

phanan commented Oct 29, 2016

The thing is isPlainObject() still determines it to be one.

@sylvainpolletvillard
Copy link

I got a similar issue on another project and replaced Array.isArray by instanceof Array.

@ghost
Copy link
Author

ghost commented Oct 30, 2016

I basically replaced the object with [] and now I'm using FancyArray.prototype.someMethod.call everywhere. It's not ideal. Thanks for the clarification @phanan.

Would instanceof Array make all tests pass?

@ClickerMonkey
Copy link

ClickerMonkey commented Oct 30, 2016

I'm working on adding an extension to a library of mine and I ran into this problem as well. My first guess was to also just override Array.isArray to use instanceof but there were still errors (VueJS replaces the extended Array with a plain array - loosing your original object). I ended up doing something similar to what VueJS already does with arrays - decorate the array functions with new functions that call this.__ob__.observeArray when needed as well as this.__ob__.notify() (if this.__ob__ was defined). My collections trigger events, so this was my solution. It could similarly be applied to anyone else's extended Array.

var $trigger = Rekord.Collection.prototype.trigger
Rekord.Collection.prototype.trigger = function (ev, args) {
  const result = $trigger.apply(this, arguments)
  const ob = this.__ob__
  if (ob) {
    switch (ev) {
      case 'add':
        ob.observeArray([args[0]])
        break
      case 'adds':
        ob.observeArray(args[0])
        break
      case 'reset':
        ob.observeArray(this.slice())
        break
    }
    ob.dep.notify()
  }
  return result
}

@HerringtonDarkholme
Copy link
Member

IMHO, the best two alternatives are:

  1. Only support native array as collection: this is what Vue currently does.
  2. Roll out a full schema for watching collections: Set, Map, Observable and user defined collections

@ClickerMonkey
Copy link

ClickerMonkey commented Oct 30, 2016

There's also another gotcha - v-for uses Array.isArray as well, so if you have your own Array class it will be passed through the Object iteration which uses Object.keys - so you need to make sure you have functions & class-level variables defined to NOT be enumerable. An inefficient work-around for this is to use slice() like so: v-for="item in arraylike.slice()"

@yyx990803
Copy link
Member

The objects you are creating are in fact array-like objects and not real arrays. Vue only supports native Arrays or arrays that are real, proper sub-class of Array (only available in ES2015+), which can pass the Array.isArray check. This is an assumption made in multiple locations of the entire framework, adapting to it involves a lot of complexity for little gain - and is unlikely to be supported.

Also - custom array-like objects likely have far worse performance than real arrays, and I personally recommend against that pattern.

ashtonmeuser pushed a commit to ashtonmeuser/vue-cloudwatch-dashboard that referenced this issue Oct 10, 2018
Honestly don't know why Array subclass worked before. Refer to vuejs/vue#4049, vuejs/vue#6943, vuejs/vue#5893.
ashtonmeuser added a commit to ashtonmeuser/vue-cloudwatch-dashboard that referenced this issue Oct 10, 2018
Honestly don't know why Array subclass worked before. Refer to vuejs/vue#4049, vuejs/vue#6943, vuejs/vue#5893.
happyman125 added a commit to happyman125/Vue_Dashboard that referenced this issue Jan 28, 2020
Honestly don't know why Array subclass worked before. Refer to vuejs/vue#4049, vuejs/vue#6943, vuejs/vue#5893.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants