spree-api-v2-js-sdk icon indicating copy to clipboard operation
spree-api-v2-js-sdk copied to clipboard

Add optional pre-request hook to Axios fetcher

Open rafalkosla101 opened this issue 3 years ago • 1 comments

Fixed the bug where error 404 was not parsed as spree error, extended sdk to have custom function that can be specified by user

rafalkosla101 avatar Aug 04 '22 06:08 rafalkosla101

A couple of quick comments:

  1. Since the parsing issue was moved to a separate PR (https://github.com/spree/spree-storefront-api-v2-js-sdk/pull/343) we could remove it here as well. Even though it won't matter if the other PR gets merged first, there's no point forcing a particular order here. You could also squash the commits into something more precise, e.g. Add optional pre-request hook parameter to Axios fetcher.
  2. This functionality could probably be tested. The tests of this SDK are pretty concise but maybe there's a nice place where this could live.
  3. Now that I think of it, the locale functionality is going to become part of Spree core. So it would probably be better if we come up with a mechanism that will allow you to explicitly set the locale instead of this very open pre-request hook function. Due to Vuestorefronts architecture this will probably boil down to a very similar approach (because of the server proxy) but maybe there are some better approaches. E.g. since VSF supports the vsf-locale cookie then it makes sense for it to support passing it to the backend somehow as well (might be tricky though because of all the abstractions, I'll think about it)

kshalot avatar Aug 08 '22 07:08 kshalot