replace axios with fetch

[jimmywarting]

(Describe your issue and goal here)

Packages:

Select all that apply:

• @slack/web-api
• @slack/rtm-api
• @slack/webhooks
• @slack/oauth
• @slack/socket-mode
• @slack/types
• I don't know

Requirements

could you replace axios with fetch instead?

And preferable don't depend on any FormData, Blob, XHR, fetch dependency? let users bring there own instances instead b/c they exist globally in NodeJS now and if they need they can polyfill this stuff by themself. using undici, node-fetch or anything else

[seratch]

Hi @jimmywarting, thanks for the suggestion!

Although are not planning to work on the replacement in the short term (due to our priorities right now), I agree that we can make the API client package simpler in the interest of broader use cases in the future.

[maxcountryman]

It would be great if this were structured in such a way so that it could then work with runtimes that don't support Node, e.g. Cloudflare Workers/V8 isolates.

[jimmywarting]

often when i code things then i have this mindset of coding things in a more cross env friendlier way and i'm also following this guidelines: https://github.com/cross-js/cross-js

The most fundamentals is to think of your application as an "onion" architecture that has different layers of supports.
at the very core of your application you shouldn't assume that you have access to any NodeJS, Deno, or any browser API's.
it shouldn't even assume that it has system IO (network, disc) access. Your internet may be down (offline) perhaps you may want to cache things or mock some data or configure the request/response before it's sent & received. maybe you want to listen to navigator.onLine and re-send the request once it's back up again. or simply just being able to debug outgoing requests for time measurement and progress listening

// Just one idea:
new WebClient({
  /**
   * - This option is required if you do not have globalThis.fetch or `globalThis.Response` 
   * - Omitting this option makes it so slack call `fetch()` for you.
   *
   * Same arguments you would use for making a `new Request(url, requestInit)`
   * the config.headers is either a 2D array or an object instead of a `Headers` class
   * 
   * @param {string} url
   * @param {RequestInit} config
   */
  onfetch (url, config) {
    config.signal = signal // add your own abort signal
    config.agent = agent // useful in NodeJS only
    // check if it exist in localStorage and if so return a new Response() manually
    return fetch(url, config)
  }
})

this way the developer can use whatever they already might have as an dependency , they might already depend on some node-fetch, got, axios, request or undici version, so you would then also avoid some duplicate code

[jimmywarting]

it would also be grate if you could change buffer for uint8array instead.

[filmaj]

I agree this is a great suggestion, as the use of axios makes this package less portable (to other runtimes like Deno, among other use cases). However, axios provides many different configuration options, like proxy support, which, if I understand correctly, is an important option used by many Slack application developers today (particularly in corporate environments).

So whatever replacement would be adequate, I just wanted to explicitly point out the need to maintain some amount of backwards compatibility for these secondary features like proxy support.

[alexbjorlig]

And security 🧐

Snyk just posted this CSRF issue with Axios.

[filmaj]

Worth revisiting this issue as I am inching towards a new major release of web-api (with things like better type safety of API arguments). In the next release, web-api will require node v18 minimum.

I wonder if in node 18+ it is easier to implement some of the more corporate-y features web-api provides like proxies.

[filmaj]

An update here is that axios 1.7 has a fetch adapter now 😮 so maybe we can keep axios, maintaining backwards compatibility, but also leveraging fetch where available? https://github.com/axios/axios/releases/tag/v1.7.0

[filmaj]

let users bring there own instances instead b/c they exist globally in NodeJS now and if they need they can polyfill this stuff by themself. using undici, node-fetch or anything else

The trick to resolving this issue and enabling ☝️ , I suppose, is the balance between:

1. Providing some kind of plugin or extension API to let developers pick their own http client (enabling runtime agnosticism), as well as
2. Either:
   a. Maintaining backwards compatibility on certain APIs web-api provides like proxying et all, or
   b. Deciding on dropping those features

2.b. sounds unreasonable to me as many Slack customers heavily rely on this feature.

Given that, I am not sure what approach could be taken to both support the more corporate-y / proxy requirements as well as enabling an http client adapter / override ability for developers. Open to suggestions, though!

[johtso]

Is there any current way to use this in the cloudflare workers runtime? I'm trying to use it with the latest axios with the fetch runtime but I get Uncaught Error: Dynamic require of "node:path" is not supported