vuejs / vuex

🗃️ Centralized State Management for Vue.js.
https://vuex.vuejs.org
MIT License
28.42k stars 9.57k forks source link

Immutability #802

Closed rchrdnsh closed 7 years ago

rchrdnsh commented 7 years ago

What problem does this feature solve?

VueX is mutable by default, but I would appreciate if the Vue community would consider adding the option for immutability in VueX as well. Maybe it would require making a separate library, similar to ReVue, or other 3rd party libraries, but I think it would be a great addition to the officially supported Vue library(module) system.

My use case is that I am trying to build a music production application with unlimited history. This means That an immutable data structure, like a trie, with shared state between snapshots of state would allow for the creation of a history panel that would allow the user to move through their project history by literally sliding along a line with a node, and see their history change backward and forward in real time. I don't think that VueX can accomplish this at the moment, but I could be incorrect.

In any case, it would be awesome if the Vue team would consider this feature to be added to VueX or made into a new library that is complimentary to Vue and VueX.

Thank you for your consideration.

What does the proposed API look like?

I don't have a proposed API as I am not sure how this would work. I would imagine something similar to Redux, or Elm, or Clojure and ClojureScript, but I would want to leave this up to your team :-)

posva commented 7 years ago

you should be able to accomplish that by storing snapshots. Vue is based on mutability, so this is very unlikely to happen. But I encourage you to embrace mutability as Vue does 🙂

ktsn commented 7 years ago

You may want some mutation enhancer to treat the store state as immutable object. https://github.com/ktsn/vuex-reducer

MVSICA-FICTA commented 7 years ago

Having the ability to change the mode of a store module to be immutable looks like a useful thing. This would be important when working with state that might not be directly tied to the DOM (which the vue devtools can manage) but rather it could be used to play sound or render to a canvas element (just two use cases among many).

So, yes there are valid situations where immutable state is required and an app would want to create their own state snapshot reloader and time-travel system. The following videos explore this further:

https://www.youtube.com/watch?v=-fPyfSAEZgk https://www.youtube.com/watch?v=y_kNdZ1_Rsk

LinusBorg commented 7 years ago

Can you explain in a few words why that requires immutability? I don't think I can watch 90 minutes of video footage to find out.

chrisnicola commented 7 years ago

@posva the problem isn't simply that Vuex is mutating state but that it is also passing it as an argument and then asking that it be mutated, which can be problematic in JS.

https://spin.atomicobject.com/2011/04/10/javascript-don-t-reassign-your-function-arguments/

For those of us that use ESLint to watch for these sorts of violations this forces us to add exceptions to Vuex stores. Even if it is just a matter of API design it would help if mutations allowed the return value to become the new state instead of simply mutating the state in place.

Also from a clean code perspective mutating the value of arguments is considered bad practice. I worry about encouraging this sort of design in practice. Objects shouldn't mutate state that doesn't belong to them. As an alternative (if we're embracing mutation) this could be hijacked:

const store = new Vuex.Store({
  state: {
    count: 1
  },
  mutations: {
    increment() {
      // mutate state
      this.state.count++
    }
  }
})
LinusBorg commented 7 years ago

In Vuex, we are not reassigning arguments. we are mutating the object referenced by an argument. So this is totally different from the problem described in the linked article, that risk simply does not exist because in vuex, it never makes sense to do state = 'something new'

And concerning eslint - that's just because of your configuration. The default would even allow what we currently do:

http://eslint.org/docs/rules/no-param-reassign#props

chrisnicola commented 7 years ago

@LinusBorg thanks you are right I misinterpreted the problem a bit and I agree nothing untoward is happening inside the Vuex store. The state object does in fact belong to the store so it seems perfectly fine for it to mutate it's own state.

However, I still understand the case for requiring immutable arguments. I have seen problems in the past with libraries that expose public methods that mutate the state of objects passed to them and this almost always creates problems. While Vuex isn't doing this, I also see the value within certain codebases for making immutable arguments a standard practice.

For our team, applying common sense in our code reviews will be sufficient. For other teams that want to enforce immutable arguments everywhere the vuex-reducer project @ktsn created looks like a perfect compromise.

andreiglingeanu commented 7 years ago

Have built something like that, by just saving snapshots of the whole state in memory, just by subscribing on the store's mutation commits. Basically, it was a time-travel feature, it was very elegant because I was relying on the mutations commit event as a point in time when I had to create a new snapshot. It was easy to achieve in userland, without the need to enforce immutability, vuex-reducer looks interesting, though.

LinusBorg commented 7 years ago

I will close this issue now for the following reasons:

So Immutability will not be implemented in vuex.

rchrdnsh commented 7 years ago

Could Vuex snapshots handle thousands of snapshots as a way of time travel through a creative project? The idea is to be able to 'scrub' though every single change that has been made over the course of an entire project, including variances in production, and hold them together in one store simultaneously. So I suppose that would require a snapshot for every single change throughout the life of a project. Is that advisable or even possible using Vuex? Or would something like Revue or even something as yet uninvented be more suited to the task?

LinusBorg commented 7 years ago

When it comes to a timetravel-like feature that is done by saving snapshots of the whole state after every mutation, the only difference between vuex and something like redux is that for each step, the whole state has to be made reactive again.

That can have performance implications if you deal with a lot of data (thousands of objects).

On the other hand, you could try and set up the whole system with a do and undo version of each mutation, which would only require to save the changes rather than the whole state. In that scenario, vuex' mutation feature would probably be cheaper than cloning the whole state on each change.

But that's something that should be discussed in another place, like the forum.

aprilmintacpineda commented 6 years ago

I'm actually experiencing difficulty with this now using a template that uses eslint air-bnb You can find the issue here -> https://github.com/vuejs-templates/pwa/issues/189

Do @mention me for any response please...

rotavele commented 4 years ago

It just feels a little weird to change the store in a mutable fashion after using Elm for a few projects.

khaledosman commented 4 years ago

the other day I ran into an issue where I needed to run a side-effect in my action and I found this very confusing having used both react's react-redux and angular's ngrx implementations which were more obsessed with function purity and immutability, pretty confusing that a function parameter is mutated / updates its value half-way through the function

function myAction({ commit}, state) {
  console.log('state before', state)
  commit('do_something', payload)
  console.log('state after', state)
  const newState = {...state, ...payload}
  //oldState = state
  runSideEffect(newState, oldState)
}

the state param was only a reference that is already updated after commit and state before was different from state after, so what I thought was the oldState was actually the same as the newState, I had to do something like this instead

function myAction({ commit}, state) {
  const oldState = {...state}
  commit('do_something', payload)
  runSideEffect(state, oldState)
}