Vuex: Wonder why not giving access to getters inside mutations

[AlexandreBonaventure]

Hi,
Just a discussion I open because it's been a long time wondering why not giving access to getters inside mutations.
My usecase, given a module:

export default {
  state: { jobs: [
    { id: 506, position: 1, name: 'Botanist'},
    { id: 104, position: 2, name: 'CEO'},
  ] },
  mutations: {
    JOB_MODIFY: (state, { id, name }) => state.find(state.jobs.find( job => job.id === id)).name = name,
  },
  getters: {
    jobs: (state) => state.jobs,
    jobsByIds: (state) => keyBy(state.jobs, 'id'),  //lodash
  },
}

With access to getters I could use jobsByIds (which I'm using elsewhere anyway), which not only is easier to write, but also more perfomant cause I don't run another loop trying to find the right id to mutate.

export default {
  state: { jobs: [
    { id: 506, position: 1, name: 'Botanist'},
    { id: 104, position: 2, name: 'CEO'},
  ] },
  mutations: {
    JOB_MODIFY: ( {state, getters: { jobsByIds }}, { id, name }) => jobsByIds[id].name = name,
  },
  getters: {
    jobs: (state) => state.jobs,
    jobsByIds: (state) => keyBy(state.jobs, 'id'),  //lodash
  },
}

I think mutations were designed with the blueprint of redux reducers, so I understand they should be dumb, pure functions, decoupled from the store (am I wrong ?). But it could be a real plus to have this in mutations since it could be easily reproduce with this kind of implementation:

const makeAction = (type) => {
  return ({ commit, getters }, payload) => commit(type, { payload, getters })
}

export default {
  state: { jobs: [
    { id: 506, position: 1, name: 'Botanist'},
    { id: 104, position: 2, name: 'CEO'},
  ] },
  actions: {
      deleteJob: makeAction(DELETE_JOB),
  },
  mutations: {
    DELETE_JOB(state, { getters, payload: id }) {
      const jobsByIds = getters.jobsByIds //cant deconstruct because its not enumerable 
      set(jobsByIds[id], '_destroy', true) //set stands as vue sets
    },
  },
  getters: {
    jobs: (state) => state.jobs,
    jobsByIds: (state) => keyBy(state.jobs, 'id'),  //lodash
  },
}

However, I feel I'm "hacking" a bit vuex. Is there any danger I should be aware of using this technique ?

I felt the need to discuss about this. So please bring me some lights if I'm misleaded.
Thanks

[ktsn]

I think using getters in mutations is a bit overkill because they are not only for querying state but also for deriving computed value. In addition, the mutation will depends on getters if we passing getters object directly. IMO, it should be avoided because if we change some getters in the future, we need to consider about its effects in mutations.

Instead, I encourage you to pass the queried state itself returned by a getter.

actions: {
  deleteJob ({ commit, getters }, id) {
    commit(DELETE_JOB, { targetJob: getters.jobsByIds[id] })
  }
},

mutations: {
  [DELETE_JOB] (state, { targetJob }) {
    set(targetJob, '_destroy', true)
  }
}

Now, the mutation does not depend on getters but we can use the getter logic in actions.

[LinusBorg]

Hi there!

I close this issue because of inactivity, and because a valid alternative to the requested fdeature was provided.

If you want to follow up on this, please comment here and we can re-open the issue.

[gggllm]

I think it is a good idea to add such an access. for example, i got a huge array as a state, and i will always use a getter to generate a map for hash-finding. However when i was trying to modify one specific item, i have no access to getters! using actions is a good idea too

[LinusBorg]

It's a better idea to save the map in state, it's an established pattern in redux etc. as well:

itemIds: [1,2,3, ...],
items: {
  1: {. .. },
  2: {. .. }
  3: {. .. }
}

[gggllm]

I have realized the problem here. thx! now I think i should write some manually data binding code to ensure better performance.

[erikweebly]

Here's how I did it:

import { isNewRecord } from './getters';

const mutations = {
  setDetails (state, details) {
    if (isNewRecord(state)) {
      state.detailsOnServer = null;
    } else {
      state.detailsOnServer = details;
    }
    state.details = clone(details);
  }
}

... I'm not sure if it's dangerous to import the getters from the mutations. 🤷‍♂️

I could pull the code out into a separate helper file and include _that_ from both getters and mutations, but this seemed clean enough, and I don't see why it should be risky.

[alsii]

@ktsn, I think this

actions: {
  deleteJob ({ commit, getters }, id) {
    commit(DELETE_JOB, { targetJob: getters.jobsByIds[id] })
  }
},

mutations: {
  [DELETE_JOB] (state, { targetJob }) {
    set(targetJob, '_destroy', true)
  }
}

could not be considered as a good solution. Mutations should mutate the state. DELETE_JOB mutation doesen't even touch the state. Really it's the same thing as trying to mutate the state immediately in an action.

[LinusBorg]

It touches the state in the sense that the object that it's mutating is the same as in the store - it cam from a getter.

Making getters accessible in the mutation and using it to get that objet in the mutation would also mean that it "doesn't even touch the state" in the sense that you used it - it used the getter, not state, right?

[alsii]

Nope. IMHO, mutations should read data from the state [with getters] and write mutated data to the state.

Explicit is better than implicit.

The code of this mutation is itself definitely _not clear_. You can not understand, whether targetJob is a part of state, or not.

The code of this mutation is definitely _not safe_. The mutation named as DELETE_JOB, but it can potentially mutate any other part of the state, and even any data not related to the state at all.

According to this code sample, mutations do not need any access to the state, as they can mutate object(s) given from actions. Logically there are two options:

• provide access to getters
• remove access to store at all

[dagadbm]

so this is a dead end zone i guess??
it makes no sense to not provide getters access.
If I want to isolate some logic on a getter so I dont need to repeat it everywhere

[pthimothee]

I agree with @dagadbm ,
it forces us to do some mental gymnastics I would rather avoid. It also makes the code less readable as it breaks with some very simple pattern: Calling a sync function from another sync function both being part of the same "class/context".

[namero999]

Agree, not being able to reference getters produces all sorts of bad effects:

1. having to bounce through actions for trivial, synchronous tasks
2. accessing getters via named properties, making most IDE wrongly mark getters methods as unused
3. having to come up with worse hacks than simply having an optional getters reference passed in mutations

Currently, the most "painless" way to do so is probably this https://stackoverflow.com/a/43259220/1382435

[zaro]

I don't know if it makes sense to comment on closed issue, but still..

I think of the getters/mutations similar to the getters/setters in JS objects.

It's perfectly valid to use the getter in a setter, why it would be different here ?