Vue: add reactive array swap

Created on 4 Dec 2016  ·  10Comments  ·  Source: vuejs/vue

I am writing a simple todo list component with:

Vue.component('todo-list', {
    props: ['todos'],
    template:
    `<ol>
        <li v-for="(td, ix) in todos">
            {{td}}
            <button title="delete"  @click="todos.splice(ix,1)">X</button>
            <button title="move up" @click="todos.swap(ix,ix-1)" v-show="ix>0">^</button>
        </li>
    </ol>`,
})

For this to work I need (in core/observer/array.js) something like:

Array.prototype.swap = function(x,y) {
  var t = this[x]
  this[x] = this[y]
  this[y] = t
}
const arrayProto = Array.prototype
export const arrayMethods = Object.create(arrayProto)

/**
 * Intercept mutating methods and emit events
 */
;[
  'push',
  'pop',
  'shift',
  'unshift',
  'splice',
  'swap', // added
  'sort',
  'reverse'
]

It works when I (lazily) fixed dist/vue.js, but did not run the tests.
I think it is worth to add swap this way because notify() is run just once, am I wrong?

feature request

Most helpful comment

Why not just write

Vue.swap = function(arr, x, y) {
   var origin = arr[x]
   arr[x] = arr[y]
   Vue.set(arr, y, origin)
}

https://jsfiddle.net/L92w2372/2/

updated for notification only once

All 10 comments

Hi @jcfg,

thanks for your suggested feature. So far, we have refrained from polluting Prototypes with custom methods - we had some in Vue 1.0 (Array.prototype.$set()) and explicitly removed them in 2.0.

I don't think we should/ want to start adding that type of code to the framwork again unless the advantage it provides is extraordinary.

So far, I don't think that's the case for your proposed .swap() method.

How about adding Vue.swap(), like Vue.set ?

Why not just write

Vue.swap = function(arr, x, y) {
   var origin = arr[x]
   arr[x] = arr[y]
   Vue.set(arr, y, origin)
}

https://jsfiddle.net/L92w2372/2/

updated for notification only once

You are doing dependency notification "twice", 1st at splice, 2nd at Vue.set, am I right?
With adding Array.prototype.swap like above, or adding Vue.swap, we will do notifications just once.

You are doing dependency notification "twice", 1st at splice, 2nd at Vue.set, am I right?

Yes, .notify() is called twice - but neither does that have any performance implications, nor is is calling any watcher's run() twice.

Why? The update that is triggered by .notify() is pushed to a scheduler, which is working asynchronously and will [skip any duplicate entries].(https://github.com/vuejs/vue/blob/dev/src/core/observer/scheduler.js#L86).

Ok, still I think it is worth to have a reactive swap in Vue. None of these are trivial to normal users.

Vue.swap also needs to do a single notify(), like the above corrected comment, since Vue developers know Vue better than normal users ;)

Ok, still I think it is worth to have a reactive swap in Vue. None of these are trivial to normal users.

I don't think that the three lines of code by @HerringtonDarkholme should be non-trivial to any dev who knows his JS enough to work with Vue successfully otherwise.

We try to keep the API surface of Vue lean. We could think of dozens of small convenvience functions like yours, but that would just inflate the API unnessessarily.

Quick question: notifications are for the whole todo list right? Single elements of the list do not have notifications of their own.

If they did, Vue.swap above would miss notifications on arr[x].

For example can we have something like:
data: {
msg: {x:1, y:2}, // reactive var
todos: [ msg, "td1", ..] // reactive list
}

We could think of dozens of small convenvience functions like yours, but that would just inflate the API unnessessarily.

Therefor one could aggregate helpers like this in a plugin and have the core clean.

Quick question: notifications are for the whole todo list right? Single elements of the list do not have notifications of their own.

In this case, they are., as we are changing an array.

Therefor one could aggregate helpers like this in a plugin and have the core clean.

Contreibution welcome.

I will close this issue now, since I think we have come to an understanding that this won't be added to core.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

guan6 picture guan6  ·  3Comments

julianxhokaxhiu picture julianxhokaxhiu  ·  3Comments

lmnsg picture lmnsg  ·  3Comments

bdedardel picture bdedardel  ·  3Comments

robertleeplummerjr picture robertleeplummerjr  ·  3Comments