twilio-node icon indicating copy to clipboard operation
twilio-node copied to clipboard

Could you replace "axios" with fetch?

Open jimmywarting opened this issue 2 years ago • 16 comments

... as part of #728 ?

let users bring in there own fetch implementation.

jimmywarting avatar Jan 20 '23 16:01 jimmywarting

Hey @jimmywarting, we'll add this to our internal backlog to be prioritized. +1s/reactions on this issue (and comments) and PRs will bump it higher up the backlog.

beebzz avatar Jan 26 '23 16:01 beebzz

@jimmywarting I don't see the point of getting rid of axios. Axios client has other stuff like httpsAgent and a dependency of Request which has a large surface. I guess you could map a adapter interface so you could use undici but...there are prolly bigger fish to fry.

It's also super small as a dependency. https://github.com/mariomui/twilio-node/tree/smaller-build. <---I shaved off 100kb to make it 1.8mb but someone else is gonna need to test this a lambda project handy.

In my opinion:

  • Deprecate the pattern of : exports = initializer
    • Basically amounts to -> module.exports = initialzer ---> Which can be done by babel.
    • also -> we can get rid of the plugin "babel-plugin-replace-ts-export-assignment"
  • use tsc to generate the types only, export them into dist/interfaces/
  • use babel to transpile typescript down to js. (performance)
  • convert existing code to get to ESM status.
  • (wait for someone higher up to spend money to rearchitect 4.0.0 to a monorepos with multiple packages)

mariomui avatar Jan 27 '23 04:01 mariomui

