Support for cloudflare workers

roopakv commented 3 years ago

Description

the web-api npm package is not usable on Cloudflare workers since it uses axios. If this SDK were to consider using something like isomorphic-fetch, one could use this package in workers too.

I am happy to make a contribution but want input before I start working on this

What type of issue is this?

[ ] bug
[x] enhancement (feature request)
[ ] question
[ ] documentation related
[ ] testing related
[ ] discussion

Requirements

[x] I've read and understood the Contributing guidelines and have done my best effort to follow them.
[x] I've read and agree to the Code of Conduct.
[x] I've searched for any related issues and avoided creating a duplicate issue.

Packages:

Select all that apply:

[x] @slack/web-api
[ ] @slack/events-api
[ ] @slack/interactive-messages
[ ] @slack/rtm-api
[ ] @slack/webhooks
[ ] @slack/oauth
[ ] @slack/socket-mode
[ ] I don't know

srajiang commented 3 years ago

@roopakv - Thanks for writing in with this suggestion and for your interest in working on this! It's kicked up a healthy discussion amongst the maintainers, and I will let others (@filmaj @seratch @stevengill) chime in with their input as well.

In general, the team isn't planning on working on enhancing of the web-api package to enable developers to use a different HTTP client, however we would be happy to work with you to review this if it's something you're interested in working on. [Edit] Or if you're thinking of creating your own solution to cover your use case and you make your work public, we are happy to share the repo out with the developer communities.

roopakv commented 3 years ago

@srajiang if i were to send PRs up moving from axios to something else would your team be open to reviewing and getting it in?

If not I would probably look into making my own solution based off of a fork of this repo :)

filmaj commented 3 years ago

Hey @roopakv,

If you want to attempt to replace axios, please feel free, though I have a feeling it is a big effort. There are many critical aspects of this package to the variety of different Slack customers, so all the various options (like proxying, file uploading, rate limit handling, request concurrency, custom TLS configurations, custom API URL) would have to be honoured and the test suites would need to pass.

The web-api package is also the foundation for many other Slack SDKs, so the test suites (including the integration tests in the root of this repo) would need to pass. Some of these require Slack Enterprise org/workspace, which, I assume, you would not have access to and would need to be coordinated with someone from Slack.

If you do attempt to take this on, may I suggest you begin working in your fork on a branch, and before sending a PR to us, you ping us (in this issue is fine) to let us know when you think it is in a workable state before opening a PR? This way I can provide some feedback before we go through a full review and back and forth process.

filmaj commented 2 years ago

BTW, in case this is helpful, I was able to get an ES Modules-based version of @slack/web-api compatible with deno by making an esbuild- based build of the project, with no code changes. Perhaps that could be useful for someone wanting to get this project working on Cloudflare Workers.

Roughly:

use esbuild
use the browser field in package.json to stub in certain npm packages to replace built-in node ones with ones built for the browser, i.e. replacing path with the path-browserify npm module. The list of native node dependencies @slack/web-api relies on is path, os, querystring and process.
(optional) Depending on your target runtime, perhaps polyfilling the Buffer global is not necessary, but if so, use the experimental Buffer compatibility package available in unstable deno together with esbuild's --define and --inject flags to stub in the Buffer global to be available in deno.
use an XMLHttpRequest polyfill (there are many available in npm and deno.land) in deno since deno only supports fetch, but axios only supports either node's HTTP API (which deno has no compatibility for) or XHR. Note: there is a concept called "adapters" in axios, which provides a clean API to implement your own network layer in axios - this is a solid backup option and should be flexible to enable running in any runtime)

Some more specifics on how a deno-compatible build was compiled:

Esbuild command:

esbuild --bundle --define:process.cwd=String --define:process.version='"v14.0.0"' --define:Buffer=dummy_buffer --inject:./buffer-shim.js --target=esnext --format=esm --outdir=./dist-esm  src/index.ts

Some package.json changes were needed, like adding the browser field, as well as add browser-compatible versions of certain native node APIs, e.g. path-browserify and os-browserify:

  "browser": {
    "os": "os-browserify",
    "path": "path-browserify",
    "querystring": "./qs-shim.js"
  }

The buffer-shim.js referenced in the esbuild command is:

import { Buffer } from "https://deno.land/std@0.112.0/node/buffer.ts"

export let dummy_buffer = Buffer

The qs-shim.js referenced in the esbuild command is:

export * from "https://deno.land/std@0.112.0/node/querystring.ts"

NB: could probably use the same shim-in-deno-compatibility-node-modules approach for path, process and os, too in case we don't want to add more dependencies via browserify packages.

github-actions[bot] commented 2 years ago

👋 It looks like this issue has been open for 30 days with no activity. We'll mark this as stale for now, and wait 10 days for an update or for further comment before closing this issue out.

jimmywarting commented 2 years ago

👍 on removing axios and replacing it with fetch (most preferable the new global built in fetch from node v18) so it dose not have any dependencies

And then also ditching form-data for the newer spec'ed FormData built in also

if ppl can't update to node v18 then i think they could add a polyfill themself

maybe add:

"engines": {
    "node": ">=18.0.0"
}

nbolton commented 1 year ago

Judging by this ticket, looks like Slack API is not supported on Cloudflare Workers. Here's the error I get:

X [ERROR] Could not resolve "querystring"

    ../../node_modules/@slack/web-api/dist/WebClient.js:49:30:
      49 │ const querystring_1 = require("querystring");
         ╵                               ~~~~~~~~~~~~~

  The package "querystring" wasn't found on the file system but is built into node.
  Add "node_compat = true" to your wrangler.toml file to enable Node.js compatibility.

