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

[RFC] Vuex naming conventions #2069

Closed
patzick opened this issue Dec 8, 2018 · 7 comments
Closed

[RFC] Vuex naming conventions #2069

patzick opened this issue Dec 8, 2018 · 7 comments
Labels
RFC Request for comments

Comments

@patzick
Copy link
Collaborator

patzick commented Dec 8, 2018

Vuex conventions

We need to establish some rules for vuex. It’s a proposition, which after some discussion will be added to our docs for possible referencing in future.

Module

Vuex module should be created for specific set of functionalities. Should also have only absolutely necessary dependencies to other modules.
Name of module should be short, quite clear about it’s destination and has words separated by dash.

Good examples:

  • products
  • product
  • user
  • checkout
  • compare-products
  • notifications
  • order

Bad examples:

  • next-module
  • compare (because it’s not saying what its compare)

State

State properties should be simple and their structure should not be nested. Their names are written in camelCase notation and indicates what they are containing.
We should avoid to have more than one instance of object, even between modules. In the vast majority of cases they can be referenced by it’s unique id property. Example:

{
  "productsMap": {
    "WS08": {
      "sku": "WS08",
      "name": "Minerva LumaTech™ V-Tee"
      // other options
    },
    "WS12": {
      "sku": "WS12",
      "name": "Radiant Tee"
      // other options
    },
    "WS08-XS-Black": {
        "sku": "WS08-XS-Black",
        "name": "Minerva LumaTech™ V-Tee"
        // other options
    }
    // maaaaaaaany more products
  },
  "currentProductId": "WS08-XS-Black",
  "wishlist": ["MP01-32-Black", "MSH05-32-Black"],
  "cartItems": [
    {
      "sku": "WH09-XS-Green",
      "qty": 3
    },
    {
      "sku": "WH09-S-Red",
      "qty": 1
    }
  ]
}

Good examples:

  • categoriesMap
  • currentCategoryId
  • order
  • productParentId

Bad examples

  • list
  • elements
filters: {
  available: {},
  chosen: {}
},

Getters

Vuex state, except of mutations, should always be accessed by getters. Including actions.
Getter should:

  • start from is when returns Boolean, or get otherwise
  • answer to question what am I returning?
  • contain module name to ensure that getter is unique through whole vuex. But I doesn’t have to start with that name - first of all it should have natural name.
    So for example we have module category and in state availableFilters. So what am I returning? -> available Filters. And this filters are category filters . Its not a Boolean, it’s array or map so we’re starting with get -> getAvailableCategoryFilters

Good examples:

  • for state user -> isUserLoggedIn, getUser
  • for state availableFilters -> getAvailableCategoryFilters
  • for state currentProductId -> getCurrentProduct (because it gets product object from map), getCurrentProductId

Bad examples:

  • totals
  • product
  • current
  • list

Actions

It’s a heart of logic for module. Every state change from outside of module should be invoked as an action. Actions are meant to:

  • fetch something from server(or cache) - in this case they have to be asynchronous (return promise)
  • mutate state of current module
  • dispatch actions from same module (to avoid repeating logic)
  • dispatch actions from another modules (only if it’s absolutely required)

Their names should most possibly be unique and in simple way says what specific action is doing. Almost every action should return promise

Good examples:

  • fetchProduct - gets product by id from server or cache, sets it in products map and returns it by getter
  • findProducts - fetches products by specific query, sets them in products map and returns them as array
  • setCurrentProduct - param could be id, it could dispatch fetchProduct, mutate it to productsMap and mutate its id to currentProductId. Also if productId is null then it removes currentProduct.
  • addCartItem
  • toggleMicrocart

Bad examples:

  • list
  • products
  • reset

Mutations

Finally we have mutations. Only mutations can change state of module. They should be synchronous (never returns promise), not contain any logic (be extremely fast) except one needed to keep state as it should be (for example sets default value for state). Mutations should be invoked only by actions from the same module. In most cases it should be only a single action which invokes specific mutation.
Types of mutation:

  • SET_ - it’s the most common type of mutation. It can set an object (or whole array), set default value of object (or maybe clean array),
  • ADD_ - it can add new element to state property which is an array or add new element to Map
  • REMOVE_ - an opposite to ADD. It can remove map element or array element by index (or by finding object which is not recommended way on big arrays, then mutation could be slow)

