thepassle / app-tools

128 stars 8 forks source link

[api] Feature discussion - Pass fetch as param #1

Closed jfsiii closed 2 years ago

jfsiii commented 2 years ago

What do you think about adding a parameter to accept the fetch implementation that's called? Defaulting to the global fetch

My use case looks something like

import instrumentedFetch from '~/tracing';
...
const response = await instrumentedFetch(resource, init)

In my case, it might be possible for me to replace instrumentedFetch with something made from this client but a) I could adopt this quicker if I didn't need to b) it seems like it could be useful for testing or other times you're using a fetch-compatible function that's not necessarily the global fetch

thepassle commented 2 years ago

You can overwrite the fetchFn from a plugin, e.g. the mock plugin and the cache plugin do this: https://github.com/thepassle/app-tools/blob/399532e22113987436980c6fb5ed0c4721fa1430/api/plugins/cache.js#L19 https://github.com/thepassle/app-tools/blob/399532e22113987436980c6fb5ed0c4721fa1430/api/plugins/mock.js#L23-L29

Do note that if you use multiple plugins that overwrite the fetchFn, the last plugin to overwrite the fetchFn will win, there can only be one fetchFn if that makes sense

jfsiii commented 2 years ago

That's great! I see it's even in the types https://github.com/thepassle/app-tools/blob/399532e22113987436980c6fb5ed0c4721fa1430/api/index.js#L49

I'm happy to close this, leave it open until it's documented, or whatever you want to do. Thanks, again.

thepassle commented 2 years ago

Didnt realize I didnt document that 😄 Added some docs for it now, thanks 🙂