Vue-storefront: Core modules[proposal, discussion]

Created on 28 May 2018  Â·  13Comments  Â·  Source: DivanteLtd/vue-storefront

Hi!
For now we are using core components and pages to develop our stores which works well but have some drawbacks that can be fixed:

  • The functionalities are grouped in component-structure-oriented way rather that feature-oriented which is not the best approach for a modular application
  • It kind of forces people to use same component structure for each store which doesn't play well with implementations that are much different than our default theme
  • Core components are rather complex, it's hard to reason about them and even harder to change only the parts that we want to work different
  • People can't tell what are the features of the component just by looking at it - you need to jump into the code and understand it
  • Methods inside core components are tightly copupled which isn't good for refactoring and modifying some functions in the future
  • It's hard to reason what comes from core and what comes from theme if both are bulky and complicated
  • We are injecting whole components even when we only want one or two features which ends up as bulky components with redundant functionalities
  • You basically can't say what you can do with the core without understanding it in-depth
  • its nearly impossible to tell people which functions are part of the CORE API so they can rely on them and use in their themes and which are just helper functions

What is the proposed fix?

The idea is to group similar functionalities into modules. Also I'll try to split current core components into smaller standalone features (like add to cart, remove from cart, items in car etc). it's still meant to be used as mixins but now you will export exactly what you want and will know exactly what you are injecting

You can find two proposal modules here ( https://github.com/filrak/vue-storefront/tree/modules-and-catalog-theme/core/modules )The implementation of this particular modules will (for sure) change but it's just to get the idea.

So now instead of bunch of core components for let's say cart we will have cart module with following mixins to import: cart_add(product), cart_remove(product), cart_items, cart_isInCart(product) etc. So we can for example import only cart_isInCart() mixin to our product listing to tell the user that this item is in cart. Now we will need to import whole microcart module or copy this method from inside of the core to achieve this.

The proposed API for using it will be as follows.

modulePrefix_methodOrDataname - so you can see from the beginning in your theme's code that this functionality is a core functionality coming from module X.

  1. You can import all parts of the module (including every data property and method related to this module) and inject:
    js import CartModule from 'core/modules/cart' ... mixins: [CartModule]
    2....or just the ones that you need (this particular set would be useful for Product page)
    js import { cart_add, cart_remove } from 'core/modules/cart' ... mixins: [cart_add, cart_remove]
    Similar modules would be created for wishlist, menu, categories, product, compare etc.

Such modularisation and abstraction will also hide the helper functions so we can refactor and improve our core much efficient without bothering about all legacy code and focusing only on keeping the core exported functions untouched with its API

Ofc we can still have some core components that will just group features from one or many modules that are commonly grouped together but it would be a little help and addition to speed up some very common taska, not the core itself that we must always use.

The core pages with their core functionalities will probably stay as they are with small refactoring but this one is still to be discussed (probably having 3-4 pages like this will be still transparent for developers)

Please let me know what do you think about this API, how do you see the improvements and what else can be improved during this API change.

You can find previous discussion here: https://github.com/DivanteLtd/vue-storefront/pull/1184

Most helpful comment

As someone 2 weeks into this code, I agree and like the Modular approach. It does get rather tricky to know what comes from where exactly. Having duplicate folder names in different places is a little confusing too. Perhaps refactoring folder names to make it clear for e.g. core-logic vs core-theme or plug-in-logic vs plug-in-theme; or something similar would help too. Thanks!

All 13 comments

I like the idea, It would make it easier to use just the parts one needs. As you said it will give way more flexibility to someone who wants to create something on top of VS with a totally different interface in mind.

As someone 2 weeks into this code, I agree and like the Modular approach. It does get rather tricky to know what comes from where exactly. Having duplicate folder names in different places is a little confusing too. Perhaps refactoring folder names to make it clear for e.g. core-logic vs core-theme or plug-in-logic vs plug-in-theme; or something similar would help too. Thanks!

Nice idea (and work!), VUE is for simplicity and development experience. I don't like second and more layer above existing in vue, because people use it for that simple approach.

I check that module directory and is clearly better approach to some problems.
One problem is the most obvious, used grouped functionality or simple one? maybe just use vuex?
Maybe "core/api" as showed in description. It's obvious that all my functions and data need to be taken from that one directory, otherwise I break site in future. Beside, more like to camelCase :D than some fancy underscore

import { cartAdd, cartRemove, cartReload, cartUpdate } from 'core/api/cart'

import * as cartAdd from 'core/api/cart/add

Internally writing test for that "api" directory will be in high demand to determine if something will break after refactor - right now, only one test :(.

I'm torn. On the one hand I like the idea of using a more modular approach for components, on the other hand we risk complicating the file and folder structure too much. Now It's quite easy to understand how components works just looking at the code, to split them into more granular components without losing the current (awesome) semplicity it will be a great challenge.
But, you are right, it's time to do it, especially for the benefit of the scalability of the framework.

The structure of the folders of the components could follow that of the vuex store (attribute, cart, category..) ? this would perhaps help to orientate within the core.

Great discussion!

I’m afraid of this approach because of the following arguments:

  • modules - it’s another layer of abstraction for the newcomers; it’s a little bit mess with the vuex modules and src/extensions
  • I agree with Irene, it’s now far easier to understand the structure of the Ui with more granulated components - we’ll probably lost it it with modules
  • I don’t see easy way to do the automatic update of the code people already have developed (custom themes)
  • the naming convention with lower case - for some it’s better than UpperCase - but the fact is that will require lot of manual changes just for sake of let’s say “visual preference” ;)
  • how to combine this approach with Vuex stores?
  • module_component - it’s very very Magento’ish :)
  • For now we have the components structure fairly easy to understand because you always start with the theme *.vue file having clear relation to the core components; with modules it’s gone; you must digg thru modules to find components you require; we don’t have capacity to maintain full blown docs at the moment to deal with this issue probably
  • the compilation process will slow down significantly I’m afraid because of the number of additional files to be processed

