It would be great to move out all the network/API communication to separate layer. The goal: make it easier to integrate VS with 3rd party backend. Now it's possible only by exposing the API in our format (https://github.com/DivanteLtd/vue-storefront-integration-boilerplate). We do have a pretty nice mechanism to add abstraction layer over ElasticSearch (https://github.com/DivanteLtd/vue-storefront/tree/master/core/lib/search) using SearchAdapters. Would be awesome if instead of set of functions "serverCartCreate" we do have something like (thinking aloud, don't bind to the naming): PlatformStrategy.CartAdapter { create, update, delete, sync, totals }, PlatformStrategy.UserAdapter { login, register ...} - typed with TS. So in these adapters user can do the mapping from Vue Storefront internal data format (we can't change the data formats that we're currently using to not break the backward compatibility!) to the specific API calls.
vue-storefront-api) in this separated layer@patzick @lukeromanowicz this is up to discussion - how best can we approach this problem?
We shouldn't have any direct network calls (fetch/syncTask API) in the Vuex actions - click for example - to keep the platform specifics separated from the ecommerce business logic
Well, in REST we call to a single repository while in GraphQL we're able to retrieve multiple entities at once. Do we plan on doing an abstraction layer capable of using them interchangeably while still utilizing fully the capabilities of GraphQL? How do we decide which service to use, based on code or configuration?
Yes, definitely we shouldn't invoke any API calls from components and Vuex actions directly.
I designed that way before and it worked nice:
Component needs data -> awaiting dispatched vuex action (action can resolve something from state, prepare request data etc) -> action invoke API layer -> here is our factory which choose GraphQL/REST/Anything -> goes back to action, which can store something (but doesn't have to!) -> resolves back for component which needed that data at the begining
API layer should be unified - means that vuex action doesn't have to know context of used API layer - it shouldn't make a difference if it is a REST or GrapgQL -> invokes call with object as params and waits for (also unified) response.
This approach keeps vuex actions a lot cleaner and separate concerns.
The architecture of vuex modules i've used:
- api.ts
- actions.ts
- getters.ts
- index.ts
- mutations.ts
@patzick's idea looks cool. I would go one step further and move api.ts out of store directory into separated one. Then we can split requests and interfaces into separated files instead of building one, oversized.
There is still one issue to solve: our GraphQL approach sounds like it will work just like REST. Please, correct me if I'm wrong, but in the current approach, it will call for the same set of data as REST. The only benefit is that it will call one and unified GraphQL endpoint instead of one of many REST endpoints.
I say we should build an abstraction layer smart enough to pull data from multiple entities at once using one GraphQL request or, interchangeably, a bunch of requests to multiple rest endpoints. Otherwise, the adding GraphQL is a moot point.
Use case:
fetchCart related data using rest:
GraphQL:
Of course, it probably won't be easy. There is some extra logic in REST version. Also, cross-module requests might be a problem, but we will achieve the most important benefit of GraphQL, which in my opinion is reducing the number of requests and the delay caused by them.
i agree with both @patzick and @lukeromanowicz. Also with moving the API calls to separate folder. As we discussed would be nice to have these adapters in one place and have this mapper as a param of invoked function or sth
+1 for separated, centralized folder for the API - we should make it easy to publish another API, not something that will be dispearsed across modules. So we should have something like:
/api/vue-storefront-api
..cart
..catalog
..user
/api/bigcommerce-api
..cart
..catalog
..user
@lukeromanowicz to push things forward I would be glad if we keep the current data formats at first phase and just try to make it abstract (the calls, separate them from vuex).
Depending on the backend platforms there could be a cases like:
a) need to call 2-3 REST endpoints to complete the product/user/cart entity as we have it in Vue Storefront
b) an option to cache more info from single graphql/rest call to use it later.
c) an option to aggregate few calls into one (graphql or dedicated rest endpoint)
I really like the example that @lukeromanowicz just gave about the shopping cart. Not sure if this will be easy or even possible to refactor the business logic of shopping cart merges into sth like this. BUT THIS WOULD BE GREAT :) The cart sync operation is a good example - it's very platform specific. It should be probably separated to single api interface method and implemented specifically to the platform. With Magento, it will be a single graphQL call for graphgQL API + about 3-4 REST calls (pull/update/totals).
Just two notes:
vue-storefront-api /other integration modules to keep it backward compatibleGenerally it would be awesome to have this possibility to call any api and just pass a mapper that will change it's data format to the one accepted by VS. It would be great tool for integrations PoC's that doesn't need ES layer or generally for stuff that doesn't require es layer. It will be much more DX firendly if the mapper/adapter will be a typed functions that must return certain data format. Moreover this functions can have built-in localstorage-based caching mechanism to cache responses.
So at the very basic level we should have:
js
interface Product {
/* this comment will be displayed after highliting 'sku' on any entity with type Product */
sku: string,
name: string,
updated_at: any,
}
js
getBigCommerceProduct(params) : BigCommerceProduct {
// fetch product from BC
}
js
BigCommerceProductAdapter(product: BigCommerceProduct) : Product {
// do the mapping
return product
}
js
getProduct(getBigCommerceProduct(params), BigCommerceProductAdapter) : Product
getArticle(getWordpressArticle(params), WordpressArticleAdapter) : Article
// ...etc
This is a very basic idea of how I would like to see this, probably with a lot of downsides and potential for simplification but would be nice to discuss our ideas and come up with something that will make dx better and what is more important - simplier ;)
One thing to keep an eye on. It's actually a first step of moving the VSAPI logic into VS core. While keeping more logic in one repo has a lot of advantages and I think it should be kept in one repo we shouldn't forget that keeping some part of the core server side actually implies less code to be downloaded by the user so some heavy (mostly in terms of amount of code) data-related operations should be still done server side.
I don鈥檛 see much value in separated mappers. I mean BigcommerceApiStrategy (keeping to the example I鈥檝e proposed) should do the mapping returning Product, Cart and other VS interface compatible entities. This adapter itself would call BigCommerce rest or graphql api.
Well, it's basically just a different taste - the same nutrients :)
I think it鈥檚 not moving the vsapi logic - as vsapi doesn鈥檛 contain much logic at all :) I mean - mapping isn鈥檛 business logic. It鈥檚 ok to have it in VS - having defacto more optimal architecture (without server-side middleware).
If somebody wants then can use the Backend For Frontends pattern building kind of more optimized vsapi / whatever and still use this switchable api adapter in the front end to just connect to it :)
By the way - some time ago I started (and haven't had time to continue the works so far) the vue-storefront-simple-api - memory based, simplest possible API to show how to integrate 3rd party systems:
https://github.com/DivanteLtd/vue-storefront-simple-api
I've also added a custom searchAdapter for this API to show how to avoid the need of using ElasticSearch: https://github.com/pkarw/vue-storefront/commits/feature/2166_simple_backend
So it doesn't use the ElasticSearch DSL either bodybuilder etc.
Related: #2166, #2167
So, I believe that there is not such a big deal regarding the catalog integration / avoiding the Elastic. By building custom searchAdapter it's pretty easy, to be honest.
The key goal for this issue was about all the other requests. However, if we're to somehow change/uprade searchAdapter keeping the backward compatibility - of course I'm up to it as well :)
I think that this is the right moment to code just some PoC @lukeromanowicz
Having easy to use abstraction over GraphQL and REST interchangeably would be very handy but using them simultaneously would use additional resources which is contrary to the goal of keeping the frontend lightweight. That's why I think we should go for separated strategies for both ways of exchanging data with backends.
My idea is to create API modules which will implement global actions for fetching and updating data with accordance to public and unified data interfaces. Standard modules would dispatch global actions like 'fetchCart', 'fetchShippingMethods' etc. that are implemented by various API strategies. API calls would still be placed in vuex actions but separated from the business logic, making our modules more platform agnostic. A strategy would be selected by enabling appropriate modules.
The structure would go as following:
src/modules/api-vue-storefront
|- store
|--- index.ts
|--- actions
|------ cart.ts
|------ checkout.ts
|------ customer.ts
|------ ...
src/modules/api-big-commerce
...
src/modules/api-project-xyz
|- store
|--- index.ts
|--- actions
|------ cart.ts //extend selected api strategy from core/modules/api-*
The drawback of this approach is that our API dedicated actions have to resolve into actual API responses which are not a common practice in Vue (usually we fire the async actions and forget about them because the result we'are after will come from reactive getters).
We've already discussed it so I'm in :)
Additional insight: https://github.com/vendure-ecommerce/deity-falcon-vendure-api/tree/master/src
Falcon API mapper - https://github.com/vendure-ecommerce/deity-falcon-vendure-api/blob/58211f8a12abd2ba08c5f1dffe1abc29f135bf3d/src/index.ts#L231
So basically the same approach described in here
@pkarw @patzick @filrak
I've done a little bit of codding and came up with the following results:
https://github.com/lukeromanowicz/vue-storefront/commit/a5a0cea52250d5bb731bfeb891e48f2e17a2b247
One of the cart's API requests, responsible for pulling cart data from a backend, has been extracted to separate api-vue-storefront module that is going to handle all core API requests.
I've added an example, how to extend an existing core API for projects requiring some core modifications, and an example for another API implementation (i.e. direct connection to SaaS backend) that could be used instead of default one.
The key is to keep one API module turned on at once.
Pros:
Cons:
Stage two could be adding some types of expected payload to let developers know what the core VS needs to work properly. The problem is that we have no way of enforcing these types using Vuex actions because of how they are designed (just dispatch what has to be dispatched and resolve a promise when ready, no matter what the result is).
The ideal solution, on the other hand, would be to use a custom API calls dispatcher instead of Vuex based one. It would require significantly more work and increase the architectural complexity by a little bit, but also let us type the results and solve the cons above. It could be either a part of VS or a standalone Vue plugin/JS module.
Although these solutions more or less help cleaning up the mess in the core code, they still don't change anything in terms of 3rd party modules development. Those developers would still have two options. It would be either splitting their modules into multiple ones (logic + API per supported platforms) if multi-platform support is required or implement API actions as regular, namespaced actions in stores of their modules.
Also, worth consideration would be getting rid of callbackEvent in API actions and dispatching callback actions within very actions that are performing those API calls.
We've just discussed this with @lukeromanowicz on Slack. We should aim at the final solution - we've got just one and only chance to refactor this right way. So We decided we should go with the separated API layer, not based on Vuex. Strongly typed, registered from the Module Registration
Hey! By this change we should also get rid off the way we're obtaining user and cart tokens:
https://github.com/pkarw/vue-storefront/blob/da58b6c59daa76b75a501aaddefcee21c6a3063d/core/lib/sync/index.ts#L52
It should be:
... and there is one more thing. I believe that would be great to have in this API layer all the operations that read data too. The assumption should be that this API layer doesn't involve any side effects (affecting in store.state changes)
Take category/list for example - the quickSearchByQuery and searchQuery building part could be extracted as a separate method - which is reusable.
Currently, our Vuex is strongly bound to the state + UI; and we won't fix it easily (not sure if it does make sense to fix it); but we can refactor and create the easy to use API for developers by implementing this issue.
This should be done within 1.11rc1 along with the catalog + cart refactor @patzick = migrated to data-resolvers
This is almost done with the data resolvers. @andrzejewsky can you please check if after #3337 we've done it?
Most helpful comment
Yes, definitely we shouldn't invoke any API calls from components and Vuex actions directly.
I designed that way before and it worked nice:
Component needs data -> awaiting dispatched vuex action (action can resolve something from state, prepare request data etc) -> action invoke API layer -> here is our factory which choose GraphQL/REST/Anything -> goes back to action, which can store something (but doesn't have to!) -> resolves back for component which needed that data at the begining
API layer should be unified - means that vuex action doesn't have to know context of used API layer - it shouldn't make a difference if it is a REST or GrapgQL -> invokes call with object as params and waits for (also unified) response.
This approach keeps vuex actions a lot cleaner and separate concerns.
The architecture of vuex modules i've used: