Vuex: Add options to mutation passed to store.subscribe()

[thebrubaker]

What problem does this feature solve?

It would enable plugin developers to check for additional, custom options on committing mutations to the store. One example would be a plugin that can cache commits into local storage if the option cache: true is passed on the commit. Another example would be a plugin for logging mutations if the option log: true is passed.

What does the proposed API look like?

This would enable the following scenarios:

store.subscribe((mutation, state) => {
  if (mutation.options.cache) {
    localStorage.setItem(mutation.type, JSON.stringify(mutation.payload));
  }
})
store.subscribe((mutation, state) => {
  if (mutation.options.log) {
    logger.push(mutation.type, mutation.payload)
  }
})

The change would be rather simple. Line 94 in vue/src/store.js currently looks like this: const mutation = { type, payload } and by changing line 94 to the following: const mutation = { type, payload, options } this feature would be enabled.

[ktsn]

Where would we set the option? Why isn't the payload enough for your use case?

[thebrubaker]

If I understand your question correctly, there is currently already the third argument in the commit function: commit(type, payload, options).

It's just not passed with the mutation object for doing store.subscribe()

[thebrubaker]

For the instance you describe where I would add it to the payload, what if the object payload also includes a value of cache: true that I'm going to be sending up to my API. Say there is an app setting to turn caching on. I would imagine the payload should be acting as data that is being passed around and set, and not as a descriptor for how a mutation might be handled.

store.commit('APP_SETTINGS', {
  name: 'Awesome App',
  timezone: 'UTC',
  cache: true
}, { cache: true })

In fact it reveals a weakness in the current commit() method where the first argument can be an object, and you can pass type as a property. That now excludes type from being a key on your payload.

[ktsn]

I'm not sure that it is a good idea because the 3rd argument of commit is meant to be used for commit itself. I feel it shouldn't be passed to the subscribe callback.

If any plugin can read the options argument, it can be broken/inconvenient for future update of Vuex. For example, some plugin expects a flag cache: true in the options, then the users set the flag on their apps. After that, what if Vuex include a new option cache: 'some string' for commit options?

I think passing plugin option in payload is OK because if there is a conflict with apps and plugin, the user can change their apps payload (and the user knows which name is reserved by plugins).

[thebrubaker]

Ah that is a fair point, and I understand if you don't proceed. But I disagree that this wouldn't be useful for different scenarios. Let's change the example. I'm creating a logger that is going to record all information about a mutation. Specifically if it's a root mutation and not a namespaced mutation, I want to show a little flag in my logger. What are the arguments for limiting my access to information that might potentially be useful?

[thebrubaker]

By the way, thanks for considering my request and discussing it so quickly!

[thebrubaker]

For now I'll follow your advice and supply it in the payload, but hopefully we can expand mutations a bit more in the future so developers can expand the power of Vuex with some cool plugins! I know state caching, and reseting default state are core features for any larger app, but these aren't built in. Trying to find a nice API way of handling these two scenarios without needing to be too verbose.

[thebrubaker]

store.subscribe(({ type, payload }, state) => {
  if (payload.cache) {
    const { cache, ...payloadWithoutCache } = payload
    localStorage.setItem(type, JSON.stringify(payloadWithoutCache))
  }
})

[thebrubaker]

I thought about it some more, and it definitely doesn't work well when the payload is an array. Really would be nice to have visibility on the entire mutation object when subscribing instead of hiding properties to protect the developer from making what you described as a bad decision.

[ktsn]

I wondar why the user cannot change their payload even though they know the plugin requires some options in the payload.

[thebrubaker]

If it were a new app, that might not be a big problem. But imagine the plugin is being installed in a very large, mature application. It would require quite a significant change across the codebase, in both the payload being sent and the action / mutation arguments. All array payloads now need to be transformed into objects, with a new property for data, and one for cache, and all actions and mutations need to be updated to incorporate this change, across the entire app.

If there was visibility on the options object, this could be a very quick and simple update. We don't have to modify payloads or actions / mutations.

Essentially it would be great to use options as a way to further describe the mutation (which is what it's being used for now. Is this a root mutation or a namespaced mutation?). It would be useful in a lot of scenarios. Do you want this mutation logged? Is this the type of mutation that should be cached? Should we perform some additional work (such as socket syncing) after the mutation?

 store.subscribe((mutation, state) => {
  if (mutation.options.sync) {
    // sync with socket
  }
})

 store.subscribe((mutation, state) => {
  if (mutation.options.log) {
    // log this mutation in dev tools
  }
})

 store.subscribe((mutation, state) => {
  if (mutation.options.cache) {
    // cache this mutation in local storage
  }
})

Even the docs currently suggest this kind of plugin, but we are limited by applying it to all mutations, or explicitly declaring every single mutation type that should be included / excluded by the plugin: https://vuex.vuejs.org/en/plugins.html#committing-mutations-inside-plugins.

The change would significantly improve our ability to write plugins. But I do see a rationale against this request. It would in fact limit vuex from being able to make a change to options without releasing it in a major version update. I guess it should be weighed against: is it realistic or probable that a lot of changes will be made to this object in the future and we will regret not being able to release it until the next major version change? Does this outweigh the benefits we provide for our plugin developers?

Still hoping this is considered because I could do some really interesting things with this, and it's a very minor update in the codebase.

Thanks again for your help!

[ktsn]

Thanks for the detailed explanation! That makes sense to me.
I think a point of compromise is adding a dedicated option in commit options instead of passing entire options to subscribed callback.

So it would look like:

// mutations object has a new property `meta`
store.subscribe((mutation, state) => {
  if (mutation.meta.cache) {
    // cache this mutation in local storage
  }
})

// the users can specify any data in `meta` option of `commit`
store.commit('mutationType', { ... }, {
  meta: { cache: true }
})

In this scenario, Vuex won't break plugin options and the users would easily utilize additional plugin options.

[thebrubaker]

That's a great idea!! Would it make sense for me to submit a PR at this
point?
On Fri, Nov 3, 2017 at 4:54 AM katashin [e-mail] wrote:

Thanks for the detailed explanation! That makes sense to me.
I think a point of compromise is adding a dedicated option in commit
options instead of passing entire options to subscribed callback.

So it would look like:

// mutations object has a new property metastore.subscribe((mutation, state) => {
if (mutation.meta.cache) {
// cache this mutation in local storage
}
})
// the users can specify any data in meta option of commitstore.commit('mutationType', { ... }, {
meta: { cache: true }
})

In this scenario, Vuex won't break plugin options and the users would
easily utilize additional plugin options.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/vuejs/vuex/issues/1035#issuecomment-341683579, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AF4T40HFVG7sthbo75AGADdZreK-RdBRks5syv8GgaJpZM4QNDS_
.

[ktsn]

Sure!

[thebrubaker]

Here is the PR: #1043

I wasn't sure which branch I'm supposed to pull into, or if I'm supposed to include build files. Is there a guide for submitting PRs?

[kiaking]

Will be closing the issue due to inactivity. Please feel free to open another issue if anyone think this is still useful 👍