Cloudflare Workers support (by moving off of axios)

[drmercer]

Enhancement description

This SDK is currently unusable in Cloudflare Workers because it uses axios, which strangely doesn't support using fetch instead of the super-old XHR api (and it doesn't seem they plan to address it anytime soon axios/axios#1219). Moving from axios to something like isomorphic-fetch would solve the issue.

Bonus: doing this would make Deno work for free, I think, because Deno also supports fetch. (#19)

Alternative, easier solution: expose an options object for the TodoistApi constructor, with an adapter option that lets me pass a custom axios adapter to use.

The problem it solves

I can't use this Todoist SDK in Cloudflare workers currently. I get this error:

TypeError: adapter is not a function

This is caused by these lines in axios - it only supports node and environments with XHR. Cloudflare Workers is neither of those, unfortunately.

https://github.com/axios/axios/blob/65977f995c7b92aeca26aa1ba50e1d6678389002/lib/defaults/index.js#L22-L32

Alternatives

I haven't found a good workaround for this. One solution I've thought of is forking axios, gutting it and replacing the internals with fetch, and then using the resolutions/overrides field to make this package use my axios-but-actually-fetch package instead of the real axios. But that would be a lot of work 😅

[henningmu]

Great proposal @drmercer, we'd love to make this at least configurable 👍

Most of the code resides in the restClient.ts, there you can see we are also using axios-retry to get automatic retries on network failures. The retry mechanism is something we could ditch for users supplying their own "network client". Would you be willing to contribute a PR for this? You could get inspired by similar work where we made the base API URL configurable: #130

[rendomnet]

Also it is not working on Browser extensions with Manifest v3 Service workers.

[drmercer]

As a workaround, I got this working by using patch-package with this patch, to replace axios' built-in adapters with this fetch-based adapter:

diff --git a/node_modules/axios/lib/defaults/index.js b/node_modules/axios/lib/defaults/index.js
index 9199818..1cfccc6 100644
--- a/node_modules/axios/lib/defaults/index.js
+++ b/node_modules/axios/lib/defaults/index.js
@@ -17,15 +17,7 @@ function setContentTypeIfUnset(headers, value) {
 }
 
 function getDefaultAdapter() {
-  var adapter;
-  if (typeof XMLHttpRequest !== 'undefined') {
-    // For browsers use XHR adapter
-    adapter = require('../adapters/xhr');
-  } else if (typeof process !== 'undefined' && Object.prototype.toString.call(process) === '[object process]') {
-    // For node use HTTP adapter
-    adapter = require('../adapters/http');
-  }
-  return adapter;
+  return require('@vespaiach/axios-fetch-adapter').default
 }
 
 function stringifySafely(rawValue, parser, encoder) {

[drmercer]

@henningmu I'm not sure if I'll have time to contribute something, but I might give it a shot. 🙂 What would the ideal interface look like for that configuration? Would the user pass in a custom function to replace the request function from restClient.ts?

[henningmu]

Awesome to hear @drmercer. Yes, we could make the whole request function configurable, that's probably the cleanest cut (as opposed to just the axiosClient).
Another alternative could be to rip out axios completely and replace it with isomorphic-fetch. We would accept a PR for that as well.

I'm off for this and next week, but if time allows I'll try my hands at the isomorphic-fetch solution after 👍

[drmercer]

Hmmm, I'm actually not sure if isomorphic-fetch or cross-fetch will solve the problem, because it doesn't seem either of them use the native fetch if available (they always reimplement it, for consistency). The ideal would be to use node-fetch in Node, and the native fetch in any other environment... Would it be too much to ask the user to load node-fetch themselves if they're running in Node?

Edit: I think I was wrong about cross-fetch and isomorphic-fetch - they both use this fetch polyfill, which uses the native fetch if available. So I think either of those would be a great solution to this. Between the two, I'd recommend cross-fetch, as it seems more up-to-date.

[nelsonjchen]

It looks like it was eventually addressed. Is this issue still applicable?

[nelsonjchen]

Not sure what happened but for my own project, it wasn't working before and it was working now.

It was failing due to some requestinfodict support thing being missing but it looks like the last of the roadblocks in the CF workers environment might have been lifted.

[henningmu]

Thank you for the update @nelsonjchen. I'll go ahead and close this issue 👍