X [ERROR] Could not resolve "path"

    ../../node_modules/@slack/web-api/dist/WebClient.js:50:23:
      50 │ const path_1 = require("path");
         ╵                        ~~~~~~

  The package "path" wasn't found on the file system but is built into node.
  Add "node_compat = true" to your wrangler.toml file to enable Node.js compatibility.

X [ERROR] Could not resolve "zlib"

    ../../node_modules/@slack/web-api/dist/WebClient.js:57:39:
      57 │ const zlib_1 = __importDefault(require("zlib"));
         ╵                                        ~~~~~~

  The package "zlib" wasn't found on the file system but is built into node.
  Add "node_compat = true" to your wrangler.toml file to enable Node.js compatibility.

X [ERROR] Could not resolve "util"

    ../../node_modules/@slack/web-api/dist/WebClient.js:58:23:
      58 │ const util_1 = require("util");
         ╵                        ~~~~~~

  The package "util" wasn't found on the file system but is built into node.
  Add "node_compat = true" to your wrangler.toml file to enable Node.js compatibility.

X [ERROR] Could not resolve "fs"

    ../../node_modules/@slack/web-api/dist/file-upload.js:4:21:
      4 │ const fs_1 = require("fs");
        ╵                      ~~~~

  The package "fs" wasn't found on the file system but is built into node.
  Add "node_compat = true" to your wrangler.toml file to enable Node.js compatibility.

X [ERROR] Could not resolve "stream"

    ../../node_modules/@slack/web-api/dist/file-upload.js:5:25:
      5 │ const stream_1 = require("stream");
        ╵                          ~~~~~~~~

  The package "stream" wasn't found on the file system but is built into node.
  Add "node_compat = true" to your wrangler.toml file to enable Node.js compatibility.

X [ERROR] Could not resolve "os"

    ../../node_modules/@slack/web-api/dist/instrument.js:27:32:
      27 │ const os = __importStar(require("os"));
         ╵                                 ~~~~

  The package "os" wasn't found on the file system but is built into node.
  Add "node_compat = true" to your wrangler.toml file to enable Node.js compatibility.

X [ERROR] Could not resolve "path"

    ../../node_modules/@slack/web-api/dist/instrument.js:28:23:
      28 │ const path_1 = require("path");
         ╵                        ~~~~~~

  The package "path" wasn't found on the file system but is built into node.
  Add "node_compat = true" to your wrangler.toml file to enable Node.js compatibility.

X [ERROR] Build failed with 8 errors:

  ../../node_modules/@slack/web-api/dist/WebClient.js:49:30: ERROR: Could not resolve "querystring"
  ../../node_modules/@slack/web-api/dist/WebClient.js:50:23: ERROR: Could not resolve "path"
  ../../node_modules/@slack/web-api/dist/WebClient.js:57:39: ERROR: Could not resolve "zlib"
  ../../node_modules/@slack/web-api/dist/WebClient.js:58:23: ERROR: Could not resolve "util"
  ../../node_modules/@slack/web-api/dist/file-upload.js:4:21: ERROR: Could not resolve "fs"
  ...

nbolton commented 1 year ago

Actually, I just spotted this in the error message!

Add "node_compat = true" to your wrangler.toml file to enable Node.js compatibility.

After adding this, the dev server starts, but then as expected, I get the error from the axios dep:

[WARN]  web-api:WebClient:0 http request failed adapter is not a function
[mf:err] TypeError: adapter is not a function
    at dispatchRequest (\path\to\project\node_modules\axios\lib\core\dispatchRequest.js:58:10)  
    at Axios.request (\path\to\project\node_modules\axios\lib\core\Axios.js:109:15)
    at Axios.httpMethod [as post] (\path\to\project\node_modules\axios\lib\core\Axios.js:144:19)
    at Function.wrap2 [as post] (\path\to\project\node_modules\axios\lib\helpers\bind.js:9:15)
    at null.<anonymous> (\path\to\project\node_modules\@slack\web-api\src\WebClient.ts:560:43)
    at run (\path\to\project\node_modules\p-queue\dist\index.js:157:104)
    at PQueue._tryToStartAnother (\path\to\project\node_modules\p-queue\dist\index.js:105:17)
    at null.<anonymous> (\path\to\project\node_modules\p-queue\dist\index.js:171:18)
    at [object Object]
    at PQueue.add (\path\to\project\node_modules\p-queue\dist\index.js:152:16)

Possibly useful: https://stackoverflow.com/a/70206333/47775

Axios accepts an adapter field in its configuration, so you can pass a fetch adapter like axios-fetch-adapter and probably, you'll need to add regenerator-runtime too, so in your background.js file:

import "regenerator-runtime/runtime.js";

seratch commented 1 year ago

:wave: I understand that many of you would like to have a 1st party / official SDK for Cloudflare Workers. However, the current SDK only supports Node.js, and our team has no plans to enhance it to support other types of runtimes, such as non-Node.js edge functions at this moment.

Recently, I began a hobby weekend project, a tool for Cloudflare and Vercel. If you’re open to using an unofficial library, my project could be useful for you. For more details, please refer to https://github.com/slackapi/node-slack-sdk/issues/1603#issuecomment-1606541205

filmaj commented 8 months ago

With @seratch providing an alternative project, going to close this issue, as web-api has no plans to officially support a runtime beyond node.

slackapi / node-slack-sdk