Good examples:

  • ADD_PRODUCT
  • SET_CURRENT_PRODUCT_ID
  • ADD_CATEGORY_FILTER
  • REMOVE_WISHLIST_PRODUCT_ID

Bad examples:

  • CATEGORY_UPD_CURRENT_CATEGORY
  • TAX_UPDATE_RULES

Thats it ;)

It’s a first draft proposition. It’ll be updated after discussions and at the end we will add it to our docs.
Please share your thoughts, every input is welcome and needed. We need to establish this ground rules to get clear and common vision for where are we're going with Vuex.

@pkarw
Copy link
Collaborator

pkarw commented Dec 10, 2018

I'm not sure about the following set of rules:

Their names are written in camelCase notation
Take the product state for example - most of the names are snake-case. We should find the common, minimal denominator for the current state of Vuex vs. this new proposition

Bad examples: list, products
I don't agree with that. Actions are called in context: products/list what makes perfect sense - much better looking (for me ;)) than product/listProducts. I would prefer to stay minimalistic and most close to English semantic as we can. Moreover, we should avoid a large number of changes across existing files + keep the backward compatibility with this refactor (therefore - having list currently we must keep it as an alias anyway).

The rest is fine with me; @filrak

@filrak
Copy link
Collaborator

filrak commented Dec 11, 2018

In my case 100% agree with @patzick .

PLUS: Our naming should be predictive and (if possible) very similar across modules for same tasks (fetching data from server/cache, filtering products on category/search, fetching single category/product etc) so by invoking similar action/getter on other module you more or less know how it works and learning curve is natural. This should be true especially for actions/getters params.

Regarding your concerns

Their names are written in camelCase notation

Usually js vars and properties aren't snake-cased. Maybe we can just keep the state like this but use proper camelCase for getters?

I don't agree with that. Actions are called in context: products/list what makes perfect sense - much better looking (for me ;)) than product/listProducts. I would prefer to stay minimalistic and most close to English semantic as we can. Moreover, we should avoid a large number of changes across existing files + keep the backward compatibility with this refactor (therefore - having list currently we must keep it as an alias anyway)

Imho we should do this anyway. If sth can be updated in a minute via simple search&replace i don't think backward-compatibility will be a problem there. @patzick point was to avoid conflicting actions under the same namespace when you use mapActions() (which is preferred to use by Vue devs and in this case product/list will be just a list ) and i totally agree with this.

@patzick
Copy link
Collaborator Author

patzick commented Dec 27, 2018

Okay, to finish this up we established two things during our last talk:

  • we'll use underscore case instead of camelCase in state variable names
  • we allow methods, which are in most of modules (like list or single) to still be produced in future modules, although they takes different parameters (so either way you need to find particular method and it's not so simple when you have many of them)

And most important fact, which i think was missed here - this is not a plan for refactor. This is naming convention, which we'll use for future reference, when we'll be planing refactor or adding new features.
IMHO every refactor and feature in vuex should be discussed first on separate RFC issue, because it's the API, which everyone use, no matter which template they use, so it should be under our special care.

@pkarw, @filrak guys, is it everything to close this? Where do we want to publish it after it's finished?

@pkarw
Copy link
Collaborator

pkarw commented Dec 28, 2018

@patzick: positive. Please go on with such a naming convention. I believe we should update docs including the coding guidelines there + we should mention it with link in CONTRIBUTING.md

@patzick
Copy link
Collaborator Author

patzick commented Jan 14, 2019

I've updated docs with PR #2049
So i'm closing this issue.

@patzick patzick closed this as completed Jan 14, 2019
@patzick patzick added the RFC Request for comments label Feb 17, 2019
nikmel2803 added a commit to codex-team/hawk.garage that referenced this issue Jun 26, 2019
@badunius
Copy link

badunius commented Jul 14, 2020

This is nice and all but, forgive my frustration, @vue/typescript/recommended is ENFORCING using camelCase in state variable names. Can't do anything about this, so right now I'm pondering should I either drop ESLint or TypeScript entirely. Or both.

I'm really sorry but it's 3 in the morning and I spent last hour trying to sort this out.

@sujin1204
Copy link

sujin1204 commented Dec 17, 2022

@patzick then how can i access state of module named including dash sign?
for example, module name is compare-products and i have to access to state of compare-products.
can i access to state like store.state.compareProducts.list ?
or should i access like store.state['compare-products'].list ?

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

No branches or pull requests

5 participants