wharfkit / antelope

Core types, client interfaces, and other tools for working with Antelope-based blockchains.
Other
44 stars 23 forks source link

Add fetch to the APIProvider and export type for use in tests #44

Closed aaroncox closed 1 year ago

ericpassmore commented 1 year ago

This is a good update, makes sense to me.

jnordberg commented 1 year ago

This removes the need for the APIProvider abstraction, for example you cannot implement a provider using the node http or axios module anymore. Fetch is pretty ubiquitous now though so we could revisit this design and just remove the entire abstraction and let the APIClient own a fetch instance (that can be overridden in options).

aaroncox commented 1 year ago

Hrm, that is a downside. I am using an axios instance within an APIProvider on Anchor desktop v1 right now 🤔

To copy over some context from Slack...

The reason I started this PR was that I want to be able to use the instance of fetch from the APIClient to retrieve things from other domains. I cannot currently do this since the APIClient instance is bound to a specific domain via this code:

https://github.com/greymass/eosio-core/blob/4b501aa0d76585d76faeb9296c09825e98078c3f/src/api/provider.ts#L51-L56

Each time call is executed, it prepends the domain from the instantiation

e.g. new APIClient({url: 'https://eos.greymass.com'})

No where through the APIClient instance can I access the fetch instance.

jnordberg commented 1 year ago

The idea of the APIProvider interface is to not dictate how communication with a node happens. For example you could speak to it via a unix socket. Trying to use the underlying fetch or call methods for other purposes then communicating with a node was never intended and adding that requirement to the provider largely negates its purpose.

aaroncox commented 1 year ago

After talking this through a bunch, we solve the problem in Wharf a different way - by instead of passing an APIClient instance into the Session Kit, we instead pass fetch in. The Session Kit is then able to store/use that instance of fetch without needing to access the fetch instance from the APIClient. Work was completed here: https://github.com/wharfkit/session/pull/11