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

Suggestion: improve use of store constants #86

Open
petervdn opened this issue Nov 18, 2018 · 0 comments
Open

Suggestion: improve use of store constants #86

petervdn opened this issue Nov 18, 2018 · 0 comments

Comments

@petervdn
Copy link

petervdn commented Nov 18, 2018

I think the latest change in the mutations/actions/getter constants has been very good (getting rid of the namespace), but i still think it can be improved upon. I have three suggestions, which is basically the system i have been using for quite a while and which not everyone seems to like, but i think we should at least implement the first one.

1. combine them into an object per store
Each constant is now exported separately, which seems a bit weird since we don't do that anywhere else (it would be like exporting each RouteName entry separately). But more practically, if i have a bunch of stores with each a bunch of mutations (SET_USERNAME, SET_SCORE), whenever i want to auto-fill/import something by typing "SET_", i get a very big list of entries to choose from, and it will be not clear to what store they belong and what they do (since the constant typically doesn't contain the store name).

I think exporting those constants like this would be more practical:

export const appStore = {
    SET_DEVICE_STATE: `${namespace}/setDeviceState`,
}

In my opinion this is a lot easier to use. You just type that storename somewhere, autoimport it and the autocomplete lets you explore whatever values it has on it.

2. categorize constants per store
I would go even further and group those values, since it's not always clear what a specific constant is (especially mutations and actions, their names can sometimes be a bit similar - although i think these two will be merged by vuex somewhere in the future).

export const appStore = {
    mutation: {
        SET_DEVICE_STATE: `${namespace}/setDeviceState`,
    },
    action: {
        LOGIN: `${namespace}/login`,
    }
}

I think using this in a component will make it very readable as well:

...mapActions({
    login: userStore.action.LOGIN,
});

3. automatically fill those values
Regardless of whether we add the 2nd suggestion, i personally don't see why you wouldn't want to automatically set all those values instead of duplicating things, although this approach wasn't universally liked :)

export const appStore = initStoreConstants({
    mutation: {
        SET_DEVICE_STATE: null,
        SET_USERNAME: null,
    },
}, namespace);

(instead of the null values, this could also be done by what seng-event does with EVENT_TYPE_PLACEHOLDER)

Let me know what you think. Like i said, i think we should at least implement the first one to avoid the big list of options whenever you start typing SET_

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

No branches or pull requests

1 participant