Maybe sth with satisfy all is to stick with the current component structure AND add this module based mixins / logic separation?

Then we should have sth like core/api or sth - just with this set of logic mixins instead of modules to not mess with vuex modules.

In that case if somebody would like to create totally custom theme he or she can avoid the core/components and use the core/api instead

Most use cases for eCommerce are - sadly ;) - I’ll just modify the look and feel / one two pages. We really MUST have a simplified starter/theme - batteries included - which was probably one of the reasons this project got popular:)

In other words - to refactor the core/components and core/pages into smaller traits/mixins but still keeping them altogether for the rapid development purposes : when somebody is not creating the theme from scratch

In this case - we probably should stick to the core/store structure of vuex modules to make it easy to get at first glance - just as @nuovecode proposed

The naming convention with the moduleName_mixinName doesn’t speak to me anyway :) and should be discussed

It’s just for discussion, thinking aloud
For sure we shouldn’t introduce any revolutionary changes to the api it must be progressive with the back/compatibility as we’re to publish 1.0 this week :-)

@filrak @Igloczek wdyt?

@pkarw i edited a little bit my first post yesterday. The idea is to still keep some of the core components:

"Ofc we can still have some core components that will just group features from one or many modules that are commonly grouped together but it would be a little help and addition to speed up some very common taska, not the core itself that we must always use."

So for example we will have cart api module with methods i listed above but you will still have microcart core component as a set of grouped functions to handle some common behavior but, what si important, you are not forced to use it and the logic for helper functions is hidden from you so it can be improved and refactored without breaking the api. The core microcart can look more or less like this
````js
import { cartAdd, cartRemove, cartItems, cartTotals, cartSubtotals } from 'core/api/cart'
import { checkoutGoTo } from 'core/api/checkout'

export default {
mixins: [ cartAdd, cartRemove, cartItems, cartTotals, cartSubtotals, checkoutGoTo ]
}
````

With this approach core components will group the core api methods so it's still very easy to create common components but we are avoiding repetition of code (similar functionalities on different components) and we are hiding the code itself so we can upgrade it, delete/rename/modify functions that are not exposed for api (helper functiosn) etc.

Abstracting the data and method properties that can be used by develoeprs is crucial for me in case of upgradability. For now we have no idea what methods from core components people are using so we can't really tell which one we can refactor and change. With modules it will be much easier (the core/api name is much better tho - thanks @Triloworld ).

@nuovecode yes, simplicity is very very important but i don't think you will loose anything with abstracting this API into modules. This modules still can be grouped into pages or components but at least there will be way to use only features that you want in a way you want instead of injecting component with 10 methods to use one of them. We need a higher level of granularity.

It will be also great for test purposes since we can just test every method separetely.

