Pass payload by value, not by reference

[Rkaede]

What problem does this feature solve?

Currently the payload passed to mutation functions is by reference, not value. This allows the mutation functions to directly update the payload data rather than the state object.

As an example see editTodo() in the todo demo:

editTodo (state, { todo, text = todo.text, done = todo.done }) {
  todo.text = text
  todo.done = done
}

There are 2 downsides to this.

The first is that the function is mutating the payload and not the state. It is not clear here what is being mutated. We now need to traverse all around the app to find out.

If the payload was instead passed by value then updating the value would not actually do anything. All mutation functions would need to update the state object. This would make vuex stores more explicit and understandable as well as ensure there is a clear boundary between it and the app.

The second is that Vuex plugins cannot log the mutations to a server and replay them back later. Here is a pseudocode example:

// replaying back mutation logs
let logs = await getLogsFromServer();
for (log in logs) {
  // any editTodo mutations passed in below will not work
  // as the mutation will update the "log" object instead of 
  // the actual todo in the state object
  this.store.commit(log.type, log.payload);
}

I'm working on a plugin that does this (similar to redux-vcr) and it would be great if there was a systematic guarantee that it would work with all Vuex stores.

What does the proposed API look like?

Example - No changes needed

mutations: {
  // current: n is passed by reference
  increment (state, n) { 
    state.count  = n
  }
}

mutations: {
  // proposed: n is passed by value
  increment (state, n) { 
    state.count  = n
  }
}

Example B - editTodo

mutations: {
  // current - can update payload
  editTodo (state, { todo, text = todo.text, done = todo.done }) {
    todo.text = text
    todo.done = done
  }
}

mutations: {
  // proposed - with pass by value we 
  // must update the state object
  editTodo (state, todo) {
     const index = state.todos.findIndex(t => t.id === todo.id)
     if (index > -1) state.todos.splice(index, 1, todo)
  }
}

[kiaking]

Interesting thoughts! Well, we allow this usage because we sometimes want to directly mutate the returned value from a getter, which is not available in a mutation. I agree that this usage is not clear and a bit confusing.

Personally, passing payload as a reference feels more natural in terms of how JS function works. Also, doing a deep copy of all payloads is just too much. If it's needed, plugin author can always do the deep copy of the payload.

However, I agree that we should update the example code to use the state instead of the passed todo object. Let's do that.

[kiaking]

And now the example are updated 👍 Will be closing this issue.