axios have many things i do not like about it

  • Dose not work in service workers, Deno or bun.js (cuz it use either xhr or node:http(s)
  • responseType = arrayBuffer is inconsistent in different env.
  • depends on the legacy form-data package
  • Don't support streams in browsers
    • b/c it depends on the legacy XMLHttpRequest
  • the stream api isn't based on whatwg stream
  • Even though i used { responseType: 'text' } on a request it still parsed it as json b/c it included application/json content-type headers
  • dose thing with node:buffer instead of using Uint8array
  • dose not support spec compatible FormData or Blob on NodeJS
  • and no i don't think axios is a "small as a dependency"
  • and you can ofc use Agent with fetch as well if you want that.

jimmywarting avatar Jan 31 '23 16:01 jimmywarting

At the end of April, Node.js v14 will be EOL and v16 will be the minimum supported version of Node.js. unidici-fetch fully supports Node.js v16 (I believe some features are missing from v14 that undici-fetch uses). Undici is an official project under the Node.js project, so it will be very will supported within Node.js, and problems will get resolved rapidly as they come up. Undici also generally has a pretty hardcore focus on performance, which will very likely benefit us here.

undici-fetch is directly included in Node.js v18 as Node.js's official fetch implementation, meaning that next year a third-party dependency could be entirely dropped since Node.js will support fetch directly, natively, on all supported release lines.

Outside of adopting undici-fetch, I do +1 allowing users to bring their own fetch implementation if they don't want to use the one we provide. There is already support for a Custom HTTP Client (thank you @beebzz for pointing this out!), so I'm not sure how different "custom fetch" (request in the OP) and "custom HTTP client" are in practice - they're not exactly compatible, but they're not totally incompatible. This request is basically asking for twilio-node to switch to using fetch rather than other HTTP request methods, which is important to note.

bnb avatar Feb 02 '23 22:02 bnb

Absolutely would love to see fetch used instead of axios.

aaronhuggins-carewell avatar Feb 10 '23 15:02 aaronhuggins-carewell

+1

IF this makes twilio usable in Deno environments instead of NodeJS, I'd love to have it too.

nileshtrivedi avatar Feb 16 '23 21:02 nileshtrivedi

Same could be said for Bun.js

jimmywarting avatar Feb 16 '23 21:02 jimmywarting

There's currently just one Twilio-related package listed on Deno.land which hasn't seen any activity for almost 2 years now.

It won't do much with a single file having less than 50 lines.

by the way, this project is only exported as CJS.

UrielCh avatar May 09 '23 07:05 UrielCh

+1

sarankumar-ns avatar Jan 09 '24 11:01 sarankumar-ns

Also had issues with this. Using bun and the promise was never resolving for messages.create

paperless avatar Jan 18 '24 00:01 paperless

+1 Unless I'm missing something, twilio-node doesn't work in my Electron app at all (node.js context). I would like to use twilio as a platform, but this is obviously a dealbreaker.

twilio gives this error when trying to run the Hello World code after signing up.

client.calls.create({
      url: "http://demo.twilio.com/docs/voice.xml",
      to: "<my_number>",
      from: "<my_twilio_number>",
})
 .then(call => console.log(call.sid));

UnhandledPromiseRejectionWarning: AxiosError: There is no suitable adapter to dispatch the request since :
- adapter xhr is not supported by the environment
- adapter http is not available in the build

riley avatar Feb 12 '24 05:02 riley

+1

OrRosenblatt avatar Apr 10 '24 18:04 OrRosenblatt

except for typescript nonsense, this seems to work:

set a custom request client but with twilio's type

  import twilio, { type RequestClient as TwilioRequestClient } from 'twilio'

const  client = twilio(
    process.env.TWILIO_ACCOUNT_SID!,
    process.env.TWILIO_AUTH_TOKEN!,
    {
      httpClient: new RequestClient() as TwilioRequestClient
    }
  )

And a custom request client implementation:

// axios (which twilio uses) breaks in bun, so override with fetch by having our
// own RequestClient implementation

// couldn't figure out out how to import the types of RequestOptions and Response
// from the twilio sdk, so copied it
type HttpMethod = 'get' | 'post' | 'put' | 'patch' | 'delete'
interface Headers {
  [header: string]: string
}

interface RequestOptions<TData = any, TParams = object> {
  method: HttpMethod
  uri: string
  username?: string
  password?: string
  headers?: Headers
  params?: TParams
  data?: TData
  timeout?: number
  allowRedirects?: boolean
  forever?: boolean
  logLevel?: string
}

class Response<TPayload> {
  constructor(
    public statusCode: number,
    public body: TPayload,
    public headers: any
  ) {}

  toString(): string {
    return 'HTTP ' + this.statusCode + ' ' + this.body
  }
}

export default class RequestClient {

  async request<TData>(opts: RequestOptions<TData>): Promise<Response<TData>> {
    const { uri, method, username, password, data, ...rest } = opts

    let headers = opts.headers || {}

    // from twilio sdk
    if (opts.username && opts.password) {
      const auth = Buffer.from(opts.username + ':' + opts.password).toString(
        'base64'
      )
      headers.Authorization = 'Basic ' + auth
    }

    let body: URLSearchParams | string | null = null
    if (
      headers['Content-Type'] === 'application/x-www-form-urlencoded' &&
      data
    ) {
      // TODO(ibash) typescript nonsense
      // Type 'import("url").URLSearchParams' is not assignable to type 'URLSearchParams'.
      body = new URLSearchParams(data) as any
    } else if (headers['Content-Type'] === 'application/json' && data) {
      body = JSON.stringify(data)
    }

    const response = await fetch(uri, {
      // TODO(ibash) uppercase method?
      method,
      headers: headers,
      body: body
    })

    // TODO(ibash) handle other response types
    let responseBody: TData | null = null
    const contentType = response.headers.get('Content-Type')
    if (contentType?.includes('application/json')) {
      responseBody = (await response.json()) as TData
    } else {
      throw new Error(`Unhandled response type: ${contentType}`)
    }

    return new Response(
      response.status,
      responseBody as TData,
      response.headers
    )
  }
}

ibash avatar Apr 24 '24 23:04 ibash

ky can also be used to make working with fetch() easier and handling common patterns (hooks, errors, etc.)

mfulton26 avatar Jun 06 '24 18:06 mfulton26

+1

tawanaj avatar Jun 07 '24 15:06 tawanaj