Now the API is very easy to work with but the goal of this change is to keep it as simple as it is but more readable, more upgradable, less complicated (developers don't need to know everything about how core component works to use it, in most cases the name is self-explainamble) and, what is most important for me, more scalable and flexible.

Exactly, core components grouping these more granular mixins - this makes sense for me!

Love to hear that ;d

I agree with you guys! Core methods as mixins to couple some basic components it's good idea 🥇
From my point of view it should be easier to maintenance, refactore and test.

@pkarw you asked how can we combine these mixins with vuex. In my opinion when we want to use some state/getter etc from vuex we should map this state in this mixin.
So for example we have method addToCart, we should map mutation from cart module. Next we have removeFromCart we should also map mutation from cart module. I think it's not problem because when we load these mixins in cart component, everything should be merged. So it's shouldn't be a problem when we would like to use the same state in two different mixins which we attach to one component.

Modularization? Of course!

One mixin per file is perfectly fine for me.
I saw what you did in core/modules and I have a few remarks:

  • Don't call your files add.js, products.js and remove.js: call them addToCart.js, productsInCart.js, removeFromCart.js (make you did this in a rush, much more readable).
  • In these files, try:
export const addToCart = {
  props: {
    product: {
      type: Object,
      required: true
    }
  },
  methods: {
    addToCart (product) {
      this.$store.dispatch('cart/addItem', { productToAdd: product })
    }
  }
}

instead of

export default {
  props: {
    product: {
      type: Object,
      required: true
    }
  },
  methods: {
    addToCart (product) {
      this.$store.dispatch('cart/addItem', { productToAdd: product })
    }
  }
}

If you use VS Code or another tool which understands ES6, the addToCart object should pop up in the autocomplete. In index.js:

import { addToCart } from './addToCart'
import { removeFromCart } from './removeFromCart'
import { productsInCart } from './productsInCart'

export {
  addToCart,
  removeFromCart,
  productsInCart
}

And when you try to use it...
import

We can also put the internal files in a internal folder. If you're a user and you're importing something from an internal folder, you do something terribly wrong, since we have no way to hide them.
arch

If we want to go further, why not putting the vuex store module directly into the cart module folder? If you're working on the cart module, it's better to have everything in the same place.
Take a look at https://medium.com/3yourmind/large-scale-vuex-application-structures-651e44863e2f and this approach:
image
"modules" can combine the vuex store module, the mixins, and even components!
I prefer the component-oriented or module-oriented approach over the classic vue approach.

@DavidRouyer good points. Regarding the Vuex decomposition into app-scoped, not just vuex scoped, modules - is probably a way to go. BUT it must be well thought off - to maintain the compatibility with the current calls etc.

Currently, the core/store is a separate module which can be used without any dependencies of the vue storefront. So you can use Vuex store to connect any Vue (or even React!) app to the Magento using vue-storefront-api. This is the cool feature for kind of landing pages, very custom integrations.

On the other hand - Vuex is still kind of code-centralization which leads to SRP rule violations and the code it's pretty hard to maintain. Having stores inside modules (like currently, one can have in the extensions) is a good way to separate concerns.

By combining core/store with UI components and mixins we're losing this feature.

A) To wrap up what we all probably agreed on so far:

  • core/components, core/pages must stay with the same structure as they have (regarding file naming, component granularity) to maintain the backward compatibility for themes
  • ... BUT we can refactor the core/components and core/pages to be just a combination of more-granular mixins
  • the mixins like "addToCart" would be single file ones,
  • it's not set where they should be placed - the suggestions are that probably kind of core/api or core/mixins should be used for a place for such a "traits" or "mixins" to be stored in

B) Then the idea of decomposing Vuex modules to came across.

Maybe we should just phase this approach. For 1.1 we can go with A) and then plan the B phase?

Keep in mind that you

  • Are not breaking anything when moving files (Vuex modules will work the same way, you import it differently)
  • You can change the folder structure and still be backward compatible by keeping the store folder (with the current index.js and exports which redirects to the new folders)
  • We can add JSDoc comments for example for depreciation warnings, or ESLint rules
Was this page helpful?
0 / 5 - 0 ratings

Related issues

slightlyoff picture slightlyoff  Â·  3Comments

revlis-x picture revlis-x  Â·  3Comments

pkarw picture pkarw  Â·  5Comments

kyvaith picture kyvaith  Â·  5Comments

cartooncatfish picture cartooncatfish  Â·  3Comments