Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Helper functions to subscribe a component to the commits/actions stream #1690

Open
asoglovo opened this issue Mar 4, 2020 · 6 comments
Open

Helper functions to subscribe a component to the commits/actions stream #1690

asoglovo opened this issue Mar 4, 2020 · 6 comments
Labels

Comments

@asoglovo
Copy link

@asoglovo asoglovo commented Mar 4, 2020

What problem does this feature solve?

Add a "mapping" layer from the mutations/actions stream to the components that want to subscribe to them.

Similarly to the existing mapActions or mapGetters functions which decouple the Vuex store from the components using it, a new pair of functions could exist to allow the components get subscribed to the dispatched actions or committed mutations in a way such that direct usage of the store is not necessary.

What does the proposed API look like?

In the components, instead of directly accessing $store to subscribe to the actions stream:

created() {
  this.unsubscribe = this.$store.subscribeAction((action, state) => {
    if (action.type === 'MY_ACTION_TYPE') { ... }
  })
},

destroyed() {
  this.unsubscribe()
}

We'd have something like:

created() {
  this.unsubscribe = subscribeAction('MY_ACTION_TYPE', (payload, state) => { ... })
},

destroyed() {
  this.unsubscribe()
}

This function would not only decouple the usage of the store from the components, but also provide a filter to only respond to the given action type. Note that, since subscribing to actions is not a widespread idiom with Vuex, we could go without a plural version, such as subscribeActions.

A similar function, subscribeMutation could do the same for mutations.

@kiaking kiaking added the proposal label Mar 8, 2020
@kiaking

This comment has been minimized.

Copy link
Member

@kiaking kiaking commented Mar 8, 2020

I think this is a simple syntax sugar that you want to add to the subscribe method, where you want action/mutation type to be able to pass as the 1st argument. Though, not so sure how helpful it is.

For subscribeAction, we have before, after hooks as well, and for the proposed API, we might need to have subscribeBeforeAction and subscribeAfterAction as well. It kinda makes api bit confusing, in my opinion.

And you could create this syntax sugar your self pretty straight forward, while I don't think it adds as much as mapXXX helpers do.

I would like to have feedback on this proposal.

@asoglovo

This comment has been minimized.

Copy link
Author

@asoglovo asoglovo commented Mar 10, 2020

It indeed looks like syntactic sugar to avoid repeating the action filtering, but the main thing we want to accomplish here is a greater decoupling between the Vuex actions/mutations stream and the component itself. By providing this new layer, components can avoid using the $store directly, and therefore be less dependent on the store itself.

In the case of the before and after, the API could be similar to the current subscribeAction:

subscribeAction('MY_ACTION_TYPE', {
   before:  (payload, stateAfter) => { ... },
  after: (payload, stateBefore) => { ... }
)
@kiaking

This comment has been minimized.

Copy link
Member

@kiaking kiaking commented Mar 11, 2020

Thank you for the clarification! But I still not sure how much benefit you'll get with this approach.

By providing this new layer, components can avoid using the $storedirectly, and therefore be less dependent on the store itself.

Well, maybe the text $store would disappear from the code base, but you still have to do import { subscribeAction } from 'vuex'
anyway. So, it feels to me that it's not really decoupling anything technically. It's just aliasing $store with a different name.

mapXXX helpers will reference store instance from this context inside Vue Component, and I think this new subscribeAction function would be the same as well. So, it highly depends on the component to have the store instance injected. Again, it's not decoupling dependencies or anything. They are indeed simple helper functions (it even lives in the file names helpers).

Another concern is that if this is something that we consider decoupling, we would have to prepare other helper functions as well, don't you think? Like watch, registerModule, and such.

I agree that it could be nice to have this kind of helper functions, but I think we need to keep them compact as possible.

@jcao02

This comment has been minimized.

Copy link

@jcao02 jcao02 commented Mar 11, 2020

mapXXX helpers will reference store instance from this context inside Vue Component, and I think this new subscribeAction function would be the same as well. So, it highly depends on the component to have the store instance injected. Again, it's not decoupling dependencies or anything

@kiaking wouldn't the helper encapsulate how the subscription's done therefore decoupling from implementation details? Like the component is not aware that the helpers's using this from the component to get the store instance, so an implementation like this would be equally valid, wouldn't it?

import { getStoreFromDependencyContainer } from '...'

function subscribeAction(name, { after, before }) {
  const store = getStoreFromDependencyContainer()
  return store.subscribeActions(...)
}

Maybe the most convenient way to implement this today is binding this to the function, but it's not the only way and what @asoglovo suggests is to provide this flexibility to the codebase

@kiaking

This comment has been minimized.

Copy link
Member

@kiaking kiaking commented Mar 11, 2020

Like the component is not aware that the helpers's using this from the component to get the store instance, so an implementation like this would be equally valid, wouldn't it?

Yea, it's valid 👍 Sorry if I sounded too aggressive, but I don't disagree with the approach, it's totally valid. Though I just don't think it should be included in the library itself.

I do understand your and @asoglovo's argument, but if we would do this due to decoupling reason, we should be doing it for other functions (watch, registerModule, etc) too right? Also, maybe other libraries like Vue Router should expose route, router, push, etc. functions as well? I don't think that's a realistic idea.

So my point is that we should be careful on what helper functions we provide. And I'm really not sure if subscribeAction is the one.

It would be cool to have these feature as a plugin though.

@jcao02

This comment has been minimized.

Copy link

@jcao02 jcao02 commented Mar 11, 2020

@kiaking I understand. You didn't sound aggressive at all 😅

I do believe doing it for the rest of the store methods may be a good idea, but it's true devs can create this helper in their projects or contribute with a plugin

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.