[RFC] Refactor Vuex API calls to separate layer

[pkarw]

What is the motivation for adding / enhancing this feature?

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.

What are the acceptance criteria

• we should design this feature with Magento 2.3 graphQL support (to be able to add graphQL support instead of vue-storefront-api) in this separated layer

[pkarw]

@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

[lukeromanowicz]

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?

[pkarw]

1. I think that in our case we are already using it (take for example product entity which consist of related items, media items, configurable children etc). We shouldn’t change the overal structure of the Vuex calls I believe - keepin the clear separation for: product/category/... operations
2. We should use kind of strategy pattern to chose the proper API connector ARN build time - per entity type / via config. However it’s not my strong preference. It can be done via user space code as well

[patzick]

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:

store
  - api.ts
  - actions.ts
  - getters.ts
  - index.ts
  - mutations.ts

[lukeromanowicz]

@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:

1. fetch current cart items
2. if there are any items fetch totals
3. if there are any non virtual items, fetch shipping methods

GraphQL:

1. fetch items, totals and shipping methods (backend will take care of the fact if there are any products in a cart or not, and what type they are, then return an appropriate result)

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.

[filrak]

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

[pkarw]

+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:

• One of the acceptance criteria for this task is also that we are NOT to modify the data formats returned by vue-storefront-api /other integration modules to keep it backward compatible
• Optimization is not the goal of this task. The goal is to make it easier for other platforms to integrate with us and pretty transparent to theme/shops developers (not modifying the data formats / either public vuex actions). Let's start small and simple :)

[filrak]

Generally 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:

• data formats accepted by VS in a form of typed interfaces with required and optional fields. All of them should be properly documented and used inside Vuexes etc since these will be an actual data formats used inside the app. These interfaces should be extendable by extension modules if they require additional fields. It's kinda single source of truth for data formats

interface Product {
    /* this comment will be displayed after highliting 'sku' on any entity with type Product */
    sku: string,
    name: string,
    updated_at: any,
}

• API call or set of API calls that returns data from CMS/eComm specific for the platform(just fetches/gql queries returning platform-specific format)

getBigCommerceProduct(params) : BigCommerceProduct {
 // fetch product from BC
}

• mappers/adapters responsible for changing the custom format to one consumed by VS.

BigCommerceProductAdapter(product: BigCommerceProduct) : Product {
// do the mapping
 return product
}

• and obviously the actual function consumed by our Vuex stores where we can pass the API call and mapper depending on which platforms are used. The users will only pass API calls to their platforms and proper adapters and the rest will be handled by VS that consumes return values of this functions (so like with VS API). All of these should be kept in one file that is exposed to users so they can change the api calls and adapters to integrate their platforms.

getProduct(getBigCommerceProduct(params), BigCommerceProductAdapter) : Product
getArticle(getWordpressArticle(params), WordpressArticleAdapter) : Article
// ...etc

With this approach itnegrations will be extremely easy (just passing api call and adapter to one function per entity) with type-checking and common format across whole app
WDYT?

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.

[pkarw]

I don’t see much value in separated mappers. I mean BigcommerceApiStrategy (keeping to the example I’ve 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 :)

[pkarw]

I think it’s not moving the vsapi logic - as vsapi doesn’t contain much logic at all :) I mean - mapping isn’t business logic. It’s 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 :)

[pkarw]

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

[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).

[pkarw]

We've already discussed it so I'm in :)

[pkarw]

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

[lukeromanowicz]

@pkarw @patzick @filrak
I've done a little bit of codding and came up with the following results:

lukeromanowicz@a5a0cea

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:

• doesn't generate breaking changes
• very easy and fast to implement
• separates API layer responsible for obtaining data from the business logic

Cons:

• no strict typing of the results
• mediocre help with multiplatform modules for 3rd party developers
• no detection of duplicated API actions

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.

[pkarw]

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

[pkarw]

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:

• loaded once (at application init) from local storage,
• we should use the getter for obtaining user and cart tokens instead

[pkarw]

... 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.

[pkarw]

This should be done within 1.11rc1 along with the catalog + cart refactor @patzick = migrated to data-resolvers

[pkarw]

This is almost done with the data resolvers. @andrzejewsky can you please check if after #3337 we've done it?