vechain / vechain-sdk-js

The official JavaScript SDK for VeChain.
24 stars 9 forks source link

💡 [REQUEST] - Restore ability to inject custom "fetch" instance #1124

Open ifavo opened 3 months ago

ifavo commented 3 months ago

Summary

In the previous SDK releases it was possible to provide a custom axios instance to HttpClient.

This made it possible to use existing modules like https://axios-cache-interceptor.js.org/, to add caching functionality for immutable http requests to nodes.

I used it to cache immutable http requests in redis and return data for quicker responses within an RPC proxy.

With beta 23 axios was replaced with fetch, which is a good alternative, but as regression it is now no longer possible to just inject the handler (in this case fetch) as dependency to achieve the same result.

I propose to add the same functionality with fetch, so its possible to add a custom fetch handler. It would also allow to send custom headers for authentification or other actions to node backends, if neccessary.

As a workaround it might be possible to provide a custom HttpClient but that would be an unnecessary overhead.

Basic Example

import fetch from 'node-fetch-cache';

const httpClient = new HttpClient(options.node, { fetch })
const thorClient = new ThorClient(httpClient)
const provider = new VeChainProvider(thorClient);
victhorbi commented 2 months ago

Hey @ifavo , this issue will be worked on after the first official release of the SDK. We will do it once the VeChain Data Model will be implemented to avoid having to adapt it after.

lucanicoladebiasi commented 14 hours ago

The VeChain SDK doesn't use node-fetch-cache nor node-fetch but the simpler Fetch API.

Albeit node-fetch-cache is based on node-fetch and the latter is based on the Fetch API, node-fetch is a library designed for NodeJS runtime hence it could be introduce runtime incompatibility when the SDK is used to develop software for other JS runtimes.

The Fetch API is tagged Baseline Widely available by MDN.

I am skeptic about the convenience to leave the availability of Fetch API.

PR at https://github.com/vechain/vechain-sdk-js/pull/1535 introduces the possibility to set HTTP request headers for the SimpleHttpClient objects using the Fetch API internally. Headers allow to manage cookies and to control the cache policies. Hoping this functionality is sufficient, this should be a convenient way to preserve the simplicity of the code, the compatibility with the existing code and the wider JS runtime panorama.

If the solution is not sufficient, we should provide a module offering an HttpClient implementation based on node-fetch-cache.