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

Allow function to be passed as namespace argument for mapState, mapGetters, mapMutations & mapActions #863

Closed
aspruds opened this issue Jul 13, 2017 · 17 comments

Comments

@aspruds
Copy link

commented Jul 13, 2017

What problem does this feature solve?

Right now I can set namespace when calling mapState. However, mapState only accepts string as a namespace. Unfortunately, I can not use Vue props values as these are only available after the component has been created. It means that I need to hardcode Vuex namespace and it is impossible to reuse the component.

What does the proposed API look like?

I would like the following functionality:

props: {
    namespace: {
        type: String,
        default: 'searchForm'
    }
},
computed: {
     // namespace in a function
  ...mapState(function() { return this.namespace }, ['username', 'password'])
},

To implement this, one would need to change mapState implementation. normalizeNamespace function would need to be called later (inside mappedState). Since mappedState is called only after the component has been initialized, it would be possible to resolve .namespace prop by calling the passed function.

I could provide a small code sample with my implementation (rewrite of current mapState) if that would help in any way.

@ktsn

This comment has been minimized.

Copy link
Member

commented Jul 14, 2017

ref #748

@LinusBorg

This comment has been minimized.

Copy link
Member

commented Jul 16, 2017

You could use mapState et al. in beforeCreate() to dynamically extend `this.$options.computed

@aspruds

This comment has been minimized.

Copy link
Author

commented Jul 17, 2017

Another workarround which I am considering is to use vuex-connect.

@ktsn

This comment has been minimized.

Copy link
Member

commented Sep 14, 2017

I think this is covered by passing a function to mapState/mapActions/mapMutations (related #924).
After releasing #924, we can dynamically change namespace of all store assets in mapXXX helpers.

{
  props: ['namespace'],

  computed: mapState({
    state (state) {
      return state[this.namespace]
    },
    someGetter (state, getters) {
      return getters[this.namespace + '/someGetter']
    }
  }),

  methods: {
    ...mapActions({
      someAction (dispatch, payload) {
        return dispatch(this.namespace + '/someAction', payload)
      }
    }),
    ...mapMutations({
      someMutation (commit, payload) {
        return commit(this.namespace + '/someMutation', payload)
      })
    })
  }
}
@men62

This comment has been minimized.

Copy link

commented Sep 16, 2017

Will the dispatch return promise? If yes, personally, is it good practice to assign the data from that?

@johnsoncheg

This comment has been minimized.

Copy link

commented Jan 31, 2018

@ktsn I'm surprised that when I use arrow function state => state[this.namespace], some errors about the this binding happen, I feel confused

@rATRIJS

This comment has been minimized.

Copy link

commented Feb 1, 2018

@ktsn it won't work neatly for deeper namespaces - e.g. level1/level2/level3 - as it would try to access it incorrectly:

mapState({
    someField: (state) => state["level1/level2/level3"].someField, // doesn't work
    someOtherField: (state) => state.level1.level2.level3.someOtherField, // works
});

To make it work you would have to replace / with . and use something like lodash.get().

@ktsn

This comment has been minimized.

Copy link
Member

commented Oct 23, 2018

@rATRIJS Yes, the nested state need to be handled by kind of lodash.get style path access.

I'm closing this issue since this usage can be covered by #863 (comment).

@ktsn ktsn closed this Oct 23, 2018
@ThomasKruegl

This comment has been minimized.

Copy link

commented Nov 15, 2018

The workaround does not really improve the code in my opinion.

Workaround:

computed: ...mapState({
    state (state) {
      return state[this.namespace]
    },
    someGetter (state, getters) {
      return getters[this.namespace + '/someGetter']
    }
  }),

Code without mapState:

computed: {
    state () {
      return this.$store.state[this.namespace]
    },
    someGetter () {
      return this.$store.getters[this.namespace + '/someGetter']
    }
  },

The only thing mapState gives me in this case is getting rid of "this.$store."

The price is adding "...mapState" and the function parameters "state" and "getters" (state even unused in the case of getters).

I think i will go with something along the lines of Linus suggestion #863 (comment) for now, or implement some mapper on my own. Please think about better support for dynamic namespaces again :)

@brophdawg11

This comment has been minimized.

Copy link

commented Feb 17, 2019

I agree with @ThomasKruegl here in that the proposed workaround is not ideal as it makes for fairly different looking code throughout the app if you use a mix of statically and dynamically named modules.

With a static namespace:

computed: {
    ...mapState('static', [ 'foo' ]),
    ...mapState('static', {
        nestedBar: state => state.foo.bar
    }),
}),

If this feature was implemented as proposed, dynamic namespaces would look quite similar:

props: ['namespace'],
computed: {
    ...mapState(vm => vm.namespace, [ 'foo' ]),
    ...mapState(vm => vm.namespace, {
        nestedBar: state => state.foo.bar
    }),
}),

Compared to the proposed workaround, which looks and feels different - and loses the ability to use the shorthands provided by the array notation:

props: ['namespace'],
computed: {
    ...mapState({
        foo(state) {
            const path = vm.namespace.split('/');
            path.push('foo');
            return _.get(state, path.join('.'));
        },
        nestedBar(state) {
            const path = vm.namespace.split('/');
            path.push('foo');
            path.push('bar');
            return _.get(state, path.join('.'));
        },
    }),
}),

Even the workaround proposed by @LinusBorg above is a fairly significant deviation from the standard pattern and usage of mapXXX. Across a large codebase and a large team, I think the consistency of being able to use mapXXX both with a static string and with a dynamic namespace function is very important to consistency and maintainability of the code. It reduces cognitive overhead and should make onboarding new members much easier - as they'll be using the patterns documented in the Vuex docs in all scenarios - not learning new workarounds for these dynamic cases.

@ktsn Is there any chance you'd reconsider?

@brophdawg11

This comment has been minimized.

Copy link

commented Feb 17, 2019

I took a stab at implementing this in #1510, and the changes don't really increase the library code footprint by very much. I also noticed that #924 had an implementation for this as well - but the diff there seems to contain a lot beyond just that change.

@brophdawg11

This comment has been minimized.

Copy link

commented Feb 17, 2019

For the time being, we've re-implemented these functions locally using something like the following:

export function mapInstanceState(getModuleNameFn, mappers) {
    return _.mapValues(mappers, function makeInstanceAware(mapper) {
        return function wrappedMapper() {
            const moduleNames = getModuleNameFn(this).split('/');
            const localState = moduleNames.reduce((acc, m) => acc[m], this.$store.state);
            return mapper.call(this, localState);
        };
    });
}

But we'd love to get rid of the variance between using mapState and mapInstanceState if this were supported out of the box.

In our specific use case, we use route-specific modules so we can use the same Vuex store across dynamic routes. Consider an e-commerce site with a route like /product/:slug - and a single product Vuex module. In order to animate between two pages, you require a route-aware store so that both the leaving and entering component can be present in the UI at the same time.

Here's a codepen showing the issue that caused us to go down this route-specific module path in the first place: https://codepen.io/brophdawg11/pen/zeyoBN

@valterbarros

This comment has been minimized.

Copy link

commented May 13, 2019

@ktsn Thanks you help me a lot

@GopherJ

This comment has been minimized.

Copy link

commented Jun 20, 2019

I have something like this which seems working for me.

 const mutationWrapper = (mutations) => {
        return mutations.reduce((ite, cur) => {
            ite[cur] = function(commit, payload) {
                return commit(this.namespace + '/' + cur, payload);
            };

            return ite;
        }, {});
    };

    const getterWrapper = (getters) => {
        return getters.reduce((ite, cur) => {
            ite[cur] = function(state, getters) {
                return getters[this.namespace + '/' + cur];
            };

            return ite;
        }, {});
    };

    const stateWrapper = (states) => {
        return states.reduce((ite, cur) => {
            ite[cur] = function(state) {
                return state[this.namespace][cur];
            };

            return ite;
        }, {});
    };

use them like:

 ...mapState(getterWrapper([
                'hourStart',
                'minuteStart',
                'secondStart',
                'yearStart',
                'monthStart',
                'dayStart',
                'hourEnd',
                'minuteEnd',
                'secondEnd',
                'yearEnd',
                'monthEnd',
                'dayEnd',
            ])),
            ...mapState(stateWrapper([
                'dateTimeStart',
                'dateTimeEnd',
            ])),

or mutations:

...mapMutations(mutationWrapper([
                'EDIT_DATE_TIME_START',
                'EDIT_DATE_TIME_END',
                'EDIT_LAST15MINUTES',
                'EDIT_LAST30MINUTES',
                'EDIT_LAST45MINUTES',
                'EDIT_LAST1HOUR',
                'EDIT_LAST4HOURS',
                'EDIT_LAST12HOURS
]))
@matteplus

This comment has been minimized.

Copy link

commented Aug 21, 2019

I have something like this which seems working for me.

 const mutationWrapper = (mutations) => {
        return mutations.reduce((ite, cur) => {
            ite[cur] = function(commit, payload) {
                return commit(this.namespace + '/' + cur, payload);
            };

            return ite;
        }, {});
    };

    const getterWrapper = (getters) => {
        return getters.reduce((ite, cur) => {
            ite[cur] = function(state, getters) {
                return getters[this.namespace + '/' + cur];
            };

            return ite;
        }, {});
    };

    const stateWrapper = (states) => {
        return states.reduce((ite, cur) => {
            ite[cur] = function(state) {
                return state[this.namespace][cur];
            };

            return ite;
        }, {});
    };

use them like:

 ...mapState(getterWrapper([
                'hourStart',
                'minuteStart',
                'secondStart',
                'yearStart',
                'monthStart',
                'dayStart',
                'hourEnd',
                'minuteEnd',
                'secondEnd',
                'yearEnd',
                'monthEnd',
                'dayEnd',
            ])),
            ...mapState(stateWrapper([
                'dateTimeStart',
                'dateTimeEnd',
            ])),

or mutations:

...mapMutations(mutationWrapper([
                'EDIT_DATE_TIME_START',
                'EDIT_DATE_TIME_END',
                'EDIT_LAST15MINUTES',
                'EDIT_LAST30MINUTES',
                'EDIT_LAST45MINUTES',
                'EDIT_LAST1HOUR',
                'EDIT_LAST4HOURS',
                'EDIT_LAST12HOURS
]))

I like your answer @GopherJ but I have a doubt: where do you define "stateWrapper" within the component script? It needs access to this.namespace so it should be a component method, right? Clearly I'm missing something here.

@BonBonSlick

This comment has been minimized.

Copy link

commented Oct 2, 2019

I see here only "workarounds" but no real support, which causes a headache. We need dynamic namespaces for better support for a big codebase.

 ...mapGetters(`lists/${this.listType}`, {
              LIST_GET_ITEMS,
                },

still does not work, We have to do crap like this

  ...mapState(myFunctionWrapper(
                [LIST_GET_ITEMS](state, getters) {
                    return getters[`${this.listType}/${LIST_GET_ITEMS}`];
                },
            )),

or else junk like this

@thopiddock

This comment has been minimized.

Copy link

commented Oct 16, 2019

I've found another style of work around by utilising beforeCreate to access the variables you want from the props passed into your component instance:

import { createNamespacedHelpers } from "vuex";
import module from '@/store/modules/mymod';

export default {
  name: "someComponent",
  props: ['namespace'],
  beforeCreate() { 
    let namespace = this.$options.propsData.namespace;
    const { mapActions, mapState } = createNamespacedHelpers(namespace);

    // register your module first
    this.$store.registerModule(namespace, module);

    // now that createNamespacedHelpers can use props we can now use neater mapping
    this.$options.computed = {
      ...mapState({
        name: state => state.name,
        description: state => state.description
      }),

      // because we use spread operator above we can still add component specifics
      aFunctionComputed(){ return this.name + "functions";},
      anArrowComputed: () => `${this.name}arrows`,
    };

    // set up your method bindings via the $options variable
    this.$options.methods = {
      ...mapActions(["initialiseModuleData"])
    };
  },

  created() {
    // call your actions passing your payloads in the first param if you need
    this.initialiseModuleData({ id: 123, name: "Tom" });
  }
}

I personally use a helper function in the module I'm importing to get a namespace, so if I hadmy module storing projects and passed a projectId of 123 to my component/page using router and/or props it would look like this:

import projectModule from '@/store/project.module';

export default{
  props['projectId'], // eg. 123
  ...
  beforeCreate() {
    // dynamic namespace built using whatever module you want:
   let namespace = projectModule.buildNamespace(this.$options.propsData.projectId); // 'project:123'
   
   // ... everything else as above
  }
}

Hope you